Messages in this thread | | | Date | Fri, 30 Jan 2009 12:31:13 -0800 (PST) | From | Linus Torvalds <> | Subject | Re: [PATCH] Fix OOPS in mmap_region() when merging adjacent VM_LOCKED file segments |
| |
On Fri, 30 Jan 2009, Lee Schermerhorn wrote: > > Ad hoc instrumentation shows that it's the VM_ACCOUNT flag that is > different between the existing file segment and the one attempting the > merge:
Gaah. I looked at VM_ACCOUNT, since it looked like a prime example of the same VM_CAN_NONLINEAR thing and was thinking that I should just add it to the "ok to merge" flags, but decided after a quick look that we do all the VM_ACCOUNT handling before we call vma_merge, so I just left it at that.
But apparently we _do_ do some VM_ACCOUNT stuff afterwards.
> is_mergeable_vma: !mergable: vma flags: 0x80020f9:0x1020f9 > | |-VM_ACCOUNT > +-----------VM_CAN_NONLINEAR > > So happens, I'm mapping with MAP_SHARED, so the VM_ACCOUNT flag gets > cleared later in mmap_region(). Comments say that this is for checking > memory availability during shmem_file_setup(). Maybe we can move the > temporary setting of VM_ACCOUNT until just before the call to > shmem_zero_setup()?
Yeah, that would probably fix it, and looks like the right thing to do.
It all looks pretty confused wrong to set the whole VM_ACCOUNT flag for a file-backed file AT ALL in the first place, but the code knows that it won't matter for a shared file, and will be cleared again later.
So it plays these temporary games with vm_flags, and it didn't matter because of how we used to call "vma_merge()" either early only for the anonymous memory case (that had VM_ACCOUNT stable and didn't have that temporary case at all) or much later (after having undone the temporary flag setting) for files.
Why do we pass in that "accountable" flag, btw? It's only ever set to 0 by a MAP_PRIVATE mapping that hits is_file_hugepages() (see do_mmap_pgoff), and we could just do that decision all inside mmap_region(). So the flag doesn't really seem to have any real meaning, and is just passed around for some odd historical reason?
Linus
| |