From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sean Christopherson Date: Mon, 22 Jun 2020 19:40:55 +0000 Subject: Re: [PATCH 10/21] KVM: x86/mmu: Make __GFP_ZERO a property of the memory cache Message-Id: <20200622194055.GC6151@linux.intel.com> List-Id: References: <20200605213853.14959-1-sean.j.christopherson@intel.com> <20200605213853.14959-11-sean.j.christopherson@intel.com> In-Reply-To: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Ben Gardon Cc: Marc Zyngier , Paul Mackerras , Christian Borntraeger , Janosch Frank , Paolo Bonzini , James Morse , Julien Thierry , Suzuki K Poulose , David Hildenbrand , Cornelia Huck , Claudio Imbrenda , Vitaly Kuznetsov , Wanpeng Li , Jim Mattson , Joerg Roedel , 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 , Peter Shier , Junaid Shahid , Christoffer Dall On Wed, Jun 10, 2020 at 11:57:32AM -0700, Ben Gardon wrote: > > --- > > arch/x86/include/asm/kvm_host.h | 1 + > > arch/x86/kvm/mmu/mmu.c | 7 ++++++- > > 2 files changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > > index e7a427547557..fb99e6776e27 100644 > > --- a/arch/x86/include/asm/kvm_host.h > > +++ b/arch/x86/include/asm/kvm_host.h > > @@ -251,6 +251,7 @@ struct kvm_kernel_irq_routing_entry; > > */ > > struct kvm_mmu_memory_cache { > > int nobjs; > > + gfp_t gfp_zero; > This would make more sense to me if it could be used for general extra > gfp flags and was called gfp_flags or something, or it was a boolean > that was later translated into the flag being set. Storing the > gfp_zero flag here is a little counter-intuitive. Probably not worth > changing unless you're sending out a v2 for some other reason. Ideally, this would be a generic gfp_flags field, but that's basically a non-starter for patch 5, which uses GFP_ATOMIC for the "oh crap the cache is empty" error handling. Allowing arbitrary flags would be a mess. I went with storing a full gfp_t because that produces more optimal code. This isn't a super critical path and it's only a few cycles, but it seems worthwhile given the frequency with which this code will be called, and since this happens under mmu_lock. 348 gfp_flags |= mc->gfp_zero; 0x00000000000058ab <+59>: mov 0x4(%rbx),%eax 0x00000000000058ae <+62>: or $0x400cc0,%eax versus 349 gfp_flags |= __GFP_ZERO; 0x00000000000058a7 <+55>: cmpb $0x1,0x4(%rbx) 0x00000000000058ab <+59>: mov 0x8(%rbx),%rdi <-- unrelated interleaved code 0x00000000000058af <+63>: sbb %eax,%eax 0x00000000000058b1 <+65>: xor %al,%al 0x00000000000058b3 <+67>: add $0x400dc0,%eax