Re: [PATCH bpf-next v5 07/14] bpf: Track verifier context diagnostic events

[email protected]
Newsgroups org.kernel.vger.bpf
Message-ID <a4998b9ef3040cb6ae37c6fb2ee4444c737531839d530cafbe4657294af40806@mail.kernel.org>
> bpf: Track verifier context diagnostic events
>
> Record verifier context transitions in the diagnostic history so later reports
> can anchor causal paths to the critical section that made an operation invalid.
>
> This covers lock, IRQ, RCU, and preempt regions without adding any new
> verifier error reports. Category-specific commits decide where those recorded
> events should be rendered.
>
> Use context depth when selecting scoped history so nested regions anchor at the
> outer active region, and fall back to the earliest retained event when the
> matching entry was pruned.
>
> Acked-by: Eduard Zingerman <[email protected]>
> Signed-off-by: Kumar Kartikeya Dwivedi <[email protected]>

The third paragraph describes history-selection behavior that this
commit does not implement. Does this paragraph belong in a later commit
instead?

This patch records BPF_DIAG_HISTORY_CONTEXT events via
bpf_diag_record_context(), but nothing in the diff consumes the event
log or selects a scoped history range. The code that actually does what
the paragraph describes is diag_history_context_start_idx() in
kernel/bpf/diagnostics.c:

  if (!opts->ctx_depth)
          return 0;
  /* Find the most recent outermost entry, or a depth-zero exit. */
  for (i = log->cnt; i > 0; i--) {
          ...
          if (event->ctx.enter && event->ctx.depth == 1)
                  return i - 1;      /* anchor at the outer active region */
          if (!event->ctx.enter && event->ctx.depth == 0)
                  return 0;          /* earliest retained event */
  }

and it appears in the next commit d08c774626ab ("bpf: Report Register
Type Safety errors") together with struct bpf_diag_history_opts::ctx_depth
and diag_history_start_idx(). Neither bpf_diag_history_opts nor any
scoped-history selector exists at this commit.

The recorded depths are correct and match the later consumer. But a
reviewer reading this patch will search for the described selection
logic and not find it, which conflicts with the second paragraph that
correctly states "Category-specific commits decide where those recorded
events should be rendered."

Could the third paragraph be moved to d08c774626ab, or reworded to say
the depth field is recorded here so that later commits can anchor scoped
history?


---
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/31870608176
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.