[PATCH RFC v1 0/6] rcu: fix stuck defer_qs_pending state

[PATCH RFC v1 0/6] rcu: fix stuck defer_qs_pending state

[Joel Fernandes]

This series fixes a bug where rdp->defer_qs_pending can remain stuck in
PENDING when a preempted reader's quiescent state is reported up-tree via
a path other than the deferred-QS irq-work handler (FQS scan, hotplug
transition, expedited GP IPI, context switch). Once stuck, the pending
gate in rcu_read_unlock_special() silently suppresses all future arming
attempts on that CPU. The series adds PENDING -> IDLE transitions at the
missing sites.

Also handles the case where the deferred-QS irq-work handler may run between
segments of a compound section (per Paul McKenney's counter-example):

I also handled expedited GP cases in a patch. I have not yet looked at how this
interacts with the softirq paths so I am keeping the RFC tag on.

The last patch is a debug-only detector (CONFIG_RCU_GP_CLEANUP_STALE_CHECK,
marked [TEST COMMIT], not for merge) -- applied alone on unmodified mainline
without patches 2-5 it reliably fires a WARN within 5 minutes under TREE03
rcutorture, confirming the bug exists and the detector catches it; with the
full fix applied, I could not reproduce the issue.

The git tree with all patches can be found at:
https://git.kernel.org/pub/scm/linux/kernel/git/jfern/linux.git/tag/?h=rcu-dqs-stuck-rfc-v1-20260522

Joel Fernandes (6):
  rcu: introduce rcu_defer_qs_clear() helper
  rcu: clear defer_qs_pending when notifying GP changes
  rcu: clear defer_qs_pending in handler for compounded sections
  rcu: drop redundant defer_qs_pending clear in irqrestore handler
  rcu: clear defer_qs_pending at expedited IPI entry
  [TEST COMMIT] rcu: detect stuck defer_qs_pending at GP cleanup

 kernel/rcu/Kconfig.debug | 11 +++++++++
 kernel/rcu/tree.c        | 49 ++++++++++++++++++++++++++++++++++++++++
 kernel/rcu/tree.h        | 13 +++++++++++
 kernel/rcu/tree_exp.h    |  6 +++++
 kernel/rcu/tree_plugin.h | 30 ++++++++++++------------
 5 files changed, 94 insertions(+), 15 deletions(-)

-- 
2.34.1

[PATCH RFC v1 1/6] rcu: introduce rcu_defer_qs_clear() helper

[Joel Fernandes]

Currently rdp->defer_qs_pending transitions from DEFER_QS_PENDING to
DEFER_QS_IDLE at two sites: rcu_preempt_deferred_qs_irqrestore() and
rcu_preempt_deferred_qs_handler() (depth>0 reset).  Both write the
IDLE value directly.

Introduce a single inline helper rcu_defer_qs_clear() in tree.h and
route both sites through it.  This becomes the single
PENDING->IDLE transition point for upcoming work.

Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
 kernel/rcu/tree.h        | 5 +++++
 kernel/rcu/tree_plugin.h | 4 ++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
index 7dfc57e9adb1..4069132f9d44 100644
--- a/kernel/rcu/tree.h
+++ b/kernel/rcu/tree.h
@@ -296,6 +296,11 @@ struct rcu_data {
 	int cpu;
 };
 
+static inline void rcu_defer_qs_clear(struct rcu_data *rdp)
+{
+	WRITE_ONCE(rdp->defer_qs_pending, DEFER_QS_IDLE);
+}
+
 /* Values for nocb_defer_wakeup field in struct rcu_data. */
 #define RCU_NOCB_WAKE_NOT	0
 #define RCU_NOCB_WAKE_BYPASS	1
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index 95ad967adcf3..8637f405cb47 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -488,7 +488,7 @@ rcu_preempt_deferred_qs_irqrestore(struct task_struct *t, unsigned long flags)
 
 	rdp = this_cpu_ptr(&rcu_data);
 	if (rdp->defer_qs_pending == DEFER_QS_PENDING)
-		rdp->defer_qs_pending = DEFER_QS_IDLE;
+		rcu_defer_qs_clear(rdp);
 
 	/*
 	 * If RCU core is waiting for this CPU to exit its critical section,
@@ -645,7 +645,7 @@ static void rcu_preempt_deferred_qs_handler(struct irq_work *iwp)
 	 * 5. Deferred QS reporting does not happen.
 	 */
 	if (rcu_preempt_depth() > 0)
-		WRITE_ONCE(rdp->defer_qs_pending, DEFER_QS_IDLE);
+		rcu_defer_qs_clear(rdp);
 }
 
 /*
-- 
2.34.1

[PATCH RFC v1 2/6] rcu: clear defer_qs_pending when notifying GP changes

[Joel Fernandes]

Prior to this commit, defer_qs_pending was an unbalanced flag:
rcu_read_unlock_special() set it to PENDING whenever a deferred-QS
mechanism was scheduled, but the clear paths did not cover every
up-tree quiescent-state reporting site. In those cases the flag stays
PENDING after the QS is reported, and rcu_read_unlock_special()'s
pending-gate then silently rejects all future arming attempts.

A test patch confirms TREE03 can have get into the problematic stuck
state very quickly (< 5 minutes).

Clear the flag in __note_gp_changes(), right after the nothing-to-do
early return.  This is the natural per-CPU "GP transitioned, sync local
state" hook, called from the GP-kthread's rcu_gp_init()/rcu_gp_cleanup()
paths, and other GP advancement paths.

For dynticks-idle CPUs, they do not call __note_gp_changes(), but they
also do not arm new PENDING work (no readers running), and on wake-up,
note_gp_changes() is called before any new reader runs.

Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
 kernel/rcu/tree.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 55df6d37145e..d0816468ffee 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -1281,6 +1281,8 @@ static bool __note_gp_changes(struct rcu_node *rnp, struct rcu_data *rdp)
 	if (rdp->gp_seq == rnp->gp_seq)
 		return false; /* Nothing to do. */
 
+	rcu_defer_qs_clear(rdp);
+
 	/* Handle the ends of any preceding grace periods first. */
 	if (rcu_seq_completed_gp(rdp->gp_seq, rnp->gp_seq) ||
 	    unlikely(rdp->gpwrap)) {
-- 
2.34.1

[PATCH RFC v1 3/6] rcu: clear defer_qs_pending in handler for compounded sections

[Joel Fernandes]

The deferred-QS irq-work handler previously cleared defer_qs_pending
only when the handler ran inside an active rcu_read_lock() critical
section (rcu_preempt_depth() > 0).  Paul McKenney pointed out a common
multi-segment compound pattern where the handler fires between
segments and segment N+1's arming attempt is silently suppressed by
the rcu_read_unlock_special() pending-gate:

    rcu_read_lock();           // segment 1 starts
    // may be preempted/boosted here
    local_irq_disable();
    rcu_read_unlock();          // segment 1 ends; arms defer_qs_pending
    preempt_disable();
    local_irq_enable();         // handler MAY fire here: depth==0, but
                                // but preempt is disabled, so it cant
                                // nudge.

    rcu_read_lock();            // segment 2 starts
    preempt_enable();
    local_irq_disable();
    rcu_read_unlock();     // arming attempt suppressed incorrectly -- (1)
    local_irq_enable();

Waiting for the next __note_gp_changes() clear is too slow for the
compound case, we need the deferred QS report sooner.

Therefore, make the irq_work handler clears defer_qs_pending whenever
rcu_in_compounded_section() is true so that (1) can do the arming.

Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
 kernel/rcu/tree_plugin.h | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index 8637f405cb47..2da009dbe64c 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -621,6 +621,17 @@ notrace void rcu_preempt_deferred_qs(struct task_struct *t)
 	rcu_preempt_deferred_qs_irqrestore(t, flags);
 }
 
+/*
+ * True if the current context is inside a compounded RCU read-side
+ * section, i.e. either in an active rcu_read_lock() (depth>0) or in an
+ * outer preempt-disabled / BH-disabled scope.
+ */
+static inline bool rcu_in_compounded_section(void)
+{
+	return rcu_preempt_depth() > 0 ||
+	       (preempt_count() & (PREEMPT_MASK | SOFTIRQ_MASK)) != 0;
+}
+
 /*
  * Minimal handler to give the scheduler a chance to re-evaluate.
  */
@@ -632,19 +643,10 @@ static void rcu_preempt_deferred_qs_handler(struct irq_work *iwp)
 	rdp = container_of(iwp, struct rcu_data, defer_qs_iw);
 
 	/*
-	 * If the IRQ work handler happens to run in the middle of RCU read-side
-	 * critical section, it could be ineffective in getting the scheduler's
-	 * attention to report a deferred quiescent state (the whole point of the
-	 * IRQ work). For this reason, requeue the IRQ work.
-	 *
-	 * Basically, we want to avoid following situation:
-	 * 1. rcu_read_unlock() queues IRQ work (state -> DEFER_QS_PENDING)
-	 * 2. CPU enters new rcu_read_lock()
-	 * 3. IRQ work runs but cannot report QS due to rcu_preempt_depth() > 0
-	 * 4. rcu_read_unlock() does not re-queue work (state still PENDING)
-	 * 5. Deferred QS reporting does not happen.
+	 * Clear defer_qs_pending when the handler fires inside a compounded
+	 * section as we may need to rearm the irq_work.
 	 */
-	if (rcu_preempt_depth() > 0)
+	if (rcu_in_compounded_section())
 		rcu_defer_qs_clear(rdp);
 }
 
-- 
2.34.1

[PATCH RFC v1 4/6] rcu: drop redundant defer_qs_pending clear in irqrestore handler

[Joel Fernandes]

With __note_gp_changes() now clearing defer_qs_pending at every
per-CPU GP advance, the per-irqrestore clear is redundant.  Remove it.

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 <joelagnelf@nvidia.com>
---
 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 2da009dbe64c..bb5f955a06df 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -487,8 +487,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

[PATCH RFC v1 5/6] rcu: clear defer_qs_pending at expedited IPI entry

[Joel Fernandes]

The per-CPU clear in __note_gp_changes() (from an earlier commit)
runs only when the local CPU notices a normal-GP advance.  When an
expedited GP arrives at a CPU whose defer_qs_pending is already
PENDING, rcu_read_unlock_special() may skip irq_work queuing due to the
pending gate.

Clear defer_qs_pending on the IPI target right in rcu_exp_handler().
This makes it possible for any arming attempt that follows
the IPI within the current GP to be able to queue irq_work again,
allowing completion expedited GPs quickly than waiting for one scheduler
tick.

Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
 kernel/rcu/tree_exp.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/kernel/rcu/tree_exp.h b/kernel/rcu/tree_exp.h
index 82cada459e5d..f8564a041879 100644
--- a/kernel/rcu/tree_exp.h
+++ b/kernel/rcu/tree_exp.h
@@ -763,6 +763,12 @@ static void rcu_exp_handler(void *unused)
 			 READ_ONCE(rdp->cpu_no_qs.b.exp)))
 		return;
 
+	/*
+	 * Clear defer_qs_pending so arming attempts following this IPI
+	 * within the current GP can queue irq_work again.
+	 */
+	rcu_defer_qs_clear(rdp);
+
 	/*
 	 * Second, the common case of not being in an RCU read-side
 	 * critical section.  If also enabled or idle, immediately
-- 
2.34.1

[PATCH RFC v1 6/6] [TEST COMMIT] rcu: detect stuck defer_qs_pending at GP cleanup

[Joel Fernandes]

Debug-only stuck-state detector for upstream review.  This commit is
NOT for merge; it is a review aid.  Reviewers can enable
CONFIG_RCU_GP_CLEANUP_STALE_CHECK to gain runtime confidence in the
preceding fix commits.

Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
 kernel/rcu/Kconfig.debug | 11 ++++++++++
 kernel/rcu/tree.c        | 47 ++++++++++++++++++++++++++++++++++++++++
 kernel/rcu/tree.h        |  8 +++++++
 3 files changed, 66 insertions(+)

diff --git a/kernel/rcu/Kconfig.debug b/kernel/rcu/Kconfig.debug
index 35218ba74eb5..5a40c4fe544c 100644
--- a/kernel/rcu/Kconfig.debug
+++ b/kernel/rcu/Kconfig.debug
@@ -98,6 +98,17 @@ config RCU_TORTURE_TEST_LOG_GP
 	  Say Y here if you want grace-period sequence numbers logged.
 	  Say N if you are unsure.
 
+config RCU_GP_CLEANUP_STALE_CHECK
+	bool "Detect stuck defer_qs_pending state at GP cleanup"
+	depends on RCU_TORTURE_TEST
+	default n
+	help
+	  This option adds a per-CPU instrumentation counter on every
+	  PENDING -> IDLE transition of rdp->defer_qs_pending, and a
+	  detector in rcu_gp_cleanup().
+
+	  Say N if you are unsure.
+
 config RCU_REF_SCALE_TEST
 	tristate "Scalability tests for read-side synchronization (RCU and others)"
 	depends on DEBUG_KERNEL
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index d0816468ffee..1307f3fb48ac 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -2146,6 +2146,52 @@ static noinline_for_stack void rcu_gp_fqs_loop(void)
 	}
 }
 
+#ifdef CONFIG_RCU_GP_CLEANUP_STALE_CHECK
+/*
+ * Threshold of consecutive GPs with rdp->defer_qs_pending stuck at
+ * PENDING and no observed PENDING -> IDLE transition before WARN.
+ */
+#define RCU_DEFER_QS_STUCK_GPS_THRESHOLD	5
+
+static void rcu_gp_cleanup_stale_check(void)
+{
+	int cpu;
+	unsigned long cur_gp_seq = READ_ONCE(rcu_state.gp_seq);
+
+	for_each_online_cpu(cpu) {
+		struct rcu_data *rdp = per_cpu_ptr(&rcu_data, cpu);
+		s64 clears_now;
+		int p_now;
+
+		if (READ_ONCE(rdp->gp_seq) != cur_gp_seq) {
+			rdp->defer_qs_pending_stuck_gps = 0;
+			rdp->defer_qs_pending_clears_snap =
+				atomic64_read(&rdp->defer_qs_pending_clears);
+			continue;
+		}
+
+		clears_now = atomic64_read(&rdp->defer_qs_pending_clears);
+		p_now = READ_ONCE(rdp->defer_qs_pending);
+
+		if (p_now != DEFER_QS_PENDING ||
+		    clears_now != rdp->defer_qs_pending_clears_snap) {
+			rdp->defer_qs_pending_stuck_gps = 0;
+			rdp->defer_qs_pending_clears_snap = clears_now;
+			continue;
+		}
+
+		rdp->defer_qs_pending_stuck_gps++;
+		WARN_ONCE(rdp->defer_qs_pending_stuck_gps >=
+			  RCU_DEFER_QS_STUCK_GPS_THRESHOLD,
+			"RCU: defer_qs_pending STUCK on CPU %d for %u GPs (gp_seq=%lu, clears=%lld)\n",
+			cpu, rdp->defer_qs_pending_stuck_gps,
+			cur_gp_seq, clears_now);
+	}
+}
+#else
+static inline void rcu_gp_cleanup_stale_check(void) { }
+#endif /* CONFIG_RCU_GP_CLEANUP_STALE_CHECK */
+
 /*
  * Clean up after the old grace period.
  */
@@ -2220,6 +2266,7 @@ static noinline void rcu_gp_cleanup(void)
 
 	/* Declare grace period done, trace first to use old GP number. */
 	trace_rcu_grace_period(rcu_state.name, rcu_state.gp_seq, TPS("end"));
+	rcu_gp_cleanup_stale_check();
 	rcu_seq_end(&rcu_state.gp_seq);
 	ASSERT_EXCLUSIVE_WRITER(rcu_state.gp_seq);
 	WRITE_ONCE(rcu_state.gp_state, RCU_GP_IDLE);
diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
index 4069132f9d44..29d852bbe218 100644
--- a/kernel/rcu/tree.h
+++ b/kernel/rcu/tree.h
@@ -204,6 +204,11 @@ struct rcu_data {
 					/* period it is aware of. */
 	struct irq_work defer_qs_iw;	/* Obtain later scheduler attention. */
 	int defer_qs_pending;		/* irqwork or softirq pending? */
+#ifdef CONFIG_RCU_GP_CLEANUP_STALE_CHECK
+	atomic64_t   defer_qs_pending_clears;
+	s64          defer_qs_pending_clears_snap;
+	unsigned int defer_qs_pending_stuck_gps;
+#endif
 	struct work_struct strict_work;	/* Schedule readers for strict GPs. */
 
 	/* 2) batch handling */
@@ -299,6 +304,9 @@ struct rcu_data {
 static inline void rcu_defer_qs_clear(struct rcu_data *rdp)
 {
 	WRITE_ONCE(rdp->defer_qs_pending, DEFER_QS_IDLE);
+#ifdef CONFIG_RCU_GP_CLEANUP_STALE_CHECK
+	atomic64_inc(&rdp->defer_qs_pending_clears);
+#endif
 }
 
 /* Values for nocb_defer_wakeup field in struct rcu_data. */
-- 
2.34.1