Re: [PATCH 1/3] [FIX] hugetlb: Fix broken fs quota management

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

On Wed, 2007-10-24 at 22:20 -0700, Ken Chen wrote:
> On 10/24/07, Dave Hansen <haveblue@us.ibm.com> wrote:
> > But, I think what I'm realizing is that the free paths for shared vs.
> > private are actually quite distinct.  Even more now after your patches
> > abolish using and actual put_page() (and the destructors) on private
> > pages losing their last mapping.
> >
> > I think it may make a lot of sense to have
> > {alloc,free}_{private,shared}_huge_page().  It'll really help
> > readability, and I _think_ it gives you a handy dandy place to add the
> > different quota operations needed.
> 
> Here is my version of re-factoring hugetlb_put_quota() into
> free_huge_page.  Not exactly what Dave suggested, but at least
> consolidate quota credit in one place.

I think consolidating quota into alloc/free_huge_page is a laudable
goal, but it has some problems (see below) that I feel go beyond simply
fixing the broken accounting (the initial purpose of the patches I was
targeting for -rc2).

<snip>
> @@ -369,6 +375,7 @@ static struct page *alloc_huge_page(
> 
>  	spin_unlock(&hugetlb_lock);
>  	set_page_refcounted(page);
> +	set_page_private(page, (unsigned long) vma->vm_file->f_mapping);
>  	return page;
> 
>  fail:

This seems like a layering violation to me.  We set page->private here,
but if add_to_page_cache() is called on this page later, it will set
page->mapping.  Granted it will overwrite the exact same value, but it
does so using a different field in the union.  So I don't think we
should be messing with page->private in this way.

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