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

[Linus Torvalds <torvalds@linux-foundation.org>]

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