All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Hansen <dave.hansen@intel.com>
To: Andrew Morton <akpm@linux-foundation.org>,
	liuchangcheng@inspur.com, fandd@inspur.com,
	guz.fnst@cn.fujitsu.com, hutao@cn.fujitsu.com,
	isimatu.yasuaki@jp.fujitsu.com, laijs@cn.fujitsu.com,
	qiuxishi@huawei.com, tangchen@cn.fujitsu.com, toshi.kani@hp.com,
	wangnan0@huawei.com, yanxiaofeng@inspur.com, yinghai@kernel.org,
	zhangyanfei@cn.fujitsu.com, Linux-MM <linux-mm@kvack.org>
Subject: Re: + mm-memory-hot-add-memory-can-not-be-added-to-movable-zone-defaultly.patch added to -mm tree
Date: Tue, 15 Sep 2015 16:06:09 -0700	[thread overview]
Message-ID: <55F8A461.3070309@intel.com> (raw)
In-Reply-To: <20150915145232.fb74148815fa79bfeaad88bc@linux-foundation.org>

On 09/15/2015 02:52 PM, Andrew Morton wrote:
>>  /*
>>   * If movable zone has already been setup, newly added memory should be check.
>>   * If its address is higher than movable zone, it should be added as movable.
>> + * And if system boots up with movable_node and config CONFIG_MOVABLE_NOD and
>> + * added memory does not overlap the zone before MOVABLE_ZONE,
>> + * the memory is added as movable
>>   * Without this check, movable zone may overlap with other zone.
>>   */
>>  static int should_add_memory_movable(int nid, u64 start, u64 size)
>> @@ -1208,6 +1211,11 @@ static int should_add_memory_movable(int
>>  	unsigned long start_pfn = start >> PAGE_SHIFT;
>>  	pg_data_t *pgdat = NODE_DATA(nid);
>>  	struct zone *movable_zone = pgdat->node_zones + ZONE_MOVABLE;
>> +	struct zone *pre_zone = pgdat->node_zones + (ZONE_MOVABLE - 1);
>> +
>> +	if (movable_node_is_enabled()
>> +	&& zone_end_pfn(pre_zone) <= start_pfn)
>> +		return 1;

This check seems goofy to me.  According to the description, we're
looking at a node here which has all of its zones empty.  So it
definitely has zone->spanned_pages=0.  zone_end_pfn() is looking at
zone->zone_start_pfn too, which is also 0, presumably.

So why is it bothering to look at the pfns if they're potentially
"garbage"?  It seems like we really want something like this:

	if (all_node_zones_empty(pgdat)) {
		/*
		 * We usually want a ZONE_NORMAL before we add a
		 * ZONE_MOVABLE since ZONE_MOVABLE is mildly crippled.
		 * We only want ZONE_MOVABLE first when 'movable_node'
		 * mode is on.
		 */
		return movable_node_is_enabled();
	}

Either way, this is a behavior change.  It's one that is triggered by a
config option plus a boot option, but it might surprise some users.  Is
this new behavior documented?

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

      parent reply	other threads:[~2015-09-15 23:06 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-15 21:49 + mm-memory-hot-add-memory-can-not-be-added-to-movable-zone-defaultly.patch added to -mm tree akpm
     [not found] ` <20150915145232.fb74148815fa79bfeaad88bc@linux-foundation.org>
2015-09-15 23:06   ` Dave Hansen [this message]

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=55F8A461.3070309@intel.com \
    --to=dave.hansen@intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=fandd@inspur.com \
    --cc=guz.fnst@cn.fujitsu.com \
    --cc=hutao@cn.fujitsu.com \
    --cc=isimatu.yasuaki@jp.fujitsu.com \
    --cc=laijs@cn.fujitsu.com \
    --cc=linux-mm@kvack.org \
    --cc=liuchangcheng@inspur.com \
    --cc=qiuxishi@huawei.com \
    --cc=tangchen@cn.fujitsu.com \
    --cc=toshi.kani@hp.com \
    --cc=wangnan0@huawei.com \
    --cc=yanxiaofeng@inspur.com \
    --cc=yinghai@kernel.org \
    --cc=zhangyanfei@cn.fujitsu.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 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.