All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: linux-kernel@vger.kernel.org
Cc: mingo@kernel.org, jiangshanlai@gmail.com, dipankar@in.ibm.com,
	akpm@linux-foundation.org, mathieu.desnoyers@efficios.com,
	josh@joshtriplett.org, tglx@linutronix.de, peterz@infradead.org,
	rostedt@goodmis.org, dhowells@redhat.com, edumazet@google.com,
	dvhart@linux.intel.com, fweisbec@gmail.com, oleg@redhat.com,
	bobby.prani@gmail.com,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Subject: [PATCH tip/core/rcu 3/5] doc: Update control-dependencies section of memory-barriers.txt
Date: Sat, 14 Jan 2017 00:55:37 -0800	[thread overview]
Message-ID: <1484384139-19622-3-git-send-email-paulmck@linux.vnet.ibm.com> (raw)
In-Reply-To: <20170114085032.GA18485@linux.vnet.ibm.com>

This commit adds consistency to examples, formatting, and a couple of
additional warnings.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 Documentation/memory-barriers.txt | 70 +++++++++++++++++++++------------------
 1 file changed, 38 insertions(+), 32 deletions(-)

diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt
index ba818ecce6f9..d2b0a8d81258 100644
--- a/Documentation/memory-barriers.txt
+++ b/Documentation/memory-barriers.txt
@@ -640,6 +640,10 @@ See also the subsection on "Cache Coherency" for a more thorough example.
 CONTROL DEPENDENCIES
 --------------------
 
+Control dependencies can be a bit tricky because current compilers do
+not understand them.  The purpose of this section is to help you prevent
+the compiler's ignorance from breaking your code.
+
 A load-load control dependency requires a full read memory barrier, not
 simply a data dependency barrier to make it work correctly.  Consider the
 following bit of code:
@@ -667,14 +671,15 @@ for load-store control dependencies, as in the following example:
 
 	q = READ_ONCE(a);
 	if (q) {
-		WRITE_ONCE(b, p);
+		WRITE_ONCE(b, 1);
 	}
 
-Control dependencies pair normally with other types of barriers.  That
-said, please note that READ_ONCE() is not optional! Without the
-READ_ONCE(), the compiler might combine the load from 'a' with other
-loads from 'a', and the store to 'b' with other stores to 'b', with
-possible highly counterintuitive effects on ordering.
+Control dependencies pair normally with other types of barriers.
+That said, please note that neither READ_ONCE() nor WRITE_ONCE()
+are optional! Without the READ_ONCE(), the compiler might combine the
+load from 'a' with other loads from 'a'.  Without the WRITE_ONCE(),
+the compiler might combine the store to 'b' with other stores to 'b'.
+Either can result in highly counterintuitive effects on ordering.
 
 Worse yet, if the compiler is able to prove (say) that the value of
 variable 'a' is always non-zero, it would be well within its rights
@@ -682,7 +687,7 @@ to optimize the original example by eliminating the "if" statement
 as follows:
 
 	q = a;
-	b = p;  /* BUG: Compiler and CPU can both reorder!!! */
+	b = 1;  /* BUG: Compiler and CPU can both reorder!!! */
 
 So don't leave out the READ_ONCE().
 
@@ -692,11 +697,11 @@ branches of the "if" statement as follows:
 	q = READ_ONCE(a);
 	if (q) {
 		barrier();
-		WRITE_ONCE(b, p);
+		WRITE_ONCE(b, 1);
 		do_something();
 	} else {
 		barrier();
-		WRITE_ONCE(b, p);
+		WRITE_ONCE(b, 1);
 		do_something_else();
 	}
 
@@ -705,12 +710,12 @@ optimization levels:
 
 	q = READ_ONCE(a);
 	barrier();
-	WRITE_ONCE(b, p);  /* BUG: No ordering vs. load from a!!! */
+	WRITE_ONCE(b, 1);  /* BUG: No ordering vs. load from a!!! */
 	if (q) {
-		/* WRITE_ONCE(b, p); -- moved up, BUG!!! */
+		/* WRITE_ONCE(b, 1); -- moved up, BUG!!! */
 		do_something();
 	} else {
-		/* WRITE_ONCE(b, p); -- moved up, BUG!!! */
+		/* WRITE_ONCE(b, 1); -- moved up, BUG!!! */
 		do_something_else();
 	}
 
@@ -723,10 +728,10 @@ memory barriers, for example, smp_store_release():
 
 	q = READ_ONCE(a);
 	if (q) {
-		smp_store_release(&b, p);
+		smp_store_release(&b, 1);
 		do_something();
 	} else {
-		smp_store_release(&b, p);
+		smp_store_release(&b, 1);
 		do_something_else();
 	}
 
@@ -735,10 +740,10 @@ ordering is guaranteed only when the stores differ, for example:
 
 	q = READ_ONCE(a);
 	if (q) {
-		WRITE_ONCE(b, p);
+		WRITE_ONCE(b, 1);
 		do_something();
 	} else {
-		WRITE_ONCE(b, r);
+		WRITE_ONCE(b, 2);
 		do_something_else();
 	}
 
@@ -751,10 +756,10 @@ the needed conditional.  For example:
 
 	q = READ_ONCE(a);
 	if (q % MAX) {
-		WRITE_ONCE(b, p);
+		WRITE_ONCE(b, 1);
 		do_something();
 	} else {
-		WRITE_ONCE(b, r);
+		WRITE_ONCE(b, 2);
 		do_something_else();
 	}
 
@@ -763,7 +768,7 @@ equal to zero, in which case the compiler is within its rights to
 transform the above code into the following:
 
 	q = READ_ONCE(a);
-	WRITE_ONCE(b, p);
+	WRITE_ONCE(b, 1);
 	do_something_else();
 
 Given this transformation, the CPU is not required to respect the ordering
@@ -776,10 +781,10 @@ one, perhaps as follows:
 	q = READ_ONCE(a);
 	BUILD_BUG_ON(MAX <= 1); /* Order load from a with store to b. */
 	if (q % MAX) {
-		WRITE_ONCE(b, p);
+		WRITE_ONCE(b, 1);
 		do_something();
 	} else {
-		WRITE_ONCE(b, r);
+		WRITE_ONCE(b, 2);
 		do_something_else();
 	}
 
@@ -812,30 +817,28 @@ not necessarily apply to code following the if-statement:
 
 	q = READ_ONCE(a);
 	if (q) {
-		WRITE_ONCE(b, p);
+		WRITE_ONCE(b, 1);
 	} else {
-		WRITE_ONCE(b, r);
+		WRITE_ONCE(b, 2);
 	}
-	WRITE_ONCE(c, 1);  /* BUG: No ordering against the read from "a". */
+	WRITE_ONCE(c, 1);  /* BUG: No ordering against the read from 'a'. */
 
 It is tempting to argue that there in fact is ordering because the
 compiler cannot reorder volatile accesses and also cannot reorder
-the writes to "b" with the condition.  Unfortunately for this line
-of reasoning, the compiler might compile the two writes to "b" as
+the writes to 'b' with the condition.  Unfortunately for this line
+of reasoning, the compiler might compile the two writes to 'b' as
 conditional-move instructions, as in this fanciful pseudo-assembly
 language:
 
 	ld r1,a
-	ld r2,p
-	ld r3,r
 	cmp r1,$0
-	cmov,ne r4,r2
-	cmov,eq r4,r3
+	cmov,ne r4,$1
+	cmov,eq r4,$2
 	st r4,b
 	st $1,c
 
 A weakly ordered CPU would have no dependency of any sort between the load
-from "a" and the store to "c".  The control dependencies would extend
+from 'a' and the store to 'c'.  The control dependencies would extend
 only to the pair of cmov instructions and the store depending on them.
 In short, control dependencies apply only to the stores in the then-clause
 and else-clause of the if-statement in question (including functions
@@ -843,7 +846,7 @@ invoked by those two clauses), not to code following that if-statement.
 
 Finally, control dependencies do -not- provide transitivity.  This is
 demonstrated by two related examples, with the initial values of
-x and y both being zero:
+'x' and 'y' both being zero:
 
 	CPU 0                     CPU 1
 	=======================   =======================
@@ -915,6 +918,9 @@ In summary:
   (*) Control dependencies do -not- provide transitivity.  If you
       need transitivity, use smp_mb().
 
+  (*) Compilers do not understand control dependencies.  It is therefore
+      your job to ensure that they do not break your code.
+
 
 SMP BARRIER PAIRING
 -------------------
-- 
2.5.2

  parent reply	other threads:[~2017-01-14  8:55 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-14  8:50 [PATCH tip/core/rcu 0/5] Documentation updates for 4.11 Paul E. McKenney
2017-01-14  8:55 ` [PATCH tip/core/rcu 1/5] rcu: Design documentation for expedited grace periods Paul E. McKenney
2017-01-14  8:55 ` [PATCH tip/core/rcu 2/5] doc: Fix RCU requirements typos Paul E. McKenney
2017-01-14  8:55 ` Paul E. McKenney [this message]
2017-01-14  8:55 ` [PATCH tip/core/rcu 4/5] doc: Quick-Quiz answers are now inline Paul E. McKenney
2017-01-14  8:55 ` [PATCH tip/core/rcu 5/5] doc: Add rcutree.rcu_kick_kthreads to kernel-parameters.txt Paul E. McKenney
2017-01-14 20:45 ` [PATCH tip/core/rcu 0/5] Documentation updates for 4.11 Josh Triplett
2017-01-15  5:31   ` Paul E. McKenney

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1484384139-19622-3-git-send-email-paulmck@linux.vnet.ibm.com \
    --to=paulmck@linux.vnet.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=bobby.prani@gmail.com \
    --cc=dhowells@redhat.com \
    --cc=dipankar@in.ibm.com \
    --cc=dvhart@linux.intel.com \
    --cc=edumazet@google.com \
    --cc=fweisbec@gmail.com \
    --cc=jiangshanlai@gmail.com \
    --cc=josh@joshtriplett.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mingo@kernel.org \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.