On Tue, May 30, 2023 at 1:37 PM Oliver Upton <oliver.upton@xxxxxxxxx> wrote: > > Hi Yu, > > On Sat, May 27, 2023 at 02:13:07PM -0600, Yu Zhao wrote: > > On Sat, May 27, 2023 at 12:08 PM Oliver Upton <oliver.upton@xxxxxxxxx> wrote: > > > diff --git a/arch/arm64/kvm/hyp/pgtable.c b/arch/arm64/kvm/hyp/pgtable.c > > > index 3d61bd3e591d..bfbebdcb4ef0 100644 > > > --- a/arch/arm64/kvm/hyp/pgtable.c > > > +++ b/arch/arm64/kvm/hyp/pgtable.c > > > @@ -1019,7 +1019,7 @@ static int stage2_unmap_walker(const struct kvm_pgtable_visit_ctx *ctx, > > > kvm_granule_size(ctx->level)); > > > > > > if (childp) > > > - mm_ops->put_page(childp); > > > + mm_ops->free_removed_table(childp, ctx->level); > > > > Thanks, Oliver. > > > > A couple of things I haven't had the chance to verify -- I'm hoping > > you could help clarify: > > 1. For unmapping, with free_removed_table(), wouldn't we have to look > > into the table we know it's empty unnecessarily? > > As it is currently implemented, yes. But, there's potential to fast-path > the implementation by checking page_count() before starting the walk. Do you mind posting another patch? I'd be happy to ack it, as well as the one you suggested above. > > 2. For remapping and unmapping, how does free_removed_table() put the > > final refcnt on the table passed in? (Previously we had > > put_page(childp) in stage2_map_walk_table_post(). So I'm assuming we'd > > have to do something equivalent with free_removed_table().) > > Heh, that's a bug, and an embarrassing one at that! > > Sent out a fix for that, since it would appear we leak memory on > table->block transitions. PTAL if you have a chance. > > https://lore.kernel.org/all/20230530193213.1663411-1-oliver.upton@xxxxxxxxx/ Awesome.
