Re: [PATCH bpf-next v1 1/3] bpf: Show more useful info in stack depth stats

On Sun, 2026-08-02 at 23:59 +0200, Kumar Kartikeya Dwivedi wrote:
> On Sun Aug 2, 2026 at 11:02 PM CEST, Eduard Zingerman wrote:
> > On Sun, 2026-08-02 at 01:03 +0200, Kumar Kartikeya Dwivedi wrote:
> > > Currently, the output of stack depth statistics is two crude, with a
> > > list of captured stack depths ordered by the subprog numbers. The actual
> > > subprog numbers are determined by libbpf, hence it is hard to associate
> > > the stack depth statistic back to the subprog by name.
> > > 
> > > Change the format to:
> > > stack depth <subprog>=<depth> ... max=<depth>
> > > 
> > > In case the subprog name is not specified, use subprog[N] as the string.
> > > 
> > > Suggested-by: Andrii Nakryiko <[email protected]>
> > > Signed-off-by: Kumar Kartikeya Dwivedi <[email protected]>
> > > ---
> > >  kernel/bpf/verifier.c                         | 16 ++++++++++++----
> > >  .../bpf/progs/verifier_basic_stack.c          |  4 ++--
> > >  .../bpf/progs/verifier_bpf_fastcall.c         | 19 +++++++++++--------
> > >  .../bpf/progs/verifier_private_stack.c        |  9 ++++++---
> > >  .../selftests/bpf/progs/verifier_var_off.c    |  4 ++--
> > >  5 files changed, 33 insertions(+), 19 deletions(-)
> > > 
> > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > > index 8d0635ee48c7..51ac0a9ae339 100644
> > > --- a/kernel/bpf/verifier.c
> > > +++ b/kernel/bpf/verifier.c
> > > @@ -18703,10 +18703,18 @@ static void print_verification_stats(struct bpf_verifier_env *env)
> > >  	if (env->log.level & BPF_LOG_STATS) {
> > >  		verbose(env, "verification time %lld usec\n",
> > >  			div_u64(env->verification_time, 1000));
> > > -		verbose(env, "stack depth %d", env->subprog_info[0].stack_depth);
> > > -		for (i = 1; i < subprog_cnt; i++)
> > > -			verbose(env, "+%d", env->subprog_info[i].stack_depth);
> > > -		verbose(env, " max %d\n", env->max_stack_depth);
> > > +		verbose(env, "stack depth");
> > > +		for (i = 0; i < subprog_cnt; i++) {
> > > +			const char *name = env->subprog_info[i].name;
> > > +
> > > +			if (name && name[0])
> > > +				verbose(env, " %s=%d", name,
> > > +					env->subprog_info[i].stack_depth);
> > > +			else
> > > +				verbose(env, " subprog[%d]=%d", i,
> > > +					env->subprog_info[i].stack_depth);
> > 
> > That would be not very convenient to parse by veristat,
> > but I'd print every subprogram on it's own line.
> > 
> 
> I can do that, but let's agree on the format before I get to it.
> How should it look like? Prefix with stack depth every line, and print stack
> depth max ... as the very first line? Like so:
> 
> stack depth max N
> stack depth subprog 0 <name> N
> ...
> stack depth subprog N <name> N
> 
> ?

Ideally I'd drop the "stack depth " prefix after the first one and
just add an indentation instead. But that would be hard for veristat
to parse. So let's settle on your suggested version.

> > > +		}
> > > +		verbose(env, " max=%d\n", env->max_stack_depth);
> > 
> > I think check_max_stack_depth_subprog() needs a similar update,
> > at the moment it reports a not very helpful error like this:
> > 
> >   combined stack size of 34 calls is 528. Too large
> > 
> > Printing the offending spine would be helpful.
> > 
> 
> I'll do that in verifier errors patch set. This is just for the verification
> stats, that are outside its scope.

Fine by me