Message-ID: <Pine.LNX.4.64.0901311216250.6763@blonde.anvils>
Date:	Sat, 31 Jan 2009 12:35:35 +0000 (GMT)
From:	Hugh Dickins <hugh@...itas.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
cc:	Lee Schermerhorn <Lee.Schermerhorn@...com>,
	Greg KH <gregkh@...e.de>,
	Maksim Yevmenkin <maksim.yevmenkin@...il.com>,
	linux-kernel <linux-kernel@...r.kernel.org>,
	Nick Piggin <npiggin@...e.de>,
	Andrew Morton <akpm@...ux-foundation.org>,
	will@...wder-design.com, Rik van Riel <riel@...hat.com>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	Mikos Szeredi <miklos@...redi.hu>
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists