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. >
