mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH rcu 0/4] Callback-offloading updates for v6.5
@ 2023-05-10 17:06 Paul E. McKenney
  2023-05-10 17:06 ` [PATCH rcu 1/4] rcu/nocb: Protect lazy shrinker against concurrent (de-)offloading Paul E. McKenney
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Paul E. McKenney @ 2023-05-10 17:06 UTC (permalink / raw)
  To: rcu; +Cc: linux-kernel, kernel-team, rostedt

Hello!

This series provides some nocb callback-offloading updates:

1.	rcu/nocb: Protect lazy shrinker against concurrent
	(de-)offloading, courtesy of Frederic Weisbecker.

2.	rcu/nocb: Fix shrinker race against callback enqueuer, courtesy
	of Frederic Weisbecker.

3.	rcu/nocb: Recheck lazy callbacks under the ->nocb_lock from
	shrinker, courtesy of Frederic Weisbecker.

4.	rcu/nocb: Make shrinker to iterate only NOCB CPUs, courtesy of
	Frederic Weisbecker.

						Thanx, Paul

------------------------------------------------------------------------

 b/kernel/rcu/tree_nocb.h |   25 ++++++++++++++++++++++++-
 kernel/rcu/tree_nocb.h   |   33 ++++++++++++++++++++++++++-------
 2 files changed, 50 insertions(+), 8 deletions(-)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH rcu 1/4] rcu/nocb: Protect lazy shrinker against concurrent (de-)offloading
  2023-05-10 17:06 [PATCH rcu 0/4] Callback-offloading updates for v6.5 Paul E. McKenney
@ 2023-05-10 17:06 ` Paul E. McKenney
  2023-05-10 17:06 ` [PATCH rcu 2/4] rcu/nocb: Fix shrinker race against callback enqueuer Paul E. McKenney
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Paul E. McKenney @ 2023-05-10 17:06 UTC (permalink / raw)
  To: rcu
  Cc: linux-kernel, kernel-team, rostedt, Frederic Weisbecker,
	Paul E . McKenney

From: Frederic Weisbecker <frederic@kernel.org>

The shrinker may run concurrently with callbacks (de-)offloading. As
such, calling rcu_nocb_lock() is very dangerous because it does a
conditional locking. The worst outcome is that rcu_nocb_lock() doesn't
lock but rcu_nocb_unlock() eventually unlocks, or the reverse, creating
an imbalance.

Fix this with protecting against (de-)offloading using the barrier mutex.
Although if the barrier mutex is contended, which should be rare, then
step aside so as not to trigger a mutex VS allocation
dependency chain.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
---
 kernel/rcu/tree_nocb.h | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
index f2280616f9d5..1a86883902ce 100644
--- a/kernel/rcu/tree_nocb.h
+++ b/kernel/rcu/tree_nocb.h
@@ -1336,13 +1336,33 @@ lazy_rcu_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)
 	unsigned long flags;
 	unsigned long count = 0;
 
+	/*
+	 * Protect against concurrent (de-)offloading. Otherwise nocb locking
+	 * may be ignored or imbalanced.
+	 */
+	if (!mutex_trylock(&rcu_state.barrier_mutex)) {
+		/*
+		 * But really don't insist if barrier_mutex is contended since we
+		 * can't guarantee that it will never engage in a dependency
+		 * chain involving memory allocation. The lock is seldom contended
+		 * anyway.
+		 */
+		return 0;
+	}
+
 	/* Snapshot count of all CPUs */
 	for_each_possible_cpu(cpu) {
 		struct rcu_data *rdp = per_cpu_ptr(&rcu_data, cpu);
-		int _count = READ_ONCE(rdp->lazy_len);
+		int _count;
+
+		if (!rcu_rdp_is_offloaded(rdp))
+			continue;
+
+		_count = READ_ONCE(rdp->lazy_len);
 
 		if (_count == 0)
 			continue;
+
 		rcu_nocb_lock_irqsave(rdp, flags);
 		WRITE_ONCE(rdp->lazy_len, 0);
 		rcu_nocb_unlock_irqrestore(rdp, flags);
@@ -1352,6 +1372,9 @@ lazy_rcu_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)
 		if (sc->nr_to_scan <= 0)
 			break;
 	}
+
+	mutex_unlock(&rcu_state.barrier_mutex);
+
 	return count ? count : SHRINK_STOP;
 }
 
-- 
2.40.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH rcu 2/4] rcu/nocb: Fix shrinker race against callback enqueuer
  2023-05-10 17:06 [PATCH rcu 0/4] Callback-offloading updates for v6.5 Paul E. McKenney
  2023-05-10 17:06 ` [PATCH rcu 1/4] rcu/nocb: Protect lazy shrinker against concurrent (de-)offloading Paul E. McKenney
@ 2023-05-10 17:06 ` Paul E. McKenney
  2023-05-10 17:06 ` [PATCH rcu 3/4] rcu/nocb: Recheck lazy callbacks under the ->nocb_lock from shrinker Paul E. McKenney
  2023-05-10 17:06 ` [PATCH rcu 4/4] rcu/nocb: Make shrinker to iterate only NOCB CPUs Paul E. McKenney
  3 siblings, 0 replies; 5+ messages in thread
From: Paul E. McKenney @ 2023-05-10 17:06 UTC (permalink / raw)
  To: rcu
  Cc: linux-kernel, kernel-team, rostedt, Frederic Weisbecker,
	Paul E . McKenney

From: Frederic Weisbecker <frederic@kernel.org>

The shrinker resets the lazy callbacks counter in order to trigger the
pending lazy queue flush though the rcuog kthread. The counter reset is
protected by the ->nocb_lock against concurrent accesses...except
for one of them. Here is a list of existing synchronized readers/writer:

1) The first lazy enqueuer (incrementing ->lazy_len to 1) does so under
   ->nocb_lock and ->nocb_bypass_lock.

2) The further lazy enqueuers (incrementing ->lazy_len above 1) do so
   under ->nocb_bypass_lock _only_.

3) The lazy flush checks and resets to 0 under ->nocb_lock and
	->nocb_bypass_lock.

The shrinker protects its ->lazy_len reset against cases 1) and 3) but
not against 2). As such, setting ->lazy_len to 0 under the ->nocb_lock
may be cancelled right away by an overwrite from an enqueuer, leading
rcuog to ignore the flush.

To avoid that, use the proper bypass flush API which takes care of all
those details.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
---
 kernel/rcu/tree_nocb.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
index 1a86883902ce..c321fce2af8e 100644
--- a/kernel/rcu/tree_nocb.h
+++ b/kernel/rcu/tree_nocb.h
@@ -1364,7 +1364,7 @@ lazy_rcu_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)
 			continue;
 
 		rcu_nocb_lock_irqsave(rdp, flags);
-		WRITE_ONCE(rdp->lazy_len, 0);
+		WARN_ON_ONCE(!rcu_nocb_flush_bypass(rdp, NULL, jiffies, false));
 		rcu_nocb_unlock_irqrestore(rdp, flags);
 		wake_nocb_gp(rdp, false);
 		sc->nr_to_scan -= _count;
-- 
2.40.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH rcu 3/4] rcu/nocb: Recheck lazy callbacks under the ->nocb_lock from shrinker
  2023-05-10 17:06 [PATCH rcu 0/4] Callback-offloading updates for v6.5 Paul E. McKenney
  2023-05-10 17:06 ` [PATCH rcu 1/4] rcu/nocb: Protect lazy shrinker against concurrent (de-)offloading Paul E. McKenney
  2023-05-10 17:06 ` [PATCH rcu 2/4] rcu/nocb: Fix shrinker race against callback enqueuer Paul E. McKenney
@ 2023-05-10 17:06 ` Paul E. McKenney
  2023-05-10 17:06 ` [PATCH rcu 4/4] rcu/nocb: Make shrinker to iterate only NOCB CPUs Paul E. McKenney
  3 siblings, 0 replies; 5+ messages in thread
From: Paul E. McKenney @ 2023-05-10 17:06 UTC (permalink / raw)
  To: rcu
  Cc: linux-kernel, kernel-team, rostedt, Frederic Weisbecker,
	Paul E . McKenney

From: Frederic Weisbecker <frederic@kernel.org>

The ->lazy_len is only checked locklessly. Recheck again under the
->nocb_lock to avoid spending more time on flushing/waking if not
necessary. The ->lazy_len can still increment concurrently (from 1 to
infinity) but under the ->nocb_lock we at least know for sure if there
are lazy callbacks at all (->lazy_len > 0).

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
---
 kernel/rcu/tree_nocb.h | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
index c321fce2af8e..dfa9c10d6727 100644
--- a/kernel/rcu/tree_nocb.h
+++ b/kernel/rcu/tree_nocb.h
@@ -1358,12 +1358,20 @@ lazy_rcu_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)
 		if (!rcu_rdp_is_offloaded(rdp))
 			continue;
 
-		_count = READ_ONCE(rdp->lazy_len);
-
-		if (_count == 0)
+		if (!READ_ONCE(rdp->lazy_len))
 			continue;
 
 		rcu_nocb_lock_irqsave(rdp, flags);
+		/*
+		 * Recheck under the nocb lock. Since we are not holding the bypass
+		 * lock we may still race with increments from the enqueuer but still
+		 * we know for sure if there is at least one lazy callback.
+		 */
+		_count = READ_ONCE(rdp->lazy_len);
+		if (!_count) {
+			rcu_nocb_unlock_irqrestore(rdp, flags);
+			continue;
+		}
 		WARN_ON_ONCE(!rcu_nocb_flush_bypass(rdp, NULL, jiffies, false));
 		rcu_nocb_unlock_irqrestore(rdp, flags);
 		wake_nocb_gp(rdp, false);
-- 
2.40.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH rcu 4/4] rcu/nocb: Make shrinker to iterate only NOCB CPUs
  2023-05-10 17:06 [PATCH rcu 0/4] Callback-offloading updates for v6.5 Paul E. McKenney
                   ` (2 preceding siblings ...)
  2023-05-10 17:06 ` [PATCH rcu 3/4] rcu/nocb: Recheck lazy callbacks under the ->nocb_lock from shrinker Paul E. McKenney
@ 2023-05-10 17:06 ` Paul E. McKenney
  3 siblings, 0 replies; 5+ messages in thread
From: Paul E. McKenney @ 2023-05-10 17:06 UTC (permalink / raw)
  To: rcu
  Cc: linux-kernel, kernel-team, rostedt, Frederic Weisbecker,
	Paul E . McKenney

From: Frederic Weisbecker <frederic@kernel.org>

Callbacks can only be queued as lazy on NOCB CPUs, therefore iterating
over the NOCB mask is enough for both counting and scanning. Just lock
the mostly uncontended barrier mutex on counting as well in order to
keep rcu_nocb_mask stable.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Signed-off-by: Paul E. McKenney <paulmck@kernel.org>
---
 kernel/rcu/tree_nocb.h | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h
index dfa9c10d6727..43229d2b0c44 100644
--- a/kernel/rcu/tree_nocb.h
+++ b/kernel/rcu/tree_nocb.h
@@ -1319,13 +1319,22 @@ lazy_rcu_shrink_count(struct shrinker *shrink, struct shrink_control *sc)
 	int cpu;
 	unsigned long count = 0;
 
+	if (WARN_ON_ONCE(!cpumask_available(rcu_nocb_mask)))
+		return 0;
+
+	/*  Protect rcu_nocb_mask against concurrent (de-)offloading. */
+	if (!mutex_trylock(&rcu_state.barrier_mutex))
+		return 0;
+
 	/* Snapshot count of all CPUs */
-	for_each_possible_cpu(cpu) {
+	for_each_cpu(cpu, rcu_nocb_mask) {
 		struct rcu_data *rdp = per_cpu_ptr(&rcu_data, cpu);
 
 		count +=  READ_ONCE(rdp->lazy_len);
 	}
 
+	mutex_unlock(&rcu_state.barrier_mutex);
+
 	return count ? count : SHRINK_EMPTY;
 }
 
@@ -1336,6 +1345,8 @@ lazy_rcu_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)
 	unsigned long flags;
 	unsigned long count = 0;
 
+	if (WARN_ON_ONCE(!cpumask_available(rcu_nocb_mask)))
+		return 0;
 	/*
 	 * Protect against concurrent (de-)offloading. Otherwise nocb locking
 	 * may be ignored or imbalanced.
@@ -1351,11 +1362,11 @@ lazy_rcu_shrink_scan(struct shrinker *shrink, struct shrink_control *sc)
 	}
 
 	/* Snapshot count of all CPUs */
-	for_each_possible_cpu(cpu) {
+	for_each_cpu(cpu, rcu_nocb_mask) {
 		struct rcu_data *rdp = per_cpu_ptr(&rcu_data, cpu);
 		int _count;
 
-		if (!rcu_rdp_is_offloaded(rdp))
+		if (WARN_ON_ONCE(!rcu_rdp_is_offloaded(rdp)))
 			continue;
 
 		if (!READ_ONCE(rdp->lazy_len))
-- 
2.40.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2023-05-10 17:06 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-10 17:06 [PATCH rcu 0/4] Callback-offloading updates for v6.5 Paul E. McKenney
2023-05-10 17:06 ` [PATCH rcu 1/4] rcu/nocb: Protect lazy shrinker against concurrent (de-)offloading Paul E. McKenney
2023-05-10 17:06 ` [PATCH rcu 2/4] rcu/nocb: Fix shrinker race against callback enqueuer Paul E. McKenney
2023-05-10 17:06 ` [PATCH rcu 3/4] rcu/nocb: Recheck lazy callbacks under the ->nocb_lock from shrinker Paul E. McKenney
2023-05-10 17:06 ` [PATCH rcu 4/4] rcu/nocb: Make shrinker to iterate only NOCB CPUs Paul E. McKenney

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®