Subject: Re: [PATCH] documentation: Fix two-CPU control-dependency example
To: Boqun Feng <boqun.feng@gmail.com>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
        linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
        Akira Yokosawa <akiyks@gmail.com>
References: <bb6050e9-91a8-f04c-809c-7a22aed232a8@gmail.com>
 <20170720161152.GQ3730@linux.vnet.ibm.com>
 <d2d26fb2-dd01-9532-0cfc-3c90bc024cb7@gmail.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>
 <20170724063407.74eep6pwisfjipok@tardis>
From: Akira Yokosawa <akiyks@gmail.com>
Message-ID: <431a2ea5-3467-6293-df00-066037328180@gmail.com>
Date: Mon, 24 Jul 2017 19:47:44 +0900
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101
 Thunderbird/52.2.1
MIME-Version: 1.0
In-Reply-To: <20170724063407.74eep6pwisfjipok@tardis>
Content-Type: text/plain; charset=windows-1252
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3717
Lines: 108

On 2017/07/24 14:34:07 +0800, Boqun Feng wrote:
> 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?

Looks fine. Thanks!

Akira

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