Re: [PATCH bpf v1 1/2] bpf: Fix grace period wait for tracepoint bpf_link

Kumar Kartikeya Dwivedi
2026-03-30T14:04:14Z

On Mon, 30 Mar 2026 at 12:00, Puranjay Mohan <puranjay@kernel.org> wrote:
>
> Kumar Kartikeya Dwivedi <memxor@gmail.com> writes:
>
> > On Mon, 30 Mar 2026 at 05:21, Kumar Kartikeya Dwivedi <memxor@gmail.com> wrote:
> >>
> >> Recently, tracepoints were switched from using disabled preemption
> >> (which acts as RCU read section) to SRCU-fast when they are not
> >> faultable. This means that to do a proper grace period wait for programs
> >> running in such tracepoints, we must use SRCU's grace period wait.
> >> This is only for non-faultable tracepoints, faultable ones continue
> >> using RCU Tasks Trace.
> >>
> >> However, bpf_link_free() currently does call_rcu() for all cases when
> >> the link is non-sleepable (hence, for tracepoints, non-faultable). Fix
> >> this by doing a call_srcu() grace period wait.
> >>
> >> As far RCU Tasks Trace gp -> RCU gp chaining is concerned, it is deemed
> >> unnecessary for tracepoint programs. The link and program are either
> >> accessed under RCU Tasks Trace protection, or SRCU-fast protection now.
> >>
> >> The earlier logic of chaining both RCU Tasks Trace and RCU gp waits was
> >> to generalize the logic, even if it conceded an extra RCU gp wait,
> >> however that is unnecessary for tracepoints even before this change.
> >> In practice no cost was paid since rcu_trace_implies_rcu_gp() was always
> >> true.
> >>
> >> Hence we need not chain any SRCU gp waits after RCU Tasks Trace.
> >
> > ... or chaining RCP gp after SRCU gp, rather, the commit log should
> > probably say that instead. The above might be confusing.
> > But more eyes on this would be great, I went back and read a few
> > discussions on why we were chaining RCU gp after RCU-tt gp and
> > couldn't convince myself it was necessary for the tracepoint path.
>
> Yeah the commit message is a bit hard to follow, let me try to lay out
> why chaining isn't needed for either case, let me know if you agree with
> this analysis:
>
> For non-faultable tracepoints (the call_srcu path):
>
> The tracepoint dispatch macro in __DECLARE_TRACE does:
>
> guard(srcu_fast_notrace)(&tracepoint_srcu);
> __DO_TRACE_CALL(name, args);
>
> which calls into __bpf_trace_##call, which calls bpf_trace_runN, and
> that ends up in __bpf_trace_run() where we have:
>
> struct bpf_prog *prog = link->link.prog;
> ...
> rcu_read_lock_dont_migrate();
> ...
> run_ctx.bpf_cookie = link->cookie;
> bpf_prog_run(prog, args);
> ...
> rcu_read_unlock_migrate();
>
> Both the link dereference (link->link.prog) and the
> rcu_read_lock_dont_migrate() happen inside the SRCU-fast read section
> from the tracepoint macro. So classic RCU is nested inside SRCU-fast
> here. When the SRCU grace period completes, all in-flight SRCU-fast
> readers have finished, which means all their nested classic RCU read
> sections have also finished. No need to chain a classic RCU GP after
> the SRCU GP.
>
> For faultable tracepoints (the call_rcu_tasks_trace path):
>
> __DECLARE_TRACE_SYSCALL uses guard(rcu_tasks_trace)() instead of
> SRCU-fast, so SRCU isn't involved at all on this path. The link and
> program are accessed exclusively under RCU Tasks Trace protection.
> A tasks trace GP is sufficient on its own, and since tasks trace GP
> implies classic RCU GP, there's nothing to chain.
>
> So in both cases, the outermost protection (SRCU-fast or tasks trace)
> is what we wait for in bpf_link_free(), and the inner
> rcu_read_lock_dont_migrate() in __bpf_trace_run() is subsumed by that
> outer GP wait.
>
> Am I missing something?

No, this is my thinking as well, but thanks for laying it out so clearly.
I think it was unnecessary before too but I think the purpose to chain
them was to keep things generic and not tracepoint specific.
I don't see a path to that here (or unnecessary chaining penalization
with RCU gp after SRCU gp, which isn't implied unlike with RCU-tt gp),
so I leaned toward the current change.

>
> Thanks,
> Puranjay