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@linux-foundation.org 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....