From: Jiri Olsa <olsajiri@gmail.com>
To: Nathan Chancellor <nathan@kernel.org>
Cc: Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Andrii Nakryiko <andrii@kernel.org>,
Martin KaFai Lau <martin.lau@linux.dev>,
Song Liu <song@kernel.org>, Yonghong Song <yhs@fb.com>,
John Fastabend <john.fastabend@gmail.com>,
KP Singh <kpsingh@kernel.org>,
Stanislav Fomichev <sdf@google.com>, Hao Luo <haoluo@google.com>,
Nick Desaulniers <ndesaulniers@google.com>,
Tom Rix <trix@redhat.com>,
x86@kernel.org, bpf@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, llvm@lists.linux.dev,
patches@lists.linux.dev
Subject: Re: [PATCH bpf] bpf: Mark bpf_arch_init_dispatcher_early() as __init_or_module
Date: Tue, 1 Nov 2022 08:57:11 +0100 [thread overview]
Message-ID: <Y2DRVwI4bNUppmXJ@krava> (raw)
In-Reply-To: <20221031173819.2344270-1-nathan@kernel.org>
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
>
next prev parent reply other threads:[~2022-11-01 7:57 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-31 17:38 [PATCH bpf] bpf: Mark bpf_arch_init_dispatcher_early() as __init_or_module Nathan Chancellor
2022-11-01 7:57 ` Jiri Olsa [this message]
2022-11-01 16:37 ` Nathan Chancellor
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=Y2DRVwI4bNUppmXJ@krava \
--to=olsajiri@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=haoluo@google.com \
--cc=john.fastabend@gmail.com \
--cc=kpsingh@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=llvm@lists.linux.dev \
--cc=martin.lau@linux.dev \
--cc=nathan@kernel.org \
--cc=ndesaulniers@google.com \
--cc=netdev@vger.kernel.org \
--cc=patches@lists.linux.dev \
--cc=sdf@google.com \
--cc=song@kernel.org \
--cc=trix@redhat.com \
--cc=x86@kernel.org \
--cc=yhs@fb.com \
/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 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).