Messages in this thread | | | Date | Sat, 31 Jan 2009 12:35:35 +0000 (GMT) | From | Hugh Dickins <> | Subject | Re: [PATCH] Fix OOPS in mmap_region() when merging adjacent VM_LOCKED file segments |
| |
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
| |