Re: [PATCH] do_brk() needs mmap_sem write-locked

[Marcelo Tosatti <marcelo.tosatti@cyclades.com>]

On Wed, Jan 12, 2005 at 08:03:44AM -0800, Linus Torvalds wrote:
> 
> 
> Looks good, except for a small nit:
> 
> On Tue, 11 Jan 2005, Marcelo Tosatti wrote:
> > diff -Nur linux-2.6-curr.orig/mm/mmap.c linux-2.6-curr/mm/mmap.c
> > --- linux-2.6-curr.orig/mm/mmap.c	2005-01-11 22:48:49.000000000 -0200
> > +++ linux-2.6-curr/mm/mmap.c	2005-01-11 23:43:10.704800272 -0200
> > @@ -1891,6 +1891,12 @@
> >  	}
> >  
> >  	/*
> > +	 * mm->mmap_sem is required to protect against another thread
> > +	 * changing the mappings in case we sleep.
> > +	 */
> > +	WARN_ON(down_read_trylock(&mm->mmap_sem));
> > +
> > +	/*
> >  	 * Clear old maps.  this also does some error checking for us
> >  	 */
> >   munmap_back:
> > 
> 
> if that warning ever triggers, mmap_sem will now be locked, and that will 
> cause problems. So I suspect it's better to do
> 
> 	if (down_read_trylock(&mm->mmap_sem)) {
> 		WARN_ON(1);
> 		up_read(&mm->mmap_sem);
> 	}
> 
> and move that into a helper function of its own (something like
> "verify_mmap_write_lock_held()").

OK - I've seen you just committed a fix.

I've fixed v2.4 version.