On Thu, Jul 14, 2022, Paolo Bonzini wrote: > On 7/12/22 03:55, Sean Christopherson wrote: > > Clean up the rmap helpers (mostly renames) to yield a more coherent set of > > APIs, and to purge the irritating and inconsistent "rmapp" (p is for pointer) > > nomenclature. > > > > Patch 1 is a tangentially related fix for a benign bug. > > > > Sean Christopherson (5): > > KVM: x86/mmu: Return a u64 (the old SPTE) from > > mmu_spte_clear_track_bits() > > KVM: x86/mmu: Rename rmap zap helpers to better show relationships > > KVM: x86/mmu: Remove underscores from __pte_list_remove() > > KVM: x86/mmu: Use innermost rmap zap helper when recycling rmaps > > KVM: x86/mmu: Drop the "p is for pointer" from rmap helpers > > > > arch/x86/kvm/mmu/mmu.c | 73 +++++++++++++++++++++--------------------- > > 1 file changed, 36 insertions(+), 37 deletions(-) > > > > > > base-commit: b9b71f43683ae9d76b0989249607bbe8c9eb6c5c > > I'm not sure I dig the ____, I'll take a closer look tomorrow or next week > since it's dinner time here. Yeah, I'm not a fan of it either. And rereading things, my proposed names also create an inconsistency; the zap path is the only user of kvm_handle_gfn_range() that uses a plural "rmaps". $ git grep kvm_handle_gfn_range arch/x86/kvm/mmu/mmu.c:static __always_inline bool kvm_handle_gfn_range(struct kvm *kvm, arch/x86/kvm/mmu/mmu.c: flush = kvm_handle_gfn_range(kvm, range, kvm_zap_rmaps); arch/x86/kvm/mmu/mmu.c: flush = kvm_handle_gfn_range(kvm, range, kvm_set_pte_rmap); arch/x86/kvm/mmu/mmu.c: young = kvm_handle_gfn_range(kvm, range, kvm_age_rmap); arch/x86/kvm/mmu/mmu.c: young = kvm_handle_gfn_range(kvm, range, kvm_test_age_rmap); Make "rmaps" plural is probably a mistake. The helper zaps multiple SPTEs for a given rmap list, but from a certain point of view it's just a single "rmap". What about: kvm_zap_rmapp => kvm_zap_rmap // to align with kvm_handle_gfn_range() usage kvm_zap_rmap => __kvm_zap_rmap // to pair with kvm_zap_rmap() and pte_list_remove => kvm_zap_one_rmap_spte pte_list_destroy => kvm_zap_all_rmap_sptes That will yield a better series too, as I can move patch 5 to be patch 2, then split what was patch 2 (the rename) into separate patches to first align kvm_zap_rmap() and __kvm_zap_rmap(), and then rename the pte_list_remove/destroy helpers.
