On Wed, Sep 2, 2026 at 9:26 AM Sebastian Andrzej Siewior
<[email protected]> wrote:
>
> On 2026-09-01 14:53:56 [+0200], Puranjay Mohan wrote:
> > On Wed, Aug 26, 2026 at 3:46 PM Sebastian Andrzej Siewior
> > > > +/*
> > > > + * Defer whenever interrupts are disabled, since a callback-list
> > > > operation may
> > > > + * be in flight on this CPU. Not before the scheduler is up: irq_work
> > > > is not
> > > > + * usable that early, and rcu_init() itself calls call_rcu().
> > > > + */
> > > > +static inline bool should_rcu_defer(void)
> > > > +{
> > > > + return IS_ENABLED(CONFIG_RCU_DEFER) && irqs_disabled() &&
> > > > + rcu_scheduler_active != RCU_SCHEDULER_INACTIVE;
> > > > +}
> > >
> > > Why does the description say that the defer part is for usage from NMI
> > > and the test here has irqs_disabled() instead of in_nmi()?
> >
> > We defer for both irq disabled sections, hard irq, and in_nmi() and
> > checking for irqs_disabled() covers all three.
>
> But it is wrong to talk about NMI and do this for other reasons not
> mentioning why this was needed/ made sense.
> Can this be fixed?
> Also what is the reasoning for doing it from any IRQ disabled region?
So, the main reason for this patchset is to make call_(s)rcu() work
from BPF programs, and these BPF programs can run from any context
like an NMI perf event interrupt or maybe attached to a function
through ftrace which is in the call chain of call_rcu() itself.
call_rcu() takes locks with irqs disabled or it modifies lists
non-atomically with irqs disabled. This means that a BPF program that
runs from NMI with the lock already taken will deadlock, or if it runs
between these non-atomic list modifications, it will corrupt the
lists. A BPF program can also synchronously re-enter call_rcu() if it
is attached to a function in the call chain of call_rcu() that is
called with a lock taken. So, all these cases are detected with
irqs_disabled() and on detection we do stuff that doesn't require
taking any locks and is safe even if re-entered.
> > >
> > > > +
> > > > enum rcutorture_type {
> > > > RCU_FLAVOR,
> > > > RCU_TASKS_FLAVOR,
> > > > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > > > index 21b6ce1dffb63..3bf3a250f9de8 100644
> > > > --- a/kernel/rcu/tree.c
> > > > +++ b/kernel/rcu/tree.c
> > > > @@ -3206,6 +3204,103 @@ __call_rcu_common(struct rcu_head *head,
> > > > rcu_callback_t func, bool lazy_in)
> > > > local_irq_restore(flags);
> > > > }
> > > >
> > > > +/*
> > > > + * Re-issue deferred callbacks straight to the enqueue so they cannot
> > > > defer
> > > > + * again. ->defer_lock serializes the drainers: this CPU's irq_work,
> > > > + * rcu_defer_flush() and rcutree_migrate_callbacks().
> > > > + */
> > > > +static void __rcu_defer_drain(struct rcu_data *rdp)
> > > > +{
> > > > + struct llist_node *node, *next;
> > > > + unsigned long flags;
> > > > +
> > > > + if (!IS_ENABLED(CONFIG_RCU_DEFER))
> > > > + return;
> > > > +
> > > > + raw_spin_lock_irqsave(&rdp->defer_lock, flags);
> > > > + llist_for_each_safe(node, next, llist_del_all(&rdp->defer_head)) {
> > > > + struct rcu_head *head = (struct rcu_head *)node;
> > >
> > > Why do you need the lock. This is still not clear to me despite the
> > > comment. You can do llist_del_all() towards another list and then feed
> > > it into rcu_do_enqueue() one by one. And you use the LAZY part.
> >
> > Because rcu_barrier() can call this for each cpu and at the same time
> > rcu_defer_drain() can call it too, so this would cause a race where
> > the irq work can remove the callbacks (llist_del_all) and before it
> > can enqueue them, rcu_barrier will see that the list is already empty
> > and will not wait for these callbacks. We want the llist_del_all() and
> > rcu_do_enqueue() to happen atomically so rcu_barrier() can work
> > correctly.
>
> So you collect a bunch of callbacks and spent time re-arranging
> everything with irqs off.
> What is wrong with keeping it in the llist and consuming it like the
> regular rcu_segcblist?
I don't understand your last point, all the collection and delayed
addition into rcu_segcblist is because it is not safe to do it
directly as I explained above.
>
> > >
> > > > +
> > > > + /* Bounds a node self-linked by a double call_rcu(). */
> > > > + head->next = NULL;
> > > > + rcu_do_enqueue(head, head->func, false);
> > > > + }
> > > > + raw_spin_unlock_irqrestore(&rdp->defer_lock, flags);
> > > > +}
> > > …
> > > > @@ -4231,6 +4330,9 @@ rcu_boot_init_percpu_data(int cpu)
> > > > rdp->rcu_onl_gp_state = RCU_GP_CLEANED;
> > > > rdp->last_sched_clock = jiffies;
> > > > rdp->cpu = cpu;
> > > > + init_llist_head(&rdp->defer_head);
> > > > + raw_spin_lock_init(&rdp->defer_lock);
> > > > + rdp->defer_work = IRQ_WORK_INIT_HARD(rcu_defer_drain);
> > >
> > > Why is this IRQ_WORK_INIT_HARD() instead, say, IRQ_WORK_INIT_LAZY()? Is
> > > there a requirement that the RCU callback needs to complete asap and not
> > > be delayed to the next tick? This would give kind of the LAZY part.
> > >
> >
> > In discussions with Paul, we concluded that we need HARD because in
> > low memory situations delaying rcu callbacks can be problematic. Given
> > that we are already deferring the callbacks, it would be nice to have
> > them enqueued asap. But if there are downsides to this we may change
> > it later.
>
> It does not look like you defer them for long at all. Every callback
> enqueued in an IRQ-off region will be immediately re-enqueued to the
> regular list the moment interrupts are enabled again.
> Except on architectures which don't implement irq-work interrupts where
> it will be delayed to the next HZ tick. And the next HZ tick does not
> sound like a long time either.
Yeah, I get your point, and when a CPU goes offline will its LAZY irq
works be drained or left in the queue? Because we want to make sure
this irq work is executed before a CPU goes offline.
