Re: [patch] mm, compaction: ignore watermarks when isolating free pages

[Vlastimil Babka]

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@xxxxxxxxxx>
> ---
>  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;
>  }