Re: [PATCH 17/21] KVM: arm64: Use common code's approach for __GFP_ZERO with memory caches

Sean Christopherson
2020-06-11T15:44:01Z

On Thu, Jun 11, 2020 at 08:59:05AM +0100, Marc Zyngier wrote:
> >diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> >index 9398b66f8a87..688213ef34f0 100644
> >--- a/arch/arm64/kvm/mmu.c
> >+++ b/arch/arm64/kvm/mmu.c
> >@@ -131,7 +131,8 @@ static int mmu_topup_memory_cache(struct
> >kvm_mmu_memory_cache *cache, int min)
> > if (cache->nobjs >= min)
> > return 0;
> > while (cache->nobjs < ARRAY_SIZE(cache->objects)) {
> >- page = (void *)__get_free_page(GFP_PGTABLE_USER);
> >+ page = (void *)__get_free_page(GFP_KERNEL_ACCOUNT |
>
> This is definitely a change in the way we account for guest
> page tables allocation, although I find it bizarre that not
> all architectures account for it the same way.

It's not intended to be a functional change, i.e. the allocations should
still be accounted:

#define GFP_PGTABLE_USER (GFP_PGTABLE_KERNEL | __GFP_ACCOUNT)
|
-> #define GFP_PGTABLE_KERNEL (GFP_KERNEL | __GFP_ZERO)

== GFP_KERNEL | __GFP_ACCOUNT | __GFP_ZERO

versus

#define GFP_KERNEL_ACCOUNT (GFP_KERNEL | __GFP_ACCOUNT)

with __GFP_ZERO explicitly OR'd in

== GFP_KERNEL | __GFP_ACCOUNT | __GFP_ZERO

I can put the above in the changelog, unless of course it's wrong and I've
missed something.

> It seems logical to me that nested page tables would be accounted
> against userspace, but I'm willing to be educated on the matter.
>
> Another possibility is that depending on the context, some allocations
> should be accounted on either the kernel or userspace (NV on arm64
> could definitely do something like that). If that was the case,
> maybe moving most of the GFP_* flags into the per-cache flags,
> and have the renaming that Ben suggested earlier.
>
> Thanks,
>
> M.
> --
> Jazz is not dead. It just smells funny...