Messages in this thread Patch in this message |  | | Date | Tue, 17 Feb 2015 13:45:22 -0800 | From | "Paul E. McKenney" <> | Subject | Re: [PATCH 2/2] [PATCH] sched: Add smp_rmb() in task rq locking cycles |
| |
On Tue, Feb 17, 2015 at 07:23:35PM +0100, Peter Zijlstra wrote: > On Tue, Feb 17, 2015 at 10:01:54AM -0800, Paul E. McKenney wrote: > > > > The blob under SMP BARRIER PAIRING does not mention pairing with control > > > > dependencies; and I'm rather sure I've done so. > > > > And here is a patch for the control-dependency pairing. Thoughts? > > The proposed patch does not change the blub under SMP BARRIER PAIRING, > which would be the first place I would look for this.
OK, updated below.
> > @@ -850,6 +853,19 @@ Or: > > <data dependency barrier> > > y = *x; > > > > +Or even: > > + > > + CPU 1 CPU 2 > > + =============== =============================== > > + r1 = ACCESS_ONCE(y); > > + <write barrier> > > + ACCESS_ONCE(y) = 1; if (r2 = ACCESS_ONCE(x)) { > > + <implicit control dependency> > > + ACCESS_ONCE(y) = 1; > > + } > > + > > + assert(r1 == 0 || r2 == 0); > > + > > Basically, the read barrier always has to be there, even though it can be of > > the "weaker" type. > > Does that want to be a <general barrier>; CPU1 looks to do a LOAD > followed by a STORE, separated by a WMB, which is of course odd. > > To me the pairing with a general barrier is also the most intuitive, > since the control dependency is a LOAD -> STORE order. > > Then again, I'm rather tired and maybe I'm missing the obvious :-)
Nope, you are correct. There either must be a general barrier or CPU 1's write must be an smp_store_release(). I went with the general barrier. Please see below for an update.
Thanx, Paul
------------------------------------------------------------------------
documentation: Clarify control-dependency pairing
This commit explicitly states that control dependencies pair normally with other barriers, and gives an example of such pairing.
Reported-by: Peter Zijlstra <peterz@infradead.org> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt index ca2387ef27ab..6974f1c2b4e1 100644 --- a/Documentation/memory-barriers.txt +++ b/Documentation/memory-barriers.txt @@ -592,9 +592,9 @@ See also the subsection on "Cache Coherency" for a more thorough example. CONTROL DEPENDENCIES -------------------- -A 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: +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: q = ACCESS_ONCE(a); if (q) { @@ -615,14 +615,15 @@ case what's actually required is: } However, stores are not speculated. This means that ordering -is- provided -in the following example: +for load-store control dependencies, as in the following example: q = ACCESS_ONCE(a); if (q) { ACCESS_ONCE(b) = p; } -Please note that ACCESS_ONCE() is not optional! Without the +Control dependencies pair normally with other types of barriers. +That said, please note that ACCESS_ONCE() is not optional! Without the ACCESS_ONCE(), 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. @@ -813,6 +814,8 @@ In summary: barrier() can help to preserve your control dependency. Please see the Compiler Barrier section for more information. + (*) Control dependencies pair normally with other types of barriers. + (*) Control dependencies do -not- provide transitivity. If you need transitivity, use smp_mb(). @@ -823,14 +826,14 @@ SMP BARRIER PAIRING When dealing with CPU-CPU interactions, certain types of memory barrier should always be paired. A lack of appropriate pairing is almost certainly an error. -General barriers pair with each other, though they also pair with -most other types of barriers, albeit without transitivity. An acquire -barrier pairs with a release barrier, but both may also pair with other -barriers, including of course general barriers. A write barrier pairs -with a data dependency barrier, an acquire barrier, a release barrier, -a read barrier, or a general barrier. Similarly a read barrier or a -data dependency barrier pairs with a write barrier, an acquire barrier, -a release barrier, or a general barrier: +General barriers pair with each other, though they also pair with most +other types of barriers, albeit without transitivity. An acquire barrier +pairs with a release barrier, but both may also pair with other barriers, +including of course general barriers. A write barrier pairs with a data +dependency barrier, a control dependency, an acquire barrier, a release +barrier, a read barrier, or a general barrier. Similarly a read barrier, +control dependency, or a data dependency barrier pairs with a write +barrier, an acquire barrier, a release barrier, or a general barrier: CPU 1 CPU 2 =============== =============== @@ -850,6 +853,19 @@ Or: <data dependency barrier> y = *x; +Or even: + + CPU 1 CPU 2 + =============== =============================== + r1 = ACCESS_ONCE(y); + <general barrier> + ACCESS_ONCE(y) = 1; if (r2 = ACCESS_ONCE(x)) { + <implicit control dependency> + ACCESS_ONCE(y) = 1; + } + + assert(r1 == 0 || r2 == 0); + Basically, the read barrier always has to be there, even though it can be of the "weaker" type.
|  |