From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753426AbbIRMCD (ORCPT ); Fri, 18 Sep 2015 08:02:03 -0400 Received: from casper.infradead.org ([85.118.1.10]:42708 "EHLO casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751375AbbIRMCB (ORCPT ); Fri, 18 Sep 2015 08:02:01 -0400 Date: Fri, 18 Sep 2015 13:56:37 +0200 From: Peter Zijlstra To: Dmitry Vyukov Cc: Will Deacon , Oleg Nesterov , "ebiederm@xmission.com" , Al Viro , Andrew Morton , Ingo Molnar , Paul McKenney , "mhocko@suse.cz" , LKML , "ktsan@googlegroups.com" , Kostya Serebryany , Andrey Konovalov , Alexander Potapenko , Hans Boehm Subject: Re: [PATCH] kernel: fix data race in put_pid Message-ID: <20150918115637.GM3604@twins.programming.kicks-ass.net> References: <20150917160837.GA26050@redhat.com> <20150917174456.GA30178@redhat.com> <20150917180919.GA32116@redhat.com> <20150918085156.GS3816@twins.programming.kicks-ass.net> <20150918092820.GA27377@arm.com> <20150918112252.GT3816@twins.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2012-12-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 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 Signed-off-by: Peter Zijlstra (Intel) --- 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 #include +#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. *