Re: [PATCH bpf-next v2 08/21] bpf: Move helpers out of CONFIG_HAVE_SINGLE_FTRACE_DIRECT_OPS guard in trampoline

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 3086 bytes --]

> bpf: Move helpers out of CONFIG_HAVE_SINGLE_FTRACE_DIRECT_OPS guard in trampoline

TL;DR: Moved trampoline helpers may trigger unused-function warnings (build
failure with CONFIG_WERROR) on non-x86_64 configs with CONFIG_BPF_SYSCALL
until a later patch adds callers; only this commit is affected.

> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index a2fd0bd8c685..bb832af8703f 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
> @@ -60,6 +60,32 @@ static void trampoline_unlock(struct bpf_trampoline *tr)
>  	mutex_unlock(select_trampoline_lock(tr));
>  }
>
> +#ifdef CONFIG_BPF_SYSCALL
> +
> +static void trampoline_lock_all(void)
> +{
> +	int i;
> +
> +	for (i = 0; i < TRAMPOLINE_LOCKS_TABLE_SIZE; i++)
> +		mutex_lock(&trampoline_locks[i].mutex);
> +}
> +
> +static void trampoline_unlock_all(void)
> +{
> +	int i;
> +
> +	for (i = 0; i < TRAMPOLINE_LOCKS_TABLE_SIZE; i++)
> +		mutex_unlock(&trampoline_locks[i].mutex);
> +}
> +
> +static void bpf_trampoline_multi_attach_init(struct bpf_trampoline *tr)
> +{
> +	tr->multi_attach.old_image = tr->cur_image;
> +	tr->multi_attach.old_flags = tr->flags;
> +}
> +
> +#endif /* CONFIG_BPF_SYSCALL */
> +

Can this add a new "defined but not used" warning on configs that have
CONFIG_BPF_SYSCALL but not CONFIG_HAVE_SINGLE_FTRACE_DIRECT_OPS?

These three helpers are static and are now guarded only by
CONFIG_BPF_SYSCALL, but their only callers at this point in the series,
bpf_trampoline_multi_attach() and bpf_trampoline_multi_detach(), are still
inside:

#if defined(CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS) && \
    defined(CONFIG_HAVE_SINGLE_FTRACE_DIRECT_OPS) && \
    defined(CONFIG_BPF_SYSCALL)

trampoline.o is built for CONFIG_BPF_JIT, and only x86_64 selects
HAVE_SINGLE_FTRACE_DIRECT_OPS.  On arm64, riscv, s390, x86_32, or x86_64
without DYNAMIC_FTRACE_WITH_DIRECT_CALLS, a CONFIG_BPF_JIT=y,
CONFIG_BPF_SYSCALL=y build would define trampoline_lock_all(),
trampoline_unlock_all() and bpf_trampoline_multi_attach_init() with no users,
and -Wunused-function would warn for each of them.  With CONFIG_WERROR=y that
becomes a build failure.

The commit message says the CONFIG_BPF_SYSCALL guard is kept "to avoid
compiling warning when CONFIG_BPF_SYSCALL is disabled", but this seems to
trade that warning for the same kind of warning when CONFIG_BPF_SYSCALL is
enabled.

The helpers only get callers outside the old guard in a later patch of the
series, commit 59e80b53cc6b ("bpf: Add
bpf_trampoline_multi_prog_attach/detach()"), which adds users under
#ifdef CONFIG_BPF_SYSCALL.  So the final state looks
fine, but this commit on its own does not build warning-free on those configs.

Would it make sense to fold this move into that later patch, or mark the
helpers __maybe_unused for the interim, so each commit in the series builds
cleanly?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/38273555988