linux-kernel - Re: [PATCH 2/5] mm/compaction: do not call suitable_migration

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:	Fri, 07 Feb 2014 10:36:13 +0100
From:	Vlastimil Babka <vbabka@...e.cz>
To:	Joonsoo Kim <iamjoonsoo.kim@....com>,
	Andrew Morton <akpm@...ux-foundation.org>
CC:	Mel Gorman <mgorman@...e.de>, Joonsoo Kim <js1304@...il.com>,
	Rik van Riel <riel@...hat.com>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/5] mm/compaction: do not call suitable_migration_target()
 on every page

On 02/07/2014 06:08 AM, Joonsoo Kim wrote:
> suitable_migration_target() checks that pageblock is suitable for
> migration target. In isolate_freepages_block(), it is called on every
> page and this is inefficient. So make it called once per pageblock.

Hmm but in sync compaction, compact_checklock_irqsave() may drop the zone->lock,
reschedule and reacquire it and thus possibly invalidate your previous check. Async
compaction is ok as that will quit immediately. So you could probably communicate that
this happened and invalidate checked_pageblock in such case. Or maybe this would not
happen too enough to worry about rare suboptimal migrations?

Vlastimil

> suitable_migration_target() also checks if page is highorder or not,
> but it's criteria for highorder is pageblock order. So calling it once
> within pageblock range has no problem.
> 
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@....com>
> 
> diff --git a/mm/compaction.c b/mm/compaction.c
> index bbe1260..0d821a2 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -245,6 +245,7 @@ static unsigned long isolate_freepages_block(struct compact_control *cc,
>  	unsigned long nr_strict_required = end_pfn - blockpfn;
>  	unsigned long flags;
>  	bool locked = false;
> +	bool checked_pageblock = false;
>  
>  	cursor = pfn_to_page(blockpfn);
>  
> @@ -275,8 +276,16 @@ static unsigned long isolate_freepages_block(struct compact_control *cc,
>  			break;
>  
>  		/* Recheck this is a suitable migration target under lock */
> -		if (!strict && !suitable_migration_target(page))
> -			break;
> +		if (!strict && !checked_pageblock) {
> +			/*
> +			 * We need to check suitability of pageblock only once
> +			 * and this isolate_freepages_block() is called with
> +			 * pageblock range, so just check once is sufficient.
> +			 */
> +			checked_pageblock = true;
> +			if (!suitable_migration_target(page))
> +				break;
> +		}
>  
>  		/* Recheck this is a buddy page under lock */
>  		if (!PageBuddy(page))
> 

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