Re: [PATCH] mm/mmu_notifier: Remove non_block_start/end() from notifier invocation

On Wed, 2026-08-12 at 00:50 +0100, David Woodhouse wrote:
> On Tue, 2026-08-11 at 15:58 -0700, Sean Christopherson wrote:
> >  SRCU is
> > fantastic infrastructure, but I genuinely think it's not the right fit here,
> > especially since we'll likely need a different solution for in-guest readers
> > anyways.

Actually I think it fits quite nicely into the GPC SRCU model. We add a
GUEST_USING_GPC flag to the gpc->state atomic. A vCPU which is going to
use the GPC in guest mode will do an atomic cmpxchg to set
GUEST_USING_GPC while the GPC_VALID flag is still set.

And then in the flush side, *exactly* the same loop which already sets
'cleared' when a cache has GPC_VALID, will also spot that
GUEST_USING_GPC and add the corresponding vCPU to the bitmask of vCPUs
to be kicked.

It's a perfect fit.

In fact, we could observe that the GUEST_USES_GPC concept has *always*
been closer to the RCU model of "readers do as they like; the writer
has to wait until they're done".

It was mixing that with rwlock that was odd.

> The cases where we have to hold the host physical address *during* the
> time we enter guest mode to run a vCPU were never covered by the rwlock
> either.
> 
> Fred's patches which reinstate that GUEST_USES_GPC mode are kicking the
> vCPU from the invalidate_start handler, just as the original
> implementation did. However the locking (or RCU) for the in-kernel
> users ends up changing, I don't see the in-guest part changing.
> 
> Unless you're imagining some scheme which could handle both? But the
> in-kernel readers are *brief* and will always go away on their own,
> while the in-guest needs to be kicked; they are opposites.

Honestly, if we're going to start re-inventing concurrency primitives
just because "SRCU *can* be used badly", even though we weren't *going*
to use SRCU badly in this case... maybe I should revisit my starting
assumption when I first started digging into this, which was that
"nobody wants a raw_rwlock_t as a workaround to the RT issues".

And if we want to invent new primitives, because that's *always* a
barrel of laughs, maybe we *could* try to build something really
dedicated to this use case...

Since every access is at least tenuously associated with *a* vCPU, we
could have a spinlock per vCPU and each read side takes "its own" while
the flush side takes all of them in turn, one at a time. That probably
only needs one such spinlock per vCPU (not per-GPC per-vCPU).

But maybe we can handle the GUEST_USES_GPC mode too... instead of a
spinlock, have an atomic_t of the number of readers, with a high bit
indicating that it's in-guest.... but honestly, I just don't want to.

SRCU is *fine* here. Let's not overcomplicate things.

> > > I get it. RCU is a versatile tool, and in *some* cases the *Sleepable*
> > > variant of RCU gets used in ways which would cause grace period latencies
> > > which are thoroughly unacceptable in an MMU notifier callback.
> > > 
> > > But in the specific case of a dedicated kvm->gpc_srcu whose only
> > > readers are tiny snippets of code that until today were always run with
> > > rwlock_irqsave? That is *not* Jason's worst-case bugbear. It's fine.
> > 
> > I agree it's more than likely fine for your use case, I'm not convinced it'll be
> > fine for use cases that heavily overcommit memory, or do NUMA balancing, or (close
> > your eyes) enable KSM.
> > 
> > E.g. it doesn't take all that much for even a single expedited sync to generate
> > a delay of 20ms.
> > https://lore.kernel.org/all/[email protected]
> 
> That looks to me like a pathological case where KVM is trying to use a
> single srcu_struct in two different modes, and getting sad when they
> conflict. That would never be the case for the separate gpc_srcu.
> 
> I've been playing with the soak tests, running in parallel with a load
> of about 140, with KASAN/lockdep/etc enabled.
> 
> I also implemented that thing I suggested: a try_synchronize_srcu()
> which doesn't even use the waitqueue when it can trivially determine
> that it doesn't need to wait.
> 
> Even when we're stressing invalidation vs readers, it ends up taking
> the fast path in about 99% of cases, taking around 4-16µs for those;
> 32-128µs when it does need to call srcu_synchronize_expedited().
> 
> It goes above 8ms in 4.4ppm of cases, with the p100 outlier being
> 17.6ms. My gut tells me we'd see those occasional outliers even with
> the rwlock at this load, but I'm going to kick that test off and answer
> that definitively in the morning.

That one crashed (reminding us that I was actually doing this to fix
*bugs* not just chase the last bits of performance). A rerun this
morning shows a p100 latency of ~11ms for the rwlock case too.

I redid the tests to also track the costs of the *other* thing that 
the KVM mmu_notifier does; poking pages out of the EPT and kicking
vCPUs. The p100 there was only about 2½ms for both EPT and shadow
paging mode. I wasn't able to test with ATS+PASID capable devices,
which I suspect would have led to larger numbers :)

Really, I just don't think Jason's tail-latency bugbear is actually
anything to be scared of. It's not new, and it's not dramatically
different.