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) \
>>>
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
