lkml.org 
[lkml]   [2015]   [Feb]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [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.



    \
     
     \ /
      Last update: 2015-02-17 23:21    [W:4.740 / U:0.044 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site