Re: [PATCH 0/2] RFC: READ/WRITE_ONCE vma/mm cleanups

[Vlastimil Babka <vbabka@suse.cz>]

On 3/1/19 10:37 AM, Kirill A. Shutemov wrote:
> On Thu, Feb 28, 2019 at 10:55:48PM -0500, Andrea Arcangeli wrote:
>> Hello,
>>
>> This was a well known issue for more than a decade, but until a few
>> months ago we relied on the compiler to stick to atomic accesses and
>> updates while walking and updating pagetables.
>>
>> However now the 64bit native_set_pte finally uses WRITE_ONCE and
>> gup_pmd_range uses READ_ONCE as well.
>>
>> This convert more racy VM places to avoid depending on the expected
>> compiler behavior to achieve kernel runtime correctness.
>>
>> It mostly guarantees gcc to do atomic updates at 64bit granularity
>> (practically not needed) and it also prevents gcc to emit code that
>> risks getting confused if the memory unexpectedly changes under it
>> (unlikely to ever be needed).
>>
>> The list of vm_start/end/pgoff to update isn't complete, I covered the
>> most obvious places, but before wasting too much time at doing a full
>> audit I thought it was safer to post it and get some comment. More
>> updates can be posted incrementally anyway.
> 
> The intention is described well to my eyes.
> 
> Do I understand correctly, that it's attempt to get away with modifying
> vma's fields under down_read(mmap_sem)?

If that's the intention, then IMHO it's not that well described. It
talks about "racy VM places" but e.g. the __mm_populate() changes are
for code protected by down_read(). So what's going on here?

> I'm not fan of this.
> 
> It can help with producing stable value for the one field, but it doesn't
> help if more than one thing changed under you. Like if both vm_start and
> vm_end modifed under you, it can lead to inconsistency. Like vm_end <
> vm_start.
>