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

Hugh Dickins
2009-01-31T12:36:49Z

On Fri, 30 Jan 2009, Linus Torvalds wrote:
> On Fri, 30 Jan 2009, Lee Schermerhorn wrote:
> >
> > I just verified that adding VM_ACCOUNT to VM_MERGEABLE does allow the
> > merge to happen with the test program. And the system didn't come
> > crashing down around me. But, I wouldn't trust that simple test as the
> > last word. A short run of a stress load I use held up/still running,
> > but I can't tell whether it's merging as expected there.
>
> Just ignoring VM_ACCOUNT for merging is not the right thing to do. It
> probably works in practice for just about everything, but at least in
> theory it does mean that mmap() can stop honoring MAP_NORESERVE.
>
> Admittedly the circumstances where that happens are unlikely, and nobody
> probably even really uses MAP_NORESERVE in the first place, so I doubt you
> can ever see it as a real issue, but it's still technically wrong to merge
> vma's that can differ in VM_ACCOUNT.
>
> Now, the particular test you have, VM_ACCOUNT differs only during that
> temporary window, and the vma's really do end up with the same VM_ACCOUNT
> state in the end, so merging them is correct, but if you were to privately
> map the same file (or private anonymous map) with the same permissions
> next to each other so that they -could- merge, but use MAP_NORESERVE on
> one and not on the other, then they shouldn't merge.
>
> So VM_ACCOUNT does matter - just barely - for merging, and we just happen
> to currently hit it too much due to a very odd internal reason.

It matters more than just barely - if you care about non-overcommit,
or if you care about non-wrapping Committed_AS in your /proc/meminfo.

Ignoring VM_ACCOUNT when merging is very much the wrong thing to do,
because it lets an unaccounted area be treated thereafter as accounted,
or vice versa - even forgetting the MAP_NORESERVE special case.

I have by now recalled why I chose to play those VM_ACCOUNT games:
/* We set VM_ACCOUNT in a shared mapping's vm_flags, to inform
* shmem_zero_setup (perhaps called through /dev/zero's ->mmap)
* that memory reservation must be checked; but that reservation
* belongs to shared memory object, not to vma: so now clear it.
We need a way to communicate not-MAP_NORESERVE to shmem.c, and we don't
just need it in the explicit shmem_zero_setup() case, we also need it
for the (probably rare nowadays) case when mmap() is working on file
/dev/zero (drivers/char/mem.c mmap_zero()), rather than using MAP_ANON.

Still haven't decided what's best to do about it (plenty of diversions):
perhaps we just say my error was to overload VM_ACCOUNT, and define a
new flag for the purpose, which can go into VM_MERGEABLE_FLAGS; but
I'd prefer a neater solution if it crosses my mind.

Hugh