Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752880AbdGXGeS (ORCPT ); Mon, 24 Jul 2017 02:34:18 -0400 Received: from mail-pf0-f195.google.com ([209.85.192.195]:35364 "EHLO mail-pf0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751254AbdGXGeL (ORCPT ); Mon, 24 Jul 2017 02:34:11 -0400 Date: Mon, 24 Jul 2017 14:34:07 +0800 From: Boqun Feng To: Akira Yokosawa Cc: "Paul E. McKenney" , linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] documentation: Fix two-CPU control-dependency example Message-ID: <20170724063407.74eep6pwisfjipok@tardis> References: <20170720161152.GQ3730@linux.vnet.ibm.com> <20170720214234.GY3730@linux.vnet.ibm.com> <55457ca1-a8db-213c-3b9c-ead441f97200@gmail.com> <20170720230714.GA3730@linux.vnet.ibm.com> <6ce659de-6c92-dbd8-e1dd-90f3e85521c0@gmail.com> <20170723044300.GI3730@linux.vnet.ibm.com> <20170723153936.t3fok53eoicculbc@tardis> <5e0b20c1-d0d8-9553-3f72-0217497f6852@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="tbd2acrqoa4ivhrg" Content-Disposition: inline In-Reply-To: <5e0b20c1-d0d8-9553-3f72-0217497f6852@gmail.com> User-Agent: NeoMutt/20170609 (1.8.3) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4385 Lines: 132 --tbd2acrqoa4ivhrg Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Jul 24, 2017 at 09:04:57AM +0900, Akira Yokosawa wrote: [...] > >=20 > > ----------------->8 > > Subject: [PATCH] kernel: Emphasize the return value of READ_ONCE() is h= onored > >=20 > > 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. > >=20 > > Whereas, Akira Yokosawa recently reported a problem that would be > > triggered if 2) is not achieved.=20 >=20 > 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 (">" -> ">=3D"). >=20 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. > >=20 > > 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. > >=20 > > Cc: Akira Yokosawa > > Cc: Paul E. McKenney > > Signed-off-by: Boqun Feng > > --- > > include/linux/compiler.h | 25 +++++++++++++++++++++++++ > > 1 file changed, 25 insertions(+) > >=20 > > 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(vola= tile void *p, void *res, int s > > * mutilate accesses that either do not require ordering or that inter= act > > * 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 valu= e of a > > + * READ_ONCE(). For example, the following code snippet: > > + * > > + * int a =3D 0; > > + * int x =3D 0; > > + * > > + * void some_func() { > > + * int t =3D 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? >=20 Make sense. Thanks! Regaords, Boqun > + * , should never be optimized as: > + * > + * void some_func() { > + * int t =3D READ_ONCE(a); > + * WRITE_ONCE(x, 1); > + * } >=20 > Thanks, Akira >=20 > > + * > > + * 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. > > */ > > =20 > > #define __READ_ONCE(x, check) \ > >=20 >=20 --tbd2acrqoa4ivhrg Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCAAdFiEEj5IosQTPz8XU1wRHSXnow7UH+rgFAll1lNoACgkQSXnow7UH +rh/tAf9FZJgjsaYJ8+zpdFfsM8+7KTejg4zj67ozrLARNEiJViZriWlt08lPsoy FmAA+DsMoKUqvI1GirTRJQWjDkRM2/2yw69wsnDNc4kuKDZAdmhHgP1ZXM9eeEFI viAhTx+vYMy8z1xKtwl0clWEZ3ISrXfz3+gKqXN2IYVBc9w82JBQye2NYm6YDA2C wi5v2ThXqO9cg9qeQx4Vy/akJXSuLM4mPqJzP/JvF5V5O/L1v9gvyrxigmJ+mdHn PXW5D7p266sBjelhvSvZsQnSnJJBTUez12bZyjMg3OVSA02wI/rGU7GhZhh//Mjk 2iFccK/axXyANPFWi6whdLDLWXZL+w== =5LXk -----END PGP SIGNATURE----- --tbd2acrqoa4ivhrg--