Re: [PATCH bpf-next 02/17] bpf: Use mutex lock pool for bpf trampolines

[Jiri Olsa <olsajiri@gmail.com>]

On Fri, Feb 20, 2026 at 10:57:51AM +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
> 
> [ ... ]
> 
> > @@ -71,7 +110,7 @@ static int bpf_tramp_ftrace_ops_func(struct ftrace_ops *ops, unsigned long ip,
> >  		/* This is called inside register_ftrace_direct_multi(), so
> >  		 * tr->mutex is already locked.
> >  		 */
> > -		lockdep_assert_held_once(&tr->mutex);
> > +		lockdep_assert_held_once(trampoline_locks_lookup(tr));
> 
> This comment still references tr->mutex, but the field was removed from
> struct bpf_trampoline by this patch. Should the comment say the
> trampoline pool lock is already held instead?

yes, I overlooked the comments..

> 
> [ ... ]
> 
> > @@ -102,7 +141,7 @@ static int bpf_tramp_ftrace_ops_func(struct ftrace_ops *ops, unsigned long ip,
> >  	/* The normal locking order is
> >  	 *    tr->mutex => direct_mutex (ftrace.c) => ftrace_lock (ftrace.c)
> >  	 *
> >  	 * The following two commands are called from
> >  	 *
> >  	 *   prepare_direct_functions_for_ipmodify
> >  	 *   cleanup_direct_functions_after_ipmodify
> >  	 *
> >  	 * In both cases, direct_mutex is already locked. Use
> >  	 * mutex_trylock(&tr->mutex) to avoid deadlock in race condition
> >  	 * (something else is making changes to this same trampoline).
> >  	 */
> > -	if (!mutex_trylock(&tr->mutex)) {
> > +	if (!mutex_trylock(trampoline_locks_lookup(tr))) {
> >  		/* sleep 1 ms to make sure whatever holding tr->mutex makes
> >  		 * some progress.
> >  		 */
> 
> The locking order comment and the two references to tr->mutex in this
> block also appear to need updating. The locking order is now pool lock
> rather than tr->mutex, and the trylock and sleep comments still name the
> removed field.

yes, will fix

> 
> [ ... ]
> 
> > @@ -1026,11 +1065,11 @@ struct bpf_attach_target_info tgt_info = {};
> >  	/* note, we're still holding tr refcnt from above */
> >
> > -	mutex_unlock(&tr->mutex);
> > +	trampoline_unlock(tr);
> >
> >  	return 0;
> >  err:
> > -	mutex_unlock(&tr->mutex);
> > +	trampoline_unlock(tr);
> >
> >  	if (shim_link)
> >  		bpf_link_put(&shim_link->link.link);
> >
> >  	/* have to release tr while _not_ holding its mutex */
> >  	bpf_trampoline_put(tr); /* bpf_trampoline_get above */
> 
> Similarly, this comment in bpf_trampoline_link_cgroup_shim() references
> "its mutex" which no longer exists as a per-trampoline field.

right, will fix

thanks,
jirka