From: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
To: kexec@lists.infradead.org
Subject: [PATCH] kexec_file: Drop weak attribute from arch_kexec_apply_relocations[_add]
Date: Thu, 19 May 2022 14:43:38 +0530 [thread overview]
Message-ID: <1652950421.sq10xtycz9.naveen@linux.ibm.com> (raw)
In-Reply-To: <20220518134705.7ae186d5419b24d689a38ccc@linux-foundation.org>
Andrew Morton wrote:
> On Wed, 18 May 2022 23:48:28 +0530 "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
>
>> 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.
>>
>> Address this by dropping the weak attribute from these functions:
>> - arch_kexec_apply_relocations() is not overridden by any architecture
>> today, so just drop the weak attribute.
>> - arch_kexec_apply_relocations_add() is only overridden by x86 and s390.
>> Retain the function prototype for those and move the weak
>> implementation into the header as a static inline for other
>> architectures.
>>
>> ...
>>
>
> Sigh. This patch demonstrates why I like __weak :<
>
>> --- a/include/linux/kexec.h
>> +++ b/include/linux/kexec.h
>> @@ -229,6 +225,30 @@ extern int crash_exclude_mem_range(struct crash_mem *mem,
>> unsigned long long mend);
>> extern int crash_prepare_elf64_headers(struct crash_mem *mem, int kernel_map,
>> void **addr, unsigned long *sz);
>> +
>> +#if defined(CONFIG_X86_64) || defined(CONFIG_S390)
>
> Let's avoid listing the architectures here? Better to add
>
> select ARCH_HAVE_ARCH_KEXEC_APPLY_RELOCATIONS_ADD
>
> to arch/<arch>/Kconfig?
I followed the approach used in commit 6e7b64b9dd6d96 ("elfcore: fix
building with clang") since here again, it was overridden on only two
architectures. I also wanted to avoid touching the architecture headers
so as to make it simpler to backport.
But, as Michael points out, using a #ifdef isn't too much of a change
either. I also confirmed that those changes still apply cleanly all the
way back to v5.10. I've posted a v2 which takes this approach.
>
> Please cc me on any additional work on this.
I've copied you on the v2 patch. Thanks!
- Naveen
WARNING: multiple messages have this Message-ID (diff)
From: "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: linuxppc-dev@lists.ozlabs.org, kexec@lists.infradead.org,
Eric Biederman <ebiederm@xmission.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] kexec_file: Drop weak attribute from arch_kexec_apply_relocations[_add]
Date: Thu, 19 May 2022 14:43:38 +0530 [thread overview]
Message-ID: <1652950421.sq10xtycz9.naveen@linux.ibm.com> (raw)
In-Reply-To: <20220518134705.7ae186d5419b24d689a38ccc@linux-foundation.org>
Andrew Morton wrote:
> On Wed, 18 May 2022 23:48:28 +0530 "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
>
>> 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.
>>
>> Address this by dropping the weak attribute from these functions:
>> - arch_kexec_apply_relocations() is not overridden by any architecture
>> today, so just drop the weak attribute.
>> - arch_kexec_apply_relocations_add() is only overridden by x86 and s390.
>> Retain the function prototype for those and move the weak
>> implementation into the header as a static inline for other
>> architectures.
>>
>> ...
>>
>
> Sigh. This patch demonstrates why I like __weak :<
>
>> --- a/include/linux/kexec.h
>> +++ b/include/linux/kexec.h
>> @@ -229,6 +225,30 @@ extern int crash_exclude_mem_range(struct crash_mem *mem,
>> unsigned long long mend);
>> extern int crash_prepare_elf64_headers(struct crash_mem *mem, int kernel_map,
>> void **addr, unsigned long *sz);
>> +
>> +#if defined(CONFIG_X86_64) || defined(CONFIG_S390)
>
> Let's avoid listing the architectures here? Better to add
>
> select ARCH_HAVE_ARCH_KEXEC_APPLY_RELOCATIONS_ADD
>
> to arch/<arch>/Kconfig?
I followed the approach used in commit 6e7b64b9dd6d96 ("elfcore: fix
building with clang") since here again, it was overridden on only two
architectures. I also wanted to avoid touching the architecture headers
so as to make it simpler to backport.
But, as Michael points out, using a #ifdef isn't too much of a change
either. I also confirmed that those changes still apply cleanly all the
way back to v5.10. I've posted a v2 which takes this approach.
>
> Please cc me on any additional work on this.
I've copied you on the v2 patch. Thanks!
- Naveen
WARNING: multiple messages have this Message-ID (diff)
From: "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Eric Biederman <ebiederm@xmission.com>,
kexec@lists.infradead.org, linux-kernel@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org,
Michael Ellerman <mpe@ellerman.id.au>
Subject: Re: [PATCH] kexec_file: Drop weak attribute from arch_kexec_apply_relocations[_add]
Date: Thu, 19 May 2022 14:43:38 +0530 [thread overview]
Message-ID: <1652950421.sq10xtycz9.naveen@linux.ibm.com> (raw)
In-Reply-To: <20220518134705.7ae186d5419b24d689a38ccc@linux-foundation.org>
Andrew Morton wrote:
> On Wed, 18 May 2022 23:48:28 +0530 "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
>
>> 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.
>>
>> Address this by dropping the weak attribute from these functions:
>> - arch_kexec_apply_relocations() is not overridden by any architecture
>> today, so just drop the weak attribute.
>> - arch_kexec_apply_relocations_add() is only overridden by x86 and s390.
>> Retain the function prototype for those and move the weak
>> implementation into the header as a static inline for other
>> architectures.
>>
>> ...
>>
>
> Sigh. This patch demonstrates why I like __weak :<
>
>> --- a/include/linux/kexec.h
>> +++ b/include/linux/kexec.h
>> @@ -229,6 +225,30 @@ extern int crash_exclude_mem_range(struct crash_mem *mem,
>> unsigned long long mend);
>> extern int crash_prepare_elf64_headers(struct crash_mem *mem, int kernel_map,
>> void **addr, unsigned long *sz);
>> +
>> +#if defined(CONFIG_X86_64) || defined(CONFIG_S390)
>
> Let's avoid listing the architectures here? Better to add
>
> select ARCH_HAVE_ARCH_KEXEC_APPLY_RELOCATIONS_ADD
>
> to arch/<arch>/Kconfig?
I followed the approach used in commit 6e7b64b9dd6d96 ("elfcore: fix
building with clang") since here again, it was overridden on only two
architectures. I also wanted to avoid touching the architecture headers
so as to make it simpler to backport.
But, as Michael points out, using a #ifdef isn't too much of a change
either. I also confirmed that those changes still apply cleanly all the
way back to v5.10. I've posted a v2 which takes this approach.
>
> Please cc me on any additional work on this.
I've copied you on the v2 patch. Thanks!
- Naveen
next prev parent reply other threads:[~2022-05-19 9:13 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-18 18:18 [PATCH] kexec_file: Drop weak attribute from arch_kexec_apply_relocations[_add] Naveen N. Rao
2022-05-18 18:18 ` Naveen N. Rao
2022-05-18 18:18 ` Naveen N. Rao
2022-05-18 20:47 ` Andrew Morton
2022-05-18 20:47 ` Andrew Morton
2022-05-18 20:47 ` Andrew Morton
2022-05-19 9:13 ` Naveen N. Rao [this message]
2022-05-19 9:13 ` Naveen N. Rao
2022-05-19 9:13 ` Naveen N. Rao
2022-05-18 21:59 ` Eric W. Biederman
2022-05-18 21:59 ` Eric W. Biederman
2022-05-18 21:59 ` Eric W. Biederman
2022-05-19 2:58 ` Baoquan He
2022-05-19 2:58 ` Baoquan He
2022-05-19 2:58 ` Baoquan He
2022-05-19 9:28 ` Naveen N. Rao
2022-05-19 9:28 ` Naveen N. Rao
2022-05-19 9:28 ` Naveen N. Rao
2022-05-19 17:59 ` Eric W. Biederman
2022-05-19 17:59 ` Eric W. Biederman
2022-05-19 17:59 ` Eric W. Biederman
2022-05-20 10:46 ` Baoquan He
2022-05-20 10:46 ` Baoquan He
2022-05-20 10:46 ` Baoquan He
2022-05-20 19:25 ` Eric W. Biederman
2022-05-20 19:25 ` Eric W. Biederman
2022-05-20 19:25 ` Eric W. Biederman
2022-05-25 19:56 ` Andrew Morton
2022-05-25 19:56 ` Andrew Morton
2022-05-25 19:56 ` Andrew Morton
2022-05-26 11:00 ` Naveen N. Rao
2022-05-26 11:00 ` Naveen N. Rao
2022-05-26 11:00 ` Naveen N. Rao
2022-05-19 5:41 ` Michael Ellerman
2022-05-19 5:41 ` Michael Ellerman
2022-05-19 5:41 ` Michael Ellerman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1652950421.sq10xtycz9.naveen@linux.ibm.com \
--to=naveen.n.rao@linux.vnet.ibm.com \
--cc=kexec@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.