| Message ID | 1600684166-32430-1-git-send-email-lirongqing@baidu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86/mmu: fix counting of rmap entries in pte_list_add | expand |
On Mon, Sep 21, 2020 at 06:29:26PM +0800, Li RongQing wrote: > counting of rmap entries was missed when desc->sptes is full > and desc->more is NULL > > Signed-off-by: Li RongQing <lirongqing@baidu.com> > --- > arch/x86/kvm/mmu/mmu.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c > index a5d0207e7189..8ffa4e40b650 100644 > --- a/arch/x86/kvm/mmu/mmu.c > +++ b/arch/x86/kvm/mmu/mmu.c > @@ -1280,6 +1280,7 @@ static int pte_list_add(struct kvm_vcpu *vcpu, u64 *spte, > if (desc->sptes[PTE_LIST_EXT-1]) { > desc->more = mmu_alloc_pte_list_desc(vcpu); > desc = desc->more; > + count += PTE_LIST_EXT; Kind of a nit, but what do you think about merging the two PTE_LIST_EXT-1 check? For me, that makes the resulting code more obviously correct, and it might be slightly more performant as it avoids the extra comparison, though the compiler may be smart enough to optimize that away without help. while (desc->sptes[PTE_LIST_EXIT-1]) { count += PTE_LIST_EXT; if (!desc->more) { desc->more = mmu_alloc_pte_list_desc(vcpu); desc = desc->more; break; } desc = desc->more; } > } > for (i = 0; desc->sptes[i]; ++i) > ++count; > -- > 2.16.2 >
> -----Original Message----- > From: Sean Christopherson [mailto:sean.j.christopherson@intel.com] > Sent: Tuesday, September 22, 2020 3:41 AM > To: Li,Rongqing <lirongqing@baidu.com> > Cc: kvm@vger.kernel.org; x86@kernel.org > Subject: Re: [PATCH] KVM: x86/mmu: fix counting of rmap entries in > pte_list_add > > On Mon, Sep 21, 2020 at 06:29:26PM +0800, Li RongQing wrote: > > counting of rmap entries was missed when desc->sptes is full and > > desc->more is NULL > > > > Signed-off-by: Li RongQing <lirongqing@baidu.com> > > --- > > arch/x86/kvm/mmu/mmu.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c index > > a5d0207e7189..8ffa4e40b650 100644 > > --- a/arch/x86/kvm/mmu/mmu.c > > +++ b/arch/x86/kvm/mmu/mmu.c > > @@ -1280,6 +1280,7 @@ static int pte_list_add(struct kvm_vcpu *vcpu, u64 > *spte, > > if (desc->sptes[PTE_LIST_EXT-1]) { > > desc->more = mmu_alloc_pte_list_desc(vcpu); > > desc = desc->more; > > + count += PTE_LIST_EXT; > > Kind of a nit, but what do you think about merging the two PTE_LIST_EXT-1 > check? For me, that makes the resulting code more obviously correct, and it > might be slightly more performant as it avoids the extra comparison, though > the compiler may be smart enough to optimize that away without help. > > while (desc->sptes[PTE_LIST_EXIT-1]) { > count += PTE_LIST_EXT; > > if (!desc->more) { > desc->more = mmu_alloc_pte_list_desc(vcpu); > desc = desc->more; > break; > } > desc = desc->more; > } > Ok, I will send V2 as you suggested Thanks -Li
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c index a5d0207e7189..8ffa4e40b650 100644 --- a/arch/x86/kvm/mmu/mmu.c +++ b/arch/x86/kvm/mmu/mmu.c @@ -1280,6 +1280,7 @@ static int pte_list_add(struct kvm_vcpu *vcpu, u64 *spte, if (desc->sptes[PTE_LIST_EXT-1]) { desc->more = mmu_alloc_pte_list_desc(vcpu); desc = desc->more; + count += PTE_LIST_EXT; } for (i = 0; desc->sptes[i]; ++i) ++count;
counting of rmap entries was missed when desc->sptes is full and desc->more is NULL Signed-off-by: Li RongQing <lirongqing@baidu.com> --- arch/x86/kvm/mmu/mmu.c | 1 + 1 file changed, 1 insertion(+)