Re: [PATCH v4 5/6] x86/numa: Provide a range-to-target_node lookup facility

From: Ingo Molnar
Date: Thu Feb 13 2020 - 04:37:03 EST



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

> The DEV_DAX_KMEM facility is a generic mechanism to allow device-dax
> instances, fronting performance-differentiated-memory like pmem, to be
> added to the System RAM pool. The numa node for that hot-added memory is
> derived from the device-dax instance's 'target_node' attribute.
>
> Recall that the 'target_node' is the ACPI-PXM-to-node translation for
> memory when it comes online whereas the 'numa_node' attribute of the
> device represents the closest online cpu node.
>
> Presently useful target_node information from the ACPI SRAT is discarded
> with the expectation that "Reserved" memory will never be onlined. Now,
> DEV_DAX_KMEM violates that assumption, there is a need to retain the
> translation. Move, rather than discard, numa_memblk data to a secondary
> array that memory_add_physaddr_to_target_node() may consider at a later
> point in time.
>
> 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>
> Reported-by: kbuild test robot <lkp@xxxxxxxxx>
> Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
> ---
> arch/x86/mm/numa.c | 68 +++++++++++++++++++++++++++++++++++++++++++-------
> include/linux/numa.h | 8 +++++-
> mm/mempolicy.c | 5 ++++
> 3 files changed, 70 insertions(+), 11 deletions(-)
>
> diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
> index 5289d9d6799a..f2c8fca36f28 100644
> --- a/arch/x86/mm/numa.c
> +++ b/arch/x86/mm/numa.c
> @@ -26,6 +26,7 @@ struct pglist_data *node_data[MAX_NUMNODES] __read_mostly;
> EXPORT_SYMBOL(node_data);
>
> static struct numa_meminfo numa_meminfo __initdata_numa;
> +static struct numa_meminfo numa_reserved_meminfo __initdata_numa;
>
> static int numa_distance_cnt;
> static u8 *numa_distance;
> @@ -164,6 +165,26 @@ void __init numa_remove_memblk_from(int idx, struct numa_meminfo *mi)
> (mi->nr_blks - idx) * sizeof(mi->blk[0]));
> }
>
> +/**
> + * numa_move_memblk - Move one numa_memblk from one numa_meminfo to another
> + * @dst: numa_meminfo to move block to
> + * @idx: Index of memblk to remove
> + * @src: numa_meminfo to remove memblk from
> + *
> + * If @dst is non-NULL add it at the @dst->nr_blks index and increment
> + * @dst->nr_blks, then remove it from @src.
> + */
> +static void __init numa_move_memblk(struct numa_meminfo *dst, int idx,
> + struct numa_meminfo *src)

Nit, this is obviously not how we format function definitions if
checkpatch complains about the col80 limit.


> +{
> + if (dst) {
> + memcpy(&dst->blk[dst->nr_blks], &src->blk[idx],
> + sizeof(struct numa_memblk));

This linebreak is actually unnecessary ...

Thanks,

Ingo