On Wed, 2005-10-19 at 18:56 +0100, Hugh Dickins wrote:
> On Tue, 18 Oct 2005, Badari Pulavarty wrote:
> > 
> > As you suggested, here is the patch to add SHM_NORESERVE which does 
> > same thing as MAP_NORESERVE. This flag is ignored for OVERCOMMIT_NEVER.
> > I decided to do SHM_NORESERVE instead of IPC_NORESERVE - just to limit
> > its scope.
> 
> Good, yes, SHM_NORESERVE is a better name.

Hugh, Big Thank you for review and help on this.

> 
> > BTW, there is a call to security_shm_alloc() earlier, which could
> > be modified to reject shmget() if it needs to.
> 
> Excellent.  But it can only see shp, and the
> 	shp->shm_flags = (shmflg & S_IRWXUGO);
> will conceal SHM_NORESERVE from it.

I noticed that, but didn't feel like passing it to security_shm_alloc(),
since even SHM_HUGETLB and others are not getting passed today.
That's why I said, "we could, if need to". 

> 
> Since nothing in security/ is worrying about MAP_NORESERVE at present,
> perhaps you need not bother about this for now.  But easily overlooked
> later if MAP_NORESERVE rejection is added.
> 
> > Is this reasonable ? Please review.
> 
> Looks fine as far as it goes, except for the typos in the comment
> +		 * Do not allow no accouting for OVERCOMMIT_NEVER, even
> +	 	 * its asked for.
> should be
> 		 * Do not allow no accounting for OVERCOMMIT_NEVER, even
> 		 * if it's asked for.
> (rather a lot of negatives, but okay there I think!)

Initially I wrote it as "For OVERCOMMIT_GUESS and OVERCOMMIT_ALWAYS,
allow no accounting if asked for." - which matches the code. But,
in future, if we decide to add another mode - then we need to
update the comment.

> I say "as far as it goes" because I don't think it's actually going to
> achieve the effect you said you wanted in your original post.
> 
> As you've probably noticed, switching off VM_ACCOUNT here will mean that
> the shm object is accounted page by page as it's instantiated, and I
> expect you're okay with that.  But you want madvise(DONTNEED) to free
> up those reservations: it'll unmap the pages from userspace, but it
> won't free the pages from the shm object, so the reservations will
> still be in force, and accumulate.

Darren Hart is working on patch to add madvise(DISCARD) to extend
the functionality of madvise(DONTNEED) to really drop those pages.
I was going to ask your opinion on that approach :) 

shmget(SHM_NORESERVE) + madvise(DISCARD) should do what I was
hoping for. (BTW, none of this has been tested with database stuff -
I am just concentrating on reasonable extensions.

Here is the version of patch under test. 
(Darren - I am sending this out without your permission, I hope
you are okay with it).
 
Thanks,
Badari