RE: [patch] hugetlb strict commit accounting - v3

Adam Litke
2006-03-20T20:21:59Z

On Mon, 2006-03-20 at 10:48 -0800, Chen, Kenneth W wrote:
> Adam Litke wrote on Monday, March 20, 2006 7:35 AM
> > On Thu, 2006-03-09 at 19:14 -0800, Chen, Kenneth W wrote:
> > > @@ -98,6 +98,12 @@ struct page *alloc_huge_page(struct vm_a
> > > int i;
> > >
> > > spin_lock(&hugetlb_lock);
> > > + if (vma->vm_flags & VM_MAYSHARE)
> > > + resv_huge_pages--;
> > > + else if (free_huge_pages <= resv_huge_pages) {
> > > + spin_unlock(&hugetlb_lock);
> > > + return NULL;
> > > + }
> > > page = dequeue_huge_page(vma, addr);
> > > if (!page) {
> > > spin_unlock(&hugetlb_lock);
> >
> > Unfortunately this will break down when two or more threads race to
> > allocate the same page. You end up with a double-decrement of
> > resv_huge_pages even though only one thread will win the race.
>
> Are you sure? David introduced hugetlb_instantiation_mutex to serialize
> entire hugetlb fault path, such race is not possible anymore. I
> previously
> quipped about it, and soon realized that for private mapping, such thing
> is inevitable. And even for shared mapping, that means not needing a
> back
> out path. I will add it for defensive measure.

You're right. I forgot about that patch... With it applied, everything
works correctly.

> Thanks for bring this up though, there is one place where it still have
> problem - allocation can fail under file system quota.
>
> Which brings up another interesting question: should private mapping
> hold
> file system quota? If it does as it is now, that means file system
> quota
> need to be reserved up front along with hugetlb page reservation.

I must profess my ignorance about the filesystem quota part. I've never
seen that used in practice as a resource limiting lever. That said, I
think we need to ensure either: both shared and private hold quota, or
neither hold it.

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