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

[Akira Yokosawa]

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@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)						\
>>>
>>