Re: [Libhugetlbfs-devel] [PATCH 3/5] hugetlb: Try to grow hugetlb pool for MAP_PRIVATE mappings

[Adam Litke <agl@us.ibm.com>]

On Fri, 2007-09-14 at 15:46 +1000, David Gibson wrote:
> On Thu, Sep 13, 2007 at 03:21:30PM -0500, Adam Litke wrote:
> > On Thu, 2007-09-13 at 11:06 -0700, Dave Hansen wrote:
> > > On Thu, 2007-09-13 at 10:59 -0700, Adam Litke wrote:
> > > > +static int within_locked_vm_limits(long hpage_delta)
> > > > +{
> > > > +	unsigned long locked_pages, locked_pages_limit;
> > > > +
> > > > +	/* Check locked page limits */
> > > > +	locked_pages = current->mm->locked_vm;
> > > > +	locked_pages += hpage_delta * (HPAGE_SIZE >> PAGE_SHIFT);
> > > > +	locked_pages_limit = current->signal->rlim[RLIMIT_MEMLOCK].rlim_cur;
> > > > +	locked_pages_limit >>= PAGE_SHIFT;
> > > > +
> > > > +	/* Return 0 if we would exceed locked_vm limits */
> > > > +	if (locked_pages > locked_pages_limit)
> > > > +		return 0;
> > > > +
> > > > +	/* Nice, we're within limits */
> > > > +	return 1;
> > > > +}
> > > > +
> > > > +static struct page *alloc_buddy_huge_page(struct vm_area_struct *vma,
> > > > +						unsigned long address)
> > > > +{
> > > > +	struct page *page;
> > > > +
> > > > +	/* Check we remain within limits if 1 huge page is allocated */
> > > > +	if (!within_locked_vm_limits(1))
> > > > +		return NULL;
> > > > +
> > > > +	page = alloc_pages(htlb_alloc_mask|__GFP_COMP|__GFP_NOWARN,
> > > ...
> > > 
> > > Is there locking around this operation?  Or, is there a way that a
> > > process could do this concurrently in two different threads, both appear
> > > to be within within_locked_vm_limits(), and both succeed to allocate
> > > when doing so actually takes them over the limit?
> > 
> > This case is prevented by hugetlb_instantiation_mutex.  I'll include a
> > comment to make that clearer.
> 
> Hrm... a number of people are trying to get rid of, or at least reduce
> the scope of the instatiation mutex, since it can be significant
> bottlenect when clearing large numbers of hugepages on big SMP
> systems.

Yes, and with the exception of this bit, this patch series furthers that
goal substantially.  With a dynamic hugetlb pool, the
alloc-instantiation race can be handled by stretching the pool during
the race window to accommodate the temporary overage.

As for the safety of within_locked_vm_limits() depending on
hugetlb_instantiation_mutex, perhaps this is another reason to not use
the locked ulimit as a way to manage hugetlb pool growth (since we do
have the fs quota method). 

-- 
Adam Litke - (agl at us.ibm.com)
IBM Linux Technology Center

--
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>