On Mon, Jul 24, 2017 at 09:04:57AM +0900, Akira Yokosawa wrote:
[...]
> >
> > ----------------->8
> > Subject: [PATCH] kernel: Emphasize the return value of READ_ONCE() is honored
> >
> > READ_ONCE() is used around in kernel to provide a control dependency,
> > and to make the control dependency valid, we must 1) make the load of
> > READ_ONCE() actually happen and 2) make sure compilers take the return
> > value of READ_ONCE() serious. 1) is already done and commented,
> > and in current implementation, 2) is also considered done in the
> > same way as 1): a 'volatile' load.
> >
> > Whereas, Akira Yokosawa recently reported a problem that would be
> > triggered if 2) is not achieved.
>
> To clarity the timeline, it was Paul who pointed out it would become
> easier for compilers to optimize away the "if" statements in response
> to my suggestion of partial revert (">" -> ">=").
>
Ah.. right, I missed that part. I will use proper sentences here like:
During a recent discussion brought up by Akira Yokosawa on
memory-barriers.txt, a problem is discovered, which would be
triggered if 2) is not achieved.
Works with you?
> > Moreover, according to Paul Mckenney,
> > using volatile might not actually give us what we want for 2) depending
> > on compiler writers' definition of 'volatile'. Therefore it's necessary
> > to emphasize 2) as a part of the semantics of READ_ONCE(), this not only
> > fits the conceptual semantics we have been using, but also makes the
> > implementation requirement more accurate.
> >
> > In the future, we can either make compiler writers accept our use of
> > 'volatile', or(if that fails) find another way to provide this
> > guarantee.
> >
> > Cc: Akira Yokosawa <akiyks@xxxxxxxxx>
> > Cc: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Boqun Feng <boqun.feng@xxxxxxxxx>
> > ---
> > include/linux/compiler.h | 25 +++++++++++++++++++++++++
> > 1 file changed, 25 insertions(+)
> >
> > diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> > index 219f82f3ec1a..8094f594427c 100644
> > --- a/include/linux/compiler.h
> > +++ b/include/linux/compiler.h
> > @@ -305,6 +305,31 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
> > * mutilate accesses that either do not require ordering or that interact
> > * with an explicit memory barrier or atomic instruction that provides the
> > * required ordering.
> > + *
> > + * The return value of READ_ONCE() should be honored by compilers, IOW,
> > + * compilers must treat the return value of READ_ONCE() as an unknown value at
> > + * compile time, i.e. no optimization should be done based on the value of a
> > + * READ_ONCE(). For example, the following code snippet:
> > + *
> > + * int a = 0;
> > + * int x = 0;
> > + *
> > + * void some_func() {
> > + * int t = READ_ONCE(a);
> > + * if (!t)
> > + * WRITE_ONCE(x, 1);
> > + * }
> > + *
> > + * , should never be optimized as:
> > + *
> > + * void some_func() {
> > + * WRITE_ONCE(x, 1);
> > + * }
> READ_ONCE() should still be honored. so maybe the following?
>
Make sense. Thanks!
Regaords,
Boqun
> + * , should never be optimized as:
> + *
> + * void some_func() {
> + * int t = READ_ONCE(a);
> + * WRITE_ONCE(x, 1);
> + * }
>
> Thanks, Akira
>
> > + *
> > + * because the compiler is 'smart' enough to think the value of 'a' is never
> > + * changed.
> > + *
> > + * We provide this guarantee by making READ_ONCE() a *volatile* load.
> > */
> >
> > #define __READ_ONCE(x, check) \
> >
>
Attachment:
signature.asc
Description: PGP signature
