On Fri, Jul 20, 2018 at 5:53 PM <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> vma_is_anonymous() relies on ->vm_ops being NULL to detect anonymous VMA.
> This is unreliable as ->mmap may not set ->vm_ops.
I detest this patch.
It does:
+ if (!vma->vm_ops)
+ vma->vm_ops = &dummy_vm_ops;
which seems eminently reasonable.
Except it does it in the wrong place - it should just do it in the vma
allocator, and guarantee that every vma has that initial dummy vm ops
pointer.
Yeah, yeah "the vma allocator" doesn't really exist right now, because people do
vma = kmem_cache_zalloc(vm_area_cachep, GFP_KERNEL);
by hand, but that would be a nice independent cleanup. We could also
puit the required
INIT_LIST_HEAD(&vma->anon_vma_chain);
etc into the allocation function.
(There's a couple of cases that don't do the "zalloc" version because
they will memcpy the old contents, but if people care we could do a
"vma_dup()" that does that, and also does all the other required
cleanup.
Then the small handful of actual anonymous cases could just NULL it
out, and we wouldn't need to make "vma_is_anonymous()" more complex
and expensive.
Instead, this patch forces random drivers to set a vm_ops that those
drivers very much by definition do not care about.
So instead fo doing cleanup, it adds complexity.
So I'm not going to apply this. Instead, I'll do the "let's introduce
a vma_alloc()/vma_free()". Initially doing *only* the allocation, then
we can start moving things into it (the vm_ops initialization, the
INIT_LIST_HEAD etc).
Linus
