Re: [PATCH v4 4/6] x86/mm: Introduce CONFIG_KEEP_NUMA

From: Ingo Molnar
Date: Thu Feb 13 2020 - 04:32:34 EST



* Dan Williams <dan.j.williams@xxxxxxxxx> wrote:

> Currently x86 numa_meminfo is marked __initdata in the
> CONFIG_MEMORY_HOTPLUG=n case. In support of a new facility to allow
> drivers to map reserved memory to a 'target_node'
> (phys_to_target_node()), add support for removing the __initdata
> designation for those users. Both memory hotplug and
> phys_to_target_node() users select CONFIG_KEEP_NUMA to tell the arch to
> maintain its physical address to numa mapping infrastructure post init.
>
> Cc: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
> Cc: Andy Lutomirski <luto@xxxxxxxxxx>
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> Cc: Borislav Petkov <bp@xxxxxxxxx>
> Cc: "H. Peter Anvin" <hpa@xxxxxxxxx>
> Cc: <x86@xxxxxxxxxx>
> Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> Cc: David Hildenbrand <david@xxxxxxxxxx>
> Cc: Michal Hocko <mhocko@xxxxxxxx>
> Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
> ---
> arch/x86/mm/numa.c | 6 +-----
> include/linux/numa.h | 6 ++++++
> mm/Kconfig | 5 +++++
> 3 files changed, 12 insertions(+), 5 deletions(-)

The concept and the x86 portions look sane, just a few minor nits:

>
> diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
> index 99f7a68738f0..5289d9d6799a 100644
> --- a/arch/x86/mm/numa.c
> +++ b/arch/x86/mm/numa.c
> @@ -25,11 +25,7 @@ nodemask_t numa_nodes_parsed __initdata;
> struct pglist_data *node_data[MAX_NUMNODES] __read_mostly;
> EXPORT_SYMBOL(node_data);
>
> -static struct numa_meminfo numa_meminfo
> -#ifndef CONFIG_MEMORY_HOTPLUG
> -__initdata
> -#endif
> -;
> +static struct numa_meminfo numa_meminfo __initdata_numa;
>
> static int numa_distance_cnt;
> static u8 *numa_distance;
> diff --git a/include/linux/numa.h b/include/linux/numa.h
> index 20f4e44b186c..c005ed6b807b 100644
> --- a/include/linux/numa.h
> +++ b/include/linux/numa.h
> @@ -13,6 +13,12 @@
>
> #define NUMA_NO_NODE (-1)
>
> +#ifdef CONFIG_KEEP_NUMA
> +#define __initdata_numa
> +#else
> +#define __initdata_numa __initdata
> +#endif
> +
> #ifdef CONFIG_NUMA
> int numa_map_to_online_node(int node);
> #else
> diff --git a/mm/Kconfig b/mm/Kconfig
> index ab80933be65f..001f1185eadf 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -139,6 +139,10 @@ config HAVE_FAST_GUP
> config ARCH_KEEP_MEMBLOCK
> bool
>
> +# Keep arch numa mapping infrastructure post-init.

s/numa/NUMA

Please also capitalize consistently in the rest of the series.

> +config KEEP_NUMA
> + bool


So most of our recent new NUMA options followed the naming pattern of:

CONFIG_NUMA_*

Such as CONFIG_NUMA_BALANCING or CONFIG_NUMA_EMU.

So I'd suggesting naming it to CONFIG_NUMA_KEEP, or, a bit more
descriptively, such as CONFIG_NUMA_KEEP_MAPPING or such?

'Keeping NUMA' is kind of lame - of course we keep NUMA. ;-)

Thanks,

Ingo