linux-kernel - Re: [PATCH] documentation: Fix two-CPU control-dependency example

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 24 Jul 2017 14:34:07 +0800
From:   Boqun Feng <boqun.feng@...il.com>
To:     Akira Yokosawa <akiyks@...il.com>
Cc:     "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] documentation: Fix two-CPU control-dependency example

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@...il.com>
> > Cc: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>
> > Signed-off-by: Boqun Feng <boqun.feng@...il.com>
> > ---
> >  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)						\
> > 
> 

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)