Hi Paul,
On 7/15/2026 5:35 PM, Paul E. McKenney wrote:
> On Thu, Jun 25, 2026 at 08:43:01PM -0400, Joel Fernandes wrote:
>> The compound branch of rcu_read_unlock_special() arms one of the
>> scheduler, RCU_SOFTIRQ (raise_softirq_irqoff()) or irq_work_queue_on()
>> in order to report a deferred quiescent state at a later time.
>>
>> However, that is not enough in scenarios where an interrupts-disabled
>> section spans the preempt_enable() of a preempt-disabled section:
>>
>> rcu_read_lock();
>> // receive IPI for expedited GP
>> preempt_disable();
>> rcu_read_unlock(); // sets the need-resched flag
>> local_irq_disable();
>> preempt_enable(); // cannot reschedule: IRQs are off
>> local_irq_enable();
>> // now outside the compound RCU read-side critical section, but the
>> // expedited GP is still held up
>>
>> Introduce a per-CPU bounded-delay rescue hrtimer, armed from the compound
>> branch when an expedited GP needs this CPU's quiescent state, that reports
>> the deferred QS via rcu_preempt_deferred_qs_try_report() once a clean
>> (non-reader, non-compound) context is reached.
>>
>> To keep the rescue from firing once one of the normal mechanisms has
>> already reported the quiescent state, cancel it from
>> rcu_preempt_deferred_qs_irqrestore() -- the common path through which the
>> deferred QS is reported. Without this the timer keeps firing (and
>> re-arming) only to find the work already done; cancelling on the natural
>> report path avoids the great majority of those fires (observed as a ~90%
>> reduction in rescue-timer fires under rcutorture TREE03 with
>> rcutorture.gp_exp=1).
>
> Can this replace some of the complexity in the deferred-quiescent-state
> code paths, for example, the irq-work handler? (Yes, we will need an
> irq-work handler for immediate deboosting, but that is a separate issue.)
I landed on the same conclusion you mentioned in brackets (immediacy), if
the irq_work gets there first, then we'd want to let it do its job, with
the timer as an escape-hatch for weird compounded cases (irq disabling
wrapping around outer most preempt enabling, etc.).
Also, after this series, the irqwork handler is simple IMO code complexity
wise. Let me know which part felt complex if any.
> Please see below for another question.
>
[..] (replied below)>>
>> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
>> index 6f5d31e3f1a3..324d08c7a91a 100644
>> --- a/kernel/rcu/tree_plugin.h
>> +++ b/kernel/rcu/tree_plugin.h
>> @@ -592,6 +592,18 @@ rcu_preempt_deferred_qs_irqrestore(struct task_struct
>> *t, unsigned long flags)
>> local_irq_restore(flags);
>> return;
>> }
>> +
>> + /*
>> + * A natural report path reached the deferred quiescent state before
>> + * the bounded-delay rescue hrtimer fired. Cancel any pending rescue
>> + * on this CPU so it does not fire only to find the quiescent state
>> + * already reported. Use hrtimer_try_to_cancel() rather than
>> + * hrtimer_cancel(): interrupts are disabled here and the timer is
>> + * HARD/PINNED, so a callback that is already running must not be
>> + * waited on (in that case this is a harmless no-op).
>> + */
>> + hrtimer_try_to_cancel(&rdp->defer_qs_iw_rescue);
>> +
>> t->rcu_read_unlock_special.s = 0;
>> if (special.b.need_qs) {
>> if (IS_ENABLED(CONFIG_RCU_STRICT_GRACE_PERIOD)) {
>> @@ -772,6 +784,54 @@ static void rcu_preempt_deferred_qs_handler(struct
>> irq_work *iwp)
>> rcu_defer_qs_clear(rdp);
>> }
>>
>> +/*
>> + * Bounded-delay rescue timeout for the deferred-QS reporting.
>> + *
>> + * The compound branch of rcu_read_unlock_special() arms either the
>> + * scheduler, RCU_SOFTIRQ (raise_softirq_irqoff) or irq_work_queue_on() in
>> order
>> + * to report a deferred QS at a later time.
>> + *
>> + * However, that is not enough as in scenarios where local_irq_disable()d
>> + * sections span the preempt_enable() call of a preempt-disabled section:
>> + *
>> + * rcu_read_lock();
>> + * // receive IPI for exp GP
>> + * preempt_disable();
>> + * rcu_read_unlock(); // Set the "need reschedule" flag.
>> + * local_irq_disable();
>> + * preempt_enable(); // Cannot reschedule as IRQs are off.
>> + * local_irq_enable();
>> + * // Now outside the compound RCU read-side critical section
>> + * // however, the expedited GP is still held up.
>> + *
>> + * Introduce a rescue timer, firing every 50 micro seconds after the last
>> + * rcu_read_unlock() call, to fix this.
>> + */
>> +static int defer_qs_rescue_delay_us = 50;
>> +module_param(defer_qs_rescue_delay_us, int, 0644);
>> +MODULE_PARM_DESC(defer_qs_rescue_delay_us,
>> + "Microseconds before the rescue timer fires a deferred-QS
>> report.");
>> +
>> +static enum hrtimer_restart
>> +rcu_preempt_deferred_qs_rescue(struct hrtimer *hrtp)
>> +{
>> + lockdep_assert_irqs_disabled();
>> +
>> + /*
>> + * Still inside a reader / compound section: deboosting is unsafe, so
>> + * rearm and retry after a bounded delay. Once clean,
>> + * rcu_preempt_deferred_qs_try_report() reports the deferred QS and
>> + * releases any boost in the current task's context (or is a no-op if
>> + * natural recovery already landed).
>> + */
>> + if (!rcu_preempt_deferred_qs_try_report(current)) {
>> + hrtimer_forward_now(hrtp,
>> + us_to_ktime(defer_qs_rescue_delay_us));
>
> One way we get here is if we are in an rcu_read_lock()-style RCU read-side
> critical section. But in that case, we have an rcu_read_unlock() in
> our future, so do we really need to arm the timer now?
If we do that, we run into the issue where outer most rcu_read_unlock() ran
with interrupts enabled, but outermost preempt_enable ran with interrupts
disabled? Like so:
CPU 0 (task T) CPU 1
-------------- -----
1 rcu_read_lock()
2 preempted in reader
3 rcub boosts
4 T resumes in reader
(a rescue armed earlier fires here:
depth > 0 -> don't restart as you suggest)
5 preempt_disable();
6 rcu_read_unlock();
needs_exp == false: // because outer, IRQs ON
7 local_irq_disable();
8 preempt_enable(); // NO schedule
9 local_irq_enable();
(T out of reader, still boosted)
We can do your suggestion but changing the condition for hrtimer_start in
rcu_read_unlock_special() to the following, but that could be worse because
now the timer starts more unconditionally.
if ((needs_exp || t->rcu_blocked_node) && cpu_online(rdp->cpu))
hrtimer_start(...);
> And before I forget again, thank you very much for digging into this!!!
>
> This is not a simple situation. ;-)
>
It is my pleasure, thanks! ;-)
-Joel
