On Tue, Jun 21, 2022, David Matlack wrote: > On Fri, Jun 17, 2022 at 10:01 AM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > > > On Mon, May 16, 2022, David Matlack wrote: > > > +static void kvm_rmap_zap_collapsible_sptes(struct kvm *kvm, > > > + const struct kvm_memory_slot *slot) > > > +{ > > > + /* > > > + * Note, use KVM_MAX_HUGEPAGE_LEVEL - 1 since there's no need to zap > > > + * pages that are already mapped at the maximum possible level. > > > + */ > > > + if (slot_handle_level(kvm, slot, kvm_mmu_zap_collapsible_spte, > > > + PG_LEVEL_4K, KVM_MAX_HUGEPAGE_LEVEL - 1, > > > + true)) > > > > No need to wrap, "true" fits easily on the previous line. That said, I don't see > > any point in adding a helper. It's highly unlike there will be another caller, > > and IMO it's not any more readable since I have to go look at another function > > when reading kvm_mmu_zap_collapsible_sptes(). > > I could see an argument for readability either way. Putting it in a > helper function abstracts away the details, which would aid > readability if the reader does not care about the implementation > details of the rmap case. I'm ok either way, dealer's choice.