Correct memory hotplug remove

PATCH Superseded Nathan Fontenot 2 participants 8 messages Aug 9, 2013 – Aug 12, 2013

Subsystems (3)

Memory management PowerPC MM core

lore

Share & tools

lore

Nathan Fontenot

author

2013-08-09 15:29 UTC #0 lore raw

cover Correct memory hotplug remove

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

Nathan Fontenot

author

2013-08-09 15:30 UTC #1 lore raw

PATCH 1/2 Mark memory resources as busy

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 <[email protected]>

---
 arch/powerpc/mm/mem.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/powerpc/mm/mem.c +1 -1

Index: 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);
 		}
 	}

Nathan Fontenot

author

2013-08-09 15:32 UTC #2 lore raw

PATCH 2/2 Register bootmem pages at boot on powerpc

In reply to Nathan Fontenot

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 <[email protected]>

---
 arch/powerpc/mm/init_64.c |    6 ++++++
 arch/powerpc/mm/mem.c     |    9 +++++++++
 mm/Kconfig                |    2 +-
 3 files changed, 16 insertions(+), 1 deletion(-)

arch/powerpc/mm/init_64.c +6 -0

Index: 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 */

arch/powerpc/mm/mem.c +9 -0

Index: 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();

mm/Kconfig +1 -1

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

Benjamin Herrenschmidt

2013-08-12 00:19 UTC #3 lore raw

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

arch/powerpc/mm/init_64.c void vmemmap_free(unsigned long start, u · line 309

Quoted 7 lines · Nathan Fontenot

arch/powerpc/mm/init_64.c +6 -0

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

Quoted 40 lines

Index: powerpc/arch/powerpc/mm/mem.c
===================================================================

arch/powerpc/mm/mem.c +9 -0

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

mm/Kconfig +1 -1

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

Quoted 4 lines

_______________________________________________
Linuxppc-dev mailing list
[email protected]
https://lists.ozlabs.org/listinfo/linuxppc-dev

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Nathan Fontenot

author

2013-08-12 13:02 UTC #4 lore raw

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

Quoted 14 lines · Benjamin Herrenschmidt

arch/powerpc/mm/init_64.c void vmemmap_free(unsigned long start, u · line 309
Quoted 7 lines · Nathan Fontenot
arch/powerpc/mm/init_64.c +6 -0
+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...

Quoted 1 line

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

Quoted 50 lines

Cheers,
Ben.

Quoted 44 lines

Index: powerpc/arch/powerpc/mm/mem.c
===================================================================

arch/powerpc/mm/mem.c +9 -0

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

mm/Kconfig +1 -1

@@ -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
[email protected]
https://lists.ozlabs.org/listinfo/linuxppc-dev

Benjamin Herrenschmidt

2013-08-12 21:13 UTC #5 lore raw

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

Quoted 9 lines · Nathan Fontenot

Quoted 1 line
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.

Nathan Fontenot

author

2013-08-12 21:25 UTC #6 lore raw

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

Quoted 5 lines · Benjamin Herrenschmidt

Quoted 4 lines · Nathan Fontenot
Quoted 1 line
Can you tell me a bit more, the above makes me nervous...
Ok, I agree. that message isn't quite right.

Quoted 8 lines

Quoted 5 lines
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.

Quoted 3 lines

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.

Quoted 2 lines

Shouldn't we fix it ?

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

Quoted 5 lines

Cheers,
Ben.

Benjamin Herrenschmidt

2013-08-12 21:59 UTC #7 lore raw

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

Quoted 10 lines · Nathan Fontenot

Quoted 9 lines · Benjamin Herrenschmidt
Quoted 8 lines · Nathan Fontenot
Quoted 1 line
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".

Quoted 7 lines

Quoted 4 lines
Quoted 1 line
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 ?

Quoted 11 lines

Quoted 3 lines
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.
Quoted 2 lines
Shouldn't we fix it ?
Working on it, but it's not there yet.

Ok, thanks.

Cheers,
Ben.

Quoted 11 lines

Quoted 5 lines
Cheers,
Ben.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>