On Fri, Jul 31, 2026 at 2:03 PM Zqiang <[email protected]> wrote: > > > > > call_srcu() has the same constraint as call_rcu(): its callback list and > > locks are only touched with interrupts disabled. srcu_gp_start_if_needed() > > enqueues under raw_spin_lock_irqsave() and may walk the srcu_node tree, > > and callback invocation and grace-period work do likewise. So a > > call_srcu() with interrupts disabled may be racing a list operation in > > flight on this CPU, and enqueuing directly could corrupt the list or > > deadlock. > > > > Defer as call_rcu() does: stage the callback on the srcu_data's > > ->defer_cbs, chain that srcu_data onto a per-CPU list, and kick a per-CPU > > irq_work that re-issues it straight to the enqueue helper -- never back > > through __call_srcu(), so it cannot defer again. > > > > The irq_work is global (per CPU, not per srcu_struct) and statically > > initialized, so the deferral never runs check_init_srcu_struct(). It is > > IRQ_WORK_INIT_HARD to re-issue promptly, as for call_rcu(). srcu_barrier() > > and cleanup_srcu_struct() flush it first, and rcutree_migrate_callbacks() > > calls srcu_offline_drain() to drain an outgoing CPU's deferred callbacks. > > Gated by CONFIG_RCU_DEFER, as for call_rcu(). > > > > Suggested-by: Paul E. McKenney <[email protected]> > > Signed-off-by: Puranjay Mohan <[email protected]> > > --- > > include/linux/srcutree.h | 4 ++ > > kernel/rcu/rcu.h | 3 + > > kernel/rcu/srcutree.c | 138 ++++++++++++++++++++++++++++++++++++++- > > kernel/rcu/tree.c | 2 + > > 4 files changed, 144 insertions(+), 3 deletions(-) > > > > diff --git a/include/linux/srcutree.h b/include/linux/srcutree.h > > index 75e54e4f963fa..1ce759fb70948 100644 > > --- a/include/linux/srcutree.h > > +++ b/include/linux/srcutree.h > > @@ -13,6 +13,8 @@ > > > > #include <linux/rcu_node_tree.h> > > #include <linux/completion.h> > > +#include <linux/irq_work_types.h> > > +#include <linux/llist.h> > > > > struct srcu_node; > > struct srcu_struct; > > @@ -41,6 +43,8 @@ struct srcu_data { > > bool srcu_cblist_invoking; /* Invoking these CBs? */ > > struct timer_list delay_work; /* Delay for CB invoking */ > > struct work_struct work; /* Context for CB invoking. */ > > + struct llist_head defer_cbs; /* Callbacks deferred on re-entry. */ > > + struct llist_node defer_link; /* Links onto the per-CPU deferral drain > > list */ > > struct rcu_head srcu_barrier_head; /* For srcu_barrier() use. */ > > struct rcu_head srcu_ec_head; /* For srcu_expedite_current() use. */ > > int srcu_ec_state; /* State for srcu_expedite_current(). */ > > diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h > > index ed6604445f2ac..036d28516e92d 100644 > > --- a/kernel/rcu/rcu.h > > +++ b/kernel/rcu/rcu.h > > @@ -587,6 +587,9 @@ static inline bool should_rcu_defer(void) > > return irqs_disabled() && rcu_scheduler_active != RCU_SCHEDULER_INACTIVE; > > } > > > > +/* Drain an outgoing CPU's deferred SRCU callbacks; see > > rcutree_migrate_callbacks(). */ > > +void srcu_offline_drain(int cpu); > > + > > enum rcutorture_type { > > RCU_FLAVOR, > > RCU_TASKS_FLAVOR, > > diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c > > index 304112674e8a2..b9fe57ff91000 100644 > > --- a/kernel/rcu/srcutree.c > > +++ b/kernel/rcu/srcutree.c > > @@ -20,6 +20,7 @@ > > #include <linux/percpu.h> > > #include <linux/preempt.h> > > #include <linux/irq_work.h> > > +#include <linux/llist.h> > > #include <linux/rcupdate_wait.h> > > #include <linux/sched.h> > > #include <linux/smp.h> > > @@ -79,6 +80,46 @@ static void process_srcu(struct work_struct *work); > > static void srcu_irq_work(struct irq_work *work); > > static void srcu_delay_timer(struct timer_list *t); > > > > +static void srcu_defer_drain(struct irq_work *iw); > > + > > +/* > > + * Per-CPU call_srcu() deferral state. The irq_work is global (shared by > > every > > + * srcu_struct), so it must find which srcu_structs deferred callbacks: a > > + * callback is staged on its srcu_data's ->defer_cbs, and that srcu_data is > > + * chained (->defer_link) onto this per-CPU ->list that the irq_work walks. > > + */ > > +struct srcu_defer { > > + struct llist_head list; > > + struct irq_work iw; > > + raw_spinlock_t lock; > > +}; > > + > > +static DEFINE_PER_CPU(struct srcu_defer, srcu_defer) = { > > + .lock = __RAW_SPIN_LOCK_UNLOCKED(srcu_defer.lock), > > + .iw = IRQ_WORK_INIT_HARD(srcu_defer_drain), > > +}; > > + > > +/* > > + * Register pending deferred call_srcu() callbacks so a following > > srcu_barrier() > > + * waits for them. An online CPU's own irq_work re-issues its callbacks, > > so wait > > + * it out; an offline CPU's irq_work may never run again, so drain its list > > + * directly onto this CPU instead. > > + */ > > +static void srcu_defer_flush(void) > > +{ > > + int cpu; > > + > > + if (!IS_ENABLED(CONFIG_RCU_DEFER)) > > + return; > > + > > + for_each_possible_cpu(cpu) { > > + if (cpu_online(cpu)) > > + irq_work_sync(&per_cpu(srcu_defer, cpu).iw); > > + else > > + srcu_defer_drain(&per_cpu(srcu_defer, cpu).iw); > > + } > > +} > > + > > /* > > * Initialize SRCU per-CPU data. Note that statically allocated > > * srcu_struct structures might already have srcu_read_lock() and > > @@ -107,6 +148,11 @@ static void init_srcu_struct_data(struct srcu_struct > > *ssp) > > sdp->cpu = cpu; > > INIT_WORK(&sdp->work, srcu_invoke_callbacks); > > timer_setup(&sdp->delay_work, srcu_delay_timer, 0); > > + /* > > + * ->defer_cbs and ->defer_link are valid when zeroed and are not > > + * reinitialized here, lest we clobber callbacks a reentrant > > + * call_srcu() already staged. See __call_srcu(). > > + */ > > sdp->ssp = ssp; > > } > > } > > @@ -697,6 +743,8 @@ void cleanup_srcu_struct(struct srcu_struct *ssp) > > return; /* Just leak it! */ > > /* Wait for irq_work to finish first as it may queue a new work. */ > > irq_work_sync(&sup->irq_work); > > + /* Drain any deferred callbacks (shared irq_work) before freeing ->sda. */ > > + srcu_defer_flush(); > > 1. Should we move srcu_defer_flush() before irq_work_sync(&sup->irq_work) ?
Will fix it in next version. > > 2. Will the following scenario trigger soft/hard_lockup ? > > In irq_work context, irq disbaled: > srcu_defer_drain > ->srcu_do_enqueue > ->srcu_gp_start_if_needed > ->rcu_segcblist_enqueue > fentry ebpf prog > call_rcu_tasks_trace() > ->__call_srcu() > ->should_rcu_defer() = true > ->llist_add() + irq_work_queue() > the ipi send and irq_work run again -> > srcu_defer_drain()->srcu_do_enqueue > > Did I miss something? BPF programs don't allow recursion on the same CPU for the same BPF program, but your scenario can still occur because here the BPF program doesn't recurse rather it keeps enqueueing and firing IPIs. Before this patch it would have been a deadlock. So, this is expected behaviour and to fix it we have following options: 1. Make stuff under call_srcu() notrace or keep the normal enqueue traceable (so the outer reentry still happens once and the deadlock fix is still exercised), but have srcu_defer_drain() re-issue through a notrace copy of the enqueue chain so the prog can't fire during the drain. 2. Accept it and document it, and correct BPF programs like the selftest in last patch will work because they detect reently. 3. Have a per-cpu in-drain flag set in srcu_defer_drain() and if in call_srcu should_rcu_defer() returns true and this flag is set + in_nmi() returns false, this means we are in this refeeding loop, so we can just leak that callback with a warning. I prefer option 3 but let's wait for Paul for his feedback. Thanks, Puranjay
