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

[Sean Christopherson <seanjc@google.com>]

On Tue, Aug 11, 2026, Jason Gunthorpe wrote:
> On Tue, Aug 11, 2026 at 06:59:24PM +0100, David Woodhouse wrote:
> > On 11 August 2026 18:26:27 BST, Jason Gunthorpe <jgg@ziepe.ca> wrote:
> > >On Tue, Aug 11, 2026 at 06:22:12PM +0100, David Woodhouse wrote:
> > >> On Tue, 2026-08-11 at 13:24 -0300, Jason Gunthorpe wrote:
> > >> > To be clear you should not be using any synchronize_[s]rcu() primitive
> > >> > inside the invalidation callbacks. These are well known to have
> > >> > multi-second delays on loaded systems which are a completely
> > >> > inappropriate performance characteristic for these mm callbacks.
> > >> > 
> > >> > This statement has nothing to do with deadlock.
> > >> > 
> > >> > RCU is always a trade off, you can make the read side run really fast
> > >> > and the write side is ghastly slow. If you can't handle the slow write
> > >> > you shouldn't use RCU techniques.
> > >> 
> > >> The multi-second horror stories are about the *global* RCU/SRCU
> > >> domains, where the grace period has to wait out arbitrary readers all
> > >> over the kernel.
> > >> 
> > >> This is not that. It is a dedicated srcu_struct, private to one VM,
> > >> and its entire reader population is a handful of KVM fast paths that
> > >> until now were under irqsave rwlocks.
> > >
> > >Are you sure? I've never heard that srcu has those kinds of properties.
> > >
> > >If its so fast you should just propose a non-sleeping version and
> > >leave the notifiers out of it
> >
> > I've got torture tests running for correctness on the GPC RCU
> > conversion. I'll throw in some metrics on how often even in that
> > pathological case we hit the wait case, and how long it actually
> > takes.
> 
> Well, to hit the bad RCU cases you need to usually do some other
> workload too..

Yeah, and we've had several (recent) examples of SRCU tail latencies causing
problems for KVM.

> I guess srcu does have some meaningful functional differences, but it
> is hardly guaranteed to be fast or non-sleeping out of the box.
> 
> I guess you are making an arugment that if SRCU critical sections are
> atomic themselves then the synchronize could also reasonably be
> atomic. That seems plausible, and may be worth some additional API
> surface on the SRCU side to expose this use model and drop the might
> sleep that is causing the trouble.
> 
> Some sort of "atomic RCU" that has a slower reader but a faster atomic
> writer.
> 
> I'm much happier to see a formal API under the notifiers that has
> strong properties of being reasonable than KVM using SRCU in a way
> that just happens to do that by accident, under the current
> implementation..

Agreed, I suspect shoving a synchronize_*rcu() of any kind in the mmu_notifier
invalidation path will come back to bite us, hard.

But I don't think we need an entirely new type of RCU for KVM.  Unlike (S)RCU,
KVM can and _must_ block relevant readers when an invalidation is in-flight.
I.e. the invalidation path doesn't need to ensure *all* readers go away, only
that the relevant readers have observed the invalidation.

The readers also don't need to be allowed to sleep; I suggested using SRCU instead
of RCU purely because the tail latencies for regular RCU are typically much, much
worse than SRCU (and I agree that they're bad for SRCU).

Earlier, David described KVM's GPCs as de facto software TLBs, and KVM already
has code to protect walks of what are effectively software TLBs, specifically
walk_shadow_page_lockless_{begin,end}() and the associated write-side handling
of READING_SHADOW_PAGE_TABLES in kvm_request_needs_ipi().

And looking to the future, if/when we use GPCs to track PFNs that are mapped into
the guest through control structures, i.e. not through page tables, we'll already
need to rely on kicking CPUs via IPI to ensure readers see the invalidation.

So rather than use (S)RCU, what if KVM tracks which CPUs are reading and then
blasts IPIs to complete the "TLB" shootdown?

The biggest wrinkle I can think of is that unlike READING_SHADOW_PAGE_TABLES,
there isn't a 1:1 association between vCPUs and CPUs, i.e. KVM can't walk its
array of vCPUs to see which CPUs need to be kicked.  But that should be easy enough
to solve with a cpumask.  Cache line contention might be a problem, but if so, it
seems like a solvable problem.

Very roughly and incomplete, relative to David's series to use SRCU:

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index ac961f4c91da..b4a7b613ad91 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1719,18 +1719,18 @@ static void kvm_setup_guest_pvclock(struct pvclock_vcpu_time_info *ref_hv_clock,
 {
 	struct pvclock_vcpu_time_info *guest_hv_clock;
 	struct pvclock_vcpu_time_info hv_clock;
-	int idx;
+	unsigned long flags;
 
 	memcpy(&hv_clock, ref_hv_clock, sizeof(hv_clock));
 
-	idx = srcu_read_lock(&vcpu->kvm->gpc_srcu);
+	flags = kvm_gpc_read_begin(vcpu->kvm);
 	while (!kvm_gpc_check(gpc, offset + sizeof(*guest_hv_clock))) {
-		srcu_read_unlock(&vcpu->kvm->gpc_srcu, idx);
+		kvm_gpc_read_end(vcpu->kvm, flags);
 
 		if (kvm_gpc_refresh(gpc, offset + sizeof(*guest_hv_clock)))
 			return;
 
-		idx = srcu_read_lock(&vcpu->kvm->gpc_srcu);
+		flags = kvm_gpc_read_begin(vcpu->kvm);
 	}
 
 	guest_hv_clock = (void *)(gpc->khva + offset);
@@ -1755,7 +1755,7 @@ static void kvm_setup_guest_pvclock(struct pvclock_vcpu_time_info *ref_hv_clock,
 	guest_hv_clock->version = ++hv_clock.version;
 
 	kvm_gpc_mark_dirty_in_slot(gpc);
-	srcu_read_unlock(&vcpu->kvm->gpc_srcu, idx);
+	kvm_gpc_read_end(vcpu->kvm, flags);
 
 	trace_kvm_pvclock_update(vcpu->vcpu_id, &hv_clock);
 }
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 7b2dbbd6b104..54ec1082c5ec 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -189,6 +189,8 @@ bool kvm_make_vcpus_request_mask(struct kvm *kvm, unsigned int req,
 				 unsigned long *vcpu_bitmap);
 bool kvm_make_all_cpus_request(struct kvm *kvm, unsigned int req);
 
+void kvm_kick_many_cpus(cpumask_var_t __cpus, bool wait);
+
 #define KVM_USERSPACE_IRQ_SOURCE_ID		0
 #define KVM_IRQFD_RESAMPLE_IRQ_SOURCE_ID	1
 #define KVM_PIT_IRQ_SOURCE_ID			2
@@ -814,7 +816,7 @@ struct kvm {
 	 * A dedicated domain (rather than kvm->srcu) keeps those waits from
 	 * being lengthened by unrelated memslot readers.
 	 */
-	struct srcu_struct gpc_srcu;
+	cpumask_var_t gpc_readers;
 
 	/*
 	 * created_vcpus is protected by kvm->lock, and is incremented
@@ -1569,6 +1571,20 @@ static inline bool kvm_gpc_is_hva_active(struct gfn_to_pfn_cache *gpc)
 	return gpc->active && kvm_is_error_gpa(gpc->gpa);
 }
 
+static inline unsigned long kvm_gpc_read_begin(struct kvm *kvm)
+{
+	unsigned long flags;
+
+	local_irq_save(flags);
+	cpumask_set_cpu(smp_processor_id(), kvm->gpc_readers);
+}
+
+static inline void kvm_gpc_read_end(struct kvm *kvm, unsigned long flags)
+{
+	cpumask_clear_cpu(smp_processor_id(), kvm->gpc_readers);
+	local_irq_restore(flags);
+}
+
 void kvm_sigset_activate(struct kvm_vcpu *vcpu);
 void kvm_sigset_deactivate(struct kvm_vcpu *vcpu);
 
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index c6e1c9c28b7e..9ef14057e477 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -205,7 +205,7 @@ static void ack_kick(void *_completed)
 {
 }
 
-static inline bool kvm_kick_many_cpus(struct cpumask *cpus, bool wait)
+static inline bool __kvm_kick_many_cpus(struct cpumask *cpus, bool wait)
 {
 	if (cpumask_empty(cpus))
 		return false;
@@ -214,6 +214,18 @@ static inline bool kvm_kick_many_cpus(struct cpumask *cpus, bool wait)
 	return true;
 }
 
+void kvm_kick_many_cpus(cpumask_var_t __cpus, bool wait)
+{
+	struct cpumask *cpus;
+
+	guard(preempt)();
+
+	cpus = this_cpu_cpumask_var_ptr(cpu_kick_mask);
+	cpumask_copy(cpus, __cpus);
+
+	__kvm_kick_many_cpus(cpus, wait);
+}
+
 static void kvm_make_vcpu_request(struct kvm_vcpu *vcpu, unsigned int req,
 				  struct cpumask *tmp, int current_cpu)
 {
@@ -262,7 +274,7 @@ bool kvm_make_vcpus_request_mask(struct kvm *kvm, unsigned int req,
 		kvm_make_vcpu_request(vcpu, req, cpus, me);
 	}
 
-	called = kvm_kick_many_cpus(cpus, !!(req & KVM_REQUEST_WAIT));
+	called = __kvm_kick_many_cpus(cpus, !!(req & KVM_REQUEST_WAIT));
 	put_cpu();
 
 	return called;
@@ -284,7 +296,7 @@ bool kvm_make_all_cpus_request(struct kvm *kvm, unsigned int req)
 	kvm_for_each_vcpu(i, vcpu, kvm)
 		kvm_make_vcpu_request(vcpu, req, cpus, me);
 
-	called = kvm_kick_many_cpus(cpus, !!(req & KVM_REQUEST_WAIT));
+	called = __kvm_kick_many_cpus(cpus, !!(req & KVM_REQUEST_WAIT));
 	put_cpu();
 
 	return called;
diff --git a/virt/kvm/pfncache.c b/virt/kvm/pfncache.c
index 97958af667fb..305706ba35dd 100644
--- a/virt/kvm/pfncache.c
+++ b/virt/kvm/pfncache.c
@@ -121,7 +121,7 @@ void gfn_to_pfn_cache_invalidate_start(struct kvm *kvm, unsigned long start,
 	 * "size at init" flag, or GFP_NOWAIT in the upgrade).
 	 */
 	if (cleared)
-		synchronize_srcu(&kvm->gpc_srcu);
+		kvm_kick_many_cpus(kvm->gpc_readers, true);
 
 	/*
 	 * Note the GPC_INVALIDATING markers set above are deliberately NOT