All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
To: Nathan Zimmer <nzimmer@sgi.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Tang Chen <tangchen@cn.fujitsu.com>,
	Wen Congyang <wency@cn.fujitsu.com>,
	Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
	Hedi <hedi@sgi.com>, Mike Travis <travis@sgi.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC] hotplug, memory: move register_memory_resource out of the lock_memory_hotplug
Date: Wed, 15 Jan 2014 10:12:46 +0900	[thread overview]
Message-ID: <52D5E08E.4030309@jp.fujitsu.com> (raw)
In-Reply-To: <1389723874-32372-1-git-send-email-nzimmer@sgi.com>

(2014/01/15 3:24), Nathan Zimmer wrote:
> We don't need to do register_memory_resource() since it has its own lock and
> doesn't make any callbacks.
> 
> Also register_memory_resource return NULL on failure so we don't have anything
> to cleanup at this point.
> 
> 
> The reason for this rfc is I was doing some experiments with hotplugging of
> memory on some of our larger systems.  While it seems to work, it can be quite
> slow.  With some preliminary digging I found that lock_memory_hotplug is
> clearly ripe for breakup.
> 
> It could be broken up per nid or something but it also covers the
> online_page_callback.  The online_page_callback shouldn't be very hard to break
> out.
> 
> Also there is the issue of various structures(wmarks come to mind) that are
> only updated under the lock_memory_hotplug that would need to be dealt with.
> 
> 
> cc: Andrew Morton <akpm@linux-foundation.org>
> cc: Tang Chen <tangchen@cn.fujitsu.com>
> cc: Wen Congyang <wency@cn.fujitsu.com>
> cc: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> cc: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
> cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> cc: Hedi <hedi@sgi.com>
> cc: Mike Travis <travis@sgi.com>
> cc: linux-mm@kvack.org
> cc: linux-kernel@vger.kernel.org
> 
> 
> ---

The patch seems good to me.

Reviewed-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>

Thanks,
Yasuaki Ishimatsu

>   mm/memory_hotplug.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 1ad92b4..62a0cd1 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1097,17 +1097,18 @@ int __ref add_memory(int nid, u64 start, u64 size)
>   	struct resource *res;
>   	int ret;
>   
> -	lock_memory_hotplug();
> -
>   	res = register_memory_resource(start, size);
>   	ret = -EEXIST;
>   	if (!res)
> -		goto out;
> +		return ret;
>   
>   	{	/* Stupid hack to suppress address-never-null warning */
>   		void *p = NODE_DATA(nid);
>   		new_pgdat = !p;
>   	}
> +
> +	lock_memory_hotplug();
> +
>   	new_node = !node_online(nid);
>   	if (new_node) {
>   		pgdat = hotadd_new_pgdat(nid, start);
> 


--
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: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
To: Nathan Zimmer <nzimmer@sgi.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Tang Chen <tangchen@cn.fujitsu.com>,
	Wen Congyang <wency@cn.fujitsu.com>,
	Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>,
	"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
	Hedi <hedi@sgi.com>, Mike Travis <travis@sgi.com>,
	<linux-mm@kvack.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [RFC] hotplug, memory: move register_memory_resource out of the lock_memory_hotplug
Date: Wed, 15 Jan 2014 10:12:46 +0900	[thread overview]
Message-ID: <52D5E08E.4030309@jp.fujitsu.com> (raw)
In-Reply-To: <1389723874-32372-1-git-send-email-nzimmer@sgi.com>

(2014/01/15 3:24), Nathan Zimmer wrote:
> We don't need to do register_memory_resource() since it has its own lock and
> doesn't make any callbacks.
> 
> Also register_memory_resource return NULL on failure so we don't have anything
> to cleanup at this point.
> 
> 
> The reason for this rfc is I was doing some experiments with hotplugging of
> memory on some of our larger systems.  While it seems to work, it can be quite
> slow.  With some preliminary digging I found that lock_memory_hotplug is
> clearly ripe for breakup.
> 
> It could be broken up per nid or something but it also covers the
> online_page_callback.  The online_page_callback shouldn't be very hard to break
> out.
> 
> Also there is the issue of various structures(wmarks come to mind) that are
> only updated under the lock_memory_hotplug that would need to be dealt with.
> 
> 
> cc: Andrew Morton <akpm@linux-foundation.org>
> cc: Tang Chen <tangchen@cn.fujitsu.com>
> cc: Wen Congyang <wency@cn.fujitsu.com>
> cc: Kamezawa Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> cc: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
> cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
> cc: Hedi <hedi@sgi.com>
> cc: Mike Travis <travis@sgi.com>
> cc: linux-mm@kvack.org
> cc: linux-kernel@vger.kernel.org
> 
> 
> ---

The patch seems good to me.

Reviewed-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>

Thanks,
Yasuaki Ishimatsu

>   mm/memory_hotplug.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 1ad92b4..62a0cd1 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -1097,17 +1097,18 @@ int __ref add_memory(int nid, u64 start, u64 size)
>   	struct resource *res;
>   	int ret;
>   
> -	lock_memory_hotplug();
> -
>   	res = register_memory_resource(start, size);
>   	ret = -EEXIST;
>   	if (!res)
> -		goto out;
> +		return ret;
>   
>   	{	/* Stupid hack to suppress address-never-null warning */
>   		void *p = NODE_DATA(nid);
>   		new_pgdat = !p;
>   	}
> +
> +	lock_memory_hotplug();
> +
>   	new_node = !node_online(nid);
>   	if (new_node) {
>   		pgdat = hotadd_new_pgdat(nid, start);
> 



  parent reply	other threads:[~2014-01-15  1:13 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-14 18:24 [RFC] hotplug, memory: move register_memory_resource out of the lock_memory_hotplug Nathan Zimmer
2014-01-14 18:24 ` Nathan Zimmer
2014-01-14 23:13 ` Andrew Morton
2014-01-14 23:13   ` Andrew Morton
2014-01-15  1:05   ` David Rientjes
2014-01-15  1:05     ` David Rientjes
2014-01-15  1:16     ` Andrew Morton
2014-01-15  1:16       ` Andrew Morton
2014-01-15  0:58 ` David Rientjes
2014-01-15  0:58   ` David Rientjes
2014-01-15  1:12 ` Yasuaki Ishimatsu [this message]
2014-01-15  1:12   ` 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=52D5E08E.4030309@jp.fujitsu.com \
    --to=isimatu.yasuaki@jp.fujitsu.com \
    --cc=akpm@linux-foundation.org \
    --cc=hedi@sgi.com \
    --cc=kamezawa.hiroyu@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=nzimmer@sgi.com \
    --cc=rafael.j.wysocki@intel.com \
    --cc=tangchen@cn.fujitsu.com \
    --cc=travis@sgi.com \
    --cc=wency@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.