Re: [PATCH v3 3/7] mm/hotplug: Allow architecture to override memmap on memory support check

[John Hubbard <jhubbard@nvidia.com>]

On 7/11/23 09:09, David Hildenbrand wrote:
...
>>> Can we make that a __weak function instead?
>>
>> We can. It is confusing because we do have these two patterns within the kernel where we use
>>
>> #ifndef x
>> #endif
>>
>> vs
>>
>> __weak x
>>
>> What is the recommended way to override ? I have mostly been using #ifndef for most of the arch overrides till now.
>>
> 
> I think when placing the implementation in a C file, it's __weak. But don't ask me :)
> 
> We do this already for arch_get_mappable_range() in mm/memory_hotplug.c and IMHO it looks quite nice.
> 

It does look nice. I always forget which parts are supposed to be
__weak, so I went to check Documentation/ , and it was quite
entertaining. There are only two search hits: one trivial reference in
Documentation/conf.py, and the other in checkpatch.rst, which says:

   **WEAK_DECLARATION**
     Using weak declarations like __attribute__((weak)) or __weak
     can have unintended link defects.  Avoid using them.

...which seems deeply out of touch with how arch layers work these days,
doesn't it? (This is not rhetorical; I'm asking in order to get an
opinion or two on the topic.)


thanks,
-- 
John Hubbard
NVIDIA