diff mbox

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

Message ID 52050B80.8010602@linux.vnet.ibm.com (mailing list archive)
State Superseded
Headers show

Commit Message

Nathan Fontenot Aug. 9, 2013, 3:32 p.m. UTC 
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(-)

Comments

Benjamin Herrenschmidt Aug. 12, 2013, 12:19 a.m. UTC | #1 
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
Nathan Fontenot Aug. 12, 2013, 1:01 p.m. UTC | #2 
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
> 
>
Benjamin Herrenschmidt Aug. 12, 2013, 9:13 p.m. UTC | #3 
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.
Nathan Fontenot Aug. 12, 2013, 9:25 p.m. UTC | #4 
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.
> 
>
Benjamin Herrenschmidt Aug. 12, 2013, 9:59 p.m. UTC | #5 
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>
diff mbox

Patch

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