Messages in this thread | | | From | Dmitry Vyukov <> | Date | Tue, 22 Sep 2015 10:38:57 +0200 | Subject | Re: [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.
| |