* Re: [PATCH V2] memory-hotplug: revert register_page_bootmem_info_node() to empty when platform related code is not implemented
2013-01-15 10:20 [PATCH V2] memory-hotplug: revert register_page_bootmem_info_node() to empty when platform related code is not implemented Lin Feng
@ 2013-01-15 14:20 ` Michal Hocko
2013-01-16 2:09 ` Lin Feng
0 siblings, 1 reply; 3+ messages in thread
From: Michal Hocko @ 2013-01-15 14:20 UTC (permalink / raw)
To: Lin Feng
Cc: akpm, linux-mm, kamezawa.hiroyu, mel, minchan, aquini, wency,
jiang.liu, laijs, isimatu.yasuaki, linux-kernel
On Tue 15-01-13 18:20:03, Lin Feng wrote:
> Memory-hotplug codes for x86_64 have been implemented by patchset:
> https://lkml.org/lkml/2013/1/9/124
> While other platforms haven't been completely implemented yet.
>
> If we enable both CONFIG_MEMORY_HOTPLUG_SPARSE and CONFIG_SPARSEMEM_VMEMMAP,
> register_page_bootmem_info_node() may be buggy, which is a hotplug generic
> function but falling back to call platform related function
> register_page_bootmem_memmap().
>
> Other platforms such as powerpc it's not implemented, so on such platforms,
> revert them to empty as they were before.
>
> It's implemented by adding a new Kconfig option named
> CONFIG_HAVE_BOOTMEM_INFO_NODE, which will be automatically selected by
> supported archs(currently only on x86_64).
>
> Reported-by: Michal Hocko <mhocko@suse.cz>
> Signed-off-by: Lin Feng <linfeng@cn.fujitsu.com>
> ---
> ChangeLog v1->v2:
> - Add a Kconfig option named HAVE_BOOTMEM_INFO_NODE suggested by Michal, which
> will be automatically selected by supported archs(currently only on x86_64).
> ---
> mm/Kconfig | 8 ++++++++
> mm/memory_hotplug.c | 7 +++++++
> 2 files changed, 15 insertions(+), 0 deletions(-)
>
> diff --git a/mm/Kconfig b/mm/Kconfig
> index 278e3ab..f8c5799 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -162,10 +162,18 @@ config MOVABLE_NODE
> Say Y here if you want to hotplug a whole node.
> Say N here if you want kernel to use memory on all nodes evenly.
>
> +#
> +# Only be set on architectures that have completely implemented memory hotplug
> +# feature. If you are not sure, don't touch it.
> +#
> +config HAVE_BOOTMEM_INFO_NODE
> + def_bool n
> +
> # eventually, we can have this option just 'select SPARSEMEM'
> config MEMORY_HOTPLUG
> bool "Allow for memory hot-add"
> select MEMORY_ISOLATION
> + select HAVE_BOOTMEM_INFO_NODE if X86_64
> depends on SPARSEMEM || X86_64_ACPI_NUMA
> depends on HOTPLUG && ARCH_ENABLE_MEMORY_HOTPLUG
> depends on (IA64 || X86 || PPC_BOOK3S_64 || SUPERH || S390)
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 8aa2b56..ef7a5c8 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -189,6 +189,7 @@ static void register_page_bootmem_info_section(unsigned long start_pfn)
> }
> #endif
>
> +#ifdef CONFIG_HAVE_BOOTMEM_INFO_NODE
> void register_page_bootmem_info_node(struct pglist_data *pgdat)
> {
> unsigned long i, pfn, end_pfn, nr_pages;
> @@ -230,6 +231,12 @@ void register_page_bootmem_info_node(struct pglist_data *pgdat)
> register_page_bootmem_info_section(pfn);
> }
> }
> +#else
> +void register_page_bootmem_info_node(struct pglist_data *pgdat)
> +{
> + /* TODO */
> +}
I think that TODO is misleading here because the function should be
empty if !CONFIG_HAVE_BOOTMEM_INFO_NODE. I would also suggest updating
include/linux/memory_hotplug.h and removing the arch specific functions
without any implementation. Something like (untested) patch below:
---
diff --git a/arch/ia64/mm/discontig.c b/arch/ia64/mm/discontig.c
index 882a0fd..cb5e1ff 100644
--- a/arch/ia64/mm/discontig.c
+++ b/arch/ia64/mm/discontig.c
@@ -827,9 +827,4 @@ void vmemmap_free(struct page *memmap, unsigned long nr_pages)
{
}
-void register_page_bootmem_memmap(unsigned long section_nr,
- struct page *start_page, unsigned long size)
-{
- /* TODO */
-}
#endif
diff --git a/arch/powerpc/mm/init_64.c b/arch/powerpc/mm/init_64.c
index 2969591..7e2246f 100644
--- a/arch/powerpc/mm/init_64.c
+++ b/arch/powerpc/mm/init_64.c
@@ -302,10 +302,5 @@ void vmemmap_free(struct page *memmap, unsigned long nr_pages)
{
}
-void register_page_bootmem_memmap(unsigned long section_nr,
- struct page *start_page, unsigned long size)
-{
- /* TODO */
-}
#endif /* CONFIG_SPARSEMEM_VMEMMAP */
diff --git a/arch/s390/mm/vmem.c b/arch/s390/mm/vmem.c
index 4073156..139bb48 100644
--- a/arch/s390/mm/vmem.c
+++ b/arch/s390/mm/vmem.c
@@ -240,12 +240,6 @@ void vmemmap_free(struct page *memmap, unsigned long nr_pages)
{
}
-void register_page_bootmem_memmap(unsigned long section_nr,
- struct page *start_page, unsigned long size)
-{
- /* TODO */
-}
-
/*
* Add memory segment to the segment list if it doesn't overlap with
* an already present segment.
diff --git a/arch/sparc/mm/init_64.c b/arch/sparc/mm/init_64.c
index f0ef3c2..07571a27 100644
--- a/arch/sparc/mm/init_64.c
+++ b/arch/sparc/mm/init_64.c
@@ -2236,11 +2236,6 @@ void vmemmap_free(struct page *memmap, unsigned long nr_pages)
{
}
-void register_page_bootmem_memmap(unsigned long section_nr,
- struct page *start_page, unsigned long size)
-{
- /* TODO */
-}
#endif /* CONFIG_SPARSEMEM_VMEMMAP */
static void prot_init_common(unsigned long page_none,
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index d1b8257..defe6ee 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -1317,7 +1317,7 @@ vmemmap_populate(struct page *start_page, unsigned long size, int node)
return 0;
}
-#ifdef CONFIG_MEMORY_HOTPLUG_SPARSE
+#if defined(CONFIG_MEMORY_HOTPLUG_SPARSE) && defined(CONFIG_HAVE_BOOTMEM_INFO_NODE)
void register_page_bootmem_memmap(unsigned long section_nr,
struct page *start_page, unsigned long size)
{
diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
index f60e728..34b0511 100644
--- a/include/linux/memory_hotplug.h
+++ b/include/linux/memory_hotplug.h
@@ -174,7 +174,13 @@ static inline void arch_refresh_nodedata(int nid, pg_data_t *pgdat)
#endif /* CONFIG_NUMA */
#endif /* CONFIG_HAVE_ARCH_NODEDATA_EXTENSION */
+#ifdef CONFIG_HAVE_BOOTMEM_INFO_NODE
extern void register_page_bootmem_info_node(struct pglist_data *pgdat);
+#else
+static void register_page_bootmem_info_node(struct pglist_data *pgdat)
+{
+}
+#endif
extern void put_page_bootmem(struct page *page);
extern void get_page_bootmem(unsigned long ingo, struct page *page,
unsigned long type);
--
Michal Hocko
SUSE Labs
--
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>
^ permalink raw reply related [flat|nested] 3+ messages in thread