From: Will Deacon <will@kernel.org>
To: Gavin Shan <gshan@redhat.com>
Cc: Marc Zyngier <maz@kernel.org>,
kernel-team@android.com, kvmarm@lists.cs.columbia.edu,
linux-arm-kernel@lists.infradead.org,
Catalin Marinas <catalin.marinas@arm.com>
Subject: Re: [PATCH v2 01/22] KVM: arm64: Don't free memcache pages in kvm_phys_addr_ioremap()
Date: Wed, 19 Aug 2020 10:03:20 +0100 [thread overview]
Message-ID: <20200819090320.GA17517@willie-the-truck> (raw)
In-Reply-To: <47f7bfc2-1880-908f-4345-b25da500b0ff@redhat.com>
Hi Gavin,
Cheers for taking a look.
On Wed, Aug 19, 2020 at 02:38:39PM +1000, Gavin Shan wrote:
> On 8/18/20 11:27 PM, Will Deacon wrote:
> > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> > index 662b0c99a63d..4a24ebdc6fc6 100644
> > --- a/arch/arm64/kvm/mmu.c
> > +++ b/arch/arm64/kvm/mmu.c
> > @@ -1489,19 +1489,17 @@ int kvm_phys_addr_ioremap(struct kvm *kvm, phys_addr_t guest_ipa,
> > ret = kvm_mmu_topup_memory_cache(&cache,
> > kvm_mmu_cache_min_pages(kvm));
> > if (ret)
> > - goto out;
> > + break;
> > spin_lock(&kvm->mmu_lock);
> > ret = stage2_set_pte(&kvm->arch.mmu, &cache, addr, &pte,
> > KVM_S2PTE_FLAG_IS_IOMAP);
> > spin_unlock(&kvm->mmu_lock);
> > if (ret)
> > - goto out;
> > + break;
> > pfn++;
> > }
> > -out:
> > - kvm_mmu_free_memory_cache(&cache);
> > return ret;
> > }
>
> It seems incorrect. The cache is tracked by local variable (@cache),
> meaning the cache is only visible to kvm_phys_addr_ioremap() and its
> callee. So it's correct to free unused pages in two cases: (1) error
> is returned (2) high level of page tables were previously populated
> and not all pre-allocated pages are used. Otherwise, this leads to
> memory leakage.
Well spotted, you're completely right. I was _sure_ this was the vCPU
memcache and I even said as much in the commit meesage, but it's not, and it
never was, so I can drop this patch. If there are any other patches I can
drop in the series, please let me know! I did test with kmemleak enabled,
but I guess that doesn't track page allocations into the memcache.
It _might_ be an idea to have a per-VM memcache to handle these allocations,
as that might offer some reuse over sticking one on the stack each time, but
then again kvm_phys_addr_ioremap() is hardly a fastpath.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-08-19 9:04 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-18 13:27 [PATCH v2 00/22] KVM: arm64: Rewrite page-table code and fault handling Will Deacon
2020-08-18 13:27 ` [PATCH v2 01/22] KVM: arm64: Don't free memcache pages in kvm_phys_addr_ioremap() Will Deacon
2020-08-19 4:38 ` Gavin Shan
2020-08-19 9:03 ` Will Deacon [this message]
2020-08-18 13:27 ` [PATCH v2 02/22] KVM: arm64: Remove kvm_mmu_free_memory_caches() Will Deacon
2020-08-19 4:45 ` Gavin Shan
2020-08-18 13:27 ` [PATCH v2 03/22] KVM: arm64: Add stand-alone page-table walker infrastructure Will Deacon
2020-08-18 13:28 ` [PATCH v2 04/22] KVM: arm64: Add support for creating kernel-agnostic stage-1 page tables Will Deacon
2020-08-18 13:28 ` [PATCH v2 05/22] KVM: arm64: Use generic allocator for hyp stage-1 page-tables Will Deacon
2020-08-18 13:28 ` [PATCH v2 06/22] KVM: arm64: Add support for creating kernel-agnostic stage-2 page tables Will Deacon
2020-08-18 13:28 ` [PATCH v2 07/22] KVM: arm64: Add support for stage-2 map()/unmap() in generic page-table Will Deacon
2020-08-18 13:28 ` [PATCH v2 08/22] KVM: arm64: Convert kvm_phys_addr_ioremap() to generic page-table API Will Deacon
2020-08-18 13:28 ` [PATCH v2 09/22] KVM: arm64: Convert kvm_set_spte_hva() " Will Deacon
2020-08-18 13:28 ` [PATCH v2 10/22] KVM: arm64: Convert unmap_stage2_range() " Will Deacon
2020-08-18 13:28 ` [PATCH v2 11/22] KVM: arm64: Add support for stage-2 page-aging in generic page-table Will Deacon
2020-08-18 13:28 ` [PATCH v2 12/22] KVM: arm64: Convert page-aging and access faults to generic page-table API Will Deacon
2020-08-18 13:28 ` [PATCH v2 13/22] KVM: arm64: Add support for stage-2 write-protect in generic page-table Will Deacon
2020-08-18 13:28 ` [PATCH v2 14/22] KVM: arm64: Convert write-protect operation to generic page-table API Will Deacon
2020-08-18 13:28 ` [PATCH v2 15/22] KVM: arm64: Add support for stage-2 cache flushing in generic page-table Will Deacon
2020-08-18 13:28 ` [PATCH v2 16/22] KVM: arm64: Convert memslot cache-flushing code to generic page-table API Will Deacon
2020-08-18 13:28 ` [PATCH v2 17/22] KVM: arm64: Add support for relaxing stage-2 perms in generic page-table code Will Deacon
2020-08-18 13:28 ` [PATCH v2 18/22] KVM: arm64: Convert user_mem_abort() to generic page-table API Will Deacon
2020-08-18 13:28 ` [PATCH v2 19/22] KVM: arm64: Check the pgt instead of the pgd when modifying page-table Will Deacon
2020-08-18 13:28 ` [PATCH v2 20/22] KVM: arm64: Remove unused page-table code Will Deacon
2020-08-18 13:28 ` [PATCH v2 21/22] KVM: arm64: Remove unused 'pgd' field from 'struct kvm_s2_mmu' Will Deacon
2020-08-18 13:28 ` [PATCH v2 22/22] KVM: arm64: Don't constrain maximum IPA size based on host configuration Will Deacon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200819090320.GA17517@willie-the-truck \
--to=will@kernel.org \
--cc=catalin.marinas@arm.com \
--cc=gshan@redhat.com \
--cc=kernel-team@android.com \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=maz@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox