Messages in this thread | | | From | "Maciej S. Szmigiero" <> | Subject | Re: [PATCH v5 01/13] KVM: x86: Cache total page count to avoid traversing the memslot array | Date | Wed, 20 Oct 2021 20:41:04 +0200 |
| |
On 20.10.2021 00:24, Sean Christopherson wrote: > On Mon, Sep 20, 2021, Maciej S. Szmigiero wrote: >> From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com> >> >> There is no point in recalculating from scratch the total number of pages >> in all memslots each time a memslot is created or deleted. >> >> Just cache the value and update it accordingly on each such operation so >> the code doesn't need to traverse the whole memslot array each time. >> >> Signed-off-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com> >> --- >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >> index 28ef14155726..65fdf27b9423 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -11609,9 +11609,23 @@ void kvm_arch_commit_memory_region(struct kvm *kvm, >> const struct kvm_memory_slot *new, >> enum kvm_mr_change change) >> { >> - if (!kvm->arch.n_requested_mmu_pages) >> - kvm_mmu_change_mmu_pages(kvm, >> - kvm_mmu_calculate_default_mmu_pages(kvm)); >> + if (change == KVM_MR_CREATE) >> + kvm->arch.n_memslots_pages += new->npages; >> + else if (change == KVM_MR_DELETE) { >> + WARN_ON(kvm->arch.n_memslots_pages < old->npages); >> + kvm->arch.n_memslots_pages -= old->npages; >> + } >> + >> + if (!kvm->arch.n_requested_mmu_pages) { (..) >> + u64 memslots_pages; >> + unsigned long nr_mmu_pages; >> + >> + memslots_pages = kvm->arch.n_memslots_pages * KVM_PERMILLE_MMU_PAGES; >> + do_div(memslots_pages, 1000); >> + nr_mmu_pages = max_t(typeof(nr_mmu_pages), >> + memslots_pages, KVM_MIN_ALLOC_MMU_PAGES); > > "memslots_pages" is a bit of a misnomer. Any objection to avoiding naming problems > by explicitly casting to an "unsigned long" and simply operating on nr_mmu_pages? > > nr_mmu_pages = (unsigned long)kvm->arch.n_memslots_pages; > nr_mmu_pages *= (KVM_PERMILLE_MMU_PAGES / 1000); > nr_mmu_pages = max(nr_mmu_pages, KVM_MIN_ALLOC_MMU_PAGES); > kvm_mmu_change_mmu_pages(kvm, nr_mmu_pages); > > E.g. the whole thing can be > > if (!kvm->arch.n_requested_mmu_pages && > (change == KVM_MR_CREATE || change == KVM_MR_DELETE)) { > unsigned long nr_mmu_pages; > > if (change == KVM_MR_CREATE) { > kvm->arch.n_memslots_pages += new->npages; > } else { > WARN_ON(kvm->arch.n_memslots_pages < old->npages); > kvm->arch.n_memslots_pages -= old->npages; > } > > nr_mmu_pages = (unsigned long)kvm->arch.n_memslots_pages; > nr_mmu_pages *= (KVM_PERMILLE_MMU_PAGES / 1000);
The above line will set nr_mmu_pages to zero since KVM_PERMILLE_MMU_PAGES is 20, so when integer-divided by 1000 will result in a multiplication coefficient of zero.
> nr_mmu_pages = max(nr_mmu_pages, KVM_MIN_ALLOC_MMU_PAGES); > kvm_mmu_change_mmu_pages(kvm, nr_mmu_pages); > } > >> + kvm_mmu_change_mmu_pages(kvm, nr_mmu_pages); >> + } >> >> kvm_mmu_slot_apply_flags(kvm, old, new, change); >>
Thanks, Maciej
| |