Re: [PATCH v4 6/8] rcu: set need_resched on softirq deferred-QS arming path

[Joel Fernandes <joelagnelf@nvidia.com>]

On 7/15/2026 5:12 PM, Paul E. McKenney wrote:
> On Thu, Jun 25, 2026 at 08:42:59PM -0400, Joel Fernandes wrote:
>> The arming code in rcu_read_unlock_special() has two paths for
>> deferring QS reporting: a softirq raise and an irq_work/set_need_resched
>> combination.
>>
>> The irq_work path always calls set_need_resched_current() before
>> queuing irq_work. The softirq path does not, relying solely on the
>> softirq firing to do the rightthing.
>>
>> This results in a problem as follows:
>>
>> Consider 2 rcu_read_lock/unlock segments:
>>
>>         rcu_read_lock();            // segment 1 starts
>>         // needs_exp becomes true
>>         preempt_disable();
>>         rcu_read_unlock();          // segment 1 ends; IRQs on, preempt
>>                                     // off, needs_exp=true =>
>>                                     //   raise_softirq(RCU_SOFTIRQ);
>>                                     //   arms defer_qs_pending.
>>                                     // Before this fix: no
>>                                     // set_need_resched_current().
>>         local_irq_disable();
>>         preempt_enable();           // softirq pending but IRQs disabled
>>                                     // hold it off.
>>         rcu_read_lock();            // segment 2 starts
>>         local_irq_enable();         // softirq fires: rcu_core runs, but
>>                                     // we are inside a reader (depth>0)
>>                                     // so no QS report; on softirq-exit
>>                                     // preempt-check finds no
>>                                     // need_resched -- still no nudge.
>>         preempt_disable();
>>         rcu_read_unlock();          // arming attempt suppressed
>>                                     // incorrectly: defer_qs_pending
>>                                     // already PENDING.  Without this
>>                                     // fix, no fresh
>>                                     // set_need_resched_current() on
>>                                     // this path either.
>>         preempt_enable();
>>
>> Therefore, add set_need_resched_current() to the softirq deferral path to
>> avoid long latencies in situations where GP needs to end sooner.
> 
> Hmmm...  Doesn't this make some of the prior changes unnecessary?

No, the above nudge and the earlier mechanisms cover different gaps. The nudge
only helps where something promptly acts on need_resched. It is not effective
until the tick under CONFIG_PREEMPT_DYNAMIC=y with preempt=none/voluntary
boot; it is also not effective across the local_irq_disable(); preempt_enable();
local_irq_enable(); Those are the cases the irq_work direct
report (patch 3), the exp-IPI clear (patch 5) and the rescue timer
(patch 8) are there for.  Is there a specific patch you feel this makes
redundant? Any case, it'd be good to have extra guard rails here.

> Also, how often is this giving us unecessary context switches?

AFAICS, only one extra when an expedited GP is waiting on this
CPU, a task needs deboosting, or a special-state unlock happens in
hardirq. In other words, only when RCU is already requiring work from this CPU.

For example, it is wasted only when the softirq managed to report first. I can
add a counter under rcutorture to quantify that fraction if you would like? But
I am doubtful it will show any difference.

> Finally, what exactly does (->defer_qs_pending == DEFER_QS_PENDING)
> mean at this point?

After this series: "this CPU has already spent one softirq/irq_work
queueing since the last clear event; do not queue another." It is
purely a throttle on mechanism queueing, for the recursion safety from
b41642c87716 -- decoupled from whether the QS has been reported, and
decoupled from the need_resched nudge.

You are right that this has drifted from what the comment in tree.h
still says ("An IRQ work was scheduled but not yet run") -- I will
update that comment block in v5 to document the new meaning. Does that work for
you? Also lets rename defer_qs_pending to defer_qs_throttled. That's what it
really is, that was why it was introduced. i.e. to not have to do extra
unnecessary work (for recursion protection).

Thanks.

> 
> 							Thanx, Paul
> 
>> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
>> ---
>>  kernel/rcu/tree_plugin.h | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
>> index c37d8cfeb714..f58ae29acdef 100644
>> --- a/kernel/rcu/tree_plugin.h
>> +++ b/kernel/rcu/tree_plugin.h
>> @@ -846,6 +846,7 @@ static void rcu_read_unlock_special(struct task_struct *t)
>>  			// Using softirq, safe to awaken, and either the
>>  			// wakeup is free or there is either an expedited
>>  			// GP in flight or a potential need to deboost.
>> +			set_need_resched_current();
>>  			if (rdp->defer_qs_pending != DEFER_QS_PENDING) {
>>  				rdp->defer_qs_pending = DEFER_QS_PENDING;
>>  				raise_softirq_irqoff(RCU_SOFTIRQ);
>> -- 
>> 2.34.1
>>

-- 
Joel Fernandes