Re: [PATCHv5 bpf-next 04/28] bpf: Use mutex lock pool for bpf trampolines

On Sat, Apr 18, 2026 at 03:52:26AM +0000, [email protected] wrote:
> Sashiko AI review found 2 potential issue(s):
> - [High] False sharing in the shared mutex pool causes spurious `mutex_trylock` failures, leading to random `-EAGAIN` errors and trampoline state corruption.
> - [Medium] The `trampoline_locks` mutex pool is initialized in `late_initcall`, creating an uninitialized lock hazard for early boot BPF usage.
> --
> 
> Note: Due to system recitation restrictions, the standard inline-quoted
> patch format has been altered. The findings are summarized below in a
> free-form format without direct code quotes.
> 
> commit 8bee21cb8aa45a6b2275c87c16fbbd2fd1953e75
> Author: Jiri Olsa <[email protected]>
> 
> bpf: Use mutex lock pool for bpf trampolines
> 
> This commit replaces the per-trampoline mutex with a 32-bucket static mutex
> pool. This allows upcoming tracing_multi features to lock all involved
> trampolines simultaneously without exceeding the maximum lock depth.
> 
> 
> In bpf_tramp_ftrace_ops_func(), mutex_trylock() is used on the selected
> trampoline pool lock to avoid deadlocks with direct_mutex. Because multiple
> trampolines now share the same lock, could this lead to spurious trylock
> failures if an unrelated trampoline happens to hash to the same bucket?
> 
> If the trylock fails, the function returns -EAGAIN. Does this mean
> concurrent ftrace or kprobe registrations might randomly fail due to false
> sharing?
> 
> Also, when the upcoming tracing_multi feature locks all 32 mutexes at once,
> wouldn't that deterministically abort any concurrent ftrace IPMODIFY
> registrations system-wide?
> 
> Finally, if this trylock failure happens during unregistration, could ftrace
> cleanup routines ignore the -EAGAIN, potentially leaving the
> BPF_TRAMP_F_SHARE_IPMODIFY flag permanently set and corrupting the
> trampoline state?

need to process this one bit more, perhaps we could offload the
BPF_TRAMP_F_SHARE_IPMODIFY unset update to defered work queue
in case the trylock fails.. or something else

in any case we need to make ipmodify and trampolines attach via multi
link mutually exclusive, because we can't just update such trampolines
with bpf_trampoline_update call

> 
> 
> Regarding the pool initialization, the new trampoline_locks array is
> initialized in init_trampolines(), which runs as a late_initcall.
> 
> Since the static array resides in the bss section and is only zero-initialized
> before this phase, what happens if early boot BPF components (like early BPF
> LSM policies) try to attach or modify a trampoline before late_initcall?
> 
> Will this result in calling mutex_lock() on an uninitialized mutex, leading
> to crashes or lockdep warnings when CONFIG_DEBUG_MUTEXES is enabled?

right, in that case we should register it earlier, will check

jirka