From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wanpeng Li Subject: Re: [PATCH] Using the tlb flush util function where applicable Date: Wed, 17 Sep 2014 08:15:24 +0800 Message-ID: <20140917001524.GA11326@kernel> References: <1410556005-15639-1-git-send-email-liangchen.linux@gmail.com> <20140915193351.GA3356@potion.brq.redhat.com> Reply-To: Wanpeng Li Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Liang Chen , pbonzini@redhat.com, kvm@vger.kernel.org To: Radim =?utf-8?B?S3LEjW3DocWZ?= Return-path: Received: from mga09.intel.com ([134.134.136.24]:2112 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753268AbaIQAN0 (ORCPT ); Tue, 16 Sep 2014 20:13:26 -0400 Content-Disposition: inline In-Reply-To: <20140915193351.GA3356@potion.brq.redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: Hi Radim, On Mon, Sep 15, 2014 at 09:33:52PM +0200, Radim Kr=C4=8Dm=C3=A1=C5=99 w= rote: >2014-09-12 17:06-0400, Liang Chen: >> Using kvm_mmu_flush_tlb as the other places to make sure vcpu >> stat is incremented >>=20 >> Signed-off-by: Liang Chen >> --- > >Good catch. > >> arch/x86/kvm/vmx.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >>=20 >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >> index bfe11cf..439682e 100644 >> --- a/arch/x86/kvm/vmx.c >> +++ b/arch/x86/kvm/vmx.c >> @@ -1810,7 +1810,7 @@ static void vmx_vcpu_load(struct kvm_vcpu *vcp= u, int cpu) >> struct desc_ptr *gdt =3D &__get_cpu_var(host_gdt); >> unsigned long sysenter_esp; >> =20 >> - kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu); >> + kvm_mmu_flush_tlb(vcpu); >> local_irq_disable(); >> crash_disable_local_vmclear(cpu); >> =20 > >And to hijack this thread ... >I noticed three other deficits in stat.tlb_flush, patch below. > >Do you prefer the current behavior? > >--- 8< --- >KVM: x86: count actual tlb flushes > >- we count KVM_REQ_TLB_FLUSH requests, not actual flushes So there maybe multiple requests accumulated at the point of kvm_check_= request,=20 if your patch account these accumulations correctly? Regards, Wanpeng Li=20 > (KVM can have multiple requests for one flush) >- flushes from kvm_flush_remote_tlbs aren't counted >- it's easy to make a direct request by mistake > >Solve these by postponing the counting to kvm_check_request(). > >Signed-off-by: Radim Kr=C4=8Dm=C3=A1=C5=99 >--- >(And what about a possible followup patch that replaces > kvm_mmu_flush_tlb() with kvm_make_request() again? > It would free the namespace a bit and we could call something > similarly named from vcpu_enter_guest() to do the job.) > >diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c >index 9314678..b41fd97 100644 >--- a/arch/x86/kvm/mmu.c >+++ b/arch/x86/kvm/mmu.c >@@ -3452,7 +3452,6 @@ static void nonpaging_init_context(struct kvm_vc= pu *vcpu, >=20 > void kvm_mmu_flush_tlb(struct kvm_vcpu *vcpu) > { >- ++vcpu->stat.tlb_flush; > kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu); > } > EXPORT_SYMBOL_GPL(kvm_mmu_flush_tlb); >diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >index 916e895..0f0ad08 100644 >--- a/arch/x86/kvm/x86.c >+++ b/arch/x86/kvm/x86.c >@@ -6041,8 +6041,10 @@ static int vcpu_enter_guest(struct kvm_vcpu *vc= pu) > } > if (kvm_check_request(KVM_REQ_MMU_SYNC, vcpu)) > kvm_mmu_sync_roots(vcpu); >- if (kvm_check_request(KVM_REQ_TLB_FLUSH, vcpu)) >+ if (kvm_check_request(KVM_REQ_TLB_FLUSH, vcpu)) { >+ ++vcpu->stat.tlb_flush; > kvm_x86_ops->tlb_flush(vcpu); >+ } > if (kvm_check_request(KVM_REQ_REPORT_TPR_ACCESS, vcpu)) { > vcpu->run->exit_reason =3D KVM_EXIT_TPR_ACCESS; > r =3D 0; >-- >To unsubscribe from this list: send the line "unsubscribe kvm" in >the body of a message to majordomo@vger.kernel.org >More majordomo info at http://vger.kernel.org/majordomo-info.html