Re: [PATCH v4 4/8] rcu: drop redundant defer_qs_pending clear in irqrestore handler
Joel Fernandes <[email protected]> Mon, 20 Jul 2026 17:06:47 -0400
| Newsgroups | org.kernel.vger.rcu,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On 7/15/2026 4:58 PM, Paul E. McKenney wrote: > On Thu, Jun 25, 2026 at 08:42:57PM -0400, Joel Fernandes wrote: >> With __note_gp_changes() now clearing defer_qs_pending at every >> per-CPU GP advance, the per-irqrestore clear is redundant. Remove it. > > Wait... > > Given your patch 2/8 of this series, isn't __note_gp_changes() now > clearing ->defer_qs_pending on any new-to-this-CPU grace period start > or end? If that was a new-to-this-CPU grace period start, is that really > a one-to-once correspondence to the need for a quiescent state? The asymmetry my series relies on is: clearing this flag is always correctness-safe — a clear can only permit an extra arming attempt (softirq raise and irq_work); only a missed clear is dangerous, because a stale PENDING suppresses all arming, which is precisely the bug. So the flag doesn't need one-to-one correspondence with QS needs. It's a re-arm throttle whose only correctness requirements are: (a) cleared rarely enough to keep the recursion bound (once per boundary notice preserves it — the deadloops needed unbounded requeue), and (b) cleared often enough to bound stuck-time (the CPU noticing the GP is the natural point, so a stale flag survives at most one noticed GP). Right now it is stuck forever when I run TREE03. Put another way, you are right that it is not a one-to-one correspondence, but merely that the CPU noticing the start/end of a GP is the starting point for when we'd need to pay attention to ->defer_qs_pending, so we have to clear it first (in __note_gp_changes()). We can keep __note_gp_changes as a single point of clearing (except some special cases) as this series does, which also makes it more robust IMO. Previously, we've had issues where we forget to clear the ->defer_qs_pending flag. Did I miss something? Thanks. > > Thanx, Paul > >> Effect: PENDING now stays set from arming until the next per-CPU GP >> advance, future arming attempts on the same CPU within the same GP are >> gated by the rcu_read_unlock_special(). >> >> This serves both as an optimization (should not need new irq_work again >> this GP - for the compounded section case, we detect and clear it there), >> and reduces risks of recursion due to clearing too aggressively. >> >> Signed-off-by: Joel Fernandes <[email protected]> >> --- >> kernel/rcu/tree_plugin.h | 2 -- >> 1 file changed, 2 deletions(-) >> >> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h >> index 7768a40677a4..c37d8cfeb714 100644 >> --- a/kernel/rcu/tree_plugin.h >> +++ b/kernel/rcu/tree_plugin.h >> @@ -581,8 +581,6 @@ rcu_preempt_deferred_qs_irqrestore(struct task_struct *t, unsigned long flags) >> union rcu_special special; >> >> rdp = this_cpu_ptr(&rcu_data); >> - if (rdp->defer_qs_pending == DEFER_QS_PENDING) >> - rcu_defer_qs_clear(rdp); >> >> /* >> * If RCU core is waiting for this CPU to exit its critical section, >> -- >> 2.34.1 >>