Re: [PATCH bpf] bpf: Don't run arg-tracking analysis twice on main subprog

[Eduard Zingerman <eddyz87@gmail.com>]

On Fri, 2026-05-08 at 16:24 -0700, Alexei Starovoitov wrote:
> On Thu, May 7, 2026 at 11:22 AM Paul Chaignon <paul.chaignon@gmail.com> wrote:
> > 
> > Because subprog 0, the main subprog, is considered a global function,
> > we end up running the arg-tracking dataflow analysis twice on it. That
> > results in slightly longer verification but mostly in more verbose
> > verifier logs. This patch fixes it by keeping only the iteration over
> > global subprogs.
> > 
> > When running over all of Cilium's programs with BPF_LOG_LEVEL2, this
> > reduces verbosity by ~20% on average.
> > 
> > Fixes: bf0c571f7feb6 ("bpf: introduce forward arg-tracking dataflow analysis")
> > Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
> > ---
> >  kernel/bpf/liveness.c | 11 ++---------
> >  1 file changed, 2 insertions(+), 9 deletions(-)
> > 
> > diff --git a/kernel/bpf/liveness.c b/kernel/bpf/liveness.c
> > index 332e6e003f27..505250998f36 100644
> > --- a/kernel/bpf/liveness.c
> > +++ b/kernel/bpf/liveness.c
> > @@ -1914,15 +1914,6 @@ int bpf_compute_subprog_arg_access(struct bpf_verifier_env *env)
> >                 return -ENOMEM;
> >         }
> > 
> > -       instance = call_instance(env, NULL, 0, 0);
> > -       if (IS_ERR(instance)) {
> > -               err = PTR_ERR(instance);
> > -               goto out;
> > -       }
> > -       err = analyze_subprog(env, NULL, info, instance, callsites);
> > -       if (err)
> > -               goto out;
> > -
> >         /*
> >          * Subprogs and callbacks that don't receive FP-derived arguments
> >          * cannot access ancestor stack frames, so they were skipped during
> > @@ -1934,6 +1925,8 @@ int bpf_compute_subprog_arg_access(struct bpf_verifier_env *env)
> >          * each subprog is analyzed before its callees, allowing the
> >          * recursive walk inside analyze_subprog() to naturally
> >          * reach nested callees that also lack FP-derived args.
> > +        *
> > +        * Note the main subprog is also analyzed as part of this loop.
> >          */
> 
> Thanks for the report.
> I guess the fix is correct.
> This part of the comment needs to be adjusted:
> "they were skipped during
>          * the recursive walk above."
> 
> I wonder whether something like this is cleaner?
> -               if (info[sub].at_in && !bpf_subprog_is_global(env, sub))
> +               if (info[sub].at_in && (!bpf_subprog_is_global(env,
> sub) || sub == 0))
> 
> This part of the comment:
> "Async callbacks (timer, workqueue) are
> * also not reachable from the main program's call graph."
> 
> is also not quite correct.
> In here:
>                 } else if (bpf_calls_callback(env, idx)) {
>                         callee = find_callback_subprog(env, insn, idx,
> &caller_reg, &cb_callee_reg);
> 
> they could have been reached, but find_callback_subprog()
> will ignore timers because FP-derived args won't be passed
> into them.
> 
> Eduard,
> wdyt?

This is a nice catch. Idk if 'sub == 0' part is necessary, tbh.
Paul, please add my acked-by when you respin with fixed comments.

Acked-by: Eduard Zingerman <eddyz87@gmail.com>