Re: [PATCH v3 3/7] KVM: pfncache: Use RCU for readers instead of a rwlock

On Sat, 2026-08-08 at 10:58 -0700, Paul E. McKenney wrote:
> By the way, good point on all the SRCU instances sharing a common
> set of workqueues.  More ways to deadlock!  But I don't see having
> per-srcu_struct sets of dedicated kthreads.  ;-)

Indeed. Although I did briefly go down the rabbit hole of whether a
*reader* sleeping in an allocation could compose into the same kind of
cycle.

Conclusion: only if something on the reclaim path synchronizes the
*same* srcu_struct that the reader holds — cross-domain it's only
latency, since the GP state machine polls and requeues rather than
capturing a worker. Which becomes a design rule for GPC usage:
never allocate under srcu_read_lock(&kvm->gpc_srcu), because our
invalidator *is* on the reaper path. But that's OK because allocating
inside the existing GPC rwlock is already verboten.

> True, but shouldn't we take as much pressure off of the spare as we can
> so that it will be there for us when we really need it.
[...]
> Why not do a "GFP_NOWAIT | __GFP_NOWARN" attempt before raiding
> srcu_spare_nodes?  Wouldn't that increase the probability that there
> would be an srcu_node array available when someone really needed it?

Makes sense. Done that way below: the GFP_NOWAIT attempt comes first,
so in the common no-pressure case the spare is never touched and is
guaranteed present under the memory pressure it exists for. That also
makes the replenish latency mostly moot — it only matters after an
allocation has already failed under pressure, and nothing ever waits on
it.

> Mightn't !try_cmpxchg() be a better fit here?  You are using the returned
> pointer as a boolean anyway.  (One could also argue for xchg(), but why
> unnecessarily write to that poor cache line?)

Also done, plus a check of srcu_spare_nodes before the kzalloc as you
suggested — the collision is indeed low-probability, but the check is
free.

> And the across-SRCU shared-workqueue deadlock that you pointed out is
> avoided because the only way that gfp_flags is set to GFP_KERNEL is when
> the caller is supplying its own task, correct?

Right. After this patch the only GFP_KERNEL caller of
init_srcu_struct_nodes() is init_srcu_struct() in the caller's own
task, where blocking is permitted. srcu_gp_end() passes GFP_NOWAIT, so
nothing on the grace-period workqueue can ever block in reclaim.

In the meantime, testing found some issues in my original conversion of
the GPC code to RCU — dropping gpc->lock broke the atomicity of the
final invalidation check against the publish, and the teardown paths
could skip the grace period when an invalidation had already cleared
the valid flag — re-breaking the syzbot thing I only just fixed, but
for which thankfully I had a repro case right there ready to catch it
:)

Both reworked: the valid/becoming-valid state now lives in a single
atomic word, so the publish is a cmpxchg which an invalidation can
veto. (My old needs_invalidation flag back again!). That's now ~30
hours into a 48-hour KASAN+lockdep soak with no complaints, and syzbot
is chewing on it too.

Tree with all of that plus this SRCU preallocation patch on top:

https://git.infradead.org/?p=users/dwmw2/linux.git;a=shortlog;h=refs/heads/xen-rcu-srcu-prealloc

Patch below. Still only compile-tested — my metal test hosts are
over the big_cpu_lim threshold, so the lazy transition path this
changes never executes there; testing it properly wants a small guest
or big_cpu_lim= tweaking, which is on the list. But also it's a PITA to
actually *trigger* the OOM reaper path anyway, and I've not actually
managed it without hacking the kernel to introduce delays.

From: David Woodhouse <[email protected]>
Subject: [PATCH] srcu: Keep a spare node array so srcu_gp_end() need not block
 in reclaim

The one-time transition of an srcu_struct from SRCU_SIZE_SMALL to
SRCU_SIZE_BIG allocates the srcu_node combining tree with GFP_KERNEL
from srcu_gp_end(). That runs on the same workqueue which processes
grace periods for every srcu_struct in the system — including grace
periods awaited from OOM/reclaim contexts such as the OOM reaper
calling synchronize_srcu() via an mmu_notifier. If the allocation
blocks in direct reclaim, it can be waiting on the very OOM reaper
whose grace period is queued behind it: a deadlock.

The allocation is literally one size fits all: it depends only on
rcu_num_nodes, which is fixed once rcu_init_geometry() has run. So
keep a single preallocated spare array, primed in srcu_init() when
lazy (contention-triggered) sizing is in effect.

Allocation tries GFP_NOWAIT first, which in the common no-pressure
case succeeds and leaves the spare untouched, so that it is still
there when there really is pressure. Only when that fails is the
spare consumed (with xchg(), so double-consumption is impossible),
and the consumer kicks a replenish worker on system_wq — a clean
context where GFP_KERNEL is safe and nothing waits on the result.
The final fallback uses the caller's own flags: GFP_KERNEL only ever
from init_srcu_struct() in the caller's own task, where blocking is
permitted; srcu_gp_end() passes GFP_NOWAIT, preserving the guarantee
that the grace-period workqueue never blocks in reclaim.

Signed-off-by: David Woodhouse <[email protected]>
Assisted-by: Claude:claude-mythos-5
---
 kernel/rcu/srcutree.c | 84 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 81 insertions(+), 3 deletions(-)

diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
index 7c2f7cc131f7..23911fa71c64 100644
--- a/kernel/rcu/srcutree.c
+++ b/kernel/rcu/srcutree.c
@@ -123,6 +123,71 @@ static inline bool srcu_invl_snp_seq(unsigned long s)
 	return s == SRCU_SNP_INIT_SEQ;
 }
 
+/*
+ * A standing spare srcu_node array. The size of the allocation depends
+ * only on rcu_num_nodes, which is fixed once rcu_init_geometry() has run,
+ * so one preallocated array fits every srcu_struct in the system.
+ *
+ * This exists because srcu_gp_end() may need to allocate the array when
+ * a size transition is triggered by contention, and srcu_gp_end() runs
+ * on the same workqueue for every srcu_struct — including grace periods
+ * awaited from OOM/reclaim contexts (e.g. the OOM reaper via an
+ * mmu_notifier). Blocking there in GFP_KERNEL reclaim can deadlock: the
+ * reclaim may be waiting on the very OOM reaper whose grace period is
+ * queued behind this allocation.
+ *
+ * The allocation therefore tries GFP_NOWAIT first — which in the common
+ * no-pressure case succeeds and leaves the spare untouched — and raids
+ * the spare only when that fails, i.e. under the memory pressure the
+ * spare exists for. The spare is replenished from a clean context on
+ * system_wq. Nothing on the grace-period path ever blocks in reclaim.
+ */
+static struct srcu_node *srcu_spare_nodes;
+
+static void srcu_spare_replenish_wq(struct work_struct *work)
+{
+	struct srcu_node *spare, *expect = NULL;
+
+	if (READ_ONCE(srcu_spare_nodes))
+		return;		/* Already refilled. */
+
+	spare = kzalloc_objs(*spare, rcu_num_nodes, GFP_KERNEL);
+	if (!spare)
+		return;
+	if (!try_cmpxchg(&srcu_spare_nodes, &expect, spare))
+		kfree(spare);	/* Someone else refilled it first. */
+}
+static DECLARE_WORK(srcu_spare_replenish_work, srcu_spare_replenish_wq);
+
+static struct srcu_node *srcu_alloc_nodes(gfp_t gfp_flags)
+{
+	struct srcu_node *node;
+
+	/*
+	 * Try a non-blocking allocation first, leaving the spare untouched
+	 * in the common no-pressure case so that it is still there when
+	 * there really is pressure.
+	 */
+	node = kzalloc_objs(*node, rcu_num_nodes, GFP_NOWAIT | __GFP_NOWARN);
+	if (node)
+		return node;
+
+	node = xchg(&srcu_spare_nodes, NULL);
+	if (node) {
+		schedule_work(&srcu_spare_replenish_work);
+		return node;
+	}
+
+	/*
+	 * Spare already taken and not yet replenished. Fall back to the
+	 * caller's own flags: for init_srcu_struct() this is GFP_KERNEL in
+	 * the caller's own task, where blocking is permitted; from
+	 * srcu_gp_end() it is GFP_NOWAIT again, preserving the guarantee
+	 * that the grace-period workqueue never blocks in reclaim.
+	 */
+	return kzalloc_objs(*node, rcu_num_nodes, gfp_flags);
+}
+
 /*
  * Allocated and initialize SRCU combining tree.  Returns @true if
  * allocation succeeded and @false otherwise.
@@ -139,8 +204,7 @@ static bool init_srcu_struct_nodes(struct srcu_struct *ssp, gfp_t gfp_flags)
 
 	/* Initialize geometry if it has not already been initialized. */
 	rcu_init_geometry();
-	ssp->srcu_sup->node = kzalloc_objs(*ssp->srcu_sup->node, rcu_num_nodes,
-					   gfp_flags);
+	ssp->srcu_sup->node = srcu_alloc_nodes(gfp_flags);
 	if (!ssp->srcu_sup->node)
 		return false;
 
@@ -1004,7 +1068,7 @@ static void srcu_gp_end(struct srcu_struct *ssp)
 	/* Transition to big if needed. */
 	if (ss_state != SRCU_SIZE_SMALL && ss_state != SRCU_SIZE_BIG) {
 		if (ss_state == SRCU_SIZE_ALLOC)
-			init_srcu_struct_nodes(ssp, GFP_KERNEL);
+			init_srcu_struct_nodes(ssp, GFP_NOWAIT | __GFP_NOWARN);
 		else
 			smp_store_release(&sup->srcu_size_state, ss_state + 1);
 	}
@@ -2111,6 +2175,20 @@ void __init srcu_init(void)
 		}
 	}
 
+	/*
+	 * Prime the spare node array if lazy (contention-triggered) size
+	 * transitions are possible, so that srcu_gp_end() never needs to
+	 * allocate. Early-boot GFP_KERNEL is implicitly non-blocking
+	 * (gfp_allowed_mask strips __GFP_RECLAIM until much later), and
+	 * failure here is harmless: the GFP_NOWAIT fallback and replenish
+	 * worker remain.
+	 */
+	if (SRCU_SIZING_IS_CONTEND() || SRCU_SIZING_IS_TORTURE()) {
+		rcu_init_geometry();
+		srcu_spare_nodes = kzalloc_objs(*srcu_spare_nodes,
+						rcu_num_nodes, GFP_KERNEL);
+	}
+
 	/*
 	 * Once that is set, call_srcu() can follow the normal path and
 	 * queue delayed work. This must follow RCU workqueues creation
-- 
2.43.0