All of lore.kernel.org
 help / color / mirror / Atom feed
From: Joonsoo Kim <iamjoonsoo.kim@lge.com>
To: Mel Gorman <mgorman@suse.de>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Vlastimil Babka <vbabka@suse.cz>, 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: Tue, 11 Feb 2014 16:12:25 +0900	[thread overview]
Message-ID: <20140211071225.GA27870@lge.com> (raw)
In-Reply-To: <20140210132634.GE6732@suse.de>

On Mon, Feb 10, 2014 at 01:26:34PM +0000, Mel Gorman wrote:
> 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.

Hello,

After disallowing high-order page to be used for migration target,
we only allow pages from movable or CMA pageblock for migration target on
migrate_async_suitable() check. So checking whether page comes from reserve or
isolate pageblock is useless.

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

Okay!

Thanks.

--
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: Joonsoo Kim <iamjoonsoo.kim@lge.com>
To: Mel Gorman <mgorman@suse.de>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Vlastimil Babka <vbabka@suse.cz>, 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: Tue, 11 Feb 2014 16:12:25 +0900	[thread overview]
Message-ID: <20140211071225.GA27870@lge.com> (raw)
In-Reply-To: <20140210132634.GE6732@suse.de>

On Mon, Feb 10, 2014 at 01:26:34PM +0000, Mel Gorman wrote:
> 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.

Hello,

After disallowing high-order page to be used for migration target,
we only allow pages from movable or CMA pageblock for migration target on
migrate_async_suitable() check. So checking whether page comes from reserve or
isolate pageblock is useless.

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

Okay!

Thanks.

  reply	other threads:[~2014-02-11  7:12 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
2014-02-10 13:26     ` Mel Gorman
2014-02-11  7:12     ` Joonsoo Kim [this message]
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=20140211071225.GA27870@lge.com \
    --to=iamjoonsoo.kim@lge.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    --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.