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; -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html