* [PATCH v1 0/2] kexec: Remove "weak" annotations from headers @ 2018-04-12 18:23 Bjorn Helgaas 2018-04-12 18:23 ` [PATCH v1 1/2] kexec: Remove "weak" from kexec_file function declarations Bjorn Helgaas ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: Bjorn Helgaas @ 2018-04-12 18:23 UTC (permalink / raw) To: Eric Biederman Cc: Thiago Jung Bauermann, Dave Young, Michael Ellerman, kexec, linux-kernel "Weak" annotations in header files are error-prone because they make every definition weak. Remove them from include/linux/kexec.h. These were introduced in two separate commits, so this is in two patches so they can be easily backported to stable kernels (some of them date back to v4.3 and one only goes back to v4.10). --- Bjorn Helgaas (2): kexec: Remove "weak" from kexec_file function declarations kexec: Remove "weak" from arch_kexec_walk_mem() declaration include/linux/kexec.h | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v1 1/2] kexec: Remove "weak" from kexec_file function declarations 2018-04-12 18:23 [PATCH v1 0/2] kexec: Remove "weak" annotations from headers Bjorn Helgaas @ 2018-04-12 18:23 ` Bjorn Helgaas 2018-04-12 18:23 ` [PATCH v1 2/2] kexec: Remove "weak" from arch_kexec_walk_mem() declaration Bjorn Helgaas 2018-04-13 9:08 ` [PATCH v1 0/2] kexec: Remove "weak" annotations from headers Philipp Rudo 2 siblings, 0 replies; 8+ messages in thread From: Bjorn Helgaas @ 2018-04-12 18:23 UTC (permalink / raw) To: Eric Biederman Cc: Thiago Jung Bauermann, Dave Young, Michael Ellerman, kexec, linux-kernel From: Bjorn Helgaas <bhelgaas@google.com> Weak header file declarations are error-prone because they make every definition weak, and the linker chooses one based on link order (see 10629d711ed7 ("PCI: Remove __weak annotation from pcibios_get_phb_of_node decl")). For the following functions: arch_kexec_kernel_image_probe() arch_kexec_kernel_image_load() arch_kimage_file_post_load_cleanup() arch_kexec_kernel_verify_sig() arch_kexec_apply_relocations_add() arch_kexec_apply_relocations() kernel/kexec_file.c contains weak definitions, and x86 and powerpc arch code contains definitions intended to be non-weak. But the annotations in the header file make *all* the definitions weak, so it's unclear which ones will be used. Remove the "weak" attribute from the declarations so we always prefer non-weak definitions over the weak ones. Fixes: a43cac0d9dc2 ("kexec: split kexec_file syscall code to kexec_file.c") Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> CC: stable@vger.kernel.org # v4.3+ --- include/linux/kexec.h | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/include/linux/kexec.h b/include/linux/kexec.h index f16f6ceb3875..8bf0ff90885c 100644 --- a/include/linux/kexec.h +++ b/include/linux/kexec.h @@ -277,16 +277,16 @@ int crash_shrink_memory(unsigned long new_size); size_t crash_get_memory_size(void); void crash_free_reserved_phys_range(unsigned long begin, unsigned long end); -int __weak arch_kexec_kernel_image_probe(struct kimage *image, void *buf, - unsigned long buf_len); -void * __weak arch_kexec_kernel_image_load(struct kimage *image); -int __weak arch_kimage_file_post_load_cleanup(struct kimage *image); -int __weak arch_kexec_kernel_verify_sig(struct kimage *image, void *buf, - unsigned long buf_len); -int __weak arch_kexec_apply_relocations_add(const Elf_Ehdr *ehdr, - Elf_Shdr *sechdrs, unsigned int relsec); -int __weak arch_kexec_apply_relocations(const Elf_Ehdr *ehdr, Elf_Shdr *sechdrs, - unsigned int relsec); +int arch_kexec_kernel_image_probe(struct kimage *image, void *buf, + unsigned long buf_len); +void *arch_kexec_kernel_image_load(struct kimage *image); +int arch_kimage_file_post_load_cleanup(struct kimage *image); +int arch_kexec_kernel_verify_sig(struct kimage *image, void *buf, + unsigned long buf_len); +int arch_kexec_apply_relocations_add(const Elf_Ehdr *ehdr, Elf_Shdr *sechdrs, + unsigned int relsec); +int arch_kexec_apply_relocations(const Elf_Ehdr *ehdr, Elf_Shdr *sechdrs, + unsigned int relsec); void arch_kexec_protect_crashkres(void); void arch_kexec_unprotect_crashkres(void); ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v1 2/2] kexec: Remove "weak" from arch_kexec_walk_mem() declaration 2018-04-12 18:23 [PATCH v1 0/2] kexec: Remove "weak" annotations from headers Bjorn Helgaas 2018-04-12 18:23 ` [PATCH v1 1/2] kexec: Remove "weak" from kexec_file function declarations Bjorn Helgaas @ 2018-04-12 18:23 ` Bjorn Helgaas 2018-04-13 9:08 ` [PATCH v1 0/2] kexec: Remove "weak" annotations from headers Philipp Rudo 2 siblings, 0 replies; 8+ messages in thread From: Bjorn Helgaas @ 2018-04-12 18:23 UTC (permalink / raw) To: Eric Biederman Cc: Thiago Jung Bauermann, Dave Young, Michael Ellerman, kexec, linux-kernel From: Bjorn Helgaas <bhelgaas@google.com> Weak header file declarations are error-prone because they make every definition weak, and the linker chooses one based on link order (see 10629d711ed7 ("PCI: Remove __weak annotation from pcibios_get_phb_of_node decl")). kernel/kexec_file.c contains a weak definition of arch_kexec_walk_mem() and arch/powerpc/kernel/machine_kexec_file_64.c contains a definition intended to be non-weak. But the annotation in the header file makes *both* definitions weak, so it's unclear which one will be used. Remove the "weak" attribute from the declaration so we always prefer a non-weak definition over the weak one. Fixes: 60fe3910bb02 ("kexec_file: Allow arch-specific memory walking for kexec_add_buffer") Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> CC: stable@vger.kernel.org # v4.10+ --- include/linux/kexec.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/linux/kexec.h b/include/linux/kexec.h index 8bf0ff90885c..e7db550c5fb6 100644 --- a/include/linux/kexec.h +++ b/include/linux/kexec.h @@ -159,8 +159,8 @@ struct kexec_buf { bool top_down; }; -int __weak arch_kexec_walk_mem(struct kexec_buf *kbuf, - int (*func)(struct resource *, void *)); +int arch_kexec_walk_mem(struct kexec_buf *kbuf, + int (*func)(struct resource *, void *)); extern int kexec_add_buffer(struct kexec_buf *kbuf); int kexec_locate_mem_hole(struct kexec_buf *kbuf); #endif /* CONFIG_KEXEC_FILE */ ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v1 0/2] kexec: Remove "weak" annotations from headers 2018-04-12 18:23 [PATCH v1 0/2] kexec: Remove "weak" annotations from headers Bjorn Helgaas 2018-04-12 18:23 ` [PATCH v1 1/2] kexec: Remove "weak" from kexec_file function declarations Bjorn Helgaas 2018-04-12 18:23 ` [PATCH v1 2/2] kexec: Remove "weak" from arch_kexec_walk_mem() declaration Bjorn Helgaas @ 2018-04-13 9:08 ` Philipp Rudo 2018-04-13 9:29 ` Baoquan He 2018-04-17 14:09 ` Bjorn Helgaas 2 siblings, 2 replies; 8+ messages in thread From: Philipp Rudo @ 2018-04-13 9:08 UTC (permalink / raw) To: Bjorn Helgaas Cc: Eric Biederman, Thiago Jung Bauermann, Michael Ellerman, Dave Young, kexec, linux-kernel Hi Bjorn, in recent patches AKASHI [1] and I [2] made some changes to the declarations you are touching and already removed some of the weak statements. The patches got accepted on linux-next and will (hopefully) be pulled for v4.17. So you should prepare for some merge conflicts. Nevertheless three weak statements still remain (arch_kexec_walk_mem & arch_kexec_apply_relocations*) so your patch still makes totally sense. Thanks Philipp [1] https://lkml.org/lkml/2018/3/6/201 [2] https://lkml.org/lkml/2018/3/21/278 On Thu, 12 Apr 2018 13:23:29 -0500 Bjorn Helgaas <helgaas@kernel.org> wrote: > "Weak" annotations in header files are error-prone because they make > every definition weak. Remove them from include/linux/kexec.h. > > These were introduced in two separate commits, so this is in two > patches so they can be easily backported to stable kernels (some of > them date back to v4.3 and one only goes back to v4.10). > > --- > > Bjorn Helgaas (2): > kexec: Remove "weak" from kexec_file function declarations > kexec: Remove "weak" from arch_kexec_walk_mem() declaration > > > include/linux/kexec.h | 24 ++++++++++++------------ > 1 file changed, 12 insertions(+), 12 deletions(-) > > _______________________________________________ > kexec mailing list > kexec@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/kexec > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1 0/2] kexec: Remove "weak" annotations from headers 2018-04-13 9:08 ` [PATCH v1 0/2] kexec: Remove "weak" annotations from headers Philipp Rudo @ 2018-04-13 9:29 ` Baoquan He 2018-04-17 14:07 ` Bjorn Helgaas 2018-04-17 14:09 ` Bjorn Helgaas 1 sibling, 1 reply; 8+ messages in thread From: Baoquan He @ 2018-04-13 9:29 UTC (permalink / raw) To: Bjorn Helgaas, Philipp Rudo Cc: Michael Ellerman, kexec, linux-kernel, Eric Biederman, Thiago Jung Bauermann, Dave Young Hi Bjorn, There are changes I have made to solve 5-level conflict with kexec/kdump and also interface unification task, they will involve x86 64 only changes on these functions, I don't think we need remove them if without any obvious impact or error reported. Thanks Baoquan On 04/13/18 at 11:08am, Philipp Rudo wrote: > Hi Bjorn, > > in recent patches AKASHI [1] and I [2] made some changes to the declarations > you are touching and already removed some of the weak statements. The patches > got accepted on linux-next and will (hopefully) be pulled for v4.17. So you > should prepare for some merge conflicts. Nevertheless three weak statements > still remain (arch_kexec_walk_mem & arch_kexec_apply_relocations*) so your > patch still makes totally sense. > > Thanks > Philipp > > [1] https://lkml.org/lkml/2018/3/6/201 > [2] https://lkml.org/lkml/2018/3/21/278 > > On Thu, 12 Apr 2018 13:23:29 -0500 > Bjorn Helgaas <helgaas@kernel.org> wrote: > > > "Weak" annotations in header files are error-prone because they make > > every definition weak. Remove them from include/linux/kexec.h. > > > > These were introduced in two separate commits, so this is in two > > patches so they can be easily backported to stable kernels (some of > > them date back to v4.3 and one only goes back to v4.10). > > > > --- > > > > Bjorn Helgaas (2): > > kexec: Remove "weak" from kexec_file function declarations > > kexec: Remove "weak" from arch_kexec_walk_mem() declaration > > > > > > include/linux/kexec.h | 24 ++++++++++++------------ > > 1 file changed, 12 insertions(+), 12 deletions(-) > > > > _______________________________________________ > > kexec mailing list > > kexec@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/kexec > > > > > _______________________________________________ > kexec mailing list > kexec@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/kexec ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1 0/2] kexec: Remove "weak" annotations from headers 2018-04-13 9:29 ` Baoquan He @ 2018-04-17 14:07 ` Bjorn Helgaas 2018-04-17 14:21 ` Baoquan He 0 siblings, 1 reply; 8+ messages in thread From: Bjorn Helgaas @ 2018-04-17 14:07 UTC (permalink / raw) To: Baoquan He Cc: Philipp Rudo, Michael Ellerman, kexec, linux-kernel, Eric Biederman, Thiago Jung Bauermann, Dave Young On Fri, Apr 13, 2018 at 05:29:08PM +0800, Baoquan He wrote: > Hi Bjorn, > > There are changes I have made to solve 5-level conflict with > kexec/kdump and also interface unification task, they will involve x86 > 64 only changes on these functions, I don't think we need remove them if > without any obvious impact or error reported. Removing the weak attribute from the declaration in the header file does not prevent you from defining a weak function later in the .c file. We should remove the weak attribute from the header file declaration because it can lead to non-obvious errors, e.g., calling the wrong version of the function. There's no build-time or run-time indication that this happens, so it's a real trap. Bjorn ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1 0/2] kexec: Remove "weak" annotations from headers 2018-04-17 14:07 ` Bjorn Helgaas @ 2018-04-17 14:21 ` Baoquan He 0 siblings, 0 replies; 8+ messages in thread From: Baoquan He @ 2018-04-17 14:21 UTC (permalink / raw) To: Bjorn Helgaas Cc: Philipp Rudo, Michael Ellerman, kexec, linux-kernel, Eric Biederman, Thiago Jung Bauermann, Dave Young On 04/17/18 at 09:07am, Bjorn Helgaas wrote: > On Fri, Apr 13, 2018 at 05:29:08PM +0800, Baoquan He wrote: > > Hi Bjorn, > > > > There are changes I have made to solve 5-level conflict with > > kexec/kdump and also interface unification task, they will involve x86 > > 64 only changes on these functions, I don't think we need remove them if > > without any obvious impact or error reported. > > Removing the weak attribute from the declaration in the header file > does not prevent you from defining a weak function later in the .c > file. OK, sounds good to me. Then I have no concern to this, thanks. Will see if other people have comments. Thanks Baoquan ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v1 0/2] kexec: Remove "weak" annotations from headers 2018-04-13 9:08 ` [PATCH v1 0/2] kexec: Remove "weak" annotations from headers Philipp Rudo 2018-04-13 9:29 ` Baoquan He @ 2018-04-17 14:09 ` Bjorn Helgaas 1 sibling, 0 replies; 8+ messages in thread From: Bjorn Helgaas @ 2018-04-17 14:09 UTC (permalink / raw) To: Philipp Rudo Cc: Eric Biederman, Thiago Jung Bauermann, Michael Ellerman, Dave Young, kexec, linux-kernel On Fri, Apr 13, 2018 at 11:08:20AM +0200, Philipp Rudo wrote: > Hi Bjorn, > > in recent patches AKASHI [1] and I [2] made some changes to the declarations > you are touching and already removed some of the weak statements. The patches > got accepted on linux-next and will (hopefully) be pulled for v4.17. So you > should prepare for some merge conflicts. Nevertheless three weak statements > still remain (arch_kexec_walk_mem & arch_kexec_apply_relocations*) so your > patch still makes totally sense. Thanks, I see those changes in v4.17-rc1. I'll update my patches and post a v2. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-04-17 14:21 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-04-12 18:23 [PATCH v1 0/2] kexec: Remove "weak" annotations from headers Bjorn Helgaas 2018-04-12 18:23 ` [PATCH v1 1/2] kexec: Remove "weak" from kexec_file function declarations Bjorn Helgaas 2018-04-12 18:23 ` [PATCH v1 2/2] kexec: Remove "weak" from arch_kexec_walk_mem() declaration Bjorn Helgaas 2018-04-13 9:08 ` [PATCH v1 0/2] kexec: Remove "weak" annotations from headers Philipp Rudo 2018-04-13 9:29 ` Baoquan He 2018-04-17 14:07 ` Bjorn Helgaas 2018-04-17 14:21 ` Baoquan He 2018-04-17 14:09 ` Bjorn Helgaas
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).