linux-kernel - Re: [PATCH] mm: hugetlb: defer freeing pages when gathering surplus pages

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 14 Mar 2012 12:13:57 +0100
From:	Michal Hocko <mhocko@...e.cz>
To:	Hillf Danton <dhillf@...il.com>
Cc:	Linux-MM <linux-mm@...ck.org>, LKML <linux-kernel@...r.kernel.org>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	Hugh Dickins <hughd@...gle.com>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH] mm: hugetlb: defer freeing pages when gathering surplus
 pages

[Sorry for the late reply but I was away from email for quite sometime]

On Tue 14-02-12 20:53:51, Hillf Danton wrote:
> When gathering surplus pages, the number of needed pages is recomputed after
> reacquiring hugetlb lock to catch changes in resv_huge_pages and
> free_huge_pages. Plus it is recomputed with the number of newly allocated
> pages involved.
> 
> Thus freeing pages could be deferred a bit to see if the final page request is
> satisfied, though pages could be allocated less than needed.

The patch looks OK but I am missing a word why we need it. I guess
your primary motivation is that we want to reduce false positives when
we fail to allocate surplus pages while somebody freed some in the
background.
What is the workload that you observed such a behavior? Or is this just
from the code review?

> Signed-off-by: Hillf Danton <dhillf@...il.com>
> ---
> 
> --- a/mm/hugetlb.c	Tue Feb 14 20:10:46 2012
> +++ b/mm/hugetlb.c	Tue Feb 14 20:19:42 2012
> @@ -852,6 +852,7 @@ static int gather_surplus_pages(struct h
>  	struct page *page, *tmp;
>  	int ret, i;
>  	int needed, allocated;
> +	bool alloc_ok = true;
> 
>  	needed = (h->resv_huge_pages + delta) - h->free_huge_pages;
>  	if (needed <= 0) {
> @@ -867,17 +868,13 @@ retry:
>  	spin_unlock(&hugetlb_lock);
>  	for (i = 0; i < needed; i++) {
>  		page = alloc_buddy_huge_page(h, NUMA_NO_NODE);
> -		if (!page)
> -			/*
> -			 * We were not able to allocate enough pages to
> -			 * satisfy the entire reservation so we free what
> -			 * we've allocated so far.
> -			 */
> -			goto free;
> -
> +		if (!page) {
> +			alloc_ok = false;
> +			break;
> +		}
>  		list_add(&page->lru, &surplus_list);
>  	}
> -	allocated += needed;
> +	allocated += i;
> 
>  	/*
>  	 * After retaking hugetlb_lock, we need to recalculate 'needed'
> @@ -886,9 +883,16 @@ retry:
>  	spin_lock(&hugetlb_lock);
>  	needed = (h->resv_huge_pages + delta) -
>  			(h->free_huge_pages + allocated);
> -	if (needed > 0)
> -		goto retry;
> -
> +	if (needed > 0) {
> +		if (alloc_ok)
> +			goto retry;
> +		/*
> +		 * We were not able to allocate enough pages to
> +		 * satisfy the entire reservation so we free what
> +		 * we've allocated so far.
> +		 */
> +		goto free;
> +	}
>  	/*
>  	 * The surplus_list now contains _at_least_ the number of extra pages
>  	 * needed to accommodate the reservation.  Add the appropriate number
> @@ -914,10 +918,10 @@ retry:
>  		VM_BUG_ON(page_count(page));
>  		enqueue_huge_page(h, page);
>  	}
> +free:
>  	spin_unlock(&hugetlb_lock);
> 
>  	/* Free unnecessary surplus pages to the buddy allocator */
> -free:
>  	if (!list_empty(&surplus_list)) {
>  		list_for_each_entry_safe(page, tmp, &surplus_list, lru) {
>  			list_del(&page->lru);
> --
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/