Re: [PATCH v2 17/18] x86/numa: Provide a range-to-target_node lookup facility

[Dan Williams <dan.j.williams@intel.com>]

On Sun, Nov 17, 2019 at 10:00 AM Dan Williams <dan.j.williams@intel.com> 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.
>
> Note that memory_add_physaddr_to_nid() is currently only available on
> CONFIG_MEMORY_HOTPLUG=y platforms whereas the target node information
> may be useful on CONFIG_MEMORY_HOTPLUG=n builds, hence why it is calling
> phys_to_target_node() and optionally defined by asm/io.h rather than a
> memory_add_physaddr_to_target_nid() helper that lives in
> include/linux/memory_hotplug.h.
>
> Cc: Dave Hansen <dave.hansen@linux.intel.com>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: <x86@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Michal Hocko <mhocko@suse.com>
> Reported-by: kbuild test robot <lkp@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  arch/x86/mm/numa.c   |   76 +++++++++++++++++++++++++++++++++++++++++++++++---
>  include/linux/numa.h |    8 +++++
>  mm/mempolicy.c       |    5 +++
>  3 files changed, 83 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
> index 4123100e0eaf..f4f02ac0c465 100644
> --- a/arch/x86/mm/numa.c
> +++ b/arch/x86/mm/numa.c
> @@ -31,6 +31,24 @@ __initdata
>  #endif
>  ;
>
> +/*
> + * Presently, DEV_DAX_KMEM is the only kernel facility that might
> + * convert Reserved or Soft Reserved memory to System RAM.
> + */
> +#if IS_ENABLED(CONFIG_DEV_DAX_KMEM)
> +static struct numa_meminfo __numa_reserved_meminfo;
> +
> +static struct numa_meminfo *numa_reserved_meminfo(void)
> +{
> +       return &__numa_reserved_meminfo;
> +}
> +#else
> +static struct numa_meminfo *numa_reserved_meminfo(void)
> +{
> +       return NULL;
> +}
> +#endif
> +
>  static int numa_distance_cnt;
>  static u8 *numa_distance;
>
> @@ -168,6 +186,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)
> +{
> +       if (dst) {
> +               memcpy(&dst->blk[dst->nr_blks], &src->blk[idx],
> +                               sizeof(struct numa_memblk));
> +               dst->nr_blks++;
> +       }
> +       numa_remove_memblk_from(idx, src);
> +}
> +
>  /**
>   * numa_add_memblk - Add one numa_memblk to numa_meminfo
>   * @nid: NUMA node ID of the new memblk
> @@ -245,7 +283,7 @@ int __init numa_cleanup_meminfo(struct numa_meminfo *mi)
>                 if (bi->start >= bi->end ||
>                     !memblock_overlaps_region(&memblock.memory,
>                         bi->start, bi->end - bi->start))
> -                       numa_remove_memblk_from(i--, mi);
> +                       numa_move_memblk(numa_reserved_meminfo(), i--, mi);
>         }
>
>         /* merge neighboring / overlapping entries */
> @@ -881,16 +919,44 @@ EXPORT_SYMBOL(cpumask_of_node);
>
>  #endif /* !CONFIG_DEBUG_PER_CPU_MAPS */
>
> +static int meminfo_to_nid(struct numa_meminfo *mi, u64 start, int *nid)
> +{
> +       int i;
> +
> +       for (i = 0; mi && i < mi->nr_blks; i++)
> +               if (mi->blk[i].start <= start && mi->blk[i].end > start) {
> +                       *nid = mi->blk[i].nid;
> +                       break;
> +               }
> +       return i;
> +}
> +
> +int phys_to_target_node(phys_addr_t start)
> +{
> +       struct numa_meminfo *mi = &numa_meminfo;
> +       int nid = mi->blk[0].nid;
> +       int i = meminfo_to_nid(mi, start, &nid);
> +
> +       /*
> +        * Prefer online nodes, but if reserved memory might be
> +        * hot-added continue the search with reserved ranges.
> +        */
> +       if (i < mi->nr_blks)
> +               return nid;
> +
> +       mi = numa_reserved_meminfo();
> +       meminfo_to_nid(mi, start, &nid);
> +       return nid;
> +}

The kbuild-robot points out that this function causes a section
mismatch warning in the CONFIG_MEMORY_HOTPLUG=n case. It touches
numa_meminfo which gets marked __init in that configuration. Given the
numa information is useful independent of memory hotplug I am going to
add a patch to add a CONFIG_KEEP_NUMA configuration symbol that is
selected by CONFIG_MEMORY_HOTPLUG, or any driver that wants to use
phys_to_target_node(). Then use CONFIG_KEEP_NUMA to gate whether
numa_meminfo is marked __init, or not.