Date:   Wed, 1 Mar 2023 12:36:45 -0800
From:   "Paul E. McKenney" <paulmck@kernel.org>
To:     Steven Rostedt <rostedt@goodmis.org>
Cc:     Uros Bizjak <ubizjak@gmail.com>,
        Joel Fernandes <joel@joelfernandes.org>, rcu@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Frederic Weisbecker <frederic@kernel.org>,
        Neeraj Upadhyay <quic_neeraju@quicinc.com>,
        Josh Triplett <josh@joshtriplett.org>,
        Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
        Lai Jiangshan <jiangshanlai@gmail.com>
Subject: Re: [PATCH] rcu: use try_cmpxchg in check_cpu_stall
Message-ID: <20230301203645.GG2948950@paulmck-ThinkPad-P17-Gen-1>
Reply-To: paulmck@kernel.org
References: <20230228160324.2a7c1012@gandalf.local.home>
 <20230228212911.GX2948950@paulmck-ThinkPad-P17-Gen-1>
 <20230228164124.77c126d2@gandalf.local.home>
 <CAEXW_YQ515_DOLVUm48GvDADuaY2mSrYTaKa7u6jYDNqBncJww@mail.gmail.com>
 <20230228190846.79b06089@gandalf.local.home>
 <CAFULd4aY3Y8tyLN70oebZDagBebvH0erwRxKDaEX8L83Xo8gYw@mail.gmail.com>
 <20230301113813.4f16a689@gandalf.local.home>
 <CAFULd4aWZ+fvVZ+MEt6JD1rFaQZeO5DDWAHtPKOrx8R8DETFBw@mail.gmail.com>
 <20230301200820.GF2948950@paulmck-ThinkPad-P17-Gen-1>
 <20230301151826.014c5977@gandalf.local.home>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230301151826.014c5977@gandalf.local.home>
Precedence: bulk

On Wed, Mar 01, 2023 at 03:18:26PM -0500, Steven Rostedt wrote:
> On Wed, 1 Mar 2023 12:08:20 -0800
> "Paul E. McKenney" <paulmck@kernel.org> wrote:
> 
> > > Attached patch implements this suggestion.  
> > 
> > Please help me out here.
> > 
> > Why on earth are we even discussing making this change to code that
> > normally never executes?  Performance is not a consideration here.
> > 
> > What am I missing here?  Is there some sort of forward-progress
> > issue that this change addresses?
> 
> Well, we sorta hijacked this thread. It turned into a more general
> discussion, as there is code that this change will be useful for
> (ring_buffer.c), but we just happen to be having the discussion here.
> 
> Where it will at most remove some text and give you back a few extra bytes
> of memory ;-)
> 
> But if we do use cmpxchg_success() IMHO, it does improve readability.
> 
> > -	    cmpxchg(&rcu_state.jiffies_stall, js, jn) == js) {
> > +	    cmpxchg_success(&rcu_state.jiffies_stall, js, jn)) {

Some years down the road, should cmpxchg_success() be on the tip of
the tongue of every kernel hacker, perhaps.  Or perhaps not.

In the meantime, we have yet another abysmally documented atomic
operation that is not well known throughout the community.  And then the
people coming across this curse everyone who had anything to do with it,
as they search the source code, dig through assembly output, and so on
trying to work out exactly what this thing does.

Sorry, but no way.

Again, unless there is some sort of forward-progress argument or
similar convincing argument.

							Thanx, Paul