Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755351AbdGWPjm (ORCPT ); Sun, 23 Jul 2017 11:39:42 -0400 Received: from mail-pf0-f195.google.com ([209.85.192.195]:38549 "EHLO mail-pf0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751395AbdGWPjk (ORCPT ); Sun, 23 Jul 2017 11:39:40 -0400 Date: Sun, 23 Jul 2017 23:39:36 +0800 From: Boqun Feng To: "Paul E. McKenney" Cc: Akira Yokosawa , linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] documentation: Fix two-CPU control-dependency example Message-ID: <20170723153936.t3fok53eoicculbc@tardis> References: <20170720013112.fmrml6abdhi2nqdt@tardis> <20170720054704.GM3730@linux.vnet.ibm.com> <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> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170723044300.GI3730@linux.vnet.ibm.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: 4967 Lines: 133 On Sat, Jul 22, 2017 at 09:43:00PM -0700, Paul E. McKenney wrote: [...] > > Your priority seemed to be in reducing the chance of the "if" statement > > to be optimized away. So I suggested to use "extern" as a compromise. > Hi Akira, The problem is that, such a compromise doesn't help *developers* write good concurrent code. The document should serve as a reference book for the developers, and with the compromise you suggest, the developers will possibly add "extern" to their shared variables. This is not only unrealistic but also wrong, because "extern" means external for translation units(compiling units), not external for execution units(CPUs). And as I said, the proper semantics of READ_ONCE() should work well without using "extern", if we find a 'volatile' load doesn't work, we can find another way (writing in asm or use asm volatile("" : "+m"(var)); to indicate @var changed). And the compromise just changes the semantics... To me, it's not worth changing the semantics because the implementation might be broken in the feature ;-) > If the various tools accept the "extern", this might not be a bad thing > to do. > > But what this really means is that I need to take another tilt at > the "volatile" windmill in the committee. > > > Another way would be to express the ">=" version in a pseudo-asm form. > > > > CPU 0 CPU 1 > > ======================= ======================= > > r1 = LOAD x r2 = LOAD y > > if (r1 >= 0) if (r2 >= 0) > > STORE y = 1 STORE x = 1 > > > > assert(!(r1 == 1 && r2 == 1)); > > > > This should eliminate any concern of compiler optimization. > > In this final part of CONTROL DEPENDENCIES section, separating the > > problem of optimization and transitivity would clarify the point > > (at least for me). > > The problem is that people really do use C-language control dependencies > in the Linux kernel, so we need to describe them. Maybe someday it > will be necessary to convert them to asm, but I am hoping that we can > avoid that. > > > Thoughts? > > My hope is that the memory model can help here, but that will in any > case take time. Hi Paul, I add some comments for READ_ONCE() to emphasize compilers should honor the return value, in the future, we may need a separate document for the use/definition of volatile in kernel, but I think the comment of READ_ONCE() is good enough now? Regards, Boqun ----------------->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. 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 Cc: Paul E. McKenney Signed-off-by: Boqun Feng --- 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); + * } + * + * 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) \ -- 2.13.3