[PATCH 0/2] Correct memory hotplug remove

[PATCH 0/2] Correct memory hotplug remove

[Nathan Fontenot]

Memory hotplug on Power is currently broken, these two patches correct the
issues needed to get memory hotplug working again.

This update marks memory resources that are added at boot time are also
marked as busy. It sounds a bit counter intuitive but the core mm code will
not free memory resources if they are not marked as busy.

This also ensures that bootmem memory is is registered at boot time. A
previous commit (46723bfa540...) that enabled memory hotplug remove with
SPARSE_VMEMMAP enabled broke this on Power.

Additional patches to follow to correct the current memory hotplug
implementation on Power.

Nathan Fontenot
---

 arch/powerpc/mm/mem.c             |    2 +-
 powerpc/arch/powerpc/mm/init_64.c |    6 ++++++
 powerpc/arch/powerpc/mm/mem.c     |    9 +++++++++
 powerpc/mm/Kconfig                |    2 +-
 4 files changed, 17 insertions(+), 2 deletions(-)

[PATCH 1/2] Mark memory resources as busy

[Nathan Fontenot]

Memory resources should be marked as busy.

If memory resources are not marked as busy they do not get released during
hotplug memory remove. This seems a bit counter intuitive but the core
kernel resource code checks for the IORESOURCE_BUSY flag before releasing
the resource.

Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
 arch/powerpc/mm/mem.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: powerpc/arch/powerpc/mm/mem.c
===================================================================
--- powerpc.orig/arch/powerpc/mm/mem.c
+++ powerpc/arch/powerpc/mm/mem.c
@@ -514,7 +514,7 @@ static int add_system_ram_resources(void
 			res->name = "System RAM";
 			res->start = base;
 			res->end = base + size - 1;
-			res->flags = IORESOURCE_MEM;
+			res->flags = IORESOURCE_MEM | IORESOURCE_BUSY;
 			WARN_ON(request_resource(&iomem_resource, res) < 0);
 		}
 	}

[PATCH 2/2] Register bootmem pages at boot on powerpc

[Nathan Fontenot]

Register bootmem pages at boot time on powerpc.

Previous commit 46723bfa540... introduced a new config option,
HAVE_BOOTMEM_INFO_NODE, to enable registering of bootmem pages. As a result
the bootmem pages for powerpc are not registered since we do not define this.
This causes a BUG_ON in put_page_bootmem() when trying to hotplug remove
memory on powerpc.

This patch resolves this by doing three things;
- define HAVE_BOOTMEM_INFO_NODE for powerpc
- Add a routine to register bootmem via register_page_bootmem_info_node()
  in mem_init().
- Stub out the register_page_bootmem_memmap() routine needed for building
  with SPARSE_VMEMMAP enabled.

Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
 arch/powerpc/mm/init_64.c |    6 ++++++
 arch/powerpc/mm/mem.c     |    9 +++++++++
 mm/Kconfig                |    2 +-
 3 files changed, 16 insertions(+), 1 deletion(-)

Index: powerpc/arch/powerpc/mm/init_64.c
===================================================================
--- powerpc.orig/arch/powerpc/mm/init_64.c
+++ powerpc/arch/powerpc/mm/init_64.c
@@ -300,5 +300,11 @@ void vmemmap_free(unsigned long start, u
 {
 }

+void register_page_bootmem_memmap(unsigned long section_nr,
+				  struct page *start_page, unsigned long size)
+{
+	WARN_ONCE(1, KERN_INFO
+		  "Sparse Vmemmap not fully supported for bootmem info nodes\n");
+}
 #endif /* CONFIG_SPARSEMEM_VMEMMAP */

Index: powerpc/arch/powerpc/mm/mem.c
===================================================================
--- powerpc.orig/arch/powerpc/mm/mem.c
+++ powerpc/arch/powerpc/mm/mem.c
@@ -297,12 +297,21 @@ void __init paging_init(void)
 }
 #endif /* ! CONFIG_NEED_MULTIPLE_NODES */

+static void __init register_page_bootmem_info(void)
+{
+	int i;
+
+	for_each_online_node(i)
+		register_page_bootmem_info_node(NODE_DATA(i));
+}
+
 void __init mem_init(void)
 {
 #ifdef CONFIG_SWIOTLB
 	swiotlb_init(0);
 #endif

+	register_page_bootmem_info();
 	high_memory = (void *) __va(max_low_pfn * PAGE_SIZE);
 	set_max_mapnr(max_pfn);
 	free_all_bootmem();
Index: powerpc/mm/Kconfig
===================================================================
--- powerpc.orig/mm/Kconfig
+++ powerpc/mm/Kconfig
@@ -183,7 +183,7 @@ config MEMORY_HOTPLUG_SPARSE
 config MEMORY_HOTREMOVE
 	bool "Allow for memory hot remove"
 	select MEMORY_ISOLATION
-	select HAVE_BOOTMEM_INFO_NODE if X86_64
+	select HAVE_BOOTMEM_INFO_NODE if (X86_64 || PPC64)
 	depends on MEMORY_HOTPLUG && ARCH_ENABLE_MEMORY_HOTREMOVE
 	depends on MIGRATION

Re: [PATCH 2/2] Register bootmem pages at boot on powerpc

[Benjamin Herrenschmidt]

On Fri, 2013-08-09 at 10:32 -0500, Nathan Fontenot wrote:

> +void register_page_bootmem_memmap(unsigned long section_nr,
> +				  struct page *start_page, unsigned long size)
> +{
> +	WARN_ONCE(1, KERN_INFO
> +		  "Sparse Vmemmap not fully supported for bootmem info nodes\n");
> +}
>  #endif /* CONFIG_SPARSEMEM_VMEMMAP */

But SPARSEMEM_VMEMMAP is our default on ppc64 pseries ... and you are
select'ing the new option, so it looks like we are missing something
here...

Can you tell me a bit more, the above makes me nervous...

Cheers,
Ben.

> Index: powerpc/arch/powerpc/mm/mem.c
> ===================================================================
> --- powerpc.orig/arch/powerpc/mm/mem.c
> +++ powerpc/arch/powerpc/mm/mem.c
> @@ -297,12 +297,21 @@ void __init paging_init(void)
>  }
>  #endif /* ! CONFIG_NEED_MULTIPLE_NODES */
> 
> +static void __init register_page_bootmem_info(void)
> +{
> +	int i;
> +
> +	for_each_online_node(i)
> +		register_page_bootmem_info_node(NODE_DATA(i));
> +}
> +
>  void __init mem_init(void)
>  {
>  #ifdef CONFIG_SWIOTLB
>  	swiotlb_init(0);
>  #endif
> 
> +	register_page_bootmem_info();
>  	high_memory = (void *) __va(max_low_pfn * PAGE_SIZE);
>  	set_max_mapnr(max_pfn);
>  	free_all_bootmem();
> Index: powerpc/mm/Kconfig
> ===================================================================
> --- powerpc.orig/mm/Kconfig
> +++ powerpc/mm/Kconfig
> @@ -183,7 +183,7 @@ config MEMORY_HOTPLUG_SPARSE
>  config MEMORY_HOTREMOVE
>  	bool "Allow for memory hot remove"
>  	select MEMORY_ISOLATION
> -	select HAVE_BOOTMEM_INFO_NODE if X86_64
> +	select HAVE_BOOTMEM_INFO_NODE if (X86_64 || PPC64)
>  	depends on MEMORY_HOTPLUG && ARCH_ENABLE_MEMORY_HOTREMOVE
>  	depends on MIGRATION
> 
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 2/2] Register bootmem pages at boot on powerpc

[Nathan Fontenot]

On 08/11/2013 07:19 PM, Benjamin Herrenschmidt wrote:
> On Fri, 2013-08-09 at 10:32 -0500, Nathan Fontenot wrote:
> 
>> +void register_page_bootmem_memmap(unsigned long section_nr,
>> +				  struct page *start_page, unsigned long size)
>> +{
>> +	WARN_ONCE(1, KERN_INFO
>> +		  "Sparse Vmemmap not fully supported for bootmem info nodes\n");
>> +}
>>  #endif /* CONFIG_SPARSEMEM_VMEMMAP */
> 
> But SPARSEMEM_VMEMMAP is our default on ppc64 pseries ... and you are
> select'ing the new option, so it looks like we are missing something
> here...
> 
> Can you tell me a bit more, the above makes me nervous...

Ok, I agree. that message isn't quite right.

What I wanted to convey is that memory hotplug is not fully supported
on powerpc with SPARSE_VMEMMAP enabled.. Perhaps the message should read
"Memory hotplug is not fully supported for bootmem info nodes".

Thoughts?

-Nathan

> 
> Cheers,
> Ben.
> 
>> Index: powerpc/arch/powerpc/mm/mem.c
>> ===================================================================
>> --- powerpc.orig/arch/powerpc/mm/mem.c
>> +++ powerpc/arch/powerpc/mm/mem.c
>> @@ -297,12 +297,21 @@ void __init paging_init(void)
>>  }
>>  #endif /* ! CONFIG_NEED_MULTIPLE_NODES */
>>
>> +static void __init register_page_bootmem_info(void)
>> +{
>> +	int i;
>> +
>> +	for_each_online_node(i)
>> +		register_page_bootmem_info_node(NODE_DATA(i));
>> +}
>> +
>>  void __init mem_init(void)
>>  {
>>  #ifdef CONFIG_SWIOTLB
>>  	swiotlb_init(0);
>>  #endif
>>
>> +	register_page_bootmem_info();
>>  	high_memory = (void *) __va(max_low_pfn * PAGE_SIZE);
>>  	set_max_mapnr(max_pfn);
>>  	free_all_bootmem();
>> Index: powerpc/mm/Kconfig
>> ===================================================================
>> --- powerpc.orig/mm/Kconfig
>> +++ powerpc/mm/Kconfig
>> @@ -183,7 +183,7 @@ config MEMORY_HOTPLUG_SPARSE
>>  config MEMORY_HOTREMOVE
>>  	bool "Allow for memory hot remove"
>>  	select MEMORY_ISOLATION
>> -	select HAVE_BOOTMEM_INFO_NODE if X86_64
>> +	select HAVE_BOOTMEM_INFO_NODE if (X86_64 || PPC64)
>>  	depends on MEMORY_HOTPLUG && ARCH_ENABLE_MEMORY_HOTREMOVE
>>  	depends on MIGRATION
>>
>>
>> _______________________________________________
>> Linuxppc-dev mailing list
>> Linuxppc-dev@lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/linuxppc-dev
> 
> 

Re: [PATCH 2/2] Register bootmem pages at boot on powerpc

[Benjamin Herrenschmidt]

On Mon, 2013-08-12 at 08:01 -0500, Nathan Fontenot wrote:
> > Can you tell me a bit more, the above makes me nervous...
> 
> Ok, I agree. that message isn't quite right.
> 
> What I wanted to convey is that memory hotplug is not fully supported
> on powerpc with SPARSE_VMEMMAP enabled.. Perhaps the message should read
> "Memory hotplug is not fully supported for bootmem info nodes".
> 
> Thoughts?

Since SPARSE_VMEMMAP is our default and enabled in our distros, that mean
that memory hotplug isn't fully supported for us in general ?

What do you mean by "not fully supported" ? What precisely is missing ?
What will happen if one tries to plug or unplug memory?

Shouldn't we fix it ?

Cheers,
Ben.

Re: [PATCH 2/2] Register bootmem pages at boot on powerpc

[Nathan Fontenot]

On 08/12/2013 04:13 PM, Benjamin Herrenschmidt wrote:
> On Mon, 2013-08-12 at 08:01 -0500, Nathan Fontenot wrote:
>>> Can you tell me a bit more, the above makes me nervous...
>>
>> Ok, I agree. that message isn't quite right.
>>
>> What I wanted to convey is that memory hotplug is not fully supported
>> on powerpc with SPARSE_VMEMMAP enabled.. Perhaps the message should read
>> "Memory hotplug is not fully supported for bootmem info nodes".
>>
>> Thoughts?
> 
> Since SPARSE_VMEMMAP is our default and enabled in our distros, that mean
> that memory hotplug isn't fully supported for us in general ?

Actually... We have had the distros (at least SLES 11 and RHEL 6 releases)
disable SPARSE_VMEMMAP in their releases.

> 
> What do you mean by "not fully supported" ? What precisely is missing ?
> What will happen if one tries to plug or unplug memory?

I don't know everything that is missing, but there are several routines
that need to be defined for power to support memory hotplug with SPARSE_VMEMMAP.

> 
> Shouldn't we fix it ?

Working on it, but it's not there yet.

> 
> Cheers,
> Ben.
> 
> 

Re: [PATCH 2/2] Register bootmem pages at boot on powerpc

[Benjamin Herrenschmidt]

On Mon, 2013-08-12 at 16:25 -0500, Nathan Fontenot wrote:
> On 08/12/2013 04:13 PM, Benjamin Herrenschmidt wrote:
> > On Mon, 2013-08-12 at 08:01 -0500, Nathan Fontenot wrote:
> >>> Can you tell me a bit more, the above makes me nervous...
> >>
> >> Ok, I agree. that message isn't quite right.
> >>
> >> What I wanted to convey is that memory hotplug is not fully supported
> >> on powerpc with SPARSE_VMEMMAP enabled.. Perhaps the message should read
> >> "Memory hotplug is not fully supported for bootmem info nodes".
> >>
> >> Thoughts?
> > 
> > Since SPARSE_VMEMMAP is our default and enabled in our distros, that mean
> > that memory hotplug isn't fully supported for us in general ?
> 
> Actually... We have had the distros (at least SLES 11 and RHEL 6 releases)
> disable SPARSE_VMEMMAP in their releases.

Yuck ! That has a significant impact on performances... Additionally our
VFIO implementation for KVM requires SPARSE_VMEMMAP. Why is it that this
was never fixed in all these years ?

> > 
> > What do you mean by "not fully supported" ? What precisely is missing ?
> > What will happen if one tries to plug or unplug memory?
> 
> I don't know everything that is missing, but there are several routines
> that need to be defined for power to support memory hotplug with SPARSE_VMEMMAP.
> 
> > 
> > Shouldn't we fix it ?
> 
> Working on it, but it's not there yet.

Ok, thanks.

Cheers,
Ben.

> > 
> > Cheers,
> > Ben.
> > 
> > 
> 
> --
> 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>