Re: [PATCH v4] mm: Fix phys_to_target_node() and memory_add_physaddr_to_nid() exports

[Stephen Rothwell <sfr@canb.auug.org.au>]

[-- Attachment #1: Type: text/plain, Size: 3377 bytes --]

Hi Dan,

On Thu, 05 Nov 2020 14:20:45 -0800 Dan Williams <dan.j.williams@intel.com> wrote:
>
> The core-mm has a default __weak implementation of phys_to_target_node()
> to mirror the weak definition of memory_add_physaddr_to_nid(). That
> symbol is exported for modules. However, while the export in
> mm/memory_hotplug.c exported the symbol in the configuration cases of:
> 
> 	CONFIG_NUMA_KEEP_MEMINFO=y
> 	CONFIG_MEMORY_HOTPLUG=y
> 
> ...and:
> 
> 	CONFIG_NUMA_KEEP_MEMINFO=n
> 	CONFIG_MEMORY_HOTPLUG=y
> 
> ...it failed to export the symbol in the case of:
> 
> 	CONFIG_NUMA_KEEP_MEMINFO=y
> 	CONFIG_MEMORY_HOTPLUG=n
> 
> Not only is that broken, but Christoph points out that the kernel should
> not be exporting any __weak symbol, which means that
> memory_add_physaddr_to_nid() example that phys_to_target_node() copied
> is broken too.
> 
> Rework the definition of phys_to_target_node() and
> memory_add_physaddr_to_nid() to not require weak symbols. Move to the
> common arch override design-pattern of an asm header defining a symbol
> to replace the default implementation.
> 
> The only common header that all memory_add_physaddr_to_nid() producing
> architectures implement is asm/sparsemem.h. In fact, powerpc already
> defines its memory_add_physaddr_to_nid() helper in sparsemem.h.
> Double-down on that observation and define phys_to_target_node() where
> necessary in asm/sparsemem.h. An alternate consideration that was
> discarded was to put this override in asm/numa.h, but that entangles
> with the definition of MAX_NUMNODES relative to the inclusion of
> linux/nodemask.h, and requires powerpc to grow a new header.
> 
> The dependency on NUMA_KEEP_MEMINFO for DEV_DAX_HMEM_DEVICES is invalid
> now that the symbol is properly exported / stubbed in all combinations
> of CONFIG_NUMA_KEEP_MEMINFO and CONFIG_MEMORY_HOTPLUG.
> 
> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Tested-by: Randy Dunlap <rdunlap@infradead.org>
> Reported-by: Thomas Gleixner <tglx@linutronix.de>
> Tested-by: Thomas Gleixner <tglx@linutronix.de>
> Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Christoph Hellwig <hch@infradead.org>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Fixes: a035b6bf863e ("mm/memory_hotplug: introduce default phys_to_target_node() implementation")
> Cc: Joao Martins <joao.m.martins@oracle.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: x86@kernel.org
> Cc: Tony Luck <tony.luck@intel.com>
> Cc: Fenghua Yu <fenghua.yu@intel.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Vishal Verma <vishal.l.verma@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
> Changes since v3 [1]:
> - (Stephen) PowerPC header include dependencies make it difficult to
>   include asm/sparsemem.h in linux/numa.h due to missing definition of
>   pgprot.  Move the declaration of create_section_mapping() to
>   asm/mmzone.h. This has received a build success notification from the
>   kbuild-robot over 159 configs.

I replaced the previous version in linux-next with this today.  Thanks.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]