Re: [RFC][PATCH 6/8] mm: handle_speculative_fault()

[Paul E. McKenney]

On Thu, Jan 07, 2010 at 12:06:48PM -0800, Linus Torvalds wrote:
> 
> 
> On Thu, 7 Jan 2010, Paul E. McKenney wrote:
> > > +
> > > +	spin_lock(&mm->page_table_lock);
> > > +	if (vma->vm_end == cur_brk) {
> > > +		vma->vm_end = brk;
> > > +		mm->brk = brk;
> > > +		cur_brk = brk;
> > > +	}
> > > +	spin_unlock(&mm->page_table_lock);
> > > +
> > > +	if (cur_brk != brk)
> > 
> > Can this be "if (cur_brk < brk)"?  Seems like it should, given the
> > earlier tests, but I don't claim to understand the VM code.
> 
> It's really just a flag, to test whether the final check (inside the 
> spinlock) succeeded, or whether we perhaps raced with _another_ brk() call 
> that also had the mm_sem for reading.
> 
> We know that cur_brk was different from brk before - because otherwise 
> we'd have just returned early (or done the slow case). So testing whether 
> it's different afterwards really only tests whether that 
> 
> 	cur_brk = brk;
> 
> statment was executed or not.
> 
> I could have used a separate flag called "success" or something. 

I was (perhaps confusedly) thinking of a pair of threads both trying
to sbrk() at the same time.  One of them wins by acquiring the
->page_table_lock first.  Then the other acquires the spinlock, but
sees vma->vm_end != cur_brk.  But if the first one extended the region
at least as far as the second intended to, the second one's work is done.

Of course, we can debate the sanity of an application that actually does
concurrent sbrk() calls.

							Thanx, Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/