From mboxrd@z Thu Jan 1 00:00:00 1970 From: Xiao Guangrong Subject: Re: [RFC PATCH 3/3] KVM: MMU: Optimize guest page table walk Date: Tue, 19 Apr 2011 09:42:30 +0800 Message-ID: <4DACE886.4080405@cn.fujitsu.com> References: <20110419033220.e527bcae.takuya.yoshikawa@gmail.com> <20110419033814.3cc7ab5e.takuya.yoshikawa@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: avi@redhat.com, mtosatti@redhat.com, kvm@vger.kernel.org, yoshikawa.takuya@oss.ntt.co.jp To: Takuya Yoshikawa Return-path: Received: from cn.fujitsu.com ([222.73.24.84]:58403 "EHLO song.cn.fujitsu.com" rhost-flags-OK-FAIL-OK-OK) by vger.kernel.org with ESMTP id S1753913Ab1DSBtK convert rfc822-to-8bit (ORCPT ); Mon, 18 Apr 2011 21:49:10 -0400 In-Reply-To: <20110419033814.3cc7ab5e.takuya.yoshikawa@gmail.com> Sender: kvm-owner@vger.kernel.org List-ID: On 04/19/2011 02:38 AM, Takuya Yoshikawa wrote: > From: Takuya Yoshikawa >=20 > We optimize multi level guest page table walk as follows: >=20 > 1. We cache the memslot which, probably, includes the next guest pa= ge > tables to avoid searching for it many times. Yeah, the hit is very high, after optimizing the algorithm of memslots (http://lwn.net/Articles/429308/), maybe the advantage is not so signif= icant, could you apply this patchset and test again please? > 2. We use get_user() instead of copy_from_user(). >=20 > Note that this is kind of a restricted way of Xiao's more generic > work: "KVM: optimize memslots searching and cache GPN to GFN." >=20 > With this patch applied, paging64_walk_addr_generic() has improved > as the following tracing results show. >=20 > Before: > 3.169 us | paging64_walk_addr_generic(); > 1.880 us | paging64_walk_addr_generic(); > 1.243 us | paging64_walk_addr_generic(); > 1.517 us | paging64_walk_addr_generic(); > 3.009 us | paging64_walk_addr_generic(); > 1.814 us | paging64_walk_addr_generic(); > 1.340 us | paging64_walk_addr_generic(); > 1.659 us | paging64_walk_addr_generic(); > 1.748 us | paging64_walk_addr_generic(); > 1.488 us | paging64_walk_addr_generic(); >=20 > After: > 1.714 us | paging64_walk_addr_generic(); > 0.806 us | paging64_walk_addr_generic(); > 0.664 us | paging64_walk_addr_generic(); > 0.619 us | paging64_walk_addr_generic(); > 0.645 us | paging64_walk_addr_generic(); > 0.605 us | paging64_walk_addr_generic(); > 1.388 us | paging64_walk_addr_generic(); > 0.753 us | paging64_walk_addr_generic(); > 0.594 us | paging64_walk_addr_generic(); > 0.833 us | paging64_walk_addr_generic(); >=20 > Signed-off-by: Takuya Yoshikawa > --- > arch/x86/kvm/paging_tmpl.h | 37 ++++++++++++++++++++++++++++++++--= --- > 1 files changed, 32 insertions(+), 5 deletions(-) >=20 > diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h > index 109939a..614aa3f 100644 > --- a/arch/x86/kvm/paging_tmpl.h > +++ b/arch/x86/kvm/paging_tmpl.h > @@ -109,12 +109,37 @@ static unsigned FNAME(gpte_access)(struct kvm_v= cpu *vcpu, pt_element_t gpte) > return access; > } > =20 > +/* > + * Read the guest PTE refered to by table_gfn and offset and put it = into ptep. > + * > + * *slot_hint, if not NULL, should point to a memslot which probably= includes > + * the guest PTE. The actual memslot will be put back into this so = that > + * callers can cache it. > + */ > static int FNAME(read_guest_pte)(struct kvm_vcpu *vcpu, struct kvm_m= mu *mmu, > - gfn_t table_gfn, int offset, pt_element_t *ptep) > + gfn_t table_gfn, int offset, pt_element_t *ptep, > + struct kvm_memory_slot **slot_hint) > { > - return kvm_read_guest_page_mmu(vcpu, mmu, table_gfn, ptep, > - offset, sizeof(*ptep), > - PFERR_USER_MASK | PFERR_WRITE_MASK); > + unsigned long addr; > + pt_element_t __user *ptep_user; > + gfn_t real_gfn; > + > + real_gfn =3D mmu->translate_gpa(vcpu, gfn_to_gpa(table_gfn), > + PFERR_USER_MASK | PFERR_WRITE_MASK); > + if (real_gfn =3D=3D UNMAPPED_GVA) > + return -EFAULT; > + > + real_gfn =3D gpa_to_gfn(real_gfn); > + > + if (!(*slot_hint) || !gfn_in_memslot(*slot_hint, real_gfn)) > + *slot_hint =3D gfn_to_memslot(vcpu->kvm, real_gfn); > + You forgot to check the result. (if *slot_hint =3D=3D NULL)? ...=E3=80=80= ;-)