On Sat, Jul 21, 2018 at 10:49:57PM +0000, Linus Torvalds wrote: > On Sat, Jul 21, 2018 at 3:34 PM Linus Torvalds > <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > > > > But I basically think that with those patches in place, we can: > > > > - make vm_area_alloc() just default vm_ops to &dummy_vm_ops > > > > - just take the part of Kirill's patch that does > > > > vma->vm_ops = &anon_vm_ops; > > > > and instead of '&anon_vm_ops', set it to NULL. > > IOW, Kirill's patch now just boils down to the attached trial patch. > > I will *not* be committing this last patch, I think it needs more > testing and ack's. And honestly, authorship should go to Kirill if > he's ok with it, because all the basic "these are the anonymous vma's" > places came from Kirill's patch and work. > > But it works for me, and I did commit and push out the parts that were > just trivial cleanups with no actual semantic changes. > > Comments? Allocation helpers are great idea. But they don't cover several corner cases: allocation of VMA on stack or in data section. For instance, with proposed approach gate VMA is anon on ARM, but non-anon on x86. I don't think it causes problems, but it's inconsistent and may become a problem in the future. I think it worth manually initialize such VMAs with dummy_vm_ops. I also would rather keep ->vm_ops non-NULL for anonymous VMAs. This way it's easier to catch broken drivers: place a WARN() in __vma_link_rb(). With non-NULL vm_ops we can drop all vma->vm_ops checks. There's patch in -mm tree doing this[1]. [1] https://ozlabs.org/~akpm/mmots/broken-out/mm-drop-unneeded-vm_ops-checks-v2.patch -- Kirill A. Shutemov
