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

On Tue, 25 Aug 2026 at 08:22, Vincent Donnefort <[email protected]> wrote:
...
> > > > 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...

I think that would work, as long as the ULONG_MAX drain at the tail of
hyp_allocator_reclaim() is bounded too. The memcache is LIFO, so the
pages hyp_allocator_unmap() stages sit on top of the top-up ones, and
draining just what the chunk loop reclaimed leaves the rest alone.

It would still fall through to the top-up pages once the chunks run
out, which is where your last point comes in. hyp_allocator_map() only
raises a request when it finds the mc empty, so pages sitting in there
are ones a top-up just put there for a retry. Could the reclaim leave
them alone altogether, and drop the mc.nr_pages term v4 added to
hyp_allocator_reclaimable(), which is what advertises them to the
shrinker?

> >
> > 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.

It's the retry that fails, not the reclaim. With target == 1,
hyp_allocator_drain_memcache() pops one page and target is done, so
the chunk loop never runs. But the top-up was sized to the exact
shortfall, so the retry now runs the mc dry one page early, sets
topup_needed = 1, returns -ENOMEM again, and pkvm_call_hyp_req() goes
round for another top-up.

Nothing errors out, it just doesn't converge for as long as the
shrinker keeps pace.

Keep in mind, I am not as familiar with this code as you are. So I
might be completely off here :)

Cheers,
/fuad

> >
> > >
> > > 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
> >
> >
> > >
>
> [...]