All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Gary Hade <garyhade@us.ibm.com>
Cc: linux-mm@kvack.org, Andrew Morton <akpm@linux-foundation.org>,
	Yasunori Goto <y-goto@jp.fujitsu.com>,
	Badari Pulavarty <pbadari@us.ibm.com>, Mel Gorman <mel@csn.ul.ie>,
	Chris McDermott <lcm@us.ibm.com>,
	linux-kernel@vger.kernel.org, x86@kernel.org
Subject: Re: [PATCH] [RESEND] x86_64: add memory hotremove config option
Date: Fri, 5 Sep 2008 19:44:49 +0200	[thread overview]
Message-ID: <20080905174449.GC27395@elte.hu> (raw)
In-Reply-To: <20080905172132.GA11692@us.ibm.com>


* Gary Hade <garyhade@us.ibm.com> wrote:

> Add memory hotremove config option to x86_64
> 
> Memory hotremove functionality can currently be configured into the 
> ia64, powerpc, and s390 kernels.  This patch makes it possible to 
> configure the memory hotremove functionality into the x86_64 kernel as 
> well.

hm, why is it for 64-bit only?

> +++ linux-2.6.27-rc5/arch/x86/Kconfig	2008-09-03 13:34:55.000000000 -0700
> @@ -1384,6 +1384,9 @@
>  	def_bool y
>  	depends on X86_64 || (X86_32 && HIGHMEM)
> 
> +config ARCH_ENABLE_MEMORY_HOTREMOVE
> +	def_bool y

so this will break the build on 32-bit, if CONFIG_MEMORY_HOTREMOVE=y? 
mm/memory_hotplug.c assumes that remove_memory() is provided by the 
architecture.

> +#ifdef CONFIG_MEMORY_HOTREMOVE
> +int remove_memory(u64 start, u64 size)
> +{
> +	unsigned long start_pfn, end_pfn;
> +	unsigned long timeout = 120 * HZ;
> +	int ret;
> +	start_pfn = start >> PAGE_SHIFT;
> +	end_pfn = start_pfn + (size >> PAGE_SHIFT);
> +	ret = offline_pages(start_pfn, end_pfn, timeout);
> +	if (ret)
> +		goto out;
> +	/* Arch-specific calls go here */
> +out:
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(remove_memory);
> +#endif /* CONFIG_MEMORY_HOTREMOVE */

hm, nothing appears to be arch-specific about this trivial wrapper 
around offline_pages().

Shouldnt this be moved to the CONFIG_MEMORY_HOTREMOVE portion of 
mm/memory_hotplug.c instead, as a weak function? That way architectures 
only have to enable ARCH_ENABLE_MEMORY_HOTREMOVE - and architectures 
with different/special needs can override it.

	Ingo

WARNING: multiple messages have this Message-ID (diff)
From: Ingo Molnar <mingo@elte.hu>
To: Gary Hade <garyhade@us.ibm.com>
Cc: linux-mm@kvack.org, Andrew Morton <akpm@linux-foundation.org>,
	Yasunori Goto <y-goto@jp.fujitsu.com>,
	Badari Pulavarty <pbadari@us.ibm.com>, Mel Gorman <mel@csn.ul.ie>,
	Chris McDermott <lcm@us.ibm.com>,
	linux-kernel@vger.kernel.org, x86@kernel.org
Subject: Re: [PATCH] [RESEND] x86_64: add memory hotremove config option
Date: Fri, 5 Sep 2008 19:44:49 +0200	[thread overview]
Message-ID: <20080905174449.GC27395@elte.hu> (raw)
In-Reply-To: <20080905172132.GA11692@us.ibm.com>

* Gary Hade <garyhade@us.ibm.com> wrote:

> Add memory hotremove config option to x86_64
> 
> Memory hotremove functionality can currently be configured into the 
> ia64, powerpc, and s390 kernels.  This patch makes it possible to 
> configure the memory hotremove functionality into the x86_64 kernel as 
> well.

hm, why is it for 64-bit only?

> +++ linux-2.6.27-rc5/arch/x86/Kconfig	2008-09-03 13:34:55.000000000 -0700
> @@ -1384,6 +1384,9 @@
>  	def_bool y
>  	depends on X86_64 || (X86_32 && HIGHMEM)
> 
> +config ARCH_ENABLE_MEMORY_HOTREMOVE
> +	def_bool y

so this will break the build on 32-bit, if CONFIG_MEMORY_HOTREMOVE=y? 
mm/memory_hotplug.c assumes that remove_memory() is provided by the 
architecture.

> +#ifdef CONFIG_MEMORY_HOTREMOVE
> +int remove_memory(u64 start, u64 size)
> +{
> +	unsigned long start_pfn, end_pfn;
> +	unsigned long timeout = 120 * HZ;
> +	int ret;
> +	start_pfn = start >> PAGE_SHIFT;
> +	end_pfn = start_pfn + (size >> PAGE_SHIFT);
> +	ret = offline_pages(start_pfn, end_pfn, timeout);
> +	if (ret)
> +		goto out;
> +	/* Arch-specific calls go here */
> +out:
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(remove_memory);
> +#endif /* CONFIG_MEMORY_HOTREMOVE */

hm, nothing appears to be arch-specific about this trivial wrapper 
around offline_pages().

Shouldnt this be moved to the CONFIG_MEMORY_HOTREMOVE portion of 
mm/memory_hotplug.c instead, as a weak function? That way architectures 
only have to enable ARCH_ENABLE_MEMORY_HOTREMOVE - and architectures 
with different/special needs can override it.

	Ingo

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

  reply	other threads:[~2008-09-05 17:45 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-09-05 17:21 [PATCH] [RESEND] x86_64: add memory hotremove config option Gary Hade
2008-09-05 17:21 ` Gary Hade
2008-09-05 17:44 ` Ingo Molnar [this message]
2008-09-05 17:44   ` Ingo Molnar
2008-09-05 18:14   ` Badari Pulavarty
2008-09-05 18:14     ` Badari Pulavarty
2008-09-05 18:17     ` Ingo Molnar
2008-09-05 18:17       ` Ingo Molnar
2008-09-08 21:52       ` [PATCH] Cleanup to make remove_memory() arch neutral Badari Pulavarty
2008-09-08 21:52         ` Badari Pulavarty
2008-09-09  0:56         ` Andrew Morton
2008-09-09  0:56           ` Andrew Morton
2008-09-09  1:14           ` Randy Dunlap
2008-09-09  1:14             ` Randy Dunlap
2008-09-09  1:21           ` Yasunori Goto
2008-09-09  1:21             ` Yasunori Goto
2008-09-09 15:12             ` Badari Pulavarty
2008-09-09 15:12               ` Badari Pulavarty
2008-09-08 21:56       ` [PATCH] x86: add memory hotremove config option Badari Pulavarty
2008-09-08 21:56         ` Badari Pulavarty
2008-09-05 18:04 ` [PATCH] [RESEND] x86_64: " Andi Kleen
2008-09-05 18:04   ` Andi Kleen
2008-09-05 18:31   ` Badari Pulavarty
2008-09-05 18:31     ` Badari Pulavarty
2008-09-05 18:54     ` Andi Kleen
2008-09-05 18:54       ` Andi Kleen
2008-09-05 22:34       ` Badari Pulavarty
2008-09-05 22:34         ` Badari Pulavarty
2008-09-05 19:53   ` Gary Hade
2008-09-05 19:53     ` Gary Hade
2008-09-05 20:04     ` Andi Kleen
2008-09-05 20:04       ` Andi Kleen
2008-09-05 21:54       ` Gary Hade
2008-09-05 21:54         ` Gary Hade
2008-09-06  0:01         ` Andi Kleen
2008-09-06  0:01           ` Andi Kleen
2008-09-06  7:06           ` Yasunori Goto
2008-09-06  7:06             ` Yasunori Goto
2008-09-06  8:53             ` Andi Kleen
2008-09-06  8:53               ` Andi Kleen
2008-09-08  5:52               ` Nick Piggin
2008-09-08  5:52                 ` Nick Piggin
2008-09-08  9:36                 ` Andi Kleen
2008-09-08  9:36                   ` Andi Kleen
2008-09-08  9:46                   ` Nick Piggin
2008-09-08  9:46                     ` Nick Piggin
2008-09-08 10:30                     ` Andi Kleen
2008-09-08 10:30                       ` Andi Kleen
2008-09-08 11:19                       ` Nick Piggin
2008-09-08 11:19                         ` Nick Piggin
2008-09-08 11:30                         ` Andi Kleen
2008-09-08 11:30                           ` Andi Kleen
2008-09-08 13:48                           ` Nick Piggin
2008-09-08 13:48                             ` Nick Piggin
2008-09-06 14:33             ` Ingo Molnar
2008-09-06 14:33               ` Ingo Molnar
2008-09-06 16:00               ` kamezawa.hiroyu
2008-09-06 16:00                 ` kamezawa.hiroyu
2008-09-06 16:05                 ` kamezawa.hiroyu
2008-09-06 16:05                   ` kamezawa.hiroyu
2008-09-06 16:17                 ` Ingo Molnar
2008-09-06 16:17                   ` Ingo Molnar

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=20080905174449.GC27395@elte.hu \
    --to=mingo@elte.hu \
    --cc=akpm@linux-foundation.org \
    --cc=garyhade@us.ibm.com \
    --cc=lcm@us.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mel@csn.ul.ie \
    --cc=pbadari@us.ibm.com \
    --cc=x86@kernel.org \
    --cc=y-goto@jp.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.