From mboxrd@z Thu Jan  1 00:00:00 1970
From: Sean Christopherson <sean.j.christopherson@intel.com>
Date: Thu, 11 Jun 2020 15:43:59 +0000
Subject: Re: [PATCH 17/21] KVM: arm64: Use common code's approach for __GFP_ZERO with memory caches
Message-Id: <20200611154359.GF29918@linux.intel.com>
List-Id: <kvm-ppc.vger.kernel.org>
References: <20200605213853.14959-1-sean.j.christopherson@intel.com>
 <20200605213853.14959-18-sean.j.christopherson@intel.com>
 <6cc08074c289cbea7b9c1deeaf18c63f@kernel.org>
In-Reply-To: <6cc08074c289cbea7b9c1deeaf18c63f@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: Marc Zyngier <maz@kernel.org>
Cc: Paul Mackerras <paulus@ozlabs.org>, Christian Borntraeger <borntraeger@de.ibm.com>, Janosch Frank <frankja@linux.ibm.com>, Paolo Bonzini <pbonzini@redhat.com>, James Morse <james.morse@arm.com>, Julien Thierry <julien.thierry.kdev@gmail.com>, Suzuki K Poulose <suzuki.poulose@arm.com>, David Hildenbrand <david@redhat.com>, Cornelia Huck <cohuck@redhat.com>, Claudio Imbrenda <imbrenda@linux.ibm.com>, Vitaly Kuznetsov <vkuznets@redhat.com>, Wanpeng Li <wanpengli@tencent.com>, Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, linux-mips@vger.kernel.org, kvm@vger.kernel.org, kvm-ppc@vger.kernel.org, linux-kernel@vger.kernel.org, Peter Feiner <pfeiner@google.com>, Peter Shier <pshier@google.com>, Junaid Shahid <junaids@google.com>, Ben Gardon <bgardon@google.com>, Christoffer Dall <christoffer.dall@arm.com>

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