rcu/nocb updates

Quoted 22 lines · Frederic Weisbecker

The "nocb_bypass_timer" ends up calling wake_nocb_gp() which deletes the pending "nocb_timer" (note they are not the same timers) for the given rdp without resetting the matching state stored in nocb_defer wakeup.

As a result, a future call_rcu() on that rdp may be fooled and think the timer is armed when it's not, missing a deferred nocb_gp wakeup.

Fix this with resetting rdp->nocb_defer_wakeup when we disarm the timer.

Fixes d1b222c6be1f (rcu/nocb: Add bypass callback queueing)

Cc Stable <[email protected]>

Cc Josh Triplett <[email protected]>

Cc Lai Jiangshan <[email protected]>

Cc Joel Fernandes <[email protected]>

Cc Neeraj Upadhyay <[email protected]>

Cc Boqun Feng <[email protected]>

Signed-off-by Frederic Weisbecker <[email protected]>

---
 kernel/rcu/tree_plugin.h | 2 ++
 1 file changed, 2 insertions(+)

kernel/rcu/tree_plugin.h static bool wake_nocb_gp(struct rcu_data *rdp, bool force, · line 1709

Quoted 10 lines

kernel/rcu/tree_plugin.h +2 -0

index 7e33dae0e6ee..a44f80d7661b 100644
@@ -1705,6 +1705,8 @@ static bool wake_nocb_gp(struct rcu_data *rdp, bool force,
 		rcu_nocb_unlock_irqrestore(rdp, flags);
 		return false;
 	}
+
+	rdp->nocb_defer_wakeup = RCU_NOCB_WAKE_NOT;

Given this change, does it make sense to remove the setting of ->nocb_defer_wakeup to RCU_NOCB_WAKE_NOT from the do_nocb_deferred_wakeup_common() function?

Does the above assignment need
to be WRITE_ONCE(), in other words, are all reads of ->nocb_defer_wakeup done with either ->nocb_lock or ->nocb_gp_lock held? (I do not believe that this is the case.)

Thanx, Paul

Quoted 6 lines

kernel/rcu/tree_plugin.h +0 -0

 	del_timer(&rdp->nocb_timer);
 	rcu_nocb_unlock_irqrestore(rdp, flags);
 	raw_spin_lock_irqsave(&rdp_gp->nocb_gp_lock, flags);

Signature · 1 line

2.25.1

Toggling the NOCB state of a CPU when it is offline imply some specific issues to handle, especially making sure that the kthreads have handled all the remaining callbacks and bypass before the corresponding CPU can be set as non-offloaded while it is offline.

To prevent from such complications, simply forbid offline CPUs to perform NOCB-mode toggling. It's a simple rule to observe and after all it doesn't make much sense to switch a non working CPU to/from offloaded state.

Reported-by Paul E. McKenney <[email protected]>

Cc Josh Triplett <[email protected]>

Cc Lai Jiangshan <[email protected]>

Cc Joel Fernandes <[email protected]>

Cc Neeraj Upadhyay <[email protected]>

Cc Boqun Feng <[email protected]>

Signed-off-by Frederic Weisbecker <[email protected]>

Quoted 10 lines · Frederic Weisbecker

Simply checking if the segcblist is enabled is enough to know if we need to initialize it or not. It's safe to check within hotplug machine.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Josh Triplett <[email protected]>

Cc Lai Jiangshan <[email protected]>

Cc Joel Fernandes <[email protected]>

Cc Neeraj Upadhyay <[email protected]>

Cc Boqun Feng <[email protected]>

Hmmm...

At the start of a CPU-hotplug operation, an incoming CPU's callback list can be in a number of states:

1. Disabled and empty. This is the case when the boot CPU has
not done call_rcu(), when a non-boot CPU first comes online, and when a non-offloaded CPU comes back online. In this case, it is permissible to initialize ->cblist. Because either the CPU is currently running with interrupts disabled (boot CPU) or is not yet running at all (other CPUs), it is not necessary to acquire ->nocb_lock.

2. Disabled and non-empty. This is the case when the boot CPU has
done call_rcu(). It is not permissible to initialize ->cblist because doing so will leak any callbacks posted by early boot invocations of call_rcu().

Test for the possibility of leaking by building with CONFIG_PROVE_RCU=y and booting with rcupdate.rcu_self_test=1.

3. Enabled, whether empty or not. This is the case when an

	offloaded CPU comes back online.  This is the only case where
	the ->nocb_lock must be held to modify ->cblist.  However,
	it is not necessarily to modify ->cblist because the rcuoc
	kthread is on the job.

So I believe that it is necessary to check for both disabled and empty. But don't take my word for it! Build with CONFIG_PROVE_RCU=y and boot with rcupdate.rcu_self_test=1. ;-)

Quoted 25 lines · Paul E. McKenney

Quoted 10 lines · Frederic Weisbecker

Simply checking if the segcblist is enabled is enough to know if we need to initialize it or not. It's safe to check within hotplug machine.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Josh Triplett <[email protected]>

Cc Lai Jiangshan <[email protected]>

Cc Joel Fernandes <[email protected]>

Cc Neeraj Upadhyay <[email protected]>

Cc Boqun Feng <[email protected]>

Hmmm...

At the start of a CPU-hotplug operation, an incoming CPU's callback list can be in a number of states:

1. Disabled and empty. This is the case when the boot CPU has
not done call_rcu(), when a non-boot CPU first comes online, and when a non-offloaded CPU comes back online. In this case, it is permissible to initialize ->cblist. Because either the CPU is currently running with interrupts disabled (boot CPU) or is not yet running at all (other CPUs), it is not necessary to acquire ->nocb_lock.

Quoted 4 lines

2. Disabled and non-empty. This is the case when the boot CPU has
done call_rcu(). It is not permissible to initialize ->cblist because doing so will leak any callbacks posted by early boot invocations of call_rcu().

I don't think that's possible. In this case __call_rcu() has called rcu_segcblist_init() and has enabled the segcblist.

Quoted 9 lines

Test for the possibility of leaking by building with CONFIG_PROVE_RCU=y and booting with rcupdate.rcu_self_test=1.

3. Enabled, whether empty or not. This is the case when an

	offloaded CPU comes back online.  This is the only case where
	the ->nocb_lock must be held to modify ->cblist.  However,
	it is not necessarily to modify ->cblist because the rcuoc
	kthread is on the job.

Quoted 3 lines

So I believe that it is necessary to check for both disabled and empty. But don't take my word for it! Build with CONFIG_PROVE_RCU=y and boot with rcupdate.rcu_self_test=1. ;-)

I'm trying that :-)

Quoted 26 lines · Frederic Weisbecker

Quoted 25 lines · Paul E. McKenney

Quoted 10 lines · Frederic Weisbecker

Simply checking if the segcblist is enabled is enough to know if we need to initialize it or not. It's safe to check within hotplug machine.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Josh Triplett <[email protected]>

Cc Lai Jiangshan <[email protected]>

Cc Joel Fernandes <[email protected]>

Cc Neeraj Upadhyay <[email protected]>

Cc Boqun Feng <[email protected]>

Hmmm...

At the start of a CPU-hotplug operation, an incoming CPU's callback list can be in a number of states:

1. Disabled and empty. This is the case when the boot CPU has
not done call_rcu(), when a non-boot CPU first comes online, and when a non-offloaded CPU comes back online. In this case, it is permissible to initialize ->cblist. Because either the CPU is currently running with interrupts disabled (boot CPU) or is not yet running at all (other CPUs), it is not necessary to acquire ->nocb_lock.

Quoted 7 lines

Quoted 4 lines

2. Disabled and non-empty. This is the case when the boot CPU has
done call_rcu(). It is not permissible to initialize ->cblist because doing so will leak any callbacks posted by early boot invocations of call_rcu().

I don't think that's possible. In this case __call_rcu() has called rcu_segcblist_init() and has enabled the segcblist.

You are right, rcu_segcblist_init() would have been called in that case and it has: rcu_segcblist_set_flags(rsclp, SEGCBLIST_ENABLED).

Quoted 9 lines

Quoted 9 lines

Test for the possibility of leaking by building with CONFIG_PROVE_RCU=y and booting with rcupdate.rcu_self_test=1.

3. Enabled, whether empty or not. This is the case when an

	offloaded CPU comes back online.  This is the only case where
	the ->nocb_lock must be held to modify ->cblist.  However,
	it is not necessarily to modify ->cblist because the rcuoc
	kthread is on the job.

Quoted 5 lines

Quoted 3 lines

So I believe that it is necessary to check for both disabled and empty. But don't take my word for it! Build with CONFIG_PROVE_RCU=y and boot with rcupdate.rcu_self_test=1. ;-)

I'm trying that :-)

Even better!

Instead of flushing bypass at the very last moment in the deoffloading process, just disable bypass enqueue at soon as we start the deoffloading process and flush the pending bypass early. It's less fragile and we leave some time to the kthreads and softirqs to process quietly.

Symmetrically, only enable bypass once we safely complete the offloading process.

Reported-by Paul E. McKenney <[email protected]>

Cc Josh Triplett <[email protected]>

Cc Lai Jiangshan <[email protected]>

Cc Joel Fernandes <[email protected]>

Cc Neeraj Upadhyay <[email protected]>

Cc Boqun Feng <[email protected]>

Signed-off-by Frederic Weisbecker <[email protected]>

---
 include/linux/rcu_segcblist.h |  7 ++++---
 kernel/rcu/tree_plugin.h      | 31 +++++++++++++++++++++++--------
 2 files changed, 27 insertions(+), 11 deletions(-)

include/linux/rcu_segcblist.h +0 -0

index 8afe886e85f1..5a2d6dadd720 100644

include/linux/rcu_segcblist.h +3 -2

  *  ----------------------------------------------------------------------------
  */
 

@@ -125,7 +125,7 @@ struct rcu_cblist {
  *  |                           SEGCBLIST_KTHREAD_GP                           |
  *  |                                                                          |
  *  |   CB/GP kthreads handle callbacks holding nocb_lock, local rcu_core()    |
- *  |   ignores callbacks.                                                     |
+ *  |   ignores callbacks. Bypass enqueue is enabled.                          |
  *  ----------------------------------------------------------------------------
  *                                      |
  *                                      v

@@ -134,7 +134,8 @@ struct rcu_cblist {
  *  |                           SEGCBLIST_KTHREAD_GP                           |
  *  |                                                                          |
  *  |   CB/GP kthreads and local rcu_core() handle callbacks concurrently      |
- *  |   holding nocb_lock. Wake up CB and GP kthreads if necessary.            |
+ *  |   holding nocb_lock. Wake up CB and GP kthreads if necessary. Disable    |
+ *  |   bypass enqueue.                                                        |
  *  ----------------------------------------------------------------------------
  *                                      |
  *                                      v

kernel/rcu/tree_plugin.h +12 -1

index 732942a15f2b..7781830a3cf1 100644
@@ -1825,11 +1825,22 @@ static bool rcu_nocb_try_bypass(struct rcu_data *rdp, struct rcu_head *rhp,
 	unsigned long j = jiffies;
 	long ncbs = rcu_cblist_n_cbs(&rdp->nocb_bypass);
 
+	lockdep_assert_irqs_disabled();
+
+	// Pure softirq/rcuc based processing: no bypassing, no
+	// locking.
 	if (!rcu_rdp_is_offloaded(rdp)) {
+		*was_alldone = !rcu_segcblist_pend_cbs(&rdp->cblist);
+		return false;
+	}
+
+	// In the process of (de-)offloading: no bypassing, but
+	// locking.
+	if (!rcu_segcblist_completely_offloaded(&rdp->cblist)) {
+		rcu_nocb_lock(rdp);
 		*was_alldone = !rcu_segcblist_pend_cbs(&rdp->cblist);
 		return false; /* Not offloaded, no bypassing. */
 	}
-	lockdep_assert_irqs_disabled();
 
 	// Don't use ->nocb_bypass during early boot.
 	if (rcu_scheduler_active != RCU_SCHEDULER_RUNNING) {

kernel/rcu/tree_plugin.h +0 -0

 	ret = rdp_offload_toggle(rdp, false, flags);
 	swait_event_exclusive(rdp->nocb_state_wq,
 			      !rcu_segcblist_test_flags(cblist, SEGCBLIST_KTHREAD_CB |

@@ -2422,19 +2434,22 @@ static long rcu_nocb_rdp_deoffload(void *arg)
 	rcu_nocb_unlock_irqrestore(rdp, flags);

kernel/rcu/tree_plugin.h +1 -3

 	/*
 	 * With SEGCBLIST_SOFTIRQ_ONLY, we can't use
-	 * rcu_nocb_unlock_irqrestore() anymore. Theoretically we
-	 * could set SEGCBLIST_SOFTIRQ_ONLY with cb unlocked and IRQs
-	 * disabled now, but let's be paranoid.
+	 * rcu_nocb_unlock_irqrestore() anymore.
 	 */
 	raw_spin_unlock_irqrestore(&rdp->nocb_lock, flags);
 

Signature · 1 line

2.25.1

Quoted 5 lines · Frederic Weisbecker

---
 include/linux/rcu_segcblist.h |  7 ++++---
 kernel/rcu/tree_plugin.h      | 31 +++++++++++++++++++++++--------
 2 files changed, 27 insertions(+), 11 deletions(-)

include/linux/rcu_segcblist.h struct rcu_cblist { · line 112

Quoted 10 lines

include/linux/rcu_segcblist.h +1 -1

index 8afe886e85f1..5a2d6dadd720 100644
@@ -109,7 +109,7 @@ struct rcu_cblist {
  *  |                           SEGCBLIST_KTHREAD_GP                           |
  *  |                                                                          |
  *  |   Kthreads handle callbacks holding nocb_lock, local rcu_core() stops    |
- *  |   handling callbacks.                                                    |
+ *  |   handling callbacks. Allow bypass enqueue.                              |

"Allow bypass enqueue" as in bypass was disabled and entering this state causes it to be enabled, correct?

If so, "Enable bypass
queueing" would be less ambiguous and would match the change below.

Quoted 4 lines

include/linux/rcu_segcblist.h +0 -0

@@ -2412,6 +2423,7 @@ static long rcu_nocb_rdp_deoffload(void *arg)
 
 	rcu_nocb_lock_irqsave(rdp, flags);
 

include/linux/rcu_segcblist.h static long rcu_nocb_rdp_deoffload(void *arg) · line 2426

Quoted 1 line

include/linux/rcu_segcblist.h +1 -0

+	WARN_ON_ONCE(!rcu_nocb_flush_bypass(rdp, NULL, jiffies));

This flush suffices because we are running on the target CPU holding ->nocb_lock (thus having interrupts disabled), and because rdp_offload_toggle() invokes rcu_segcblist_offload(), which clears SEGCBLIST_OFFLOADED. Thus future calls to rcu_segcblist_completely_offloaded() will return false, which means that future calls to rcu_nocb_try_bypass() will refuse to put anything into the bypass.

I believe that this deserves a comment, particularly if I am confused about what is really happening here. ;-)

On another topic, since I saw it along the way...

The header comment for rcu_segcblist_offload() says that the structure must be empty, but that isn't really the case, is it?

Quoted 8 lines

include/linux/rcu_segcblist.h +0 -0

 	ret = rdp_offload_toggle(rdp, false, flags);
 	swait_event_exclusive(rdp->nocb_state_wq,
 			      !rcu_segcblist_test_flags(cblist, SEGCBLIST_KTHREAD_CB |

@@ -2422,19 +2434,22 @@ static long rcu_nocb_rdp_deoffload(void *arg)
 	rcu_nocb_unlock_irqrestore(rdp, flags);
 	del_timer_sync(&rdp->nocb_timer);
 

include/linux/rcu_segcblist.h static long rcu_nocb_rdp_deoffload(void *arg) · line 2441

Quoted 8 lines

include/linux/rcu_segcblist.h +4 -3

+	/* Sanity check */
+	WARN_ON_ONCE(rcu_cblist_n_cbs(&rdp->nocb_bypass));
+
 	/*
-	 * Flush bypass. While IRQs are disabled and once we set
-	 * SEGCBLIST_SOFTIRQ_ONLY, no callback is supposed to be
-	 * enqueued on bypass.
+	 * Lock one last time so we see latest updates from kthreads and timer

You lost me here. What updates are we seeing from kthreads and timers?

include/linux/rcu_segcblist.h static long rcu_nocb_rdp_deoffload(void *arg) · line 2449

Quoted 10 lines

include/linux/rcu_segcblist.h +5 -1

+	 * so that we can later run callbacks locally without the lock.
 	 */
 	rcu_nocb_lock_irqsave(rdp, flags);
-	rcu_nocb_flush_bypass(rdp, NULL, jiffies);
+	/*
+	 * Theoretically we could set SEGCBLIST_SOFTIRQ_ONLY after the nocb
+	 * LOCK/UNLOCK but let's be paranoid.
+	 */
 	rcu_segcblist_set_flags(cblist, SEGCBLIST_SOFTIRQ_ONLY);

As long as we are being paranoid, should we also check that the bypass remains empty?

Quoted 13 lines

include/linux/rcu_segcblist.h +1 -3

 	/*
 	 * With SEGCBLIST_SOFTIRQ_ONLY, we can't use
-	 * rcu_nocb_unlock_irqrestore() anymore. Theoretically we
-	 * could set SEGCBLIST_SOFTIRQ_ONLY with cb unlocked and IRQs
-	 * disabled now, but let's be paranoid.
+	 * rcu_nocb_unlock_irqrestore() anymore.
 	 */
 	raw_spin_unlock_irqrestore(&rdp->nocb_lock, flags);
 

Signature · 1 line

2.25.1

Quoted 6 lines · Paul E. McKenney

Quoted 5 lines · Frederic Weisbecker

---
 include/linux/rcu_segcblist.h |  7 ++++---
 kernel/rcu/tree_plugin.h      | 31 +++++++++++++++++++++++--------
 2 files changed, 27 insertions(+), 11 deletions(-)

Quoted 13 lines

include/linux/rcu_segcblist.h struct rcu_cblist { · line 112

Quoted 10 lines

include/linux/rcu_segcblist.h +1 -1

index 8afe886e85f1..5a2d6dadd720 100644
@@ -109,7 +109,7 @@ struct rcu_cblist {
  *  |                           SEGCBLIST_KTHREAD_GP                           |
  *  |                                                                          |
  *  |   Kthreads handle callbacks holding nocb_lock, local rcu_core() stops    |
- *  |   handling callbacks.                                                    |
+ *  |   handling callbacks. Allow bypass enqueue.                              |

"Allow bypass enqueue" as in bypass was disabled and entering this state causes it to be enabled, correct?

Right.

Quoted 2 lines

If so, "Enable bypass
queueing" would be less ambiguous and would match the change below.

Ok I'll fix that.

Quoted 4 lines

Quoted 4 lines

include/linux/rcu_segcblist.h +0 -0

@@ -2412,6 +2423,7 @@ static long rcu_nocb_rdp_deoffload(void *arg)
 
 	rcu_nocb_lock_irqsave(rdp, flags);
 

Quoted 9 lines

include/linux/rcu_segcblist.h static long rcu_nocb_rdp_deoffload(void *arg) · line 2426

Quoted 1 line

include/linux/rcu_segcblist.h +1 -0

+	WARN_ON_ONCE(!rcu_nocb_flush_bypass(rdp, NULL, jiffies));

This flush suffices because we are running on the target CPU holding ->nocb_lock (thus having interrupts disabled), and because rdp_offload_toggle() invokes rcu_segcblist_offload(), which clears SEGCBLIST_OFFLOADED. Thus future calls to rcu_segcblist_completely_offloaded() will return false, which means that future calls to rcu_nocb_try_bypass() will refuse to put anything into the bypass.

Exactly!

Quoted 2 lines

I believe that this deserves a comment, particularly if I am confused about what is really happening here. ;-)

Yes indeed I've been greedy there, will comment this :o)

Quoted 4 lines

On another topic, since I saw it along the way...

The header comment for rcu_segcblist_offload() says that the structure must be empty, but that isn't really the case, is it?

Ah strange indeed, must be a leftover. I'll remove it.

Quoted 7 lines

Quoted 7 lines

include/linux/rcu_segcblist.h +0 -0

 	ret = rdp_offload_toggle(rdp, false, flags);
 	swait_event_exclusive(rdp->nocb_state_wq,
 			      !rcu_segcblist_test_flags(cblist, SEGCBLIST_KTHREAD_CB |

@@ -2422,19 +2434,22 @@ static long rcu_nocb_rdp_deoffload(void *arg)
 	rcu_nocb_unlock_irqrestore(rdp, flags);
 	del_timer_sync(&rdp->nocb_timer);
 

Quoted 10 lines

include/linux/rcu_segcblist.h static long rcu_nocb_rdp_deoffload(void *arg) · line 2441

Quoted 8 lines

include/linux/rcu_segcblist.h +4 -3

+	/* Sanity check */
+	WARN_ON_ONCE(rcu_cblist_n_cbs(&rdp->nocb_bypass));
+
 	/*
-	 * Flush bypass. While IRQs are disabled and once we set
-	 * SEGCBLIST_SOFTIRQ_ONLY, no callback is supposed to be
-	 * enqueued on bypass.
+	 * Lock one last time so we see latest updates from kthreads and timer

You lost me here. What updates are we seeing from kthreads and timers?

We want to make sure that, whatever change has been made on the segcblist by kthreads such as length or callbacks dequeue, this is visible on the current CPU. The swait_event_exclusive() doesn't guarantee that everything from the kthreads is visible here as the flags are checked lockless and I haven't added specific barriers.

That said right after the swait_event there is a nocb_lock LOCK/UNLOCK cycle to disable the timer, so that's enough for the local CPU to see those updates. What remains is the updates made by pending timers flushed in del_timer_sync(). There is nothing special there to be visible here but out of paranoia...

In fact this matters later in the series as the above timer disablement and flush will disappear and the LOCK/UNLOCK cycle that comes along.

Quoted 4 lines

Quoted 4 lines

include/linux/rcu_segcblist.h +1 -1

+	 * so that we can later run callbacks locally without the lock.
 	 */
 	rcu_nocb_lock_irqsave(rdp, flags);
-	rcu_nocb_flush_bypass(rdp, NULL, jiffies);

Quoted 8 lines

include/linux/rcu_segcblist.h static long rcu_nocb_rdp_deoffload(void *arg) · line 2449

Quoted 5 lines

include/linux/rcu_segcblist.h +4 -0

+	/*
+	 * Theoretically we could set SEGCBLIST_SOFTIRQ_ONLY after the nocb
+	 * LOCK/UNLOCK but let's be paranoid.
+	 */
 	rcu_segcblist_set_flags(cblist, SEGCBLIST_SOFTIRQ_ONLY);

As long as we are being paranoid, should we also check that the bypass remains empty?

You missed it, check above for sanity check :-)

Thanks.

Quoted 13 lines

Quoted 13 lines

include/linux/rcu_segcblist.h +1 -3

 	/*
 	 * With SEGCBLIST_SOFTIRQ_ONLY, we can't use
-	 * rcu_nocb_unlock_irqrestore() anymore. Theoretically we
-	 * could set SEGCBLIST_SOFTIRQ_ONLY with cb unlocked and IRQs
-	 * disabled now, but let's be paranoid.
+	 * rcu_nocb_unlock_irqrestore() anymore.
 	 */
 	raw_spin_unlock_irqrestore(&rdp->nocb_lock, flags);
 

Signature · 1 line

2.25.1

rdp->nocb_cb_sleep is first set to true by default after processing the callbacks then set back to false if we still find ready callbacks to invoke.

This is confusing and even unsafe if it ever happens to be read locklessly at some point. So make sure we write it only once per nocb_cb_wait() loop.

Reported-by Paul E. McKenney <[email protected]>

Cc Josh Triplett <[email protected]>

Cc Lai Jiangshan <[email protected]>

Cc Joel Fernandes <[email protected]>

Cc Neeraj Upadhyay <[email protected]>

Cc Boqun Feng <[email protected]>

Signed-off-by Frederic Weisbecker <[email protected]>

Unconfuse a bit the name of this function which suggests returning true when the state is updated. It actually returns true when the rdp is in the process of deoffloading and we must ignore it.

Reported-by Paul E. McKenney <[email protected]>

Cc Josh Triplett <[email protected]>

Cc Lai Jiangshan <[email protected]>

Cc Joel Fernandes <[email protected]>

Cc Neeraj Upadhyay <[email protected]>

Cc Boqun Feng <[email protected]>

Signed-off-by Frederic Weisbecker <[email protected]>

Those tracing calls don't need to be under the nocb lock. Move them outside.

Signed-off-by Frederic Weisbecker <[email protected]>

Cc Josh Triplett <[email protected]>

Cc Lai Jiangshan <[email protected]>

Cc Joel Fernandes <[email protected]>

Cc Neeraj Upadhyay <[email protected]>

Cc Boqun Feng <[email protected]>

---
 kernel/rcu/tree_plugin.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

kernel/rcu/tree_plugin.h +3 -3

index 86c3bcceede6..8c5fea58ee80 100644
@@ -1700,9 +1700,9 @@ static bool wake_nocb_gp(struct rcu_data *rdp, bool force,
 
 	lockdep_assert_held(&rdp->nocb_lock);
 	if (!READ_ONCE(rdp_gp->nocb_gp_kthread)) {
+		rcu_nocb_unlock_irqrestore(rdp, flags);
 		trace_rcu_nocb_wake(rcu_state.name, rdp->cpu,
 				    TPS("AlreadyAwake"));
-		rcu_nocb_unlock_irqrestore(rdp, flags);
 		return false;
 	}
 

@@ -1950,9 +1950,9 @@ static void __call_rcu_nocb_wake(struct rcu_data *rdp, bool was_alldone,
 	// If we are being polled or there is no kthread, just leave.
 	t = READ_ONCE(rdp->nocb_gp_kthread);
 	if (rcu_nocb_poll || !t) {
+		rcu_nocb_unlock_irqrestore(rdp, flags);
 		trace_rcu_nocb_wake(rcu_state.name, rdp->cpu,
 				    TPS("WakeNotPoll"));
-		rcu_nocb_unlock_irqrestore(rdp, flags);
 		return;
 	}
 	// Need to actually to a wakeup.

@@ -1987,8 +1987,8 @@ static void __call_rcu_nocb_wake(struct rcu_data *rdp, bool was_alldone,
 					   TPS("WakeOvfIsDeferred"));
 		rcu_nocb_unlock_irqrestore(rdp, flags);
 	} else {
+		rcu_nocb_unlock_irqrestore(rdp, flags);
 		trace_rcu_nocb_wake(rcu_state.name, rdp->cpu, TPS("WakeNot"));
-		rcu_nocb_unlock_irqrestore(rdp, flags);
 	}
 	return;
 }

Signature · 1 line

2.25.1