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

On Thu, 2026-08-06 at 19:59 +0200, Woodhouse, David wrote:
> On Thu, 2026-08-06 at 09:53 -0700, Sean Christopherson wrote:
> > On Wed, Aug 05, 2026, [email protected] wrote:
> > > Replace the per-cache rwlock with RCU for the read side.
> > 
> > I don't hate the idea, but I am very against using RCU.  Unless it's "impossible",
> > e.g. because synchronize_srcu() allocates memory and breaks OOM kill, I would
> > strongly prefer to use SRCU, probably with a dedicated kvm->gpc_srcu, so that
> > synchronization doesn't need to wait on all CPUs in the system.  The tail latencies
> > for synchronize_rcu() are horrendous, especially for many-CPU systems.  If it
> > were only mmu_notifiers that got hit, it miiiight be acceptable, but since this
> > will affect vCPU tasks in the refresh() path as well, normal RCU is pretty much
> > a non-starter.
> 
> Yeah, the refresh() path got pretty slow in my first attempt, before
> optimising that *not* to have a grace period if the memslot generation
> changed but the actual GPA→uHVA (and memslot) don't *change*.
> 
> > Even SRCU could be problematic: if synchronize_srcu_expedited() is forced to wait,
> > the wait time can easily get to 20+ milliseconds, which again is a non-starter for
> > things like steal-time updates and nVMX pages.
> 
> But we don't have to synchronize from the read side. The code path
> which will do so most often is gfn_to_pfn_cache_invalidate_start(). And
> the refresh() path which is already the fallback slow path which can
> sleep. Although in my tree I've optimised that *not* to incur a grace
> period when it's avoidable, as noted above.
> 
> I'm more concerned by the fact that srcu_gp_end() might have to
> *allocate*, while the OOM reaper path waits for it. I'll see how we can
> deal with that...

I think I need to invoke Paul et al for that one...

Paul, Boqun, please could I trouble you to look at the top commit in
https://git.infradead.org/?p=users/dwmw2/linux.git;a=shortlog;h=refs/heads/xen-rcu-srcu

The "gfn_to_pfn_cache" (GPC) here is for all extents and purposes a
TLB. It's caching the full two-stage translation through KVM memslots
which map a guest physical address to a userspace virtual address, and
then through the standard process page tables to a host physical
address.

It is the second stage we are interested in here. Invalidation happens
through an MMU notifier, which (after I obey Sean's request to switch
to SRCU) calls synchronize_srcu_expedited().

My problem:

 1. The MMU notifier invalidation can be called from the OOM reaper
    path. In this case it MUST NOT sleep to allocate memory, or block
    on anything which does so. cf.
    https://lore.kernel.org/all/[email protected]/

 2. srcu_gp_end() does so, through init_srcu_struct_nodes(), while
    synchronize_srcu_expedited() effectively waits for it — precisely
    the thing we must not do. (Strictly, the waiter's completion is
    queued to the same workqueue on which process_srcu() could be
    blocked in init_srcu_struct_nodes(…, GFP_KERNEL)).

On a system with >= 128 CPUs it's actually OK because that allocation
path never happens; everything's allocated in advance ("rcu: srcu_init:
Setting srcu_struct sizes to big"). But on smaller systems I don't see
that we have a way to ensure that things are safe. (I don't count
setting big_cpu_lim to zero!)

Could we change that init_srcu_struct_nodes(…, GFP_KERNEL) to a non-
sleeping allocation? If the allocation fails, that isn't fatal, is it?
Or some other way to trigger the transition to big under controlled
circumstances in advance...?