Date:	Mon, 6 Jun 2016 10:28:24 -0700
From:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>
To:	Will Deacon <will.deacon@....com>
Cc:	Peter Zijlstra <peterz@...radead.org>,
	Vineet Gupta <Vineet.Gupta1@...opsys.com>,
	Waiman Long <waiman.long@....com>,
	linux-kernel@...r.kernel.org, torvalds@...ux-foundation.org,
	manfred@...orfullife.com, dave@...olabs.net, boqun.feng@...il.com,
	tj@...nel.org, pablo@...filter.org, kaber@...sh.net,
	davem@...emloft.net, oleg@...hat.com,
	netfilter-devel@...r.kernel.org, sasha.levin@...cle.com,
	hofrat@...dl.org
Subject: Re: [RFC][PATCH 1/3] locking: Introduce smp_acquire__after_ctrl_dep

On Sat, Jun 04, 2016 at 08:29:29AM -0700, Paul E. McKenney wrote:
> On Fri, Jun 03, 2016 at 02:45:53PM +0100, Will Deacon wrote:
> > On Fri, Jun 03, 2016 at 06:32:38AM -0700, Paul E. McKenney wrote:
> > > On Fri, Jun 03, 2016 at 02:23:10PM +0200, Peter Zijlstra wrote:
> > > > On Fri, Jun 03, 2016 at 05:08:27AM -0700, Paul E. McKenney wrote:
> > > > > On Fri, Jun 03, 2016 at 11:38:34AM +0200, Peter Zijlstra wrote:
> > > > > > On Fri, Jun 03, 2016 at 02:48:38PM +0530, Vineet Gupta wrote:
> > > > > > > On Wednesday 25 May 2016 09:27 PM, Paul E. McKenney wrote:
> > > > > > > > For your example, but keeping the compiler in check:
> > > > > > > > 
> > > > > > > > 	if (READ_ONCE(a))
> > > > > > > > 		WRITE_ONCE(b, 1);
> > > > > > > > 	smp_rmb();
> > > > > > > > 	WRITE_ONCE(c, 2);
> > > > > > 
> > > > > > So I think it example is broken. The store to @c is not in fact
> > > > > > dependent on the condition of @a.
> > > > > 
> > > > > At first glance, the compiler could pull the write to "c" above the
> > > > > conditional, but the "memory" constraint in smp_rmb() prevents this.
> > > > > From a hardware viewpoint, the write to "c" does depend on the "if",
> > > > > as the conditional branch does precede that write in execution order.
> > > > > 
> > > > > But yes, this is using smp_rmb() in a very strange way, if that is
> > > > > what you are getting at.
> > > > 
> > > > Well, the CPU could decide that the store to C happens either way around
> > > > the branch. I'm not sure I'd rely on CPUs not being _that_ clever.
> > > 
> > > If I remember correctly, both Power and ARM guarantee that the CPU won't
> > > be that clever.  Not sure about Itanium.
> > 
> > I wouldn't be so sure about ARM. On 32-bit, at least, we have conditional
> > store instructions so if the compiler could somehow use one of those for
> > the first WRITE_ONCE then there's very obviously no control dependency
> > on the second WRITE_ONCE and they could be observed out of order.
> 
> OK, good to know...
> 
> > I note that smp_rmb() on ARM and arm64 actually orders against subsequent
> > (in program order) writes, so this is still pretty theoretical for us.
> 
> So the combined control-dependency/smp_rmb() still works, but I should
> re-examine the straight control dependency stuff.

And how about the patch below?

							Thanx, Paul

------------------------------------------------------------------------

commit 43672d15aeb69b1a196c06cbc071cbade8d247fd
Author: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>
Date:   Mon Jun 6 10:19:42 2016 -0700

    documentation: Clarify limited control-dependency scope
    
    Nothing in the control-dependencies section of memory-barriers.txt
    says that control dependencies don't extend beyond the end of the
    if-statement containing the control dependency.  Worse yet, in many
    situations, they do extend beyond that if-statement.  In particular,
    the compiler cannot destroy the control dependency given proper use of
    READ_ONCE() and WRITE_ONCE().  However, a weakly ordered system having
    a conditional-move instruction provides the control-dependency guarantee
    only to code within the scope of the if-statement itself.
    
    This commit therefore adds words and an example demonstrating this
    limitation of control dependencies.
    
    Reported-by: Will Deacon <will.deacon@....com>
    Signed-off-by: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>

diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt
index 147ae8ec836f..a4d0a99de04d 100644
--- a/Documentation/memory-barriers.txt
+++ b/Documentation/memory-barriers.txt
@@ -806,6 +806,41 @@ out-guess your code.  More generally, although READ_ONCE() does force
 the compiler to actually emit code for a given load, it does not force
 the compiler to use the results.
 
+In addition, control dependencies apply only to the then-clause and
+else-clause of the if-statement in question.  In particular, it does
+not necessarily apply to code following the if-statement:
+
+	q = READ_ONCE(a);
+	if (q) {
+		WRITE_ONCE(b, p);
+	} else {
+		WRITE_ONCE(b, r);
+	}
+	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
+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
+	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
+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
+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:
@@ -869,6 +904,12 @@ In summary:
       atomic{,64}_read() can help to preserve your control dependency.
       Please see the COMPILER BARRIER section for more information.
 
+  (*) Control dependencies apply only to the then-clause and else-clause
+      of the if-statement containing the control dependency, including
+      any functions that these two clauses call.  Control dependencies
+      do -not- apply to code following the if-statement containing the
+      control dependency.
+
   (*) Control dependencies pair normally with other types of barriers.
 
   (*) Control dependencies do -not- provide transitivity.  If you

Powered by blists - more mailing lists