Re: [PATCH] hugetlb: Fix dynamic pool resize failure case

[Dave Hansen <haveblue@us.ibm.com>]

On Tue, 2007-10-09 at 08:58 -0700, Adam Litke wrote:
> index 9b3dfac..f349c16 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -281,8 +281,11 @@ free:
>  		list_del(&page->lru);
>  		if ((--needed) >= 0)
>  			enqueue_huge_page(page);
> -		else
> -			update_and_free_page(page);
> +		else {
> +			spin_unlock(&hugetlb_lock);
> +			put_page(page);
> +			spin_lock(&hugetlb_lock);
> +		}
>  	}

update_and_free_page() does several things:
1. it decrements nr_huge_pages(_node[])
2. it resets the member page flags to some known values
3. clears the compound page destructor
4. clears the page refcount (to 1)
5. actually frees the page back to the allocator

put_page() does several things, too:
1. put_page() hits PageCompound(), then calls put_compound_page()
2. put_compound_page() calls the compound page destructor which is set
   to free_huge_page() (this was set in alloc_buddy_huge_page())
3. free_huge_page() checks page_count(), takes the hugetlb_lock, and
   calls enqueue_huge_page()
4. enqueue_huge_page() puts the page back in hugepage_freelists[nid],
   then _increments_ nr_huge_pages(_node[])

This seems weird to me that you're replacing a function with something
that eventually does the opposite.  update_and_free_page() also did
nothing with the hugepage_freelists[], which enqueue_huge_page() does.
Something doesn't quite add up here.  Did you realize that the destuctor
was going to get called?  Or, did I misread it, and the destructor is
_not_ called?

I also think it's a crime that alloc_buddy_huge_page() doesn't share
code with alloc_fresh_huge_page().  

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