Re: [PATCH] mm/mmu_notifier: Remove non_block_start/end() from notifier invocation
David Woodhouse <[email protected]>
| Newsgroups | dev.linux.lists.linux-rt-devel,org.kernel.vger.kvm,org.kernel.vger.linux-kernel,org.kvack.linux-mm |
|---|---|
| Message-ID | <[email protected]> |
On Tue, 2026-08-11 at 15:58 -0700, Sean Christopherson wrote: > On Tue, Aug 11, 2026, David Woodhouse wrote: > > On Tue, 2026-08-11 at 22:21 +0200, Paolo Bonzini wrote: > > > > > > for_each_present_cpu(cpu) > > > if (per_cpu(kvm_gpc_reader, cpu) == gpc) > > > cpumask_set_cpu(cpu, gpc_readers); > > > __kvm_kick_many_cpus(cpus, wait); > > > > > > I'm literally sitting here watching you reinvent what SRCU already does. > > The problem, for me at least, is that SRCU does this and a _lot_ more. > > > Which, again, is *fine* given the behaviour of *these* read-side sections. > > Well, yeah. Why use a nailgun to put a nail into the wall to hang a picture > frame? (other than the indisputable fact that power tools are fun). It's not a nailgun, it's a drill. And in this analogy, my observation that it works absolutely fine for us with *these* SRCU read-side users ... is equivalent to finding the torque setting that lets you put it into screwdriver mode. Power tools *are* fun. But I try to avoid buying a new one when I already have one that does the job correctly with care. > 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. 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. > > 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.
smime.p7s
(application/pkcs7-signature, 6 KB) - not displayed