Re: [PATCH 1/7] rcu: Make call_rcu() safe to call from any context

[Paul E. McKenney]

On Mon, Sep 21, 2026 at 08:30:42PM +0200, Boqun Feng wrote:
> On Mon, Sep 21, 2026 at 07:12:02PM +0100, Puranjay Mohan wrote:
> > On Sat, Sep 19, 2026 at 3:07 PM Boqun Feng <boqun@xxxxxxxxxx> wrote:
> > >
> > > On Fri, Sep 18, 2026 at 05:32:52PM -0700, Paul E. McKenney wrote:
> > > > From: Puranjay Mohan <puranjay@xxxxxxxxxx>
> > > >
> > >
> > > Hi,
> > >
> > > Sorry for a bit late repsonse.
> > >
> > > > RCU's per-CPU callback list is only touched with interrupts disabled: the
> > > > enqueue runs under local_irq_save() (and the nocb locks when offloaded),
> > > > as do callback invocation and grace-period work.  A call_rcu() that
> > > > arrives with interrupts already disabled, whether from an NMI or from
> > > > instrumentation that re-enters RCU, can interrupt one of those and corrupt
> > > > the list or deadlock.
> > > >
> > > > Defer instead: stage the callback on a per-CPU llist and raise an irq_work
> > > > that re-issues it once interrupts are on, straight to the enqueue so it
> > > > cannot defer again.  The gate is bare irqs_disabled(), so callers that
> > > > merely hold interrupts off are deferred too and pay one irq_work hop.
> > > > Skip it while the scheduler is down (RCU_SCHEDULER_INACTIVE): irq_work is
> > > > not usable that early, rcu_init() already calls call_rcu(), and the per-CPU
> > > > deferral state is not initialised until rcu_init_one() runs later in it.
> > > >
> > > > rcu_barrier() drains every CPU's ->defer_head before it scans the lists,
> > > > and rcutree_migrate_callbacks() drains an outgoing CPU's.  A drain
> > > > re-issues onto the draining CPU, so a barrier moves other CPUs' staged
> > > > callbacks onto
> > > > its own ->cblist; call_rcu() promises no CPU affinity for invocation.
> > > > ->defer_lock is held across llist_del_all() and the whole re-issue so the
> > > > drainers
> > > > serialize: one that finds the list empty can conclude that everything
> > > > staged before it is already on a callback list.  Interrupts stay off for
> > > > the batch.  Where the arch has an irq_work self-IPI that is what one
> > > > interrupts-disabled region could stage, normally a single callback; where
> > > > arch_irq_work_has_interrupt() is false the drain waits for the tick, so
> > > > several regions can accumulate first.
> > > >
> > > > The drain clears ->next before re-issuing.  A double call_rcu() on a head
> > > > that is already debug-object-active self-links the staged node, and
> > > > rcu_do_enqueue()'s duplicate path returns without clearing it, so the
> > > > drain would spin.  A re-add behind other staged callbacks makes a longer
> > > > cycle, which that does not bound; a double call_rcu() stays undefined.
> > > > llist_del_all() yields newest-first, so a batch is re-issued in reverse
> > > > call order; nothing depends on call_rcu() ordering.  The re-issue drops
> > > > the lazy hint, since staging records only ->func, so a deferred callback
> > > > loses its batching on CONFIG_RCU_LAZY.  kasan_record_aux_stack() moves to
> > >
> > > I'm not sure this is a good idea, because it effectively remove LAZY
> > > support when DEFER is enabled. Since the goal of this patchset supports
> > > BPF and NMI, would it be nicer that we skip the whole defer logic if the
> > > callback is LAZY? Alternatively, you can have two llist (one for hurry
> > > and one for lazy).
> > 
> > I had made this trade-off of removing the Lazy tag as I thought it is
> > not necessary to support lazy when call_rcu() is called from nmi and
> > bpf based instrumentation as they should not be frequent. But I like
> 
> Ah, I missed that you only defer if irqs_disabled() is true, so this is
> much better than I used to think. So..
> 
> > the idea of two lists (skipping the defer logic is not possible as it
> > could lead to deadlocks/corruption). I will also investigate if we can
> > put the Lazy tag on the ->next pointer. But will it be acceptable if I
> > do that as a follow up? I want to get the base support fully validated

I agree that you need a lazy flag that indicates that all deferred
callbacks in this llist are lazy.  As soon as the first non-lazy callback
arrives, the flag must be cleared.  Because this is the way laziness
works without deferral, correct?

Then in the irq-work handler, enqueue them all lazily if the flag is
still set, or non-lazily otherwise.  If the flag is not set, enqueue
them lazily.  Either way, set the lazy flag.

Of course, as described, this is quite racy.  To make it work, the
enqueuing and the clearing of the flag for that first non-lazy
callback must be atomic.  Which can be accomplished by having this
be the low-order bit of the llist_head structure's ->first pointer,
as opposed to the llist_node structure's ->next pointer.

Or am I missing something subtle here?

And of course, either way, this requires llist primitives that clear
that low-order bit at the appropriate places.

> .. definitely a follow-up would do, thank you!

Very much agreed on this being a follow-up item!

							Thanx, Paul

> Regards,
> Boqun
> 
> > with the BPF side changes. I also have more optimizations planned as
> > suggested by Sebastian.
> > 
> > Thanks,
> > Puranjay
> >