lkml.org 
[lkml]   [2016]   [Jan]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [v3,11/41] mips: reuse asm-generic/barrier.h
    On Tue, Jan 26, 2016 at 03:29:21PM -0800, Paul E. McKenney wrote:
    > On Tue, Jan 26, 2016 at 02:33:40PM -0800, Linus Torvalds wrote:
    > > On Tue, Jan 26, 2016 at 2:15 PM, Linus Torvalds
    > > <torvalds@linux-foundation.org> wrote:
    > > >
    > > > You might as well just write it as
    > > >
    > > > struct foo x = READ_ONCE(*ptr);
    > > > x->bar = 5;
    > > >
    > > > because that "smp_read_barrier_depends()" does NOTHING wrt the second write.
    > >
    > > Just to clarify: on alpha it adds a memory barrier, but that memory
    > > barrier is useless.
    >
    > No trailing data-dependent read, so agreed, no smp_read_barrier_depends()
    > needed. That said, I believe that we should encourage rcu_dereference*()
    > or lockless_dereference() instead of READ_ONCE() for documentation
    > reasons, though.
    >
    > > On non-alpha, it is a no-op, and obviously does nothing simply because
    > > it generates no code.
    > >
    > > So if anybody believes that the "smp_read_barrier_depends()" does
    > > something, they are *wrong*.
    >
    > The other problem with smp_read_barrier_depends() is that it is often
    > a pain figuring out which prior load it is supposed to apply to.
    > Hence my preference for rcu_dereference*() and lockless_dereference().
    >

    Because semantically speaking, rcu_derefence*() and
    lockless_dereference() are CONSUME(i.e. data/address dependent
    read->read and read->write pairs are ordered), whereas
    smp_read_barrier_depends() only guarantees read->read pairs with data
    dependency are ordered, right?

    If so, maybe we need to call it out in memory-barriers.txt, for example:

    diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt
    index 904ee42..6b262c2 100644
    --- a/Documentation/memory-barriers.txt
    +++ b/Documentation/memory-barriers.txt
    @@ -1703,8 +1703,8 @@ There are some more advanced barrier functions:


    (*) lockless_dereference();
    - This can be thought of as a pointer-fetch wrapper around the
    - smp_read_barrier_depends() data-dependency barrier.
    + This is a load, and any load or store that has a data dependency on the
    + value returned by this load won't be reordered before this load.

    This is also similar to rcu_dereference(), but in cases where
    object lifetime is handled by some mechanism other than RCU, for

    Regards,
    Boqun

    > > And if anybody sends out an email with that smp_read_barrier_depends()
    > > in an example, they are actively just confusing other people, which is
    > > even worse than just being wrong. Which is why I jumped in.
    > >
    > > So stop perpetuating the myth that smp_read_barrier_depends() does
    > > something here. It does not. It's a bug, and it has become this "mind
    > > virus" for some people that seem to believe that it does something.
    >
    > It looks like I should add words to memory-barriers.txt de-emphasizing
    > smp_read_barrier_depends(). I will take a look at that.
    >
    > > I had to remove this crap once from the kernel already, see commit
    > > 105ff3cbf225 ("atomic: remove all traces of READ_ONCE_CTRL() and
    > > atomic*_read_ctrl()").
    > >
    > > I don't want to ever see that broken construct again. And I want to
    > > make sure that everybody is educated about how broken it was. I'm
    > > extremely unhappy that it came up again.
    >
    > Well, if it makes you feel better, that was control dependencies and this
    > was data dependencies. So it was not -exactly- the same. ;-)
    >
    > (Sorry, couldn't resist...)
    >
    > > If it turns out that some architecture does actually need a barrier
    > > between a read and a dependent write, then that will mean that
    > >
    > > (a) we'll have to make up a _new_ barrier, because
    > > "smp_read_barrier_depends()" is not that barrier. We'll presumably
    > > then have to make that new barrier part of "rcu_derefence()" and
    > > friends.
    >
    > Agreed. We can worry about whether or not we replace the current
    > smp_read_barrier_depends() with that new barrier when and if such
    > hardware appears.
    >
    > > (b) we will have found an architecture with even worse memory
    > > ordering semantics than alpha, and we'll have to stop castigating
    > > alpha for being the worst memory ordering ever.
    >
    > ;-) ;-) ;-)
    >
    > > but I sincerely hope that we'll never find that kind of broken architecture.
    >
    > Apparently at least some hardware vendors are reading memory-barriers.txt,
    > so perhaps the odds of that kind of breakage have reduced.
    >
    > Thanx, Paul
    >
    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2016-01-27 03:21    [W:5.742 / U:0.752 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site