From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PATCH 4/5] KVM: x86: MMU: Remove mapping_level_dirty_bitmap() Date: Thu, 15 Oct 2015 17:35:27 +0200 Message-ID: <561FC7BF.8040902@redhat.com> References: <20151015193953.7a277f640c7dfd19e8b8dcee@lab.ntt.co.jp> <20151015194320.b184ec92049ef7e597f57851@lab.ntt.co.jp> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org To: Takuya Yoshikawa Return-path: Received: from mx1.redhat.com ([209.132.183.28]:53599 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751805AbbJOPfd (ORCPT ); Thu, 15 Oct 2015 11:35:33 -0400 In-Reply-To: <20151015194320.b184ec92049ef7e597f57851@lab.ntt.co.jp> Sender: kvm-owner@vger.kernel.org List-ID: On 15/10/2015 12:43, Takuya Yoshikawa wrote: > +static inline bool memslot_invalid(struct kvm_memory_slot *slot) Can you make this function memslot_valid_for_gpte(struct kvm_memory_slot *slot, bool no_dirty_log), and have it return slot && !(slot->flags & KVM_MEMSLOT_INVALID) && (!no_dirty_log || !slot->dirty_bitmap) ? If gfn_to_memslot_dirty_bitmap and mapping_level call the same function, it helps highlighting the similarity between them. Your optimization loses that similarity in the name, but I think we can bring it back somehow. Otherwise, the patches are great. Thanks! Paolo > +{ > + if (!slot || slot->flags & KVM_MEMSLOT_INVALID) > + return true; > + > + return false; > +} > + > static struct kvm_memory_slot * > gfn_to_memslot_dirty_bitmap(struct kvm_vcpu *vcpu, gfn_t gfn, > bool no_dirty_log) > @@ -858,25 +866,22 @@ gfn_to_memslot_dirty_bitmap(struct kvm_vcpu *vcpu, gfn_t gfn, > struct kvm_memory_slot *slot; > > slot = kvm_vcpu_gfn_to_memslot(vcpu, gfn); > - if (!slot || slot->flags & KVM_MEMSLOT_INVALID || > - (no_dirty_log && slot->dirty_bitmap)) > + if (memslot_invalid(slot) || (no_dirty_log && slot->dirty_bitmap)) > slot = NULL; > > return slot; > } > > -static bool mapping_level_dirty_bitmap(struct kvm_vcpu *vcpu, gfn_t large_gfn) > -{ > - return !gfn_to_memslot_dirty_bitmap(vcpu, large_gfn, true); > -} > - > static int mapping_level(struct kvm_vcpu *vcpu, gfn_t large_gfn, > bool *force_pt_level) > { > int host_level, level, max_level; > + struct kvm_memory_slot *slot; > + > + slot = kvm_vcpu_gfn_to_memslot(vcpu, large_gfn); > > if (likely(!*force_pt_level)) > - *force_pt_level = mapping_level_dirty_bitmap(vcpu, large_gfn); > + *force_pt_level = memslot_invalid(slot) || slot->dirty_bitmap;