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

Kumar Kartikeya Dwivedi
2026-03-30T14:02:39Z

On Mon, 30 Mar 2026 at 11:52, Puranjay Mohan <puranjay@kernel.org> wrote:
>
> Kumar Kartikeya Dwivedi <memxor@gmail.com> writes:
>
> > 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.
> >
> > Fixes: a46023d5616e ("tracing: Guard __DECLARE_TRACE() use of __DO_TRACE_CALL() with SRCU-fast")
> > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
>
> With a nit below which is mostly informational.
>
> Reviewed-by: Puranjay Mohan <puranjay@kernel.org>
>
> > ---
> > include/linux/tracepoint.h | 8 ++++++++
> > kernel/bpf/syscall.c | 22 ++++++++++++++++++++--
> > 2 files changed, 28 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
> > index 22ca1c8b54f3..8227102a771f 100644
> > --- a/include/linux/tracepoint.h
> > +++ b/include/linux/tracepoint.h
> > @@ -113,6 +113,10 @@ void for_each_tracepoint_in_module(struct module *mod,
> > */
> > #ifdef CONFIG_TRACEPOINTS
> > extern struct srcu_struct tracepoint_srcu;
> > +static inline struct srcu_struct *tracepoint_srcu_ptr(void)
> > +{
> > + return &tracepoint_srcu;
> > +}
> > static inline void tracepoint_synchronize_unregister(void)
> > {
> > synchronize_rcu_tasks_trace();
> > @@ -123,6 +127,10 @@ static inline bool tracepoint_is_faultable(struct tracepoint *tp)
> > return tp->ext && tp->ext->faultable;
> > }
> > #else
> > +static inline struct srcu_struct *tracepoint_srcu_ptr(void)
> > +{
> > + return NULL;
> > +}
> > static inline void tracepoint_synchronize_unregister(void)
> > { }
> > static inline bool tracepoint_is_faultable(struct tracepoint *tp)
> > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > index 274039e36465..ab61a5ce35af 100644
> > --- a/kernel/bpf/syscall.c
> > +++ b/kernel/bpf/syscall.c
> > @@ -3261,6 +3261,13 @@ static void bpf_link_defer_dealloc_rcu_gp(struct rcu_head *rcu)
> > bpf_link_dealloc(link);
> > }
> >
> > +static bool bpf_link_is_tracepoint(struct bpf_link *link)
> > +{
> > + /* Only these combinations support a tracepoint bpf_link. */
> > + return link->type == BPF_LINK_TYPE_RAW_TRACEPOINT ||
> > + (link->type == BPF_LINK_TYPE_TRACING && link->attach_type == BPF_TRACE_RAW_TP);
>
> nit: this second check is never true here, because BPF_LINK_TYPE_TRACING
> uses bpf_tracing_link_lops, which has .dealloc (not .dealloc_deferred)
> and this function is only called from dealloc_deferred() path.

bpf_raw_tp_link_attach has a special case for this, where this
combination ends up using bpf_raw_tp_link_lops instead.
I will add a comment since it's non-obvious. If you think about it, if
this wasn't the case, we'd have another bug.

>
> …