All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Wen Congyang <wency@cn.fujitsu.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	Rob Landley <rob@landley.net>,
	Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>,
	Lai Jiangshan <laijs@cn.fujitsu.com>,
	Jiang Liu <jiang.liu@huawei.com>,
	KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	Minchan Kim <minchan.kim@gmail.com>, Mel Gorman <mgorman@suse.de>,
	David Rientjes <rientjes@google.com>,
	Yinghai Lu <yinghai@kernel.org>,
	Rusty Russell <rusty@rustcorp.com.au>
Subject: Re: [PART4 Patch v2 1/2] numa: add CONFIG_MOVABLE_NODE for movable-dedicated node
Date: Tue, 20 Nov 2012 14:25:50 -0800	[thread overview]
Message-ID: <20121120142550.5c126194.akpm@linux-foundation.org> (raw)
In-Reply-To: <1353067090-19468-2-git-send-email-wency@cn.fujitsu.com>

On Fri, 16 Nov 2012 19:58:09 +0800
Wen Congyang <wency@cn.fujitsu.com> wrote:

> From: Lai Jiangshan <laijs@cn.fujitsu.com>
> 
> All are prepared, we can actually introduce N_MEMORY.
> add CONFIG_MOVABLE_NODE make we can use it for movable-dedicated node

This description is far too short on details.

I grabbed this from the [0/n] email:

: We need a node which only contains movable memory.  This feature is very
: important for node hotplug.  If a node has normal/highmem, the memory may
: be used by the kernel and can't be offlined.  If the node only contains
: movable memory, we can offline the memory and the node.

which helps a bit, but it's still pretty thin.

Why is this option made configurable?  Why not enable it unconditionally?

Please send a patch which adds the Kconfig help text for
CONFIG_MOVABLE_NODE.  Let's make that text nice and detailed.

The name MOVABLE_NODE is not a good one.  It means "a node which is
movable", whereas the concept is actually "a node whcih contains only
movable memory".  I suppose we could change it to something like
CONFIG_MOVABLE_MEMORY_ONLY_NODE or similar.  But I suppose that
CONFIG_MOVABLE_NODE is good enough, as long as it is well-described in
associated comments or help text.  This is not the case at present.

> +#ifdef CONFIG_MOVABLE_NODE
> +	N_MEMORY,		/* The node has memory(regular, high, movable) */
> +#else

I think the comment should be "The node has only movable memory"?


--
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: Andrew Morton <akpm@linux-foundation.org>
To: Wen Congyang <wency@cn.fujitsu.com>
Cc: linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	Rob Landley <rob@landley.net>,
	Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>,
	Lai Jiangshan <laijs@cn.fujitsu.com>,
	Jiang Liu <jiang.liu@huawei.com>,
	KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	Minchan Kim <minchan.kim@gmail.com>, Mel Gorman <mgorman@suse.de>,
	David Rientjes <rientjes@google.com>,
	Yinghai Lu <yinghai@kernel.org>,
	Rusty Russell <rusty@rustcorp.com.au>
Subject: Re: [PART4 Patch v2 1/2] numa: add CONFIG_MOVABLE_NODE for movable-dedicated node
Date: Tue, 20 Nov 2012 14:25:50 -0800	[thread overview]
Message-ID: <20121120142550.5c126194.akpm@linux-foundation.org> (raw)
In-Reply-To: <1353067090-19468-2-git-send-email-wency@cn.fujitsu.com>

On Fri, 16 Nov 2012 19:58:09 +0800
Wen Congyang <wency@cn.fujitsu.com> wrote:

> From: Lai Jiangshan <laijs@cn.fujitsu.com>
> 
> All are prepared, we can actually introduce N_MEMORY.
> add CONFIG_MOVABLE_NODE make we can use it for movable-dedicated node

This description is far too short on details.

I grabbed this from the [0/n] email:

: We need a node which only contains movable memory.  This feature is very
: important for node hotplug.  If a node has normal/highmem, the memory may
: be used by the kernel and can't be offlined.  If the node only contains
: movable memory, we can offline the memory and the node.

which helps a bit, but it's still pretty thin.

Why is this option made configurable?  Why not enable it unconditionally?

Please send a patch which adds the Kconfig help text for
CONFIG_MOVABLE_NODE.  Let's make that text nice and detailed.

The name MOVABLE_NODE is not a good one.  It means "a node which is
movable", whereas the concept is actually "a node whcih contains only
movable memory".  I suppose we could change it to something like
CONFIG_MOVABLE_MEMORY_ONLY_NODE or similar.  But I suppose that
CONFIG_MOVABLE_NODE is good enough, as long as it is well-described in
associated comments or help text.  This is not the case at present.

> +#ifdef CONFIG_MOVABLE_NODE
> +	N_MEMORY,		/* The node has memory(regular, high, movable) */
> +#else

I think the comment should be "The node has only movable memory"?



  reply	other threads:[~2012-11-20 22:25 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-16 11:58 [PART4 Patch v2 0/2] memory-hotplug: allow online/offline memory to result movable node Wen Congyang
2012-11-16 11:58 ` Wen Congyang
2012-11-16 11:58 ` [PART4 Patch v2 1/2] numa: add CONFIG_MOVABLE_NODE for movable-dedicated node Wen Congyang
2012-11-16 11:58   ` Wen Congyang
2012-11-20 22:25   ` Andrew Morton [this message]
2012-11-20 22:25     ` Andrew Morton
2012-11-21  3:27     ` Wen Congyang
2012-11-21  3:27       ` Wen Congyang
2012-11-16 11:58 ` [PART4 Patch v2 2/2] memory_hotplug: allow online/offline memory to result movable node Wen Congyang
2012-11-16 11:58   ` Wen Congyang
2012-11-20 22:29   ` Andrew Morton
2012-11-20 22:29     ` Andrew Morton
2012-12-17  2:23     ` Tang Chen
2012-12-17  2:23       ` Tang Chen

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=20121120142550.5c126194.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=isimatu.yasuaki@jp.fujitsu.com \
    --cc=jiang.liu@huawei.com \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=laijs@cn.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    --cc=minchan.kim@gmail.com \
    --cc=rientjes@google.com \
    --cc=rob@landley.net \
    --cc=rusty@rustcorp.com.au \
    --cc=wency@cn.fujitsu.com \
    --cc=yinghai@kernel.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.