All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mel Gorman <mgorman@suse.de>
To: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Vlastimil Babka <vbabka@suse.cz>, Joonsoo Kim <js1304@gmail.com>,
	Rik van Riel <riel@redhat.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/5] mm/compaction: disallow high-order page for migration target
Date: Mon, 10 Feb 2014 13:26:34 +0000	[thread overview]
Message-ID: <20140210132634.GE6732@suse.de> (raw)
In-Reply-To: <1391749726-28910-2-git-send-email-iamjoonsoo.kim@lge.com>

On Fri, Feb 07, 2014 at 02:08:42PM +0900, Joonsoo Kim wrote:
> Purpose of compaction is to get a high order page. Currently, if we find
> high-order page while searching migration target page, we break it to
> order-0 pages and use them as migration target. It is contrary to purpose
> of compaction, so disallow high-order page to be used for
> migration target.
> 
> Additionally, clean-up logic in suitable_migration_target() to simply.
> There is no functional changes from this clean-up.
> 
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> 
> diff --git a/mm/compaction.c b/mm/compaction.c
> index 3a91a2e..bbe1260 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -217,21 +217,12 @@ static inline bool compact_trylock_irqsave(spinlock_t *lock,
>  /* Returns true if the page is within a block suitable for migration to */
>  static bool suitable_migration_target(struct page *page)
>  {
> -	int migratetype = get_pageblock_migratetype(page);
> -
> -	/* Don't interfere with memory hot-remove or the min_free_kbytes blocks */
> -	if (migratetype == MIGRATE_RESERVE)
> -		return false;
> -

Why is this check removed? The reservation blocks are preserved as
short-lived high-order atomic allocations depend on them.

> -	if (is_migrate_isolate(migratetype))
> -		return false;
> -

Why is this check removed?

> -	/* If the page is a large free page, then allow migration */
> +	/* If the page is a large free page, then disallow migration */
>  	if (PageBuddy(page) && page_order(page) >= pageblock_order)
> -		return true;
> +		return false;
>  

The reason why this was originally allowed was to allow pageblocks that were
marked MIGRATE_UNMOVABLE or MIGRATE_RECLAIMABLE to be used as compaction
targets. However, compaction should not even be running if this is the
case so the change makes sense.

>  	/* If the block is MIGRATE_MOVABLE or MIGRATE_CMA, allow migration */
> -	if (migrate_async_suitable(migratetype))
> +	if (migrate_async_suitable(get_pageblock_migratetype(page)))
>  		return true;
>  
>  	/* Otherwise skip the block */
> -- 
> 1.7.9.5
> 

-- 
Mel Gorman
SUSE Labs

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

WARNING: multiple messages have this Message-ID (diff)
From: Mel Gorman <mgorman@suse.de>
To: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Vlastimil Babka <vbabka@suse.cz>, Joonsoo Kim <js1304@gmail.com>,
	Rik van Riel <riel@redhat.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/5] mm/compaction: disallow high-order page for migration target
Date: Mon, 10 Feb 2014 13:26:34 +0000	[thread overview]
Message-ID: <20140210132634.GE6732@suse.de> (raw)
In-Reply-To: <1391749726-28910-2-git-send-email-iamjoonsoo.kim@lge.com>

On Fri, Feb 07, 2014 at 02:08:42PM +0900, Joonsoo Kim wrote:
> Purpose of compaction is to get a high order page. Currently, if we find
> high-order page while searching migration target page, we break it to
> order-0 pages and use them as migration target. It is contrary to purpose
> of compaction, so disallow high-order page to be used for
> migration target.
> 
> Additionally, clean-up logic in suitable_migration_target() to simply.
> There is no functional changes from this clean-up.
> 
> Signed-off-by: Joonsoo Kim <iamjoonsoo.kim@lge.com>
> 
> diff --git a/mm/compaction.c b/mm/compaction.c
> index 3a91a2e..bbe1260 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -217,21 +217,12 @@ static inline bool compact_trylock_irqsave(spinlock_t *lock,
>  /* Returns true if the page is within a block suitable for migration to */
>  static bool suitable_migration_target(struct page *page)
>  {
> -	int migratetype = get_pageblock_migratetype(page);
> -
> -	/* Don't interfere with memory hot-remove or the min_free_kbytes blocks */
> -	if (migratetype == MIGRATE_RESERVE)
> -		return false;
> -

Why is this check removed? The reservation blocks are preserved as
short-lived high-order atomic allocations depend on them.

> -	if (is_migrate_isolate(migratetype))
> -		return false;
> -

Why is this check removed?

> -	/* If the page is a large free page, then allow migration */
> +	/* If the page is a large free page, then disallow migration */
>  	if (PageBuddy(page) && page_order(page) >= pageblock_order)
> -		return true;
> +		return false;
>  

The reason why this was originally allowed was to allow pageblocks that were
marked MIGRATE_UNMOVABLE or MIGRATE_RECLAIMABLE to be used as compaction
targets. However, compaction should not even be running if this is the
case so the change makes sense.

>  	/* If the block is MIGRATE_MOVABLE or MIGRATE_CMA, allow migration */
> -	if (migrate_async_suitable(migratetype))
> +	if (migrate_async_suitable(get_pageblock_migratetype(page)))
>  		return true;
>  
>  	/* Otherwise skip the block */
> -- 
> 1.7.9.5
> 

-- 
Mel Gorman
SUSE Labs

  parent reply	other threads:[~2014-02-10 13:26 UTC|newest]

Thread overview: 34+ 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 ` Joonsoo Kim
2014-02-07  5:08 ` [PATCH 1/5] mm/compaction: disallow high-order page for migration target Joonsoo Kim
2014-02-07  5:08   ` Joonsoo Kim
2014-02-07  9:20   ` Vlastimil Babka
2014-02-07  9:20     ` Vlastimil Babka
2014-02-10 13:26   ` Mel Gorman [this message]
2014-02-10 13:26     ` Mel Gorman
2014-02-11  7:12     ` Joonsoo Kim
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  5:08   ` Joonsoo Kim
2014-02-07  9:36   ` Vlastimil Babka
2014-02-07  9:36     ` Vlastimil Babka
2014-02-10  0:41     ` Joonsoo Kim
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  5:08   ` Joonsoo Kim
2014-02-07  9:50   ` Vlastimil Babka
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  5:08   ` Joonsoo Kim
2014-02-07 10:30   ` Vlastimil Babka
2014-02-07 10:30     ` Vlastimil Babka
2014-02-10  0:46     ` Joonsoo Kim
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  5:08   ` Joonsoo Kim
2014-02-07 10:33   ` Vlastimil Babka
2014-02-07 10:33     ` Vlastimil Babka
2014-02-07  9:14 ` [PATCH 0/5] compaction related commits Vlastimil Babka
2014-02-07  9:14   ` Vlastimil Babka
2014-02-10  0:24   ` Joonsoo Kim
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=20140210132634.GE6732@suse.de \
    --to=mgorman@suse.de \
    --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=riel@redhat.com \
    --cc=vbabka@suse.cz \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.