Received-SPF: pass (google.com: domain of linux-kernel+bounces-203063-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Wed, 5 Jun 2024 11:36:15 -0700
From: "Paul E. McKenney" <paulmck@kernel.org>
To: Frederic Weisbecker <frederic@kernel.org>
Cc: rcu@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-team@meta.com,
	rostedt@goodmis.org, Rao Shoaib <rao.shoaib@oracle.com>
Subject: Re: [PATCH rcu 2/2] doc: Clarify rcu_assign_pointer() and
 rcu_dereference() ordering
Message-ID: <4035a57f-9873-4178-8d60-3ab07f709e2e@paulmck-laptop>
Reply-To: paulmck@kernel.org
References: <e5dc5f14-04c1-494b-9044-e837279a7bcd@paulmck-laptop>
 <20240604222155.2370541-2-paulmck@kernel.org>
 <ZmBSZ3AAp9TN5YaP@localhost.localdomain>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <ZmBSZ3AAp9TN5YaP@localhost.localdomain>

On Wed, Jun 05, 2024 at 01:56:23PM +0200, Frederic Weisbecker wrote:
> Le Tue, Jun 04, 2024 at 03:21:55PM -0700, Paul E. McKenney a ?crit :
> > This commit expands on the ordering properties of rcu_assign_pointer()
> > and rcu_dereference(), outlining their constraints on CPUs and compilers.
> > 
> > Reported-by: Rao Shoaib <rao.shoaib@oracle.com>
> > Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
> > ---
> >  Documentation/RCU/whatisRCU.rst | 30 +++++++++++++++++++-----------
> >  1 file changed, 19 insertions(+), 11 deletions(-)
> > 
> > diff --git a/Documentation/RCU/whatisRCU.rst b/Documentation/RCU/whatisRCU.rst
> > index 94838c65c7d97..d585a5490aeec 100644
> > --- a/Documentation/RCU/whatisRCU.rst
> > +++ b/Documentation/RCU/whatisRCU.rst
> > @@ -250,21 +250,25 @@ rcu_assign_pointer()
> >  ^^^^^^^^^^^^^^^^^^^^
> >  	void rcu_assign_pointer(p, typeof(p) v);
> >  
> > -	Yes, rcu_assign_pointer() **is** implemented as a macro, though it
> > -	would be cool to be able to declare a function in this manner.
> > -	(Compiler experts will no doubt disagree.)
> > +	Yes, rcu_assign_pointer() **is** implemented as a macro, though
> > +	it would be cool to be able to declare a function in this manner.
> > +	(And there has been some discussion of adding overloaded functions
> > +	to the C language, so who knows?)
> >  
> >  	The updater uses this spatial macro to assign a new value to an
> >  	RCU-protected pointer, in order to safely communicate the change
> >  	in value from the updater to the reader.  This is a spatial (as
> >  	opposed to temporal) macro.  It does not evaluate to an rvalue,
> > -	but it does execute any memory-barrier instructions required
> > -	for a given CPU architecture.  Its ordering properties are that
> > -	of a store-release operation.
> > -
> > -	Perhaps just as important, it serves to document (1) which
> > -	pointers are protected by RCU and (2) the point at which a
> > -	given structure becomes accessible to other CPUs.  That said,
> > +	but it does provide any compiler directives and memory-barrier
> > +	instructions required for a given compile or CPU architecture.
> > +	Its ordering properties are that of a store-release operation,
> > +	that is, any prior loads and stores required to initialize the
> > +	structure are ordered before the store that publishes the pointer
> > +	to that structure.
> 
> About that, why rcu_dereference() isn't a matching load-acquire?

Here is an example showing the difference:

	p = rcu_dereference(gp);
	r1 = READ_ONCE(x);
	r2 = p->a;

The READ_ONCE() is not ordered against the rcu_dereference(), only the
read from p->a.  In contrast, if that rcu_dereference() was instead an
smp_load_acquire(), both of the two later statements would be ordered.

Ah.  You are suggesting that this be added to the description of
rcu_dereference()?

Or are you asking that this documentation state that an rcu_dereference()
memory-barrier-pairs with an rcu_assign_pointer()?

Or something else completely?

							Thanx, Paul