From: Paul Chaignon <paul.chaignon@gmail.com>
To: Eduard Zingerman <eddyz87@gmail.com>
Cc: bpf@vger.kernel.org, ast@kernel.org, andrii@kernel.org,
daniel@iogearbox.net, martin.lau@linux.dev, kernel-team@fb.com,
yonghong.song@linux.dev
Subject: Re: [PATCH bpf-next v4 10/14] bpf: change logging scheme for live stack analysis
Date: Fri, 8 May 2026 01:29:32 +0200 [thread overview]
Message-ID: <af0gXEqD3FPFYZDj@mail.gmail.com> (raw)
In-Reply-To: <20260410-patch-set-v4-10-5d4eecb343db@gmail.com>
On Fri, Apr 10, 2026 at 01:56:01PM -0700, Eduard Zingerman wrote:
> Instead of breadcrumbs like:
>
> (d2,cs15) frame 0 insn 18 +live -16
> (d2,cs15) frame 0 insn 17 +live -16
>
> Print final accumulated stack use/def data per-func_instance
> per-instruction. printed func_instance's are ordered by callsite and
> depth. For example:
Hi Eduard,
Sorry to revive an old thread.
We've started running a kernel with this patchset in Cilium's CI and
noticed a big increase of log verbosity for BPF_LOG_LEVEL2. To get an
idea, a full dump of all (uncompressed) verifier logs for our
tested programs used to take ~8.4G. With this patchset, it now takes
15G. [1] helps a bit to reduce it, but still only getting to 12G.
The increase seems to come from print_subprog_arg_access(), which prints
full functions with the results of the fixed-point analysis, from
print_instances(), which prints full functions with the use/def slots,
and to a lesser extent from compute_subprog_args(), which prints the
fixed-point iterations.
I'm wondering if maybe there are other opportunities to reduce
verbosity here (besides [1]). Maybe we don't need to print the
fixed-point iterations if we're already printing the results? Or maybe
we could put the more detailed liveness-related logs behind
BPF_LOG_LEVEL3 or BPF_LOG_LIVENESS?
1: https://lore.kernel.org/bpf/e4d7b53d4963ef520541a782f5fc8108a168877c.1778176504.git.paul.chaignon@gmail.com/
>
> stack use/def subprog#0 shared_instance_must_write_overwrite (d0,cs0):
> 0: (b7) r1 = 1
> 1: (7b) *(u64 *)(r10 -8) = r1 ; def: fp0-8
> 2: (7b) *(u64 *)(r10 -16) = r1 ; def: fp0-16
> 3: (bf) r1 = r10
> 4: (07) r1 += -8
> 5: (bf) r2 = r10
> 6: (07) r2 += -16
> 7: (85) call pc+7 ; use: fp0-8 fp0-16
> 8: (bf) r1 = r10
> 9: (07) r1 += -16
> 10: (bf) r2 = r10
> 11: (07) r2 += -8
> 12: (85) call pc+2 ; use: fp0-8 fp0-16
> 13: (b7) r0 = 0
> 14: (95) exit
> stack use/def subprog#1 forwarding_rw (d1,cs7):
> 15: (85) call pc+1 ; use: fp0-8 fp0-16
> 16: (95) exit
> stack use/def subprog#1 forwarding_rw (d1,cs12):
> 15: (85) call pc+1 ; use: fp0-8 fp0-16
> 16: (95) exit
> stack use/def subprog#2 write_first_read_second (d2,cs15):
> 17: (7a) *(u64 *)(r1 +0) = 42
> 18: (79) r0 = *(u64 *)(r2 +0) ; use: fp0-8 fp0-16
> 19: (95) exit
>
> For groups of three or more consecutive stack slots, abbreviate as
> follows:
>
> 25: (85) call bpf_loop#181 ; use: fp2-8..-512 fp1-8..-512 fp0-8..-512
>
> Signed-off-by: Eduard Zingerman <eddyz87@gmail.com>
> ---
> include/linux/bpf_verifier.h | 5 -
> kernel/bpf/liveness.c | 241 +++++++++++++++++++++++++++++--------------
> 2 files changed, 165 insertions(+), 81 deletions(-)
>
> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index d7fbc0e1559b..d3dc46aae2e7 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h
> @@ -256,11 +256,6 @@ static inline spis_t spis_and(spis_t a, spis_t b)
> return (spis_t){{ a.v[0] & b.v[0], a.v[1] & b.v[1] }};
> }
>
> -static inline spis_t spis_xor(spis_t a, spis_t b)
> -{
> - return (spis_t){{ a.v[0] ^ b.v[0], a.v[1] ^ b.v[1] }};
> -}
> -
> static inline spis_t spis_not(spis_t s)
> {
> return (spis_t){{ ~s.v[0], ~s.v[1] }};
> diff --git a/kernel/bpf/liveness.c b/kernel/bpf/liveness.c
> index 1c8477069a43..b8005094c6a6 100644
> --- a/kernel/bpf/liveness.c
> +++ b/kernel/bpf/liveness.c
> @@ -6,6 +6,7 @@
> #include <linux/hashtable.h>
> #include <linux/jhash.h>
> #include <linux/slab.h>
> +#include <linux/sort.h>
>
> #define verbose(env, fmt, args...) bpf_verifier_log_write(env, fmt, ##args)
>
> @@ -259,64 +260,6 @@ static int mark_stack_write(struct func_instance *instance, u32 frame, u32 insn_
> return 0;
> }
>
> -static char *fmt_instance(struct bpf_verifier_env *env, struct func_instance *instance)
> -{
> - snprintf(env->tmp_str_buf, sizeof(env->tmp_str_buf),
> - "(d%d,cs%d)", instance->depth, instance->callsite);
> - return env->tmp_str_buf;
> -}
> -
> -/*
> - * When both halves of an 8-byte SPI are set, print as "-8","-16",...
> - * When only one half is set, print as "-4h","-8h",...
> - */
> -static void bpf_fmt_spis_mask(char *buf, ssize_t buf_sz, spis_t spis)
> -{
> - bool first = true;
> - int spi, n;
> -
> - buf[0] = '\0';
> -
> - for (spi = 0; spi < STACK_SLOTS / 2 && buf_sz > 0; spi++) {
> - bool lo = spis_test_bit(spis, spi * 2);
> - bool hi = spis_test_bit(spis, spi * 2 + 1);
> -
> - if (!lo && !hi)
> - continue;
> - n = snprintf(buf, buf_sz, "%s%d%s",
> - first ? "" : ",",
> - -(spi + 1) * BPF_REG_SIZE + (lo && !hi ? BPF_HALF_REG_SIZE : 0),
> - lo && hi ? "" : "h");
> - first = false;
> - buf += n;
> - buf_sz -= n;
> - }
> -}
> -
> -static void log_mask_change(struct bpf_verifier_env *env, struct func_instance *instance,
> - char *pfx, u32 frame, u32 insn_idx,
> - spis_t old, spis_t new)
> -{
> - spis_t changed_bits, new_ones, new_zeros;
> -
> - changed_bits = spis_xor(old, new);
> - new_ones = spis_and(new, changed_bits);
> - new_zeros = spis_and(spis_not(new), changed_bits);
> -
> - if (spis_is_zero(changed_bits))
> - return;
> - bpf_log(&env->log, "%s frame %d insn %d ", fmt_instance(env, instance), frame, insn_idx);
> - if (!spis_is_zero(new_ones)) {
> - bpf_fmt_spis_mask(env->tmp_str_buf, sizeof(env->tmp_str_buf), new_ones);
> - bpf_log(&env->log, "+%s %s ", pfx, env->tmp_str_buf);
> - }
> - if (!spis_is_zero(new_zeros)) {
> - bpf_fmt_spis_mask(env->tmp_str_buf, sizeof(env->tmp_str_buf), new_zeros);
> - bpf_log(&env->log, "-%s %s", pfx, env->tmp_str_buf);
> - }
> - bpf_log(&env->log, "\n");
> -}
> -
> int bpf_jmp_offset(struct bpf_insn *insn)
> {
> u8 code = insn->code;
> @@ -392,7 +335,6 @@ __diag_pop();
> static inline bool update_insn(struct bpf_verifier_env *env,
> struct func_instance *instance, u32 frame, u32 insn_idx)
> {
> - struct bpf_insn_aux_data *aux = env->insn_aux_data;
> spis_t new_before, new_after;
> struct per_frame_masks *insn, *succ_insn;
> struct bpf_iarray *succ;
> @@ -419,13 +361,6 @@ static inline bool update_insn(struct bpf_verifier_env *env,
> new_before = spis_or(spis_and(new_after, spis_not(insn->must_write)),
> insn->may_read);
> changed |= !spis_equal(new_before, insn->live_before);
> - if (unlikely(env->log.level & BPF_LOG_LEVEL2) &&
> - (!spis_is_zero(insn->may_read) || !spis_is_zero(insn->must_write) ||
> - insn_idx == instance->subprog_start ||
> - aux[insn_idx].prune_point)) {
> - log_mask_change(env, instance, "live",
> - frame, insn_idx, insn->live_before, new_before);
> - }
> insn->live_before = new_before;
> return changed;
> }
> @@ -455,10 +390,6 @@ static void update_instance(struct bpf_verifier_env *env, struct func_instance *
> changed |= update_insn(env, instance, frame, insn_postorder[i]);
> }
> } while (changed);
> -
> - if (env->log.level & BPF_LOG_LEVEL2)
> - bpf_log(&env->log, "%s live stack update done in %d iterations\n",
> - fmt_instance(env, instance), cnt);
> }
>
> static bool is_live_before(struct func_instance *instance, u32 insn_idx, u32 frameno, u32 half_spi)
> @@ -538,6 +469,166 @@ bool bpf_stack_slot_alive(struct bpf_verifier_env *env, u32 frameno, u32 half_sp
> return false;
> }
>
> +static char *fmt_subprog(struct bpf_verifier_env *env, int subprog)
> +{
> + const char *name = env->subprog_info[subprog].name;
> +
> + snprintf(env->tmp_str_buf, sizeof(env->tmp_str_buf),
> + "subprog#%d%s%s", subprog, name ? " " : "", name ? name : "");
> + return env->tmp_str_buf;
> +}
> +
> +static char *fmt_instance(struct bpf_verifier_env *env, struct func_instance *instance)
> +{
> + snprintf(env->tmp_str_buf, sizeof(env->tmp_str_buf),
> + "(d%d,cs%d)", instance->depth, instance->callsite);
> + return env->tmp_str_buf;
> +}
> +
> +static int spi_off(int spi)
> +{
> + return -(spi + 1) * BPF_REG_SIZE;
> +}
> +
> +/*
> + * When both halves of an 8-byte SPI are set, print as "-8","-16",...
> + * When only one half is set, print as "-4h","-8h",...
> + * Runs of 3+ consecutive fully-set SPIs are collapsed: "fp0-8..-24"
> + */
> +static char *fmt_spis_mask(struct bpf_verifier_env *env, int frame, bool first, spis_t spis)
> +{
> + int buf_sz = sizeof(env->tmp_str_buf);
> + char *buf = env->tmp_str_buf;
> + int spi, n, run_start;
> +
> + buf[0] = '\0';
> +
> + for (spi = 0; spi < STACK_SLOTS / 2 && buf_sz > 0; spi++) {
> + bool lo = spis_test_bit(spis, spi * 2);
> + bool hi = spis_test_bit(spis, spi * 2 + 1);
> + const char *space = first ? "" : " ";
> +
> + if (!lo && !hi)
> + continue;
> +
> + if (!lo || !hi) {
> + /* half-spi */
> + n = scnprintf(buf, buf_sz, "%sfp%d%d%s",
> + space, frame, spi_off(spi) + (lo ? STACK_SLOT_SZ : 0), "h");
> + } else if (spi + 2 < STACK_SLOTS / 2 &&
> + spis_test_bit(spis, spi * 2 + 2) &&
> + spis_test_bit(spis, spi * 2 + 3) &&
> + spis_test_bit(spis, spi * 2 + 4) &&
> + spis_test_bit(spis, spi * 2 + 5)) {
> + /* 3+ consecutive full spis */
> + run_start = spi;
> + while (spi + 1 < STACK_SLOTS / 2 &&
> + spis_test_bit(spis, (spi + 1) * 2) &&
> + spis_test_bit(spis, (spi + 1) * 2 + 1))
> + spi++;
> + n = scnprintf(buf, buf_sz, "%sfp%d%d..%d",
> + space, frame, spi_off(run_start), spi_off(spi));
> + } else {
> + /* just a full spi */
> + n = scnprintf(buf, buf_sz, "%sfp%d%d", space, frame, spi_off(spi));
> + }
> + first = false;
> + buf += n;
> + buf_sz -= n;
> + }
> + return env->tmp_str_buf;
> +}
> +
> +static void print_instance(struct bpf_verifier_env *env, struct func_instance *instance)
> +{
> + int start = env->subprog_info[instance->subprog].start;
> + struct bpf_insn *insns = env->prog->insnsi;
> + struct per_frame_masks *masks;
> + int len = instance->insn_cnt;
> + int insn_idx, frame, i;
> + bool has_use, has_def;
> + u64 pos, insn_pos;
> +
> + if (!(env->log.level & BPF_LOG_LEVEL2))
> + return;
> +
> + verbose(env, "stack use/def %s ", fmt_subprog(env, instance->subprog));
> + verbose(env, "%s:\n", fmt_instance(env, instance));
> + for (i = 0; i < len; i++) {
> + insn_idx = start + i;
> + has_use = false;
> + has_def = false;
> + pos = env->log.end_pos;
> + verbose(env, "%3d: ", insn_idx);
> + bpf_verbose_insn(env, &insns[insn_idx]);
> + bpf_vlog_reset(&env->log, env->log.end_pos - 1); /* remove \n */
> + insn_pos = env->log.end_pos;
> + verbose(env, "%*c;", bpf_vlog_alignment(insn_pos - pos), ' ');
> + pos = env->log.end_pos;
> + verbose(env, " use: ");
> + for (frame = instance->depth; frame >= 0; --frame) {
> + masks = get_frame_masks(instance, frame, insn_idx);
> + if (!masks || spis_is_zero(masks->may_read))
> + continue;
> + verbose(env, "%s", fmt_spis_mask(env, frame, !has_use, masks->may_read));
> + has_use = true;
> + }
> + if (!has_use)
> + bpf_vlog_reset(&env->log, pos);
> + pos = env->log.end_pos;
> + verbose(env, " def: ");
> + for (frame = instance->depth; frame >= 0; --frame) {
> + masks = get_frame_masks(instance, frame, insn_idx);
> + if (!masks || spis_is_zero(masks->must_write))
> + continue;
> + verbose(env, "%s", fmt_spis_mask(env, frame, !has_def, masks->must_write));
> + has_def = true;
> + }
> + if (!has_def)
> + bpf_vlog_reset(&env->log, has_use ? pos : insn_pos);
> + verbose(env, "\n");
> + if (bpf_is_ldimm64(&insns[insn_idx]))
> + i++;
> + }
> +}
> +
> +static int cmp_instances(const void *pa, const void *pb)
> +{
> + struct func_instance *a = *(struct func_instance **)pa;
> + struct func_instance *b = *(struct func_instance **)pb;
> + int dcallsite = (int)a->callsite - b->callsite;
> + int ddepth = (int)a->depth - b->depth;
> +
> + if (dcallsite)
> + return dcallsite;
> + if (ddepth)
> + return ddepth;
> + return 0;
> +}
> +
> +/* print use/def slots for all instances ordered by callsite first, then by depth */
> +static int print_instances(struct bpf_verifier_env *env)
> +{
> + struct func_instance *instance, **sorted_instances;
> + struct bpf_liveness *liveness = env->liveness;
> + int i, bkt, cnt;
> +
> + cnt = 0;
> + hash_for_each(liveness->func_instances, bkt, instance, hl_node)
> + cnt++;
> + sorted_instances = kvmalloc_objs(*sorted_instances, cnt, GFP_KERNEL_ACCOUNT);
> + if (!sorted_instances)
> + return -ENOMEM;
> + cnt = 0;
> + hash_for_each(liveness->func_instances, bkt, instance, hl_node)
> + sorted_instances[cnt++] = instance;
> + sort(sorted_instances, cnt, sizeof(*sorted_instances), cmp_instances, NULL);
> + for (i = 0; i < cnt; i++)
> + print_instance(env, sorted_instances[i]);
> + kvfree(sorted_instances);
> + return 0;
> +}
> +
> /*
> * Per-register tracking state for compute_subprog_args().
> * Tracks which frame's FP a value is derived from
> @@ -1425,12 +1516,7 @@ static void print_subprog_arg_access(struct bpf_verifier_env *env,
> if (!(env->log.level & BPF_LOG_LEVEL2))
> return;
>
> - verbose(env, "subprog#%d %s:\n", subprog,
> - env->prog->aux->func_info
> - ? btf_name_by_offset(env->prog->aux->btf,
> - btf_type_by_id(env->prog->aux->btf,
> - env->prog->aux->func_info[subprog].type_id)->name_off)
> - : "");
> + verbose(env, "%s:\n", fmt_subprog(env, subprog));
> for (i = 0; i < len; i++) {
> int idx = start + i;
> bool has_extra = false;
> @@ -1918,6 +2004,9 @@ int bpf_compute_subprog_arg_access(struct bpf_verifier_env *env)
> goto out;
> }
>
> + if (env->log.level & BPF_LOG_LEVEL2)
> + err = print_instances(env);
> +
> out:
> for (k = 0; k < insn_cnt; k++)
> kvfree(env->callsite_at_stack[k]);
>
> --
> 2.53.0
>
next prev parent reply other threads:[~2026-05-07 23:29 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-10 20:55 [PATCH bpf-next v4 00/14] bpf: static stack liveness data flow analysis Eduard Zingerman
2026-04-10 20:55 ` [PATCH bpf-next v4 01/14] bpf: share several utility functions as internal API Eduard Zingerman
2026-04-10 20:55 ` [PATCH bpf-next v4 02/14] bpf: save subprogram name in bpf_subprog_info Eduard Zingerman
2026-04-10 20:55 ` [PATCH bpf-next v4 03/14] bpf: Add spis_*() helpers for 4-byte stack slot bitmasks Eduard Zingerman
2026-04-10 20:55 ` [PATCH bpf-next v4 04/14] bpf: make liveness.c track stack with 4-byte granularity Eduard Zingerman
2026-04-10 20:55 ` [PATCH bpf-next v4 05/14] bpf: 4-byte precise clean_verifier_state Eduard Zingerman
2026-04-10 20:55 ` [PATCH bpf-next v4 06/14] bpf: prepare liveness internal API for static analysis pass Eduard Zingerman
2026-04-10 20:55 ` [PATCH bpf-next v4 07/14] bpf: introduce forward arg-tracking dataflow analysis Eduard Zingerman
2026-04-10 21:44 ` bot+bpf-ci
2026-04-10 21:46 ` Eduard Zingerman
2026-04-10 22:17 ` Alexei Starovoitov
2026-04-10 20:55 ` [PATCH bpf-next v4 08/14] bpf: record arg tracking results in bpf_liveness masks Eduard Zingerman
2026-04-10 20:56 ` [PATCH bpf-next v4 09/14] bpf: simplify liveness to use (callsite, depth) keyed func_instances Eduard Zingerman
2026-04-10 21:39 ` Paul Chaignon
2026-04-10 21:42 ` Eduard Zingerman
2026-04-13 13:31 ` Paul Chaignon
2026-04-10 21:44 ` bot+bpf-ci
2026-04-10 22:33 ` Alexei Starovoitov
2026-04-10 20:56 ` [PATCH bpf-next v4 10/14] bpf: change logging scheme for live stack analysis Eduard Zingerman
2026-05-07 23:29 ` Paul Chaignon [this message]
2026-05-08 13:46 ` Eduard Zingerman
2026-04-10 20:56 ` [PATCH bpf-next v4 11/14] selftests/bpf: update existing tests due to liveness changes Eduard Zingerman
2026-04-10 20:56 ` [PATCH bpf-next v4 12/14] selftests/bpf: adjust verifier_log buffers Eduard Zingerman
2026-04-10 20:56 ` [PATCH bpf-next v4 13/14] selftests/bpf: add new tests for static stack liveness analysis Eduard Zingerman
2026-04-10 20:56 ` [PATCH bpf-next v4 14/14] bpf: poison dead stack slots Eduard Zingerman
2026-04-10 22:40 ` [PATCH bpf-next v4 00/14] bpf: static stack liveness data flow analysis patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=af0gXEqD3FPFYZDj@mail.gmail.com \
--to=paul.chaignon@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=eddyz87@gmail.com \
--cc=kernel-team@fb.com \
--cc=martin.lau@linux.dev \
--cc=yonghong.song@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox