[PATCH -next 0/6] Patches for v6.17

[PATCH -next 0/6] Patches for v6.17

[Joel Fernandes]

Just a repost of patches with tags, for our consideration into v6.17.

All have tags, and the last commit is a fixup for the deadloop patch which can
be squashed into the original patch.

Joel Fernandes (6):
  smp: Document preemption and stop_machine() mutual exclusion
  rcu: Refactor expedited handling check in rcu_read_unlock_special()
  rcu: Document GP init vs hotplug-scan ordering requirements
  rcu: Document separation of rcu_state and rnp's gp_seq
  rcu: Document concurrent quiescent state reporting for offline CPUs
  [please squash] fixup! rcu: Fix rcu_read_unlock() deadloop due to IRQ
    work

 .../Data-Structures/Data-Structures.rst       |  32 +++++
 .../RCU/Design/Requirements/Requirements.rst  | 128 ++++++++++++++++++
 kernel/rcu/tree.c                             |  31 ++++-
 kernel/rcu/tree.h                             |  10 +-
 kernel/rcu/tree_plugin.h                      |  90 ++++++++++--
 kernel/smp.c                                  |  26 ++--
 6 files changed, 293 insertions(+), 24 deletions(-)

-- 
2.34.1

[PATCH -next 1/6] smp: Document preemption and stop_machine() mutual exclusion

[Joel Fernandes]

Recently while revising RCU's cpu online checks, there was some discussion
around how IPIs synchronize with hotplug.

Add comments explaining how preemption disable creates mutual exclusion with
CPU hotplug's stop_machine mechanism. The key insight is that stop_machine()
atomically updates CPU masks and flushes IPIs with interrupts disabled, and
cannot proceed while any CPU (including the IPI sender) has preemption
disabled.

[ Apply peterz feedback. ]

Cc: Andrea Righi <arighi@nvidia.com>
Cc: Paul E. McKenney <paulmck@kernel.org>
Cc: Frederic Weisbecker <frederic@kernel.org>
Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: rcu@vger.kernel.org
Acked-by: Paul E. McKenney <paulmck@kernel.org>
Co-developed-by: Frederic Weisbecker <frederic@kernel.org>
Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
 kernel/smp.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/kernel/smp.c b/kernel/smp.c
index 974f3a3962e8..23d51a8e582d 100644
--- a/kernel/smp.c
+++ b/kernel/smp.c
@@ -86,13 +86,15 @@ int smpcfd_dead_cpu(unsigned int cpu)
 int smpcfd_dying_cpu(unsigned int cpu)
 {
 	/*
-	 * The IPIs for the smp-call-function callbacks queued by other
-	 * CPUs might arrive late, either due to hardware latencies or
-	 * because this CPU disabled interrupts (inside stop-machine)
-	 * before the IPIs were sent. So flush out any pending callbacks
-	 * explicitly (without waiting for the IPIs to arrive), to
-	 * ensure that the outgoing CPU doesn't go offline with work
-	 * still pending.
+	 * The IPIs for the smp-call-function callbacks queued by other CPUs
+	 * might arrive late, either due to hardware latencies or because this
+	 * CPU disabled interrupts (inside stop-machine) before the IPIs were
+	 * sent. So flush out any pending callbacks explicitly (without waiting
+	 * for the IPIs to arrive), to ensure that the outgoing CPU doesn't go
+	 * offline with work still pending.
+	 *
+	 * This runs with interrupts disabled inside the stopper task invoked by
+	 * stop_machine(), ensuring mutually exclusive CPU offlining and IPI flush.
 	 */
 	__flush_smp_call_function_queue(false);
 	irq_work_run();
@@ -418,6 +420,10 @@ void __smp_call_single_queue(int cpu, struct llist_node *node)
  */
 static int generic_exec_single(int cpu, call_single_data_t *csd)
 {
+	/*
+	 * Preemption already disabled here so stopper cannot run on this CPU,
+	 * ensuring mutually exclusive CPU offlining and last IPI flush.
+	 */
 	if (cpu == smp_processor_id()) {
 		smp_call_func_t func = csd->func;
 		void *info = csd->info;
@@ -638,8 +644,10 @@ int smp_call_function_single(int cpu, smp_call_func_t func, void *info,
 	int err;
 
 	/*
-	 * prevent preemption and reschedule on another processor,
-	 * as well as CPU removal
+	 * Prevent preemption and reschedule on another CPU, as well as CPU
+	 * removal. This prevents stopper from running on this CPU, thus
+	 * providing mutual exclusion of the below cpu_online() check and
+	 * IPI sending ensuring IPI are not missed by CPU going offline.
 	 */
 	this_cpu = get_cpu();
 
-- 
2.34.1

[PATCH -next 2/6] rcu: Refactor expedited handling check in rcu_read_unlock_special()

[Joel Fernandes]

Extract the complex expedited handling condition in rcu_read_unlock_special()
into a separate function rcu_unlock_needs_exp_handling() with detailed
comments explaining each condition.

This improves code readability. No functional change intended.

Reviewed-by: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
 kernel/rcu/tree_plugin.h | 83 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 74 insertions(+), 9 deletions(-)

diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index 1ee0d34ec333..ffe6eb5d8e34 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -646,6 +646,75 @@ static void rcu_preempt_deferred_qs_handler(struct irq_work *iwp)
 	local_irq_restore(flags);
 }
 
+/*
+ * Check if expedited grace period processing during unlock is needed.
+ *
+ * This function determines whether expedited handling is required based on:
+ * 1. Task blocking an expedited grace period (based on a heuristic, could be
+ *    false-positive, see below.)
+ * 2. CPU participating in an expedited grace period
+ * 3. Strict grace period mode requiring expedited handling
+ * 4. RCU priority deboosting needs when interrupts were disabled
+ *
+ * @t: The task being checked
+ * @rdp: The per-CPU RCU data
+ * @rnp: The RCU node for this CPU
+ * @irqs_were_disabled: Whether interrupts were disabled before rcu_read_unlock()
+ *
+ * Returns true if expedited processing of the rcu_read_unlock() is needed.
+ */
+static bool rcu_unlock_needs_exp_handling(struct task_struct *t,
+				      struct rcu_data *rdp,
+				      struct rcu_node *rnp,
+				      bool irqs_were_disabled)
+{
+	/*
+	 * Check if this task is blocking an expedited grace period. If the
+	 * task was preempted within an RCU read-side critical section and is
+	 * on the expedited grace period blockers list (exp_tasks), we need
+	 * expedited handling to unblock the expedited GP. This is not an exact
+	 * check because 't' might not be on the exp_tasks list at all - its
+	 * just a fast heuristic that can be false-positive sometimes.
+	 */
+	if (t->rcu_blocked_node && READ_ONCE(t->rcu_blocked_node->exp_tasks))
+		return true;
+
+	/*
+	 * Check if this CPU is participating in an expedited grace period.
+	 * The expmask bitmap tracks which CPUs need to check in for the
+	 * current expedited GP. If our CPU's bit is set, we need expedited
+	 * handling to help complete the expedited GP.
+	 */
+	if (rdp->grpmask & READ_ONCE(rnp->expmask))
+		return true;
+
+	/*
+	 * In CONFIG_RCU_STRICT_GRACE_PERIOD=y kernels, all grace periods
+	 * are treated as short for testing purposes even if that means
+	 * disturbing the system more. Check if either:
+	 * - This CPU has not yet reported a quiescent state, or
+	 * - This task was preempted within an RCU critical section
+	 * In either case, require expedited handling for strict GP mode.
+	 */
+	if (IS_ENABLED(CONFIG_RCU_STRICT_GRACE_PERIOD) &&
+	    ((rdp->grpmask & READ_ONCE(rnp->qsmask)) || t->rcu_blocked_node))
+		return true;
+
+	/*
+	 * RCU priority boosting case: If a task is subject to RCU priority
+	 * boosting and exits an RCU read-side critical section with interrupts
+	 * disabled, we need expedited handling to ensure timely deboosting.
+	 * Without this, a low-priority task could incorrectly run at high
+	 * real-time priority for an extended period degrading real-time
+	 * responsiveness. This applies to all CONFIG_RCU_BOOST=y kernels,
+	 * not just to PREEMPT_RT.
+	 */
+	if (IS_ENABLED(CONFIG_RCU_BOOST) && irqs_were_disabled && t->rcu_blocked_node)
+		return true;
+
+	return false;
+}
+
 /*
  * Handle special cases during rcu_read_unlock(), such as needing to
  * notify RCU core processing or task having blocked during the RCU
@@ -665,18 +734,14 @@ static void rcu_read_unlock_special(struct task_struct *t)
 	local_irq_save(flags);
 	irqs_were_disabled = irqs_disabled_flags(flags);
 	if (preempt_bh_were_disabled || irqs_were_disabled) {
-		bool expboost; // Expedited GP in flight or possible boosting.
+		bool needs_exp; // Expedited handling needed.
 		struct rcu_data *rdp = this_cpu_ptr(&rcu_data);
 		struct rcu_node *rnp = rdp->mynode;
 
-		expboost = (t->rcu_blocked_node && READ_ONCE(t->rcu_blocked_node->exp_tasks)) ||
-			   (rdp->grpmask & READ_ONCE(rnp->expmask)) ||
-			   (IS_ENABLED(CONFIG_RCU_STRICT_GRACE_PERIOD) &&
-			   ((rdp->grpmask & READ_ONCE(rnp->qsmask)) || t->rcu_blocked_node)) ||
-			   (IS_ENABLED(CONFIG_RCU_BOOST) && irqs_were_disabled &&
-			    t->rcu_blocked_node);
+		needs_exp = rcu_unlock_needs_exp_handling(t, rdp, rnp, irqs_were_disabled);
+	
 		// Need to defer quiescent state until everything is enabled.
-		if (use_softirq && (in_hardirq() || (expboost && !irqs_were_disabled))) {
+		if (use_softirq && (in_hardirq() || (needs_exp && !irqs_were_disabled))) {
 			// 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.
@@ -689,7 +754,7 @@ static void rcu_read_unlock_special(struct task_struct *t)
 			set_tsk_need_resched(current);
 			set_preempt_need_resched();
 			if (IS_ENABLED(CONFIG_IRQ_WORK) && irqs_were_disabled &&
-			    expboost && rdp->defer_qs_iw_pending != DEFER_QS_PENDING &&
+			    needs_exp && rdp->defer_qs_iw_pending != DEFER_QS_PENDING &&
 			    cpu_online(rdp->cpu)) {
 				// Get scheduler to re-evaluate and call hooks.
 				// If !IRQ_WORK, FQS scan will eventually IPI.
-- 
2.34.1

[PATCH -next 3/6] rcu: Document GP init vs hotplug-scan ordering requirements

[Joel Fernandes]

Add detailed comments explaining the critical ordering constraints
during RCU grace period initialization, based on discussions with
Frederic.

Reviewed-by: Paul E. McKenney <paulmck@kernel.org>
Co-developed-by: Frederic Weisbecker <frederic@kernel.org>
Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
 .../RCU/Design/Requirements/Requirements.rst  | 41 +++++++++++++++++++
 kernel/rcu/tree.c                             |  8 ++++
 2 files changed, 49 insertions(+)

diff --git a/Documentation/RCU/Design/Requirements/Requirements.rst b/Documentation/RCU/Design/Requirements/Requirements.rst
index 6125e7068d2c..771a1ce4c84d 100644
--- a/Documentation/RCU/Design/Requirements/Requirements.rst
+++ b/Documentation/RCU/Design/Requirements/Requirements.rst
@@ -1970,6 +1970,47 @@ corresponding CPU's leaf node lock is held. This avoids race conditions
 between RCU's hotplug notifier hooks, the grace period initialization
 code, and the FQS loop, all of which refer to or modify this bookkeeping.
 
+Note that grace period initialization (rcu_gp_init()) must carefully sequence
+CPU hotplug scanning with grace period state changes. For example, the
+following race could occur in rcu_gp_init() if rcu_seq_start() were to happen
+after the CPU hotplug scanning.
+
+.. code-block:: none
+
+   CPU0 (rcu_gp_init)                   CPU1                          CPU2
+   ---------------------                ----                          ----
+   // Hotplug scan first (WRONG ORDER)
+   rcu_for_each_leaf_node(rnp) {
+       rnp->qsmaskinit = rnp->qsmaskinitnext;
+   }
+                                        rcutree_report_cpu_starting()
+                                            rnp->qsmaskinitnext |= mask;
+                                        rcu_read_lock()
+                                        r0 = *X;
+                                                                      r1 = *X;
+                                                                      X = NULL;
+                                                                      cookie = get_state_synchronize_rcu();
+                                                                      // cookie = 8 (future GP)
+   rcu_seq_start(&rcu_state.gp_seq);
+   // gp_seq = 5
+
+   // CPU1 now invisible to this GP!
+   rcu_for_each_node_breadth_first() {
+       rnp->qsmask = rnp->qsmaskinit;
+       // CPU1 not included!
+   }
+
+   // GP completes without CPU1
+   rcu_seq_end(&rcu_state.gp_seq);
+   // gp_seq = 8
+                                                                      poll_state_synchronize_rcu(cookie);
+                                                                      // Returns true!
+                                                                      kfree(r1);
+                                        r2 = *r0; // USE-AFTER-FREE!
+
+By incrementing gp_seq first, CPU1's RCU read-side critical section
+is guaranteed to not be missed by CPU2.
+
 Scheduler and RCU
 ~~~~~~~~~~~~~~~~~
 
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 08be3df95e68..040e853758df 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -1837,6 +1837,14 @@ static noinline_for_stack bool rcu_gp_init(void)
 	start_new_poll = rcu_sr_normal_gp_init();
 	/* Record GP times before starting GP, hence rcu_seq_start(). */
 	old_gp_seq = rcu_state.gp_seq;
+	/*
+	 * Critical ordering: rcu_seq_start() must happen BEFORE the CPU hotplug
+	 * scan below. Otherwise we risk a race where a newly onlining CPU could
+	 * be missed by the current grace period, potentially leading to
+	 * use-after-free errors. For a detailed explanation of this race, see
+	 * Documentation/RCU/Design/Requirements/Requirements.rst in the
+	 * "Hotplug CPU" section.
+	 */
 	rcu_seq_start(&rcu_state.gp_seq);
 	/* Ensure that rcu_seq_done_exact() guardband doesn't give false positives. */
 	WARN_ON_ONCE(IS_ENABLED(CONFIG_PROVE_RCU) &&
-- 
2.34.1

[PATCH -next 4/6] rcu: Document separation of rcu_state and rnp's gp_seq

[Joel Fernandes]

The details of this are subtle and was discussed recently. Add a
quick-quiz about this and refer to it from the code, for more clarity.

Reviewed-by: Paul E. McKenney <paulmck@kernel.org>
Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
 .../Data-Structures/Data-Structures.rst       | 32 +++++++++++++++++++
 kernel/rcu/tree.c                             |  4 +++
 2 files changed, 36 insertions(+)

diff --git a/Documentation/RCU/Design/Data-Structures/Data-Structures.rst b/Documentation/RCU/Design/Data-Structures/Data-Structures.rst
index 04e16775c752..930535f076b4 100644
--- a/Documentation/RCU/Design/Data-Structures/Data-Structures.rst
+++ b/Documentation/RCU/Design/Data-Structures/Data-Structures.rst
@@ -286,6 +286,38 @@ in order to detect the beginnings and ends of grace periods in a
 distributed fashion. The values flow from ``rcu_state`` to ``rcu_node``
 (down the tree from the root to the leaves) to ``rcu_data``.
 
++-----------------------------------------------------------------------+
+| **Quick Quiz**:                                                       |
++-----------------------------------------------------------------------+
+| Given that the root rcu_node structure has a gp_seq field,            |
+| why does RCU maintain a separate gp_seq in the rcu_state structure?   |
+| Why not just use the root rcu_node's gp_seq as the official record    |
+| and update it directly when starting a new grace period?              |
++-----------------------------------------------------------------------+
+| **Answer**:                                                           |
++-----------------------------------------------------------------------+
+| On single-node RCU trees (where the root node is also a leaf),        |
+| updating the root node's gp_seq immediately would create unnecessary  |
+| lock contention. Here's why:                                          |
+|                                                                       |
+| If we did rcu_seq_start() directly on the root node's gp_seq:         |
+| 1. All CPUs would immediately see their node's gp_seq from their rdp's|
+|    gp_seq, in rcu_pending(). They would all then invoke the RCU-core. |
+| 2. Which calls note_gp_changes() and try to acquire the node lock.    |
+| 3. But rnp->qsmask isn't initialized yet (happens later in            |
+|    rcu_gp_init())                                                     |
+| 4. So each CPU would acquire the lock, find it can't determine if it  |
+|    needs to report quiescent state (no qsmask), update rdp->gp_seq,   |
+|    and release the lock.                                              |
+| 5. Result: Lots of lock acquisitions with no grace period progress    |
+|                                                                       |
+| By having a separate rcu_state.gp_seq, we can increment the official  |
+| grace period counter without immediately affecting what CPUs see in   |
+| their nodes. The hierarchical propagation in rcu_gp_init() then       |
+| updates the root node's gp_seq and qsmask together under the same lock|
+| acquisition, avoiding this useless contention.                        |
++-----------------------------------------------------------------------+
+
 Miscellaneous
 '''''''''''''
 
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 040e853758df..aa6cbd1501cb 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -1844,6 +1844,10 @@ static noinline_for_stack bool rcu_gp_init(void)
 	 * use-after-free errors. For a detailed explanation of this race, see
 	 * Documentation/RCU/Design/Requirements/Requirements.rst in the
 	 * "Hotplug CPU" section.
+	 *
+	 * Also note that the root rnp's gp_seq is kept separate from, and lags,
+	 * the rcu_state's gp_seq, for a reason. See the Quick-Quiz on
+	 * Single-node systems for more details (in Data-Structures.rst).
 	 */
 	rcu_seq_start(&rcu_state.gp_seq);
 	/* Ensure that rcu_seq_done_exact() guardband doesn't give false positives. */
-- 
2.34.1

[PATCH -next 5/6] rcu: Document concurrent quiescent state reporting for offline CPUs

[Joel Fernandes]

The synchronization of CPU offlining with GP initialization is confusing
to put it mildly (rightfully so as the issue it deals with is complex).
Recent discussions brought up a question -- what prevents the
rcu_implicit_dyntick_qs() from warning about QS reports for offline
CPUs (missing QS reports for offline CPUs causing indefinite hangs).

QS reporting for now-offline CPUs should only happen from:
- gp_init()
- rcutree_cpu_report_dead()

Add some documentation on this and refer to it from comments in the code
explaining how QS reporting is not missed when these functions are
concurrently running.

I referred heavily to this post [1] about the need for the ofl_lock.
[1] https://lore.kernel.org/all/20180924164443.GF4222@linux.ibm.com/

[ Applied paulmck feedback on moving documentation to Requirements.rst ]

Link: https://lore.kernel.org/all/01b4d228-9416-43f8-a62e-124b92e8741a@paulmck-laptop/
Co-developed-by: Paul E. McKenney <paulmck@kernel.org>
Reviewed-by: Frederic Weisbecker <frederic@kernel.org>
Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
 .../RCU/Design/Requirements/Requirements.rst  | 87 +++++++++++++++++++
 kernel/rcu/tree.c                             | 19 +++-
 2 files changed, 105 insertions(+), 1 deletion(-)

diff --git a/Documentation/RCU/Design/Requirements/Requirements.rst b/Documentation/RCU/Design/Requirements/Requirements.rst
index 771a1ce4c84d..841326d9358d 100644
--- a/Documentation/RCU/Design/Requirements/Requirements.rst
+++ b/Documentation/RCU/Design/Requirements/Requirements.rst
@@ -2011,6 +2011,93 @@ after the CPU hotplug scanning.
 By incrementing gp_seq first, CPU1's RCU read-side critical section
 is guaranteed to not be missed by CPU2.
 
+**Concurrent Quiescent State Reporting for Offline CPUs**
+
+RCU must ensure that CPUs going offline report quiescent states to avoid
+blocking grace periods. This requires careful synchronization to handle
+race conditions
+
+**Race condition causing Offline CPU to hang GP**
+
+A race between CPU offlining and new GP initialization (gp_init) may occur
+because `rcu_report_qs_rnp()` in `rcutree_report_cpu_dead()` must temporarily
+release the `rcu_node` lock to wake the RCU grace-period kthread:
+
+.. code-block:: none
+
+   CPU1 (going offline)                 CPU0 (GP kthread)
+   --------------------                 -----------------
+   rcutree_report_cpu_dead()
+     rcu_report_qs_rnp()
+       // Must release rnp->lock to wake GP kthread
+       raw_spin_unlock_irqrestore_rcu_node()
+                                        // Wakes up and starts new GP
+                                        rcu_gp_init()
+                                          // First loop:
+                                          copies qsmaskinitnext->qsmaskinit
+                                          // CPU1 still in qsmaskinitnext!
+                                          
+                                          // Second loop:
+                                          rnp->qsmask = rnp->qsmaskinit
+                                          mask = rnp->qsmask & ~rnp->qsmaskinitnext
+                                          // mask is 0! CPU1 still in both masks
+       // Reacquire lock (but too late)
+     rnp->qsmaskinitnext &= ~mask       // Finally clears bit
+
+Without `ofl_lock`, the new grace period includes the offline CPU and waits
+forever for its quiescent state causing a GP hang.
+
+**A solution with ofl_lock**
+
+The `ofl_lock` (offline lock) prevents `rcu_gp_init()` from running during
+the vulnerable window when `rcu_report_qs_rnp()` has released `rnp->lock`:
+
+.. code-block:: none
+
+   CPU0 (rcu_gp_init)                   CPU1 (rcutree_report_cpu_dead)
+   ------------------                   ------------------------------
+   rcu_for_each_leaf_node(rnp) {
+       arch_spin_lock(&ofl_lock) -----> arch_spin_lock(&ofl_lock) [BLOCKED]
+       
+       // Safe: CPU1 can't interfere
+       rnp->qsmaskinit = rnp->qsmaskinitnext
+       
+       arch_spin_unlock(&ofl_lock) ---> // Now CPU1 can proceed
+   }                                    // But snapshot already taken
+
+**Another race causing GP hangs in rcu_gpu_init(): Reporting QS for Now-offline CPUs**
+
+After the first loop takes an atomic snapshot of online CPUs, as shown above,
+the second loop in `rcu_gp_init()` detects CPUs that went offline between
+releasing `ofl_lock` and acquiring the per-node `rnp->lock`. This detection is
+crucial because:
+
+1. The CPU might have gone offline after the snapshot but before the second loop
+2. The offline CPU cannot report its own QS if it's already dead
+3. Without this detection, the grace period would wait forever for CPUs that
+   are now offline.
+
+The second loop performs this detection safely:
+
+.. code-block:: none
+
+   rcu_for_each_node_breadth_first(rnp) {
+       raw_spin_lock_irqsave_rcu_node(rnp, flags);
+       rnp->qsmask = rnp->qsmaskinit;  // Apply the snapshot
+       
+       // Detect CPUs offline after snapshot
+       mask = rnp->qsmask & ~rnp->qsmaskinitnext;
+       
+       if (mask && rcu_is_leaf_node(rnp))
+           rcu_report_qs_rnp(mask, ...)  // Report QS for offline CPUs
+   }
+
+This approach ensures atomicity: quiescent state reporting for offline CPUs
+happens either in `rcu_gp_init()` (second loop) or in `rcutree_report_cpu_dead()`,
+never both and never neither. The `rnp->lock` held throughout the sequence
+prevents races - `rcutree_report_cpu_dead()` also acquires this lock when
+clearing `qsmaskinitnext`, ensuring mutual exclusion.
+
 Scheduler and RCU
 ~~~~~~~~~~~~~~~~~
 
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index aa6cbd1501cb..174ee243b349 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -1885,6 +1885,10 @@ static noinline_for_stack bool rcu_gp_init(void)
 	/* Exclude CPU hotplug operations. */
 	rcu_for_each_leaf_node(rnp) {
 		local_irq_disable();
+		/*
+		 * Serialize with CPU offline. See Requirements.rst > Hotplug CPU >
+		 * Concurrent Quiescent State Reporting for Offline CPUs.
+		 */
 		arch_spin_lock(&rcu_state.ofl_lock);
 		raw_spin_lock_rcu_node(rnp);
 		if (rnp->qsmaskinit == rnp->qsmaskinitnext &&
@@ -1959,7 +1963,12 @@ static noinline_for_stack bool rcu_gp_init(void)
 		trace_rcu_grace_period_init(rcu_state.name, rnp->gp_seq,
 					    rnp->level, rnp->grplo,
 					    rnp->grphi, rnp->qsmask);
-		/* Quiescent states for tasks on any now-offline CPUs. */
+		/*
+		 * Quiescent states for tasks on any now-offline CPUs. Since we
+		 * released the ofl and rnp lock before this loop, CPUs might
+		 * have gone offline and we have to report QS on their behalf.
+		 * See Requirements.rst > Hotplug CPU > Concurrent QS Reporting.
+		 */
 		mask = rnp->qsmask & ~rnp->qsmaskinitnext;
 		rnp->rcu_gp_init_mask = mask;
 		if ((mask || rnp->wait_blkd_tasks) && rcu_is_leaf_node(rnp))
@@ -4390,6 +4399,13 @@ void rcutree_report_cpu_dead(void)
 
 	/* Remove outgoing CPU from mask in the leaf rcu_node structure. */
 	mask = rdp->grpmask;
+
+	/*
+	 * Hold the ofl_lock and rnp lock to avoid races between CPU going
+	 * offline and doing a QS report (as below), versus rcu_gp_init().
+	 * See Requirements.rst > Hotplug CPU > Concurrent QS Reporting section
+	 * for more details.
+	 */
 	arch_spin_lock(&rcu_state.ofl_lock);
 	raw_spin_lock_irqsave_rcu_node(rnp, flags); /* Enforce GP memory-order guarantee. */
 	rdp->rcu_ofl_gp_seq = READ_ONCE(rcu_state.gp_seq);
@@ -4400,6 +4416,7 @@ void rcutree_report_cpu_dead(void)
 		rcu_report_qs_rnp(mask, rnp, rnp->gp_seq, flags);
 		raw_spin_lock_irqsave_rcu_node(rnp, flags);
 	}
+	/* Clear from ->qsmaskinitnext to mark offline. */
 	WRITE_ONCE(rnp->qsmaskinitnext, rnp->qsmaskinitnext & ~mask);
 	raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
 	arch_spin_unlock(&rcu_state.ofl_lock);
-- 
2.34.1

[PATCH -next 6/6] [please squash] fixup! rcu: Fix rcu_read_unlock() deadloop due to IRQ work

[Joel Fernandes]

Please squash few comment-related changes courtesy of review from
Frederic.

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

diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
index f8f612269e6e..de6ca13a7b5f 100644
--- a/kernel/rcu/tree.h
+++ b/kernel/rcu/tree.h
@@ -175,10 +175,12 @@ struct rcu_snap_record {
 };
 
 /*
- * The IRQ work (deferred_qs_iw) is used by RCU to get scheduler's attention.
- * It can be in one of the following states:
- * - DEFER_QS_IDLE: An IRQ work was never scheduled.
- * - DEFER_QS_PENDING: An IRQ work was scheduler but never run.
+ * An IRQ work (deferred_qs_iw) is used by RCU to get the scheduler's attention.
+ * to report quiescent states at the soonest possible time.
+ * The request can be in one of the following states:
+ * - DEFER_QS_IDLE: An IRQ work is yet to be scheduled.
+ * - DEFER_QS_PENDING: An IRQ work was scheduled but either not yet run, or it
+ *                     ran and we still haven't reported a quiescent state.
  */
 #define DEFER_QS_IDLE		0
 #define DEFER_QS_PENDING	1
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index ffe6eb5d8e34..1b9403505c42 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -633,7 +633,12 @@ static void rcu_preempt_deferred_qs_handler(struct irq_work *iwp)
 	local_irq_save(flags);
 
 	/*
-	 * Requeue the IRQ work on next unlock in following situation:
+	 * 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
-- 
2.34.1

Re: [PATCH -next 0/6] Patches for v6.17

[Neeraj upadhyay]

Hi Joel,

On Wed, Jul 16, 2025 at 1:32 AM Joel Fernandes <joelagnelf@nvidia.com> wrote:
>
> Just a repost of patches with tags, for our consideration into v6.17.
>
> All have tags, and the last commit is a fixup for the deadloop patch which can
> be squashed into the original patch.
>
> Joel Fernandes (6):
>   smp: Document preemption and stop_machine() mutual exclusion
>   rcu: Refactor expedited handling check in rcu_read_unlock_special()
>   rcu: Document GP init vs hotplug-scan ordering requirements
>   rcu: Document separation of rcu_state and rnp's gp_seq
>   rcu: Document concurrent quiescent state reporting for offline CPUs
>   [please squash] fixup! rcu: Fix rcu_read_unlock() deadloop due to IRQ
>     work
>

I have included patches 2-6 here:
https://git.kernel.org/pub/scm/linux/kernel/git/neeraj.upadhyay/linux-rcu.git/log/?h=rcu.merge.16.07.2025c
for testing

Fixed few whitespaces errors in the documentation.

I see below warnings while doing `make htmldocs`. Can you suggest a
fix for these?

Documentation/RCU/Design/Data-Structures/Data-Structures.rst:305:
ERROR: Unexpected indentation.
Documentation/RCU/Design/Data-Structures/Data-Structures.rst:307:
WARNING: Block quote ends without a blank line; unexpected unindent.


- Neeraj

Re: [PATCH -next 0/6] Patches for v6.17

[Akira Yokosawa]

[+CC Frederic]

Hi Joel and Neeraj,

Belated drive-by review.  Please see below.

On Tue, 15 Jul 2025 16:01:50 -0400, Joel Fernandes wrote:
> Just a repost of patches with tags, for our consideration into v6.17.
> 
> All have tags, and the last commit is a fixup for the deadloop patch which can
> be squashed into the original patch.
> 
> Joel Fernandes (6):
>   smp: Document preemption and stop_machine() mutual exclusion
>   rcu: Refactor expedited handling check in rcu_read_unlock_special()
>   rcu: Document GP init vs hotplug-scan ordering requirements
>   rcu: Document separation of rcu_state and rnp's gp_seq
>   rcu: Document concurrent quiescent state reporting for offline CPUs
>   [please squash] fixup! rcu: Fix rcu_read_unlock() deadloop due to IRQ
>     work

There seems to be a couple of issues in the S-O-B chains of commits listed
below (in rcu/next):

 * dcf1668449c9 ("rcu: Document GP init vs hotplug-scan ordering requirements")
 * bb1c373934db ("rcu: Document concurrent quiescent state reporting for offline CPUs")

They have a "Co-developed-by:" tag without a corresponding "Signed-off-by:"
tag [1].

Or, if the contribution is too minor to have a "Signed-off-by:",
then a "Suggested-by:" tag with a "Link:" to the relevant message should
suffice.

I have no idea which approach suits better in each commit above.

[1]: Documentation/process/submitting-patches.rst
     section "When to use Acked-by:, Cc:, and Co-developed-by:"

Quoting relevant paragraph:

  Co-developed-by: states that the patch was co-created by multiple developers;
  it is used to give attribution to co-authors (in addition to the author
  attributed by the From: tag) when several people work on a single patch.  Since
  Co-developed-by: denotes authorship, every Co-developed-by: must be immediately
  followed by a Signed-off-by: of the associated co-author.  Standard sign-off
  procedure applies, i.e. the ordering of Signed-off-by: tags should reflect the
  chronological history of the patch insofar as possible, regardless of whether
  the author is attributed via From: or Co-developed-by:.  Notably, the last
  Signed-off-by: must always be that of the developer submitting the patch.

Side note:
  scripts/checkpatch.pl would have complained about those missing
  Signed-off-by: tags.

> 
>  .../Data-Structures/Data-Structures.rst       |  32 +++++
>  .../RCU/Design/Requirements/Requirements.rst  | 128 ++++++++++++++++++

I'm seeing sub-optimal uses of reST markups in Requirements.rst from kernel
documentation stand point.

I'm going to submit a patch or two to improve them, but I can't promise when.
They will likely be only cosmetic cleanups and I'm OK with it upstreamed as
it is.

        Thanks, Akira

>  kernel/rcu/tree.c                             |  31 ++++-
>  kernel/rcu/tree.h                             |  10 +-
>  kernel/rcu/tree_plugin.h                      |  90 ++++++++++--
>  kernel/smp.c                                  |  26 ++--
>  6 files changed, 293 insertions(+), 24 deletions(-)
> 
> -- 
> 2.34.1

Re: [PATCH -next 0/6] Patches for v6.17

[Neeraj upadhyay]

Hi,

On Sun, Jul 20, 2025 at 10:01 AM Akira Yokosawa <akiyks@gmail.com> wrote:

...

> There seems to be a couple of issues in the S-O-B chains of commits listed
> below (in rcu/next):
>
>  * dcf1668449c9 ("rcu: Document GP init vs hotplug-scan ordering requirements")
>  * bb1c373934db ("rcu: Document concurrent quiescent state reporting for offline CPUs")
>
> They have a "Co-developed-by:" tag without a corresponding "Signed-off-by:"
> tag [1].
>
> Or, if the contribution is too minor to have a "Signed-off-by:",
> then a "Suggested-by:" tag with a "Link:" to the relevant message should
> suffice.
>
> I have no idea which approach suits better in each commit above.
>
> [1]: Documentation/process/submitting-patches.rst
>      section "When to use Acked-by:, Cc:, and Co-developed-by:"
>

Thanks for reviewing this! I will fix the tags.


> Quoting relevant paragraph:
>
>   Co-developed-by: states that the patch was co-created by multiple developers;
>   it is used to give attribution to co-authors (in addition to the author
>   attributed by the From: tag) when several people work on a single patch.  Since
>   Co-developed-by: denotes authorship, every Co-developed-by: must be immediately
>   followed by a Signed-off-by: of the associated co-author.  Standard sign-off
>   procedure applies, i.e. the ordering of Signed-off-by: tags should reflect the
>   chronological history of the patch insofar as possible, regardless of whether
>   the author is attributed via From: or Co-developed-by:.  Notably, the last
>   Signed-off-by: must always be that of the developer submitting the patch.
>
> Side note:
>   scripts/checkpatch.pl would have complained about those missing
>   Signed-off-by: tags.
>
> >
> >  .../Data-Structures/Data-Structures.rst       |  32 +++++
> >  .../RCU/Design/Requirements/Requirements.rst  | 128 ++++++++++++++++++
>
> I'm seeing sub-optimal uses of reST markups in Requirements.rst from kernel
> documentation stand point.
>
> I'm going to submit a patch or two to improve them, but I can't promise when.
> They will likely be only cosmetic cleanups and I'm OK with it upstreamed as
> it is.
>

Thanks!


- Neeraj

Re: [PATCH -next 0/6] Patches for v6.17

[Joel Fernandes]

> On Jul 21, 2025, at 12:29 AM, Neeraj upadhyay <neeraj.iitr10@gmail.com> wrote:
> 
> Hi,
> 
> On Sun, Jul 20, 2025 at 10:01 AM Akira Yokosawa <akiyks@gmail.com> wrote:
> 
> ...
> 
>> There seems to be a couple of issues in the S-O-B chains of commits listed
>> below (in rcu/next):
>> 
>> * dcf1668449c9 ("rcu: Document GP init vs hotplug-scan ordering requirements")
>> * bb1c373934db ("rcu: Document concurrent quiescent state reporting for offline CPUs")
>> 
>> They have a "Co-developed-by:" tag without a corresponding "Signed-off-by:"
>> tag [1].
>> 
>> Or, if the contribution is too minor to have a "Signed-off-by:",
>> then a "Suggested-by:" tag with a "Link:" to the relevant message should
>> suffice.
>> 
>> I have no idea which approach suits better in each commit above.
>> 
>> [1]: Documentation/process/submitting-patches.rst
>>     section "When to use Acked-by:, Cc:, and Co-developed-by:"
>> 
> 
> Thanks for reviewing this! I will fix the tags.
> 
> 
>> Quoting relevant paragraph:
>> 
>>  Co-developed-by: states that the patch was co-created by multiple developers;
>>  it is used to give attribution to co-authors (in addition to the author
>>  attributed by the From: tag) when several people work on a single patch.  Since
>>  Co-developed-by: denotes authorship, every Co-developed-by: must be immediately
>>  followed by a Signed-off-by: of the associated co-author.  Standard sign-off
>>  procedure applies, i.e. the ordering of Signed-off-by: tags should reflect the
>>  chronological history of the patch insofar as possible, regardless of whether
>>  the author is attributed via From: or Co-developed-by:.  Notably, the last
>>  Signed-off-by: must always be that of the developer submitting the patch.
>> 
>> Side note:
>>  scripts/checkpatch.pl would have complained about those missing
>>  Signed-off-by: tags.
>> 
>>> 
>>> .../Data-Structures/Data-Structures.rst       |  32 +++++
>>> .../RCU/Design/Requirements/Requirements.rst  | 128 ++++++++++++++++++
>> 
>> I'm seeing sub-optimal uses of reST markups in Requirements.rst from kernel
>> documentation stand point.
>> 
>> I'm going to submit a patch or two to improve them, but I can't promise when.
>> They will likely be only cosmetic cleanups and I'm OK with it upstreamed as
>> it is.
>> 
> 
> Thanks!

Thanks, I appreciate that!

- Joel

> 
> 
> - Neeraj