Re: [PATCH] Fix OOPS in mmap_region() when merging adjacent VM_LOCKED file segments

Linus Torvalds
2009-01-30T21:38:51Z

On Fri, 30 Jan 2009, Hugh Dickins wrote:
>
> By the way, there's an argument to say that you should add
> VM_MIXEDMAP to VM_CAN_NONLINEAR in VM_MERGEABLE_FLAGS: I don't
> really care whether we merge the odd filemap_xip vma or not,
> but it used to do so and now won't.
>
> By the same (used to merge, now won't) argument, one could say
> VM_INSERTPAGE should be there too; but whereas VM_MIXEDMAP is used
> in one place only, quite a lot of drivers use vm_insert_page(), so
> I feel more comfortable with the idea that it's stopping merges -
> though in that case, shouldn't we add it to VM_SPECIAL?

VM_MIXEDMAP, yes. It's a special case.

But VM_INSERTPAGE - no.

Why? Because VM_INSERTPAGE implies that mmap() itself quite possibly
actually _does_ something (it may have inserted _all_ the pages, and may
not even have any ->fault handler at all).

So we can't just expand a current mapping and forget about it, we need to
do the mmap(). So the "only do merges early" fundamentally cannot merge
such a vma, even if the old code did.

Of course, we could look at whether it has a ->fault handler as an
indication of whether it's possible to merge or not. We already do that
for ->close, and in many ways ->fault would likely be a better indicator
of whether something is mergeable or not.

But there's really no point. VM_INSERTPAGE implies a very special mapping:
it's used by things like the SCSI target user space ring map, or the magic
network packet mmap thing. If you use those things, you really don't care
about merging adjacent VM's anyway, and at least the two I looked at
really do the whole "pre-populate at mmap time" thing.

And at least the packet one really does have a ->close function, and lacks
a ->fault function, so it wouldn't have merged before either (and looking
at ->fault again seems to be as valid as looking at ->close).

Linus