lkml.org 
[lkml]   [2015]   [Sep]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH] kernel: fix data race in put_pid
    Should I wait until atomic_read_ctrl patch is landed and update my patch?


    On Fri, Sep 18, 2015 at 1:56 PM, Peter Zijlstra <peterz@infradead.org> wrote:
    > On Fri, Sep 18, 2015 at 01:50:01PM +0200, Dmitry Vyukov wrote:
    >> > +#ifndef atomic_read_ctrl
    >> > +static inline int atomic_read_ctrl(atomic_t *v)
    >> > +{
    >> > + int val = atomic_read(v);
    >> > + smp_read_barrier_depends(); /* Enforce control dependency. */
    >> > + return val;
    >> > +}
    >> > +#endif
    >> > +
    >> > /*
    >> > * Relaxed variants of xchg, cmpxchg and some atomic operations.
    >> > *
    >>
    >> Looks good to me.
    >> Should we add atomic64_read_ctrl for completeness? I have not seen
    >> cases where it was needed, though.
    >
    > Sure, and while doing another spin, let me go update the documentation
    > too.
    >
    > ---
    > Subject: atomic: Implement atomic_read_ctrl()
    > From: Peter Zijlstra <peterz@infradead.org>
    > Date: Fri, 18 Sep 2015 13:22:52 +0200
    >
    > Provide atomic_read_ctrl() to mirror READ_ONCE_CTRL(), such that we can
    > more conveniently use atomics in control dependencies.
    >
    > Since we can assume atomic_read() implies a READ_ONCE(), we must only
    > emit an extra smp_read_barrier_depends() in order to upgrade to
    > READ_ONCE_CTRL() semantics.
    >
    > Cc: oleg@redhat.com
    > Cc: torvalds@linux-foundation.org
    > Cc: will.deacon@arm.com
    > Cc: paulmck@linux.vnet.ibm.com
    > Requested-by: Dmitry Vyukov <dvyukov@google.com>
    > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
    > ---
    > Documentation/memory-barriers.txt | 17 +++++++++--------
    > include/linux/atomic.h | 18 ++++++++++++++++++
    > 2 files changed, 27 insertions(+), 8 deletions(-)
    >
    > --- a/Documentation/memory-barriers.txt
    > +++ b/Documentation/memory-barriers.txt
    > @@ -637,7 +637,8 @@ to optimize the original example by elim
    > b = p; /* BUG: Compiler and CPU can both reorder!!! */
    >
    > Finally, the READ_ONCE_CTRL() includes an smp_read_barrier_depends()
    > -that DEC Alpha needs in order to respect control depedencies.
    > +that DEC Alpha needs in order to respect control depedencies. Alternatively
    > +use one of atomic{,64}_read_ctrl().
    >
    > So don't leave out the READ_ONCE_CTRL().
    >
    > @@ -796,9 +797,9 @@ site: https://www.cl.cam.ac.uk/~pes20/pp
    >
    > In summary:
    >
    > - (*) Control dependencies must be headed by READ_ONCE_CTRL().
    > - Or, as a much less preferable alternative, interpose
    > - smp_read_barrier_depends() between a READ_ONCE() and the
    > + (*) Control dependencies must be headed by READ_ONCE_CTRL(),
    > + atomic{,64}_read_ctrl(). Or, as a much less preferable alternative,
    > + interpose smp_read_barrier_depends() between a READ_ONCE() and the
    > control-dependent write.
    >
    > (*) Control dependencies can order prior loads against later stores.
    > @@ -820,10 +821,10 @@ site: https://www.cl.cam.ac.uk/~pes20/pp
    > and WRITE_ONCE() can help to preserve the needed conditional.
    >
    > (*) Control dependencies require that the compiler avoid reordering the
    > - dependency into nonexistence. Careful use of READ_ONCE_CTRL()
    > - or smp_read_barrier_depends() can help to preserve your control
    > - dependency. Please see the Compiler Barrier section for more
    > - information.
    > + dependency into nonexistence. Careful use of READ_ONCE_CTRL(),
    > + atomic{,64}_read_ctrl() or smp_read_barrier_depends() 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.
    >
    > --- a/include/linux/atomic.h
    > +++ b/include/linux/atomic.h
    > @@ -4,6 +4,24 @@
    > #include <asm/atomic.h>
    > #include <asm/barrier.h>
    >
    > +#ifndef atomic_read_ctrl
    > +static inline int atomic_read_ctrl(atomic_t *v)
    > +{
    > + int val = atomic_read(v);
    > + smp_read_barrier_depends(); /* Enforce control dependency. */
    > + return val;
    > +}
    > +#endif
    > +
    > +#ifndef atomic64_read_ctrl
    > +static inline int atomic64_read_ctrl(atomic64_t *v)
    > +{
    > + int val = atomic64_read(v);
    > + smp_read_barrier_depends(); /* Enforce control dependency. */
    > + return val;
    > +}
    > +#endif
    > +
    > /*
    > * Relaxed variants of xchg, cmpxchg and some atomic operations.
    > *



    --
    Dmitry Vyukov, Software Engineer, dvyukov@google.com
    Google Germany GmbH, Dienerstraße 12, 80331, München
    Geschäftsführer: Graham Law, Christine Elizabeth Flores
    Registergericht und -nummer: Hamburg, HRB 86891
    Sitz der Gesellschaft: Hamburg
    Diese E-Mail ist vertraulich. Wenn Sie nicht der richtige Adressat
    sind, leiten Sie diese bitte nicht weiter, informieren Sie den
    Absender und löschen Sie die E-Mail und alle Anhänge. Vielen Dank.
    This e-mail is confidential. If you are not the right addressee please
    do not forward it, please inform the sender, and please erase this
    e-mail including any attachments. Thanks.


    \
     
     \ /
      Last update: 2015-09-22 11:01    [W:3.119 / U:0.028 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site