linux-kernel - Re: [patch] mm, compaction: ignore watermarks when isolating free 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]

Message-ID: <4f5ba93e-8bf0-151e-57eb-cad1a4823b9e@suse.cz>
Date:	Thu, 16 Jun 2016 09:15:42 +0200
From:	Vlastimil Babka <vbabka@...e.cz>
To:	David Rientjes <rientjes@...gle.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Mel Gorman <mgorman@...hsingularity.net>
Cc:	Hugh Dickins <hughd@...gle.com>,
	Joonsoo Kim <iamjoonsoo.kim@....com>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	Michal Hocko <mhocko@...e.cz>
Subject: Re: [patch] mm, compaction: ignore watermarks when isolating free
 pages

On 06/16/2016 12:34 AM, David Rientjes wrote:
> The goal of memory compaction is to defragment memory by moving migratable
> pages to free pages at the end of the zone.  No additional memory is being
> allocated.
>
> Ignore per-zone low watermarks in __isolate_free_page() because memory is
> either fully migrated or isolated free pages are returned when migration
> fails.

Michal Hocko suggested that too, but I didn't think it safe that 
compaction should go below the min watermark, even temporarily. It means 
the system is struggling with order-0 allocations, so making it worse 
for the benefit of high-order allocations doesn't make sense. The 
high-order allocation would likely fail anyway due to watermark checks, 
even if the page of sufficient order was formed by compaction. So in my 
series, I just changed the low watermark check to min [1].

> This fixes an issue where the compaction freeing scanner can isolate
> memory but the zone drops below its low watermark for that page order, so
> the scanner must continue to scan all memory pointlessly.

Good point, looks like failing the watermark is the only reason when 
__isolate_free_page() can fail. isolate_freepages_block() and its 
callers should take this as an indication that compaction should return 
with failure immediately.

[1] http://article.gmane.org/gmane.linux.kernel/2231369

> Signed-off-by: David Rientjes <rientjes@...gle.com>
> ---
>  mm/page_alloc.c | 14 ++------------
>  1 file changed, 2 insertions(+), 12 deletions(-)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -2484,23 +2484,14 @@ EXPORT_SYMBOL_GPL(split_page);
>
>  int __isolate_free_page(struct page *page, unsigned int order)
>  {
> -	unsigned long watermark;
>  	struct zone *zone;
> -	int mt;
> +	const int mt = get_pageblock_migratetype(page);
>
>  	BUG_ON(!PageBuddy(page));
> -
>  	zone = page_zone(page);
> -	mt = get_pageblock_migratetype(page);
> -
> -	if (!is_migrate_isolate(mt)) {
> -		/* Obey watermarks as if the page was being allocated */
> -		watermark = low_wmark_pages(zone) + (1 << order);
> -		if (!zone_watermark_ok(zone, 0, watermark, 0, 0))
> -			return 0;
>
> +	if (!is_migrate_isolate(mt))
>  		__mod_zone_freepage_state(zone, -(1UL << order), mt);
> -	}
>
>  	/* Remove page from free list */
>  	list_del(&page->lru);
> @@ -2520,7 +2511,6 @@ int __isolate_free_page(struct page *page, unsigned int order)
>  		}
>  	}
>
> -
>  	return 1UL << order;
>  }
>
>