linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Vlastimil Babka <vbabka@suse.cz>
To: Joonsoo Kim <iamjoonsoo.kim@lge.com>,
	Andrew Morton <akpm@linux-foundation.org>
Cc: Mel Gorman <mgorman@suse.de>, Joonsoo Kim <js1304@gmail.com>,
	Rik van Riel <riel@redhat.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/5] mm/compaction: do not call suitable_migration_target() on every page
Date: Fri, 07 Feb 2014 10:36:13 +0100	[thread overview]
Message-ID: <52F4A90D.20804@suse.cz> (raw)
In-Reply-To: <1391749726-28910-3-git-send-email-iamjoonsoo.kim@lge.com>

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@lge.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, 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>

  reply	other threads:[~2014-02-07  9:36 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-07  5:08 [PATCH 0/5] compaction related commits Joonsoo Kim
2014-02-07  5:08 ` [PATCH 1/5] mm/compaction: disallow high-order page for migration target Joonsoo Kim
2014-02-07  9:20   ` Vlastimil Babka
2014-02-10 13:26   ` Mel Gorman
2014-02-11  7:12     ` Joonsoo Kim
2014-02-07  5:08 ` [PATCH 2/5] mm/compaction: do not call suitable_migration_target() on every page Joonsoo Kim
2014-02-07  9:36   ` Vlastimil Babka [this message]
2014-02-10  0:41     ` Joonsoo Kim
2014-02-07  5:08 ` [PATCH 3/5] mm/compaction: change the timing to check to drop the spinlock Joonsoo Kim
2014-02-07  9:50   ` Vlastimil Babka
2014-02-07  5:08 ` [PATCH 4/5] mm/compaction: check pageblock suitability once per pageblock Joonsoo Kim
2014-02-07 10:30   ` Vlastimil Babka
2014-02-10  0:46     ` Joonsoo Kim
2014-02-07  5:08 ` [PATCH 5/5] mm/compaction: clean-up code on success of ballon isolation Joonsoo Kim
2014-02-07 10:33   ` Vlastimil Babka
2014-02-07  9:14 ` [PATCH 0/5] compaction related commits Vlastimil Babka
2014-02-10  0:24   ` Joonsoo Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52F4A90D.20804@suse.cz \
    --to=vbabka@suse.cz \
    --cc=akpm@linux-foundation.org \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=js1304@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    --cc=riel@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).