Re: [PATCH bpf-next v2 1/3] bpf: Show more useful info in stack depth stats
"Kumar Kartikeya Dwivedi" <[email protected]> Mon, 03 Aug 2026 02:40:26 +0200
| Newsgroups | org.kernel.vger.bpf |
|---|---|
| Message-ID | <[email protected]> |
On Mon Aug 3, 2026 at 2:21 AM CEST, bot+bpf-ci wrote:
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index b274004fccfd9..3b61897ed0d2a 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -18777,10 +18777,15 @@ 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 max %d\n", env->max_stack_depth);
>> + for (i = 0; i < subprog_cnt; i++) {
>> + const char *name = env->subprog_info[i].name;
>> +
>> + if (!name || !name[0])
>> + name = "<unknown>";
>> + verbose(env, "stack depth subprog %d %s %d\n", i, name,
>> + env->subprog_info[i].stack_depth);
>> + }
>
> This isn't a bug, but could the name lookup here reuse subprog_name()?
> It is already in this file and several other verbose() sites use it,
> for example in do_check_common()'s caller:
>
> kernel/bpf/verifier.c
> ...
> } else if (env->log.level & BPF_LOG_LEVEL) {
> verbose(env, "Func#%d ('%s') is safe for any args that match its prototype\n",
> i, subprog_name(env, i));
> }
> ...
>
> subprog_name() returns "" when there is no func_info, so the
> !name[0] fallback would still catch it:
>
This nit is fine.
> kernel/bpf/verifier.c:subprog_name() {
> if (!env->prog->aux->func_info)
> return "";
>
> info = &env->prog->aux->func_info[subprog];
> return btf_type_name(env->prog->aux->btf, info->type_id);
> }
>
> There is also fmt_subprog() in kernel/bpf/liveness.c doing the same
> name-or-fallback formatting, so this is a third variant of the same
> lookup.
>
That's too far fetched, and changes the format, I'll go with using subprog_name().
>> verbose(env, "insns processed %d", env->subprog_info[0].insn_processed);
>> for (i = 1; i < subprog_cnt; i++)
>> if (bpf_subprog_is_global(env, i))
>
> [ ... ]
>
>
> ---
> 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/30771845943