[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CACT4Y+YgypNXRpwO5OwvDrA9hycmFV64h5aPQ4wn66WqQC5BeA@mail.gmail.com>
Date: Fri, 18 Sep 2015 13:50:01 +0200
From: Dmitry Vyukov <dvyukov@...gle.com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: Will Deacon <will.deacon@....com>, Oleg Nesterov <oleg@...hat.com>,
"ebiederm@...ssion.com" <ebiederm@...ssion.com>,
Al Viro <viro@...iv.linux.org.uk>,
Andrew Morton <akpm@...ux-foundation.org>,
Ingo Molnar <mingo@...nel.org>,
Paul McKenney <paulmck@...ux.vnet.ibm.com>,
"mhocko@...e.cz" <mhocko@...e.cz>,
LKML <linux-kernel@...r.kernel.org>,
"ktsan@...glegroups.com" <ktsan@...glegroups.com>,
Kostya Serebryany <kcc@...gle.com>,
Andrey Konovalov <andreyknvl@...gle.com>,
Alexander Potapenko <glider@...gle.com>,
Hans Boehm <hboehm@...gle.com>
Subject: Re: [PATCH] kernel: fix data race in put_pid
On Fri, Sep 18, 2015 at 1:22 PM, Peter Zijlstra <peterz@...radead.org> wrote:
> On Fri, Sep 18, 2015 at 10:28:20AM +0100, Will Deacon wrote:
>> On Fri, Sep 18, 2015 at 10:06:46AM +0100, Dmitry Vyukov wrote:
>
>> > Can we have something along the lines of:
>> >
>> > #define atomic_read_ctrl(v) READ_ONCE_CTRL(&(v)->counter)
>>
>> Funnily enough, I had this exact same discussion off-list yesterday
>> afternoon, since I wrote some code relying on a ctrl dependency from
>> an atomic_read to an atomic_xchg_relaxed.
>
> Something a little like so should work fine I suppose.
>
> ---
> Subject: atomic: Implement atomic_read_ctrl()
>
> 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.
>
> Requested-by: Dmitry Vyukov <dvyukov@...gle.com>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> ---
> include/linux/atomic.h | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/include/linux/atomic.h b/include/linux/atomic.h
> index 00a5763e850e..c4072421ea7f 100644
> --- a/include/linux/atomic.h
> +++ b/include/linux/atomic.h
> @@ -4,6 +4,15 @@
> #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
> +
> /*
> * 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.
--
Dmitry Vyukov, Software Engineer, dvyukov@...gle.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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists