Re: [RFC PATCH v3] mm: use READ_ONCE in page_cpupid_xchg_last()

From: Rasmus Villemoes
Date: Wed Dec 07 2016 - 17:17:03 EST


On Wed, Dec 07 2016, Christian Borntraeger <borntraeger@xxxxxxxxxx> wrote:

> On 12/07/2016 10:59 AM, Michal Hocko wrote:
>> On Wed 07-12-16 10:40:47, Christian Borntraeger wrote:
>>> On 12/07/2016 10:29 AM, Vlastimil Babka wrote:
>>>> On 12/07/2016 09:58 AM, Michal Hocko wrote:
>>>>> On Wed 07-12-16 09:48:52, Vlastimil Babka wrote:
>>>>> Anyway, this could be addressed easily by
>>>>
>>>> Yes, that way there should be no doubt.
>>>
>>> That change would make it clearer, but the code is correct anyway,
>>> as assignments in C are done from right to left, so
>>> old_flags = flags = READ_ONCE(page->flags);
>>>
>>> is equivalent to
>>>
>>> flags = READ_ONCE(page->flags);
>>> old_flags = flags;
>>
>> OK, I guess you are right. For some reason I thought that the compiler
>> is free to bypass flags and split an assignment
>> a = b = c; into b = c; a = c
>> which would still follow from right to left rule. I guess I am over
>> speculating here though, so sorry for the noise.
>
> Hmmm, just rereading C, I am no longer sure...
> I cannot find anything right now, that adds a sequence point in here.
> Still looking...

C99 6.5.16.3: ... An assignment expression has the value of the left
operand after the assignment, ....

So if the expression c can have side effects or is for any reason
(e.g. volatile) not guaranteed to produce the same value if it's
evaluated again, there's no way the compiler would be allowed to change
a=b=c; into b=c; a=c;. (Also, this means that in "int a, c = 256;
char b; a=b=c;", a ends up with the value 0.)

Somewhat related: https://lwn.net/Articles/233902/