[PATCH 1/3] Add remove_memory() for ppc64

[PATCH 1/3] Add remove_memory() for ppc64

[Badari Pulavarty]

Supply arch specific remove_memory() for PPC64. There is nothing
ppc specific code here and its exactly same as ia64 version. 
For now, lets keep it arch specific - so each arch can add
its own special things if needed.

Signed-off-by: Badari Pulavarty <pbadari@us.ibm.com>
---
 arch/powerpc/mm/mem.c |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

Index: linux-2.6.23/arch/powerpc/mm/mem.c
===================================================================
--- linux-2.6.23.orig/arch/powerpc/mm/mem.c	2007-10-25 11:34:54.000000000 -0700
+++ linux-2.6.23/arch/powerpc/mm/mem.c	2007-10-25 11:35:24.000000000 -0700
@@ -131,6 +131,20 @@ int __devinit arch_add_memory(int nid, u
 
 #endif /* CONFIG_MEMORY_HOTPLUG */
 
+#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);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(remove_memory);
+#endif /* CONFIG_MEMORY_HOTREMOVE */
+
 void show_mem(void)
 {
 	unsigned long total = 0, reserved = 0;

Re: [PATCH 1/3] Add remove_memory() for ppc64

[Dave Hansen]

On Wed, 2007-10-31 at 08:49 -0800, Badari Pulavarty wrote:
> +#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);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(remove_memory);
> +#endif /* CONFIG_MEMORY_HOTREMOVE */

Did someone go and copy the ia64 verion?  Tsk.  Tsk.  Bad Badari.  :)

Can we just make this a weak symbol in the generic mm/memory_hotplug.c?
Or, make this the generic memory_remove() function int there and have an
arch_remove_memory() hook called from there if the architectures need to
tweak it?

-- Dave

Re: [PATCH 1/3] Add remove_memory() for ppc64

[Badari Pulavarty]

On Wed, 2007-10-31 at 13:45 -0700, Dave Hansen wrote:
> On Wed, 2007-10-31 at 08:49 -0800, Badari Pulavarty wrote:
> > +#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);
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(remove_memory);
> > +#endif /* CONFIG_MEMORY_HOTREMOVE */
> 
> Did someone go and copy the ia64 verion?  Tsk.  Tsk.  Bad Badari.  :)
> 
> Can we just make this a weak symbol in the generic mm/memory_hotplug.c?
> Or, make this the generic memory_remove() function int there and have an
> arch_remove_memory() hook called from there if the architectures need to
> tweak it?

Well, We don't need arch-specific remove_memory() for ia64 and ppc64.
x86_64, I don't know. We will know, only when some one does the
verification. I don't need arch_remove_memory() hook also at this time.

KAME and I agreed that, we will kill all this if no arch needs it (after
verifying it with x86/x86-64). No point adding all the infrastructure,
if no one needs it at the end.

Thanks,
Badari

Re: [PATCH 1/3] Add remove_memory() for ppc64

[Dave Hansen]

On Wed, 2007-10-31 at 14:11 -0800, Badari Pulavarty wrote:
> 
> Well, We don't need arch-specific remove_memory() for ia64 and ppc64.
> x86_64, I don't know. We will know, only when some one does the
> verification. I don't need arch_remove_memory() hook also at this
> time.

I wasn't being very clear.  I say, add the arch hook only if you need
it.  But, for now, just take the ia64 code and make it generic.  

-- Dave

Re: [PATCH 1/3] Add remove_memory() for ppc64

[KAMEZAWA Hiroyuki]

On Wed, 31 Oct 2007 14:55:03 -0700
Dave Hansen <haveblue@us.ibm.com> wrote:

> On Wed, 2007-10-31 at 14:11 -0800, Badari Pulavarty wrote:
> > 
> > Well, We don't need arch-specific remove_memory() for ia64 and ppc64.
> > x86_64, I don't know. We will know, only when some one does the
> > verification. I don't need arch_remove_memory() hook also at this
> > time.
> 
> I wasn't being very clear.  I say, add the arch hook only if you need
> it.  But, for now, just take the ia64 code and make it generic.  
> 

remove_memory() has been arch-specific since there was no piece of unplug
code. And I didn't merge it to be generic when I implemented ia64 ver.

Hmm...I have no objection to merge them. But let's see how memory hotremove
for ppc64 works for a while. We can merge them later.

I'm glad to have new testers :)

Thanks,
-Kame

Re: [PATCH 1/3] Add remove_memory() for ppc64

[Badari Pulavarty]

On Wed, 2007-10-31 at 13:45 -0700, Dave Hansen wrote:
> On Wed, 2007-10-31 at 08:49 -0800, Badari Pulavarty wrote:
> > +#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);
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(remove_memory);
> > +#endif /* CONFIG_MEMORY_HOTREMOVE */
> 
> Did someone go and copy the ia64 verion?  Tsk.  Tsk.  Bad Badari.  :)
> 
> Can we just make this a weak symbol in the generic mm/memory_hotplug.c?
> Or, make this the generic memory_remove() function int there and have an
> arch_remove_memory() hook called from there if the architectures need to
> tweak it?

BTW, we do have generic remove_memory() which returns -EINVAL, if
CONFIG_HOTPLUG_MEMORY_REMOVE is not set to cover all the arch
that doesn't support it.

Thanks,
Badari

Re: [PATCH 1/3] Add remove_memory() for ppc64

[Kumar Gala]

On Oct 31, 2007, at 11:49 AM, Badari Pulavarty wrote:

> Supply arch specific remove_memory() for PPC64. There is nothing
> ppc specific code here and its exactly same as ia64 version.
> For now, lets keep it arch specific - so each arch can add
> its own special things if needed.
>
> Signed-off-by: Badari Pulavarty <pbadari@us.ibm.com>
> ---

What's ppc64 specific about these patches?

- k

Re: [PATCH 1/3] Add remove_memory() for ppc64

[Badari Pulavarty]

On Thu, 2007-11-01 at 01:26 -0500, Kumar Gala wrote:
> On Oct 31, 2007, at 11:49 AM, Badari Pulavarty wrote:
> 
> > Supply arch specific remove_memory() for PPC64. There is nothing
> > ppc specific code here and its exactly same as ia64 version.
> > For now, lets keep it arch specific - so each arch can add
> > its own special things if needed.
> >
> > Signed-off-by: Badari Pulavarty <pbadari@us.ibm.com>
> > ---
> 
> What's ppc64 specific about these patches?

Like I mentioned, nothing. When KAME did the hotplug memory
remove, he kept this remove_memory() arch-specific - so
each arch can provide its own, if it needs to something
special. So far, there is no need for arch-specific 
remove_memory(). If other archs (x86-64 and others)
agree we can merge this into arch neutral code.

I have to provide this for ppc64 to plug into general
frame work.

Thanks,
Badari

Re: [PATCH 1/3] Add remove_memory() for ppc64

[Dale Farnsworth]

Badari Pulavarty wrote:
> On Thu, 2007-11-01 at 01:26 -0500, Kumar Gala wrote:
> > On Oct 31, 2007, at 11:49 AM, Badari Pulavarty wrote:
> > 
> > > Supply arch specific remove_memory() for PPC64. There is nothing
> > > ppc specific code here and its exactly same as ia64 version.
> > > For now, lets keep it arch specific - so each arch can add
> > > its own special things if needed.
> > >
> > > Signed-off-by: Badari Pulavarty <pbadari@us.ibm.com>
> > > ---
> > 
> > What's ppc64 specific about these patches?
> 
> Like I mentioned, nothing. When KAME did the hotplug memory
> remove, he kept this remove_memory() arch-specific - so
> each arch can provide its own, if it needs to something
> special. So far, there is no need for arch-specific 
> remove_memory(). If other archs (x86-64 and others)
> agree we can merge this into arch neutral code.
> 
> I have to provide this for ppc64 to plug into general
> frame work.

You've added it to arch/powerpc for both PPC32 and PPC64, so your
commit description is misleading.

-Dale