diff mbox series

[bpf] bpf: Mark bpf_arch_init_dispatcher_early() as __init_or_module

Message ID 20221031173819.2344270-1-nathan@kernel.org
State New
Headers
Series [bpf] bpf: Mark bpf_arch_init_dispatcher_early() as __init_or_module |

Commit Message

Nathan Chancellor Oct. 31, 2022, 5:38 p.m. UTC 
  After commit dbe69b299884 ("bpf: Fix dispatcher patchable function entry
to 5 bytes nop"), building kernel/bpf/dispatcher.c in certain
configurations with LLVM's integrated assembler results in a known
recordmcount bug:

  Cannot find symbol for section 4: .init.text.
  kernel/bpf/dispatcher.o: failed

This occurs when there are only weak symbols in a particular section in
the translation unit; in this case, bpf_arch_init_dispatcher_early() is
marked '__weak __init' and it is the only symbol in the .init.text
section. recordmcount expects there to be a symbol for a particular
section but LLVM's integrated assembler (and GNU as after 2.37) do not
generated section symbols. This has been worked around in the kernel
before in commit 55d5b7dd6451 ("initramfs: fix clang build failure")
and commit 6e7b64b9dd6d ("elfcore: fix building with clang").

Fixing recordmcount has been brought up before but there is no clear
solution that does not break ftrace outright.

Unfortunately, working around this issue by removing the '__init' from
bpf_arch_init_dispatcher_early() is not an option, as the x86 version of
bpf_arch_init_dispatcher_early() calls text_poke_early(), which is
marked '__init_or_module', meaning that when CONFIG_MODULES is disabled,
bpf_arch_init_dispatcher_early() has to be marked '__init' as well to
avoid a section mismatch warning from modpost.

However, bpf_arch_init_dispatcher_early() can be marked
'__init_or_module' as well, which would resolve the recordmcount warning
for configurations that support modules (i.e., the vast majority of
them) while not introducing any new warnings for all configurations. Do
so to clear up the build failure for CONFIG_MODULES=y configurations.

Link: https://github.com/ClangBuiltLinux/linux/issues/981
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
 arch/x86/net/bpf_jit_comp.c | 2 +-
 include/linux/bpf.h         | 2 +-
 kernel/bpf/dispatcher.c     | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)


base-commit: 8bdc2acd420c6f3dd1f1c78750ec989f02a1e2b9

Comments

Jiri Olsa Nov. 1, 2022, 7:57 a.m. UTC | #1 
On Mon, Oct 31, 2022 at 10:38:19AM -0700, Nathan Chancellor wrote:
> After commit dbe69b299884 ("bpf: Fix dispatcher patchable function entry
> to 5 bytes nop"), building kernel/bpf/dispatcher.c in certain
> configurations with LLVM's integrated assembler results in a known
> recordmcount bug:
> 
>   Cannot find symbol for section 4: .init.text.
>   kernel/bpf/dispatcher.o: failed
> 
> This occurs when there are only weak symbols in a particular section in
> the translation unit; in this case, bpf_arch_init_dispatcher_early() is
> marked '__weak __init' and it is the only symbol in the .init.text
> section. recordmcount expects there to be a symbol for a particular
> section but LLVM's integrated assembler (and GNU as after 2.37) do not
> generated section symbols. This has been worked around in the kernel
> before in commit 55d5b7dd6451 ("initramfs: fix clang build failure")
> and commit 6e7b64b9dd6d ("elfcore: fix building with clang").
> 
> Fixing recordmcount has been brought up before but there is no clear
> solution that does not break ftrace outright.
> 
> Unfortunately, working around this issue by removing the '__init' from
> bpf_arch_init_dispatcher_early() is not an option, as the x86 version of
> bpf_arch_init_dispatcher_early() calls text_poke_early(), which is
> marked '__init_or_module', meaning that when CONFIG_MODULES is disabled,
> bpf_arch_init_dispatcher_early() has to be marked '__init' as well to
> avoid a section mismatch warning from modpost.
> 
> However, bpf_arch_init_dispatcher_early() can be marked
> '__init_or_module' as well, which would resolve the recordmcount warning
> for configurations that support modules (i.e., the vast majority of
> them) while not introducing any new warnings for all configurations. Do
> so to clear up the build failure for CONFIG_MODULES=y configurations.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/981
> Signed-off-by: Nathan Chancellor <nathan@kernel.org>

LGTM but the whole thing might be actually going away:
  https://lore.kernel.org/bpf/Y2BD6xZ108lv3j7J@krava/T/#u

because it won't compile on gcc 7

jirka

> ---
>  arch/x86/net/bpf_jit_comp.c | 2 +-
>  include/linux/bpf.h         | 2 +-
>  kernel/bpf/dispatcher.c     | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> index 00127abd89ee..4145939bbb6a 100644
> --- a/arch/x86/net/bpf_jit_comp.c
> +++ b/arch/x86/net/bpf_jit_comp.c
> @@ -389,7 +389,7 @@ static int __bpf_arch_text_poke(void *ip, enum bpf_text_poke_type t,
>  	return ret;
>  }
>  
> -int __init bpf_arch_init_dispatcher_early(void *ip)
> +int __init_or_module bpf_arch_init_dispatcher_early(void *ip)
>  {
>  	const u8 *nop_insn = x86_nops[5];
>  
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 0566705c1d4e..4aa7bde406f5 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -971,7 +971,7 @@ struct bpf_trampoline *bpf_trampoline_get(u64 key,
>  					  struct bpf_attach_target_info *tgt_info);
>  void bpf_trampoline_put(struct bpf_trampoline *tr);
>  int arch_prepare_bpf_dispatcher(void *image, void *buf, s64 *funcs, int num_funcs);
> -int __init bpf_arch_init_dispatcher_early(void *ip);
> +int __init_or_module bpf_arch_init_dispatcher_early(void *ip);
>  
>  #define BPF_DISPATCHER_INIT(_name) {				\
>  	.mutex = __MUTEX_INITIALIZER(_name.mutex),		\
> diff --git a/kernel/bpf/dispatcher.c b/kernel/bpf/dispatcher.c
> index 04f0a045dcaa..e14a68e9a74f 100644
> --- a/kernel/bpf/dispatcher.c
> +++ b/kernel/bpf/dispatcher.c
> @@ -91,7 +91,7 @@ int __weak arch_prepare_bpf_dispatcher(void *image, void *buf, s64 *funcs, int n
>  	return -ENOTSUPP;
>  }
>  
> -int __weak __init bpf_arch_init_dispatcher_early(void *ip)
> +int __weak __init_or_module bpf_arch_init_dispatcher_early(void *ip)
>  {
>  	return -ENOTSUPP;
>  }
> 
> base-commit: 8bdc2acd420c6f3dd1f1c78750ec989f02a1e2b9
> -- 
> 2.38.1
>
Nathan Chancellor Nov. 1, 2022, 4:37 p.m. UTC | #2 
On Tue, Nov 01, 2022 at 08:57:11AM +0100, Jiri Olsa wrote:
> On Mon, Oct 31, 2022 at 10:38:19AM -0700, Nathan Chancellor wrote:
> > After commit dbe69b299884 ("bpf: Fix dispatcher patchable function entry
> > to 5 bytes nop"), building kernel/bpf/dispatcher.c in certain
> > configurations with LLVM's integrated assembler results in a known
> > recordmcount bug:
> > 
> >   Cannot find symbol for section 4: .init.text.
> >   kernel/bpf/dispatcher.o: failed
> > 
> > This occurs when there are only weak symbols in a particular section in
> > the translation unit; in this case, bpf_arch_init_dispatcher_early() is
> > marked '__weak __init' and it is the only symbol in the .init.text
> > section. recordmcount expects there to be a symbol for a particular
> > section but LLVM's integrated assembler (and GNU as after 2.37) do not
> > generated section symbols. This has been worked around in the kernel
> > before in commit 55d5b7dd6451 ("initramfs: fix clang build failure")
> > and commit 6e7b64b9dd6d ("elfcore: fix building with clang").
> > 
> > Fixing recordmcount has been brought up before but there is no clear
> > solution that does not break ftrace outright.
> > 
> > Unfortunately, working around this issue by removing the '__init' from
> > bpf_arch_init_dispatcher_early() is not an option, as the x86 version of
> > bpf_arch_init_dispatcher_early() calls text_poke_early(), which is
> > marked '__init_or_module', meaning that when CONFIG_MODULES is disabled,
> > bpf_arch_init_dispatcher_early() has to be marked '__init' as well to
> > avoid a section mismatch warning from modpost.
> > 
> > However, bpf_arch_init_dispatcher_early() can be marked
> > '__init_or_module' as well, which would resolve the recordmcount warning
> > for configurations that support modules (i.e., the vast majority of
> > them) while not introducing any new warnings for all configurations. Do
> > so to clear up the build failure for CONFIG_MODULES=y configurations.
> > 
> > Link: https://github.com/ClangBuiltLinux/linux/issues/981
> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> 
> LGTM but the whole thing might be actually going away:
>   https://lore.kernel.org/bpf/Y2BD6xZ108lv3j7J@krava/T/#u
> 
> because it won't compile on gcc 7

Ah, if that is the case, I suppose we can just wait until it is reverted
and replaced with the better fix. It seems like that will be the case
based on Bjorn's testing:

https://lore.kernel.org/87iljyyes6.fsf@all.your.base.are.belong.to.us/

Cheers,
Nathan

> > ---
> >  arch/x86/net/bpf_jit_comp.c | 2 +-
> >  include/linux/bpf.h         | 2 +-
> >  kernel/bpf/dispatcher.c     | 2 +-
> >  3 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
> > index 00127abd89ee..4145939bbb6a 100644
> > --- a/arch/x86/net/bpf_jit_comp.c
> > +++ b/arch/x86/net/bpf_jit_comp.c
> > @@ -389,7 +389,7 @@ static int __bpf_arch_text_poke(void *ip, enum bpf_text_poke_type t,
> >  	return ret;
> >  }
> >  
> > -int __init bpf_arch_init_dispatcher_early(void *ip)
> > +int __init_or_module bpf_arch_init_dispatcher_early(void *ip)
> >  {
> >  	const u8 *nop_insn = x86_nops[5];
> >  
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index 0566705c1d4e..4aa7bde406f5 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -971,7 +971,7 @@ struct bpf_trampoline *bpf_trampoline_get(u64 key,
> >  					  struct bpf_attach_target_info *tgt_info);
> >  void bpf_trampoline_put(struct bpf_trampoline *tr);
> >  int arch_prepare_bpf_dispatcher(void *image, void *buf, s64 *funcs, int num_funcs);
> > -int __init bpf_arch_init_dispatcher_early(void *ip);
> > +int __init_or_module bpf_arch_init_dispatcher_early(void *ip);
> >  
> >  #define BPF_DISPATCHER_INIT(_name) {				\
> >  	.mutex = __MUTEX_INITIALIZER(_name.mutex),		\
> > diff --git a/kernel/bpf/dispatcher.c b/kernel/bpf/dispatcher.c
> > index 04f0a045dcaa..e14a68e9a74f 100644
> > --- a/kernel/bpf/dispatcher.c
> > +++ b/kernel/bpf/dispatcher.c
> > @@ -91,7 +91,7 @@ int __weak arch_prepare_bpf_dispatcher(void *image, void *buf, s64 *funcs, int n
> >  	return -ENOTSUPP;
> >  }
> >  
> > -int __weak __init bpf_arch_init_dispatcher_early(void *ip)
> > +int __weak __init_or_module bpf_arch_init_dispatcher_early(void *ip)
> >  {
> >  	return -ENOTSUPP;
> >  }
> > 
> > base-commit: 8bdc2acd420c6f3dd1f1c78750ec989f02a1e2b9
> > -- 
> > 2.38.1
> >
diff mbox series

Patch

diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 00127abd89ee..4145939bbb6a 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -389,7 +389,7 @@  static int __bpf_arch_text_poke(void *ip, enum bpf_text_poke_type t,
 	return ret;
 }
 
-int __init bpf_arch_init_dispatcher_early(void *ip)
+int __init_or_module bpf_arch_init_dispatcher_early(void *ip)
 {
 	const u8 *nop_insn = x86_nops[5];
 
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 0566705c1d4e..4aa7bde406f5 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -971,7 +971,7 @@  struct bpf_trampoline *bpf_trampoline_get(u64 key,
 					  struct bpf_attach_target_info *tgt_info);
 void bpf_trampoline_put(struct bpf_trampoline *tr);
 int arch_prepare_bpf_dispatcher(void *image, void *buf, s64 *funcs, int num_funcs);
-int __init bpf_arch_init_dispatcher_early(void *ip);
+int __init_or_module bpf_arch_init_dispatcher_early(void *ip);
 
 #define BPF_DISPATCHER_INIT(_name) {				\
 	.mutex = __MUTEX_INITIALIZER(_name.mutex),		\
diff --git a/kernel/bpf/dispatcher.c b/kernel/bpf/dispatcher.c
index 04f0a045dcaa..e14a68e9a74f 100644
--- a/kernel/bpf/dispatcher.c
+++ b/kernel/bpf/dispatcher.c
@@ -91,7 +91,7 @@  int __weak arch_prepare_bpf_dispatcher(void *image, void *buf, s64 *funcs, int n
 	return -ENOTSUPP;
 }
 
-int __weak __init bpf_arch_init_dispatcher_early(void *ip)
+int __weak __init_or_module bpf_arch_init_dispatcher_early(void *ip)
 {
 	return -ENOTSUPP;
 }