Re: [PATCH v2 2/7] srcu: Rule for using synchronize_srcu_atomic() from interrupt handlers

[Paul E. McKenney]

On Wed, Nov 11, 2026 at 09:59:44AM -0500, Mathieu Desnoyers wrote:
> This is about the update side.  A user that calls
> synchronize_srcu_atomic() from an interrupt handler must disable
> interrupts around all its calls on that srcu_struct, task context
> included.  Document this rule, and warn when it is broken.  The rules
> for readers are unchanged.
> 
> synchronize_srcu_atomic() spins rather than sleeps, so it may be called
> from atomic context, interrupt handlers included.  Its callers on one
> srcu_struct exclude each other with ->srcu_atomic_gp_flag.  One of them
> drives the grace period.  The others spin until a grace period ends.
> 
> The flag owner disables only preemption.  So an interrupt can arrive on
> its CPU while it holds the flag.  If that interrupt handler calls
> synchronize_srcu_atomic() on the same srcu_struct, it spins forever:
> 
>   CPU 0, task                        CPU 0, hardirq
>   synchronize_srcu_atomic()
>     takes ->srcu_atomic_gp_flag
>                                      synchronize_srcu_atomic()
>                                        spins until a grace period ends
>                                        it never ends: its driver is the
>                                        task this interrupt interrupted
> 
> Nothing reports it.  lockdep cannot, because lock_sync() records no
> interrupt state.
> 
> synchronize_srcu_atomic() cannot disable interrupts itself.  A reader
> that disables only preemption may wait for an IPI to the waiter's CPU.
> That IPI never runs while the waiter spins with interrupts disabled.
> The kernel-doc already lets the caller choose the waiter's context.
> It then asks the readers to run in that context, or a stricter one.
> 
> So the caller disables interrupts.  The existing reader rule then
> requires the readers of that srcu_struct to disable interrupts too.
> 
> Also store the owner's CPU number plus one in the flag, instead of 1.
> A waiter that finds its own CPU as the owner warns once, rather than
> hanging silently.
> 
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx>

Nice!!!

Reviewed-by: Paul E. McKenney <paulmck@xxxxxxxxxx>

> ---
>  kernel/rcu/srcutree.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> index 2b0e2b2d2c18..8075da4b1bb5 100644
> --- a/kernel/rcu/srcutree.c
> +++ b/kernel/rcu/srcutree.c
> @@ -2108,6 +2108,13 @@ static void srcu_advance_state(struct srcu_struct *ssp, bool is_atomic)
>   * and srcu_read_unlock_atomic() must be invoked from that same context,
>   * or one that is even more strict.
>   *
> + * If synchronize_srcu_atomic() is invoked from an interrupt handler for
> + * a given srcu_struct structure, then for that structure, all calls to
> + * synchronize_srcu_atomic() must be invoked with interrupts disabled,
> + * including those from task context.  Its callers take turns to drive a
> + * grace period, and the others spin until it ends.  An interrupt handler
> + * that calls it on the CPU of an interrupted caller would spin forever.
> + *
>   * If synchronize_srcu_atomic() is invoked on a given srcu_struct
>   * structure, then none of call_srcu(), synchronize_srcu(),
>   * synchronize_srcu_expedited(), start_poll_synchronize_srcu(),
> @@ -2144,7 +2151,13 @@ void synchronize_srcu_atomic(struct srcu_struct *ssp)
>  	srcu_state = get_state_synchronize_srcu(ssp);
>  	preempt_disable();
>  	while (atomic_read(&sup->srcu_atomic_gp_flag) ||
> -	       atomic_xchg(&sup->srcu_atomic_gp_flag, 1)) {
> +	       atomic_cmpxchg(&sup->srcu_atomic_gp_flag, 0,
> +			      smp_processor_id() + 1)) {
> +		// The flag holds its owner's CPU number plus one.  An owner on
> +		// this CPU is a caller that this one interrupted.  It cannot
> +		// release the flag, so this caller would spin forever.
> +		WARN_ON_ONCE(atomic_read(&sup->srcu_atomic_gp_flag) ==
> +			     smp_processor_id() + 1);
>  		preempt_enable();
>  		if (poll_state_synchronize_srcu(ssp, srcu_state))
>  			return;
> -- 
> 2.47.3
>