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

Puranjay Mohan
2026-03-30T09:52:24Z

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.

> +}
> +
> static void bpf_link_defer_dealloc_mult_rcu_gp(struct rcu_head *rcu)
> {
> if (rcu_trace_implies_rcu_gp())
> @@ -3279,16 +3286,27 @@ static void bpf_link_free(struct bpf_link *link)
> if (link->prog)
> ops->release(link);
> if (ops->dealloc_deferred) {
> - /* Schedule BPF link deallocation, which will only then
> + struct srcu_struct *tp_srcu = tracepoint_srcu_ptr();
> +
> + /*
> + * Schedule BPF link deallocation, which will only then
> * trigger putting BPF program refcount.
> * If underlying BPF program is sleepable or BPF link's target
> * attach hookpoint is sleepable or otherwise requires RCU GPs
> * to ensure link and its underlying BPF program is not
> * reachable anymore, we need to first wait for RCU tasks
> - * trace sync, and then go through "classic" RCU grace period
> + * trace sync, and then go through "classic" RCU grace period.
> + *
> + * For tracepoint BPF links, we need to go through SRCU grace
> + * period wait instead when non-faultable tracepoint is used. We
> + * don't need to chain SRCU grace period waits, however, for the
> + * faultable case, since it exclusively uses RCU Tasks Trace.
> */
> if (link->sleepable || (link->prog && link->prog->sleepable))
> call_rcu_tasks_trace(&link->rcu, bpf_link_defer_dealloc_mult_rcu_gp);
> + /* We need to do a SRCU grace period wait for tracepoint-based BPF links. */
> + else if (bpf_link_is_tracepoint(link) && tp_srcu)
> + call_srcu(tp_srcu, &link->rcu, bpf_link_defer_dealloc_rcu_gp);
> else
> call_rcu(&link->rcu, bpf_link_defer_dealloc_rcu_gp);
> } else if (ops->dealloc) {
> --
> 2.52.0