[PATCH v1 0/2] kexec: Remove "weak" annotations from headers

[PATCH v1 0/2] kexec: Remove "weak" annotations from headers

[Bjorn Helgaas]

"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(-)

[PATCH v1 0/2] kexec: Remove "weak" annotations from headers

[Bjorn Helgaas]

"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

[PATCH v1 1/2] kexec: Remove "weak" from kexec_file function declarations

[Bjorn Helgaas]

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);
 

[PATCH v1 1/2] kexec: Remove "weak" from kexec_file function declarations

[Bjorn Helgaas]

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);
 


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v1 2/2] kexec: Remove "weak" from arch_kexec_walk_mem() declaration

[Bjorn Helgaas]

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 */

[PATCH v1 2/2] kexec: Remove "weak" from arch_kexec_walk_mem() declaration

[Bjorn Helgaas]

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 */


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v1 0/2] kexec: Remove "weak" annotations from headers

[Philipp Rudo]

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
> 

Re: [PATCH v1 0/2] kexec: Remove "weak" annotations from headers

[Philipp Rudo]

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

Re: [PATCH v1 0/2] kexec: Remove "weak" annotations from headers

[Baoquan He]

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

Re: [PATCH v1 0/2] kexec: Remove "weak" annotations from headers

[Baoquan He]

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

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v1 0/2] kexec: Remove "weak" annotations from headers

[Bjorn Helgaas]

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

Re: [PATCH v1 0/2] kexec: Remove "weak" annotations from headers

[Bjorn Helgaas]

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

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v1 0/2] kexec: Remove "weak" annotations from headers

[Baoquan He]

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

Re: [PATCH v1 0/2] kexec: Remove "weak" annotations from headers

[Baoquan He]

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

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v1 0/2] kexec: Remove "weak" annotations from headers

[Bjorn Helgaas]

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.

Re: [PATCH v1 0/2] kexec: Remove "weak" annotations from headers

[Bjorn Helgaas]

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.

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec