Re: [PATCHv2 bpf-next 02/23] bpf: Use mutex lock pool for bpf trampolines

[Jiri Olsa <olsajiri@gmail.com>]

On Wed, Mar 04, 2026 at 11:02:28PM +0000, bot+bpf-ci@kernel.org wrote:
> > diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> > --- a/kernel/bpf/trampoline.c
> > +++ b/kernel/bpf/trampoline.c
> 
> [ ... ]
> 
> > @@ -99,12 +127,12 @@ static int bpf_tramp_ftrace_ops_func(struct ftrace_ops *ops, unsigned long ip,
> >  	 *   cleanup_direct_functions_after_ipmodify
> >  	 *
> >  	 * In both cases, direct_mutex is already locked. Use
> > -	 * mutex_trylock(&tr->mutex) to avoid deadlock in race condition
> > +	 * mutex_trylock(select_trampoline_lock(tr)) to avoid deadlock in race condition
> >  	 * (something else is making changes to this same trampoline).
> >  	 */
> > -	if (!mutex_trylock(&tr->mutex)) {
> > -		/* sleep 1 ms to make sure whatever holding tr->mutex makes
> > -		 * some progress.
> > +	if (!mutex_trylock(select_trampoline_lock(tr))) {
> > +		/* sleep 1 ms to make sure whatever holding select_trampoline_lock(tr)
> > +		 * makes some progress.
> >  		 */
> 
> With a pool of 32 mutexes, the trylock can now also fail because a
> different trampoline that hashes to the same pool entry holds the lock.
> Should the comment say "something else holds the same pool lock" instead
> of "something else is making changes to this same trampoline"?

right, will change the comment, thnx

> 
> Similarly, the sleep comment says the holder "makes some progress" but
> with a pool lock the holder could be operating on an entirely unrelated
> trampoline.

this one seems ok to me.. holder is 'whatever holding select_trampoline_lock'
which is correct

jirka

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