Re: [PATCH v4 10/17] KVM: arm64: Add a shrinker for pKVM

[Vincent Donnefort <vdonnefort@google.com>]

On Mon, Aug 24, 2026 at 07:10:37PM +0100, Fuad Tabba wrote:
> On Mon, 24 Aug 2026 at 17:38, Vincent Donnefort <vdonnefort@google.com> wrote:
> >
> > On Tue, Aug 18, 2026 at 04:28:38PM +0100, Fuad Tabba wrote:
> > > Hi Vincent,
> > >
> > > On Fri, 31 Jul 2026 at 15:36, 'Vincent Donnefort' via kernel-team
> > > <kernel-team@android.com> wrote:
> > > >
> > > > Integrate the pKVM memory reclaim interface with the host's memory
> > > > management subsystem.
> > > >
> > > > This allows the host to automatically recover unused memory fom the
> > > > hypervisor's heap allocator when the host is under memory pressure.
> > > >
> > > > Tested-by: Fuad Tabba <fuad.tabba@linux.dev>
> > > > Signed-off-by: Vincent Donnefort <vdonnefort@google.com>
> > > >
> > > > diff --git a/arch/arm64/kvm/pkvm.c b/arch/arm64/kvm/pkvm.c
> > > > index d28422f5c3d6..bfbb1266491d 100644
> > > > --- a/arch/arm64/kvm/pkvm.c
> > > > +++ b/arch/arm64/kvm/pkvm.c
> > > > @@ -115,7 +115,7 @@ static int pkvm_hyp_topup(enum pkvm_topup_id id, unsigned long nr_pages)
> > > >         return ret;
> > > >  }
> > > >
> > > > -static __maybe_unused unsigned long pkvm_hyp_reclaim(enum pkvm_topup_id id, unsigned long target)
> > > > +static unsigned long pkvm_hyp_reclaim(enum pkvm_topup_id id, unsigned long target)
> > >
> > > This is the first caller of these, so it is where reclaim starts
> > > running against a concurrent top-up.
> > >
> > > Nothing marks the pages a top-up just put in allocator->mc as spoken
> > > for, and hyp_allocator_reclaim() ends with an unbounded drain of it,
> > > so a shrink with target 1 hands back the lot. Land that between a
> > > top-up and the retry it was for, and the retry asks again, and
> > > pkvm_call_hyp_req() goes round.
> >
> > Sorry, I am not sure I follow here.
> >
> > IIRC, the shrinker will only reclaim half of what is available. So the pressure
> > should be proportional to what is available and limit races with topup!
> 
> It's the ordering, not the amount.

Do you think we should first try to reclaim from the mapped pages before
draining the allocator->mc? 

Mapped pages are more valuable hence why I have started with allocator->mc.

But, it is true it might make sense. It is unlikely to have pages left unused
into that mc. If that mc has been topped-up that's because it is about to be
allocated from...

> 
> topup and its retry are separate hypercalls, lock dropped between
> them, so a shrink on another CPU can slip in, right? .
> hyp_allocator_reclaim() drains allocator->mc, where the topup pages
> sit, before any chunk, so half still comes out of them first: a target
> of 1 fails the retry.

I do not see where a target == 1 fails.

> 
> >
> > However now looking at it. I wonder if I don't want to ratelimit here the number
> > of pages reclaimed in one go to limit the time spent at EL2. Especially we do
> > all that with the allocator lock taken...
> 
> Ratelimiting would cap the time under the lock, but it wouldn't stop a
> concurrent shrink from taking the topup pages, would it?

Yes, my only intent is to avoid blocking at EL2 for too long.

-- 
Vincent

> 
> Cheers,
> /fuad
> 
> 
> >

[...]