On 11/2/20 3:52 PM, Dan Williams wrote: > On Sat, Oct 31, 2020 at 2:10 AM Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote: >> >> On Thu, Oct 29, 2020 at 07:29:45PM -0700, Dan Williams wrote: >>> The core-mm has a default __weak implementation of phys_to_target_node() >>> when the architecture does not override it. That symbol is exported >>> for modules. However, while the export in mm/memory_hotplug.c exported >>> the symbol in the configuration cases of: >> >> Which just means that we should never export weak symbols. So instead >> of hacking around this introduce a symbol that indicates that the >> architecture impements phys_to_target_node, and don't defined it at all >> in common code for that case. > > So I agree with this, but it made me realize that the way > memory_add_physaddr_to_nid() was defined as an exported weak symbol is > similarly broken. > >>> --- a/mm/memory_hotplug.c >>> +++ b/mm/memory_hotplug.c >>> @@ -365,9 +365,14 @@ int __weak phys_to_target_node(u64 start) >>> start); >>> return 0; >>> } >>> + >>> +/* If the arch did not export a strong symbol, export the weak one. */ >>> +#ifndef CONFIG_NUMA_KEEP_MEMINFO >>> EXPORT_SYMBOL_GPL(phys_to_target_node); >>> #endif >>> >>> +#endif >> >> i.e. move the ifdef to include the actual phys_to_target_node >> definition, and remove the __weak from it here. > > The trick is finding an arch common way to pick up the presence of the > phys_to_target_node() override, and it still has the wart of ifdefery > in C code. > > I went a bit deeper and moved all the fallback routines to > linux/numa.h and the overrides in all archs that care to > asm/sparsemem.h. Note that asm/sparsemem.h was not my first choice, > but it happened to be where powerpc was already defining its > phys-addr-to-node-id infrastructure, and my first choice header, > asm/numa.h, is not universally available. > > The attached patch is going through some kbuild-robot exposure to make > sure I did not break anything else. > Works for me. Thanks. Acked-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx> # build-tested -- ~Randy
