Re: mm/hugetlb: GFP_KERNEL allocation under spinlock?

[Mel Gorman <mel@csn.ul.ie>]

On Sun, Aug 02, 2009 at 11:54:13AM +0100, Hugh Dickins wrote:
> On Sun, 2 Aug 2009, Jiri Slaby wrote:
> > 
> > could anybody please confirm this cannot happen?
> 
> I'm no authority on hugetlb.c nowadays: you'll have studied this
> in more detail than I have, so please don't believe me.  (And I'm
> no longer at my old address, but lkml's enjoying a quiet Sunday.)
> 
> > 
> > hugetlb_fault()
> > -> spin_lock()
> > -> hugetlb_cow()
> >    -> alloc_huge_page()
> >       -> vma_needs_reservation()
> >          -> region_chg() (either of the 2)
> >             -> kmalloc(*, GFP_KERNEL)
> > 
> > Thanks.
> 
> That should be taken care of by the successful vma_needs_reservation()
> on the same address in hugetlb_fault(), before taking page_table_lock,
> shouldn't it?
> 

Yes, any kmalloc() required should be happening outside spinlocks. The
region_chg and region_add acts like a prepare,commit pair except the naming
is diabolical. There was a mistake made at one point where kmalloc() was
called within a spinlock but enabling lock debugging caught it.

> It is possible that a hugetlb_vmtruncate() comes in between that
> vma_needs_reservation() and taking the page_table_lock, which could
> remove the region (or "nrg") needed.
> 
> But if that's the case then the pte_same test immediately after taking
> page_table_lock should catch it: we're in the part of hugetlb_fault()
> dealing with !huge_pte_none, whereas truncation would have made it
> huge_pte_none (and it won't get to freeing the reservations before
> it's nullified the page tables, holding page_table_lock).
> 

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>