Date:   Sun, 3 Nov 2019 12:05:22 +0530
From:   Amol Grover <frextrite@...il.com>
To:     "Paul E. McKenney" <paulmck@...nel.org>
Cc:     Josh Triplett <josh@...htriplett.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        Lai Jiangshan <jiangshanlai@...il.com>,
        Joel Fernandes <joel@...lfernandes.org>,
        Jonathan Corbet <corbet@....net>, rcu@...r.kernel.org,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-kernel-mentees@...ts.linuxfoundation.org,
        Shuah Khan <skhan@...uxfoundation.org>,
        madhuparnabhowmik04@...il.com, tranmanphong@...il.com
Subject: Re: [PATCH] Documentation: RCU: rcu_dereference: Convert to
 rcu_dereference.rst

On Sat, Nov 02, 2019 at 09:57:34AM -0700, Paul E. McKenney wrote:
> On Sat, Nov 02, 2019 at 01:31:07PM +0530, Amol Grover wrote:
> > This patch converts rcu_dereference.txt to rcu_dereference.rst and
> > adds it to index.rst
> > 
> > Signed-off-by: Amol Grover <frextrite@...il.com>
> 
> Queued and pushed for review and testing.  Assuming all goes well, I will
> push it for the v5.6 merge window.
> 
> Could you please take a look at the similar commits from Madhuparna and
> Phong?  No time like the present to also gain experience with review
> and testing!
> 
> 							Thanx, Paul

Sure thing Paul!

Thank you
Amol

> 
> > ---
> >  Documentation/RCU/index.rst                   |  1 +
> >  ...cu_dereference.txt => rcu_dereference.rst} | 75 ++++++++++---------
> >  2 files changed, 42 insertions(+), 34 deletions(-)
> >  rename Documentation/RCU/{rcu_dereference.txt => rcu_dereference.rst} (88%)
> > 
> > diff --git a/Documentation/RCU/index.rst b/Documentation/RCU/index.rst
> > index 627128c230dc..585f3d8abd76 100644
> > --- a/Documentation/RCU/index.rst
> > +++ b/Documentation/RCU/index.rst
> > @@ -8,6 +8,7 @@ RCU concepts
> >     :maxdepth: 3
> >  
> >     arrayRCU
> > +   rcu_dereference
> >     rcu
> >     listRCU
> >     NMI-RCU
> > diff --git a/Documentation/RCU/rcu_dereference.txt b/Documentation/RCU/rcu_dereference.rst
> > similarity index 88%
> > rename from Documentation/RCU/rcu_dereference.txt
> > rename to Documentation/RCU/rcu_dereference.rst
> > index bf699e8cfc75..c9667eb0d444 100644
> > --- a/Documentation/RCU/rcu_dereference.txt
> > +++ b/Documentation/RCU/rcu_dereference.rst
> > @@ -1,4 +1,7 @@
> > +.. _rcu_dereference_doc:
> > +
> >  PROPER CARE AND FEEDING OF RETURN VALUES FROM rcu_dereference()
> > +===============================================================
> >  
> >  Most of the time, you can use values from rcu_dereference() or one of
> >  the similar primitives without worries.  Dereferencing (prefix "*"),
> > @@ -8,7 +11,7 @@ subtraction of constants, and casts all work quite naturally and safely.
> >  It is nevertheless possible to get into trouble with other operations.
> >  Follow these rules to keep your RCU code working properly:
> >  
> > -o	You must use one of the rcu_dereference() family of primitives
> > +-	You must use one of the rcu_dereference() family of primitives
> >  	to load an RCU-protected pointer, otherwise CONFIG_PROVE_RCU
> >  	will complain.  Worse yet, your code can see random memory-corruption
> >  	bugs due to games that compilers and DEC Alpha can play.
> > @@ -25,24 +28,24 @@ o	You must use one of the rcu_dereference() family of primitives
> >  	for an example where the compiler can in fact deduce the exact
> >  	value of the pointer, and thus cause misordering.
> >  
> > -o	You are only permitted to use rcu_dereference on pointer values.
> > +-	You are only permitted to use rcu_dereference on pointer values.
> >  	The compiler simply knows too much about integral values to
> >  	trust it to carry dependencies through integer operations.
> >  	There are a very few exceptions, namely that you can temporarily
> >  	cast the pointer to uintptr_t in order to:
> >  
> > -	o	Set bits and clear bits down in the must-be-zero low-order
> > +	-	Set bits and clear bits down in the must-be-zero low-order
> >  		bits of that pointer.  This clearly means that the pointer
> >  		must have alignment constraints, for example, this does
> >  		-not- work in general for char* pointers.
> >  
> > -	o	XOR bits to translate pointers, as is done in some
> > +	-	XOR bits to translate pointers, as is done in some
> >  		classic buddy-allocator algorithms.
> >  
> >  	It is important to cast the value back to pointer before
> >  	doing much of anything else with it.
> >  
> > -o	Avoid cancellation when using the "+" and "-" infix arithmetic
> > +-	Avoid cancellation when using the "+" and "-" infix arithmetic
> >  	operators.  For example, for a given variable "x", avoid
> >  	"(x-(uintptr_t)x)" for char* pointers.	The compiler is within its
> >  	rights to substitute zero for this sort of expression, so that
> > @@ -54,16 +57,16 @@ o	Avoid cancellation when using the "+" and "-" infix arithmetic
> >  	"p+a-b" is safe because its value still necessarily depends on
> >  	the rcu_dereference(), thus maintaining proper ordering.
> >  
> > -o	If you are using RCU to protect JITed functions, so that the
> > +-	If you are using RCU to protect JITed functions, so that the
> >  	"()" function-invocation operator is applied to a value obtained
> >  	(directly or indirectly) from rcu_dereference(), you may need to
> >  	interact directly with the hardware to flush instruction caches.
> >  	This issue arises on some systems when a newly JITed function is
> >  	using the same memory that was used by an earlier JITed function.
> >  
> > -o	Do not use the results from relational operators ("==", "!=",
> > +-	Do not use the results from relational operators ("==", "!=",
> >  	">", ">=", "<", or "<=") when dereferencing.  For example,
> > -	the following (quite strange) code is buggy:
> > +	the following (quite strange) code is buggy::
> >  
> >  		int *p;
> >  		int *q;
> > @@ -81,11 +84,11 @@ o	Do not use the results from relational operators ("==", "!=",
> >  	after such branches, but can speculate loads, which can again
> >  	result in misordering bugs.
> >  
> > -o	Be very careful about comparing pointers obtained from
> > +-	Be very careful about comparing pointers obtained from
> >  	rcu_dereference() against non-NULL values.  As Linus Torvalds
> >  	explained, if the two pointers are equal, the compiler could
> >  	substitute the pointer you are comparing against for the pointer
> > -	obtained from rcu_dereference().  For example:
> > +	obtained from rcu_dereference().  For example::
> >  
> >  		p = rcu_dereference(gp);
> >  		if (p == &default_struct)
> > @@ -93,7 +96,7 @@ o	Be very careful about comparing pointers obtained from
> >  
> >  	Because the compiler now knows that the value of "p" is exactly
> >  	the address of the variable "default_struct", it is free to
> > -	transform this code into the following:
> > +	transform this code into the following::
> >  
> >  		p = rcu_dereference(gp);
> >  		if (p == &default_struct)
> > @@ -105,14 +108,14 @@ o	Be very careful about comparing pointers obtained from
> >  
> >  	However, comparisons are OK in the following cases:
> >  
> > -	o	The comparison was against the NULL pointer.  If the
> > +	-	The comparison was against the NULL pointer.  If the
> >  		compiler knows that the pointer is NULL, you had better
> >  		not be dereferencing it anyway.  If the comparison is
> >  		non-equal, the compiler is none the wiser.  Therefore,
> >  		it is safe to compare pointers from rcu_dereference()
> >  		against NULL pointers.
> >  
> > -	o	The pointer is never dereferenced after being compared.
> > +	-	The pointer is never dereferenced after being compared.
> >  		Since there are no subsequent dereferences, the compiler
> >  		cannot use anything it learned from the comparison
> >  		to reorder the non-existent subsequent dereferences.
> > @@ -124,31 +127,31 @@ o	Be very careful about comparing pointers obtained from
> >  		dereferenced, rcu_access_pointer() should be used in place
> >  		of rcu_dereference().
> >  
> > -	o	The comparison is against a pointer that references memory
> > +	-	The comparison is against a pointer that references memory
> >  		that was initialized "a long time ago."  The reason
> >  		this is safe is that even if misordering occurs, the
> >  		misordering will not affect the accesses that follow
> >  		the comparison.  So exactly how long ago is "a long
> >  		time ago"?  Here are some possibilities:
> >  
> > -		o	Compile time.
> > +		-	Compile time.
> >  
> > -		o	Boot time.
> > +		-	Boot time.
> >  
> > -		o	Module-init time for module code.
> > +		-	Module-init time for module code.
> >  
> > -		o	Prior to kthread creation for kthread code.
> > +		-	Prior to kthread creation for kthread code.
> >  
> > -		o	During some prior acquisition of the lock that
> > +		-	During some prior acquisition of the lock that
> >  			we now hold.
> >  
> > -		o	Before mod_timer() time for a timer handler.
> > +		-	Before mod_timer() time for a timer handler.
> >  
> >  		There are many other possibilities involving the Linux
> >  		kernel's wide array of primitives that cause code to
> >  		be invoked at a later time.
> >  
> > -	o	The pointer being compared against also came from
> > +	-	The pointer being compared against also came from
> >  		rcu_dereference().  In this case, both pointers depend
> >  		on one rcu_dereference() or another, so you get proper
> >  		ordering either way.
> > @@ -159,13 +162,13 @@ o	Be very careful about comparing pointers obtained from
> >  		of such an RCU usage bug is shown in the section titled
> >  		"EXAMPLE OF AMPLIFIED RCU-USAGE BUG".
> >  
> > -	o	All of the accesses following the comparison are stores,
> > +	-	All of the accesses following the comparison are stores,
> >  		so that a control dependency preserves the needed ordering.
> >  		That said, it is easy to get control dependencies wrong.
> >  		Please see the "CONTROL DEPENDENCIES" section of
> >  		Documentation/memory-barriers.txt for more details.
> >  
> > -	o	The pointers are not equal -and- the compiler does
> > +	-	The pointers are not equal -and- the compiler does
> >  		not have enough information to deduce the value of the
> >  		pointer.  Note that the volatile cast in rcu_dereference()
> >  		will normally prevent the compiler from knowing too much.
> > @@ -175,7 +178,7 @@ o	Be very careful about comparing pointers obtained from
> >  		comparison will provide exactly the information that the
> >  		compiler needs to deduce the value of the pointer.
> >  
> > -o	Disable any value-speculation optimizations that your compiler
> > +-	Disable any value-speculation optimizations that your compiler
> >  	might provide, especially if you are making use of feedback-based
> >  	optimizations that take data collected from prior runs.  Such
> >  	value-speculation optimizations reorder operations by design.
> > @@ -188,11 +191,12 @@ o	Disable any value-speculation optimizations that your compiler
> >  
> >  
> >  EXAMPLE OF AMPLIFIED RCU-USAGE BUG
> > +----------------------------------
> >  
> >  Because updaters can run concurrently with RCU readers, RCU readers can
> >  see stale and/or inconsistent values.  If RCU readers need fresh or
> >  consistent values, which they sometimes do, they need to take proper
> > -precautions.  To see this, consider the following code fragment:
> > +precautions.  To see this, consider the following code fragment::
> >  
> >  	struct foo {
> >  		int a;
> > @@ -244,7 +248,7 @@ to some reordering from the compiler and CPUs is beside the point.
> >  
> >  But suppose that the reader needs a consistent view?
> >  
> > -Then one approach is to use locking, for example, as follows:
> > +Then one approach is to use locking, for example, as follows::
> >  
> >  	struct foo {
> >  		int a;
> > @@ -299,6 +303,7 @@ As always, use the right tool for the job!
> >  
> >  
> >  EXAMPLE WHERE THE COMPILER KNOWS TOO MUCH
> > +-----------------------------------------
> >  
> >  If a pointer obtained from rcu_dereference() compares not-equal to some
> >  other pointer, the compiler normally has no clue what the value of the
> > @@ -308,7 +313,7 @@ guarantees that RCU depends on.  And the volatile cast in rcu_dereference()
> >  should prevent the compiler from guessing the value.
> >  
> >  But without rcu_dereference(), the compiler knows more than you might
> > -expect.  Consider the following code fragment:
> > +expect.  Consider the following code fragment::
> >  
> >  	struct foo {
> >  		int a;
> > @@ -354,6 +359,7 @@ dereference the resulting pointer.
> >  
> >  
> >  WHICH MEMBER OF THE rcu_dereference() FAMILY SHOULD YOU USE?
> > +------------------------------------------------------------
> >  
> >  First, please avoid using rcu_dereference_raw() and also please avoid
> >  using rcu_dereference_check() and rcu_dereference_protected() with a
> > @@ -370,7 +376,7 @@ member of the rcu_dereference() to use in various situations:
> >  
> >  2.	If the access might be within an RCU read-side critical section
> >  	on the one hand, or protected by (say) my_lock on the other,
> > -	use rcu_dereference_check(), for example:
> > +	use rcu_dereference_check(), for example::
> >  
> >  		p1 = rcu_dereference_check(p->rcu_protected_pointer,
> >  					   lockdep_is_held(&my_lock));
> > @@ -378,14 +384,14 @@ member of the rcu_dereference() to use in various situations:
> >  
> >  3.	If the access might be within an RCU read-side critical section
> >  	on the one hand, or protected by either my_lock or your_lock on
> > -	the other, again use rcu_dereference_check(), for example:
> > +	the other, again use rcu_dereference_check(), for example::
> >  
> >  		p1 = rcu_dereference_check(p->rcu_protected_pointer,
> >  					   lockdep_is_held(&my_lock) ||
> >  					   lockdep_is_held(&your_lock));
> >  
> >  4.	If the access is on the update side, so that it is always protected
> > -	by my_lock, use rcu_dereference_protected():
> > +	by my_lock, use rcu_dereference_protected()::
> >  
> >  		p1 = rcu_dereference_protected(p->rcu_protected_pointer,
> >  					       lockdep_is_held(&my_lock));
> > @@ -410,18 +416,19 @@ member of the rcu_dereference() to use in various situations:
> >  
> >  
> >  SPARSE CHECKING OF RCU-PROTECTED POINTERS
> > +-----------------------------------------
> >  
> >  The sparse static-analysis tool checks for direct access to RCU-protected
> >  pointers, which can result in "interesting" bugs due to compiler
> >  optimizations involving invented loads and perhaps also load tearing.
> > -For example, suppose someone mistakenly does something like this:
> > +For example, suppose someone mistakenly does something like this::
> >  
> >  	p = q->rcu_protected_pointer;
> >  	do_something_with(p->a);
> >  	do_something_else_with(p->b);
> >  
> >  If register pressure is high, the compiler might optimize "p" out
> > -of existence, transforming the code to something like this:
> > +of existence, transforming the code to something like this::
> >  
> >  	do_something_with(q->rcu_protected_pointer->a);
> >  	do_something_else_with(q->rcu_protected_pointer->b);
> > @@ -435,7 +442,7 @@ Load tearing could of course result in dereferencing a mashup of a pair
> >  of pointers, which also might fatally disappoint your code.
> >  
> >  These problems could have been avoided simply by making the code instead
> > -read as follows:
> > +read as follows::
> >  
> >  	p = rcu_dereference(q->rcu_protected_pointer);
> >  	do_something_with(p->a);
> > @@ -448,7 +455,7 @@ or as a formal parameter, with "__rcu", which tells sparse to complain if
> >  this pointer is accessed directly.  It will also cause sparse to complain
> >  if a pointer not marked with "__rcu" is accessed using rcu_dereference()
> >  and friends.  For example, ->rcu_protected_pointer might be declared as
> > -follows:
> > +follows::
> >  
> >  	struct foo __rcu *rcu_protected_pointer;
> >  
> > -- 
> > 2.20.1
> > 

Powered by blists - more mailing lists