Re: [PATCH] x86/mm: Fix phys_to_target_node() export

[Randy Dunlap <rdunlap@infradead.org>]

On 11/2/20 3:52 PM, Dan Williams wrote:
> On Sat, Oct 31, 2020 at 2:10 AM Christoph Hellwig <hch@infradead.org> 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@infradead.org> # build-tested

-- 
~Randy