All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mel Gorman <mgorman@suse.de>
To: KOSAKI Motohiro <kosaki.motohiro@gmail.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	akpm@linux-foundation.org,
	KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
Subject: Re: [PATCH] mm: get rid of unnecessary pageblock scanning in setup_zone_migrate_reserve
Date: Thu, 31 Oct 2013 10:15:25 +0000	[thread overview]
Message-ID: <20131031101525.GT2400@suse.de> (raw)
In-Reply-To: <527169BB.8020104@gmail.com>

On Wed, Oct 30, 2013 at 04:19:07PM -0400, KOSAKI Motohiro wrote:
> >@@ -3926,11 +3929,11 @@ static void setup_zone_migrate_reserve(struct zone *zone)
> >  	/*
> >  	 * Reserve blocks are generally in place to help high-order atomic
> >  	 * allocations that are short-lived. A min_free_kbytes value that
> >-	 * would result in more than 2 reserve blocks for atomic allocations
> >-	 * is assumed to be in place to help anti-fragmentation for the
> >-	 * future allocation of hugepages at runtime.
> >+	 * would result in more than MAX_MIGRATE_RESERVE_BLOCKS reserve blocks
> >+	 * for atomic allocations is assumed to be in place to help
> >+	 * anti-fragmentation for the future allocation of hugepages at runtime.
> >  	 */
> >-	reserve = min(2, reserve);
> >+	reserve = min(MAX_MIGRATE_RESERVE_BLOCKS, reserve);
> >
> >  	for (pfn = start_pfn; pfn < end_pfn; pfn += pageblock_nr_pages) {
> >  		if (!pfn_valid(pfn))
> >@@ -3956,6 +3959,7 @@ static void setup_zone_migrate_reserve(struct zone *zone)
> >  			/* If this block is reserved, account for it */
> >  			if (block_migratetype == MIGRATE_RESERVE) {
> >  				reserve--;
> >+				found++;
> >  				continue;
> >  			}
> >
> >@@ -3970,6 +3974,10 @@ static void setup_zone_migrate_reserve(struct zone *zone)
> >  			}
> >  		}
> >
> >+		/* If all possible reserve blocks have been found, we're done */
> >+		if (found >= MAX_MIGRATE_RESERVE_BLOCKS)
> >+			break;
> >+
> >  		/*
> >  		 * If the reserve is met and this is a previous reserved block,
> >  		 * take it back
> 
> Nit. I would like to add following hunk. This is just nit because moving
> reserve pageblock is extreme rare.
> 
> 		if (block_migratetype == MIGRATE_RESERVE) {
> +                       found++;
> 			set_pageblock_migratetype(page, MIGRATE_MOVABLE);
> 			move_freepages_block(zone, page, MIGRATE_MOVABLE);
> 		}

I don't really see the advantage but if you think it is necessary then I
do not object either.

-- 
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: KOSAKI Motohiro <kosaki.motohiro@gmail.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	akpm@linux-foundation.org,
	KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
Subject: Re: [PATCH] mm: get rid of unnecessary pageblock scanning in setup_zone_migrate_reserve
Date: Thu, 31 Oct 2013 10:15:25 +0000	[thread overview]
Message-ID: <20131031101525.GT2400@suse.de> (raw)
In-Reply-To: <527169BB.8020104@gmail.com>

On Wed, Oct 30, 2013 at 04:19:07PM -0400, KOSAKI Motohiro wrote:
> >@@ -3926,11 +3929,11 @@ static void setup_zone_migrate_reserve(struct zone *zone)
> >  	/*
> >  	 * Reserve blocks are generally in place to help high-order atomic
> >  	 * allocations that are short-lived. A min_free_kbytes value that
> >-	 * would result in more than 2 reserve blocks for atomic allocations
> >-	 * is assumed to be in place to help anti-fragmentation for the
> >-	 * future allocation of hugepages at runtime.
> >+	 * would result in more than MAX_MIGRATE_RESERVE_BLOCKS reserve blocks
> >+	 * for atomic allocations is assumed to be in place to help
> >+	 * anti-fragmentation for the future allocation of hugepages at runtime.
> >  	 */
> >-	reserve = min(2, reserve);
> >+	reserve = min(MAX_MIGRATE_RESERVE_BLOCKS, reserve);
> >
> >  	for (pfn = start_pfn; pfn < end_pfn; pfn += pageblock_nr_pages) {
> >  		if (!pfn_valid(pfn))
> >@@ -3956,6 +3959,7 @@ static void setup_zone_migrate_reserve(struct zone *zone)
> >  			/* If this block is reserved, account for it */
> >  			if (block_migratetype == MIGRATE_RESERVE) {
> >  				reserve--;
> >+				found++;
> >  				continue;
> >  			}
> >
> >@@ -3970,6 +3974,10 @@ static void setup_zone_migrate_reserve(struct zone *zone)
> >  			}
> >  		}
> >
> >+		/* If all possible reserve blocks have been found, we're done */
> >+		if (found >= MAX_MIGRATE_RESERVE_BLOCKS)
> >+			break;
> >+
> >  		/*
> >  		 * If the reserve is met and this is a previous reserved block,
> >  		 * take it back
> 
> Nit. I would like to add following hunk. This is just nit because moving
> reserve pageblock is extreme rare.
> 
> 		if (block_migratetype == MIGRATE_RESERVE) {
> +                       found++;
> 			set_pageblock_migratetype(page, MIGRATE_MOVABLE);
> 			move_freepages_block(zone, page, MIGRATE_MOVABLE);
> 		}

I don't really see the advantage but if you think it is necessary then I
do not object either.

-- 
Mel Gorman
SUSE Labs

  reply	other threads:[~2013-10-31 10:15 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-23 21:01 [PATCH] mm: get rid of unnecessary pageblock scanning in setup_zone_migrate_reserve kosaki.motohiro
2013-10-23 21:01 ` kosaki.motohiro
2013-10-30 15:19 ` Mel Gorman
2013-10-30 15:19   ` Mel Gorman
2013-10-30 19:26   ` KOSAKI Motohiro
2013-10-30 19:26     ` KOSAKI Motohiro
2013-10-30 20:19   ` KOSAKI Motohiro
2013-10-30 20:19     ` KOSAKI Motohiro
2013-10-31 10:15     ` Mel Gorman [this message]
2013-10-31 10:15       ` Mel Gorman
2013-10-31 17:14       ` KOSAKI Motohiro
2013-10-31 17:14         ` KOSAKI Motohiro
2013-11-01  9:00         ` Yasuaki Ishimatsu
2013-11-01  9:00           ` Yasuaki Ishimatsu

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=20131031101525.GT2400@suse.de \
    --to=mgorman@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=isimatu.yasuaki@jp.fujitsu.com \
    --cc=kosaki.motohiro@gmail.com \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    /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.