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

John Hubbard jhubbard at nvidia.com
Sat Jul 15 09:14:11 AEST 2023

On 7/13/23 02:08, David Hildenbrand wrote:
>>       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.)
> Did some digging:
> commit 65d9a9a60fd71be964effb2e94747a6acb6e7015
> Author: Naveen N. Rao <naveen.n.rao at linux.vnet.ibm.com>
> Date:   Fri Jul 1 13:04:04 2022 +0530
>      kexec_file: drop weak attribute from functions
>      As requested
>      (http://lkml.kernel.org/r/87ee0q7b92.fsf@email.froward.int.ebiederm.org),
>      this series converts weak functions in kexec to use the #ifdef approach.
>      Quoting the 3e35142ef99fe ("kexec_file: drop weak attribute from
>      arch_kexec_apply_relocations[_add]") changelog:
>      : Since commit d1bcae833b32f1 ("ELF: Don't generate unused section symbols")
>      : [1], binutils (v2.36+) started dropping section symbols that it thought
>      : were unused.  This isn't an issue in general, but with kexec_file.c, gcc
>      : is placing kexec_arch_apply_relocations[_add] into a separate
>      : .text.unlikely section and the section symbol ".text.unlikely" is being
>      : dropped.  Due to this, recordmcount is unable to find a non-weak symbol in
>      : .text.unlikely to generate a relocation record against.
>      This patch (of 2);
>      Drop __weak attribute from functions in kexec_file.c:
>      - arch_kexec_kernel_image_probe()
>      - arch_kimage_file_post_load_cleanup()
>      - arch_kexec_kernel_image_load()
>      - arch_kexec_locate_mem_hole()
>      - arch_kexec_kernel_verify_sig()
>      arch_kexec_kernel_image_load() calls into kexec_image_load_default(), so
>      drop the static attribute for the latter.
>      arch_kexec_kernel_verify_sig() is not overridden by any architecture, so
>      drop the __weak attribute.
>      Link: https://lkml.kernel.org/r/cover.1656659357.git.naveen.n.rao@linux.vnet.ibm.com
>      Link: https://lkml.kernel.org/r/2cd7ca1fe4d6bb6ca38e3283c717878388ed6788.1656659357.git.naveen.n.rao@linux.vnet.ibm.com
>      Signed-off-by: Naveen N. Rao <naveen.n.rao at linux.vnet.ibm.com>
>      Suggested-by: Eric Biederman <ebiederm at xmission.com>
>      Signed-off-by: Andrew Morton <akpm at linux-foundation.org>
>      Signed-off-by: Mimi Zohar <zohar at linux.ibm.com>
> So, in general, it's use seems to be fine (unless some tool actually bails out).
> https://lore.kernel.org/all/87ee0q7b92.fsf@email.froward.int.ebiederm.org/T/#u
> Also mentions that__weak and non __weak variants ending up in the vmlinux. Did not
> check if that's actually (still) the case.

OK, I looked at that commit and the associated discussion, and now have a
pretty clear picture of the preferred ways to do arch overrides.

Thanks for taking the time to look into it, and also to explain it.
Much appreciated!

John Hubbard

More information about the Linuxppc-dev mailing list