All of lore.kernel.org
 help / color / mirror / Atom feed
From: Baoquan He <bhe@redhat.com>
To: kexec@lists.infradead.org
Subject: [PATCH] kexec_file: Drop weak attribute from arch_kexec_apply_relocations[_add]
Date: Thu, 19 May 2022 10:58:19 +0800	[thread overview]
Message-ID: <YoWySwbszfdZS9LU@MiWiFi-R3L-srv> (raw)
In-Reply-To: <87ee0q7b92.fsf@email.froward.int.ebiederm.org>

Hi Eric,

On 05/18/22 at 04:59pm, Eric W. Biederman wrote:
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:
> 
> > 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.
> >
> > [1] https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=d1bcae833b32f1
> 
> Any chance you can also get machine_kexec_post_load,
> crash_free_reserved_phys_range, arch_kexec_protect_protect_crashkres,
> arch_kexec_unprotect_crashkres, arch_kexec_kernel_image_probe,
> arch_kexec_kernel_image_probe, arch_kimage_file_post_load_cleanup,
> arch_kexec_kernel_verify_sig, and arch_kexec_locate_mem_hole as well.
> 
> That is everything in kexec that uses a __weak symbol.  If we can't
> count on them working we might as well just get rid of the rest
> preemptively.

Is there a new rule that __weak is not suggested in kernel any more?
Please help provide a pointer if yes, so that I can learn that.

In my mind, __weak is very simple and clear as a mechanism to add
ARCH related functionality.

Thanks
Baoquan



WARNING: multiple messages have this Message-ID (diff)
From: Baoquan He <bhe@redhat.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: kexec@lists.infradead.org, linux-kernel@vger.kernel.org,
	"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>,
	linuxppc-dev@lists.ozlabs.org,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH] kexec_file: Drop weak attribute from arch_kexec_apply_relocations[_add]
Date: Thu, 19 May 2022 10:58:19 +0800	[thread overview]
Message-ID: <YoWySwbszfdZS9LU@MiWiFi-R3L-srv> (raw)
In-Reply-To: <87ee0q7b92.fsf@email.froward.int.ebiederm.org>

Hi Eric,

On 05/18/22 at 04:59pm, Eric W. Biederman wrote:
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:
> 
> > 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.
> >
> > [1] https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=d1bcae833b32f1
> 
> Any chance you can also get machine_kexec_post_load,
> crash_free_reserved_phys_range, arch_kexec_protect_protect_crashkres,
> arch_kexec_unprotect_crashkres, arch_kexec_kernel_image_probe,
> arch_kexec_kernel_image_probe, arch_kimage_file_post_load_cleanup,
> arch_kexec_kernel_verify_sig, and arch_kexec_locate_mem_hole as well.
> 
> That is everything in kexec that uses a __weak symbol.  If we can't
> count on them working we might as well just get rid of the rest
> preemptively.

Is there a new rule that __weak is not suggested in kernel any more?
Please help provide a pointer if yes, so that I can learn that.

In my mind, __weak is very simple and clear as a mechanism to add
ARCH related functionality.

Thanks
Baoquan


WARNING: multiple messages have this Message-ID (diff)
From: Baoquan He <bhe@redhat.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>,
	Michael Ellerman <mpe@ellerman.id.au>,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
	kexec@lists.infradead.org,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH] kexec_file: Drop weak attribute from arch_kexec_apply_relocations[_add]
Date: Thu, 19 May 2022 10:58:19 +0800	[thread overview]
Message-ID: <YoWySwbszfdZS9LU@MiWiFi-R3L-srv> (raw)
In-Reply-To: <87ee0q7b92.fsf@email.froward.int.ebiederm.org>

Hi Eric,

On 05/18/22 at 04:59pm, Eric W. Biederman wrote:
> "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> writes:
> 
> > 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.
> >
> > [1] https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=d1bcae833b32f1
> 
> Any chance you can also get machine_kexec_post_load,
> crash_free_reserved_phys_range, arch_kexec_protect_protect_crashkres,
> arch_kexec_unprotect_crashkres, arch_kexec_kernel_image_probe,
> arch_kexec_kernel_image_probe, arch_kimage_file_post_load_cleanup,
> arch_kexec_kernel_verify_sig, and arch_kexec_locate_mem_hole as well.
> 
> That is everything in kexec that uses a __weak symbol.  If we can't
> count on them working we might as well just get rid of the rest
> preemptively.

Is there a new rule that __weak is not suggested in kernel any more?
Please help provide a pointer if yes, so that I can learn that.

In my mind, __weak is very simple and clear as a mechanism to add
ARCH related functionality.

Thanks
Baoquan


  reply	other threads:[~2022-05-19  2:58 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
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 [this message]
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=YoWySwbszfdZS9LU@MiWiFi-R3L-srv \
    --to=bhe@redhat.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.