On Thu, Jul 25, 2024, David Matlack wrote: > On 2024-07-24 01:10 AM, James Houghton wrote: > > Optimize both kvm_age_gfn and kvm_test_age_gfn's interaction with the > > nit: Use () when referring to functions. > > > shadow MMU by, rather than checking if our memslot has rmaps, check if > > there are any indirect_shadow_pages at all. > > What is optimized by checking indirect_shadow_pages instead of > have_rmaps and what's the benefit? Smells like a premature optimization. Checking indirect_shadow_pages avoids taking mmu_lock for write when KVM doesn't currently have shadow MMU pages, but did at some point in the past, whereas kvm_memslots_have_rmaps() is sticky and will return true forever. > > Also, for kvm_test_age_gfn, reorder the TDP MMU check to be first. If we > > find that the range is young, we do not need to check the shadow MMU. > > This should be a separate commit since it's a logically distinct change > and no dependency on the other change in this commit (other than both > touch the same function). > > Splitting the commits up will also make it easier to write more specific > short logs (instead of "optimize a little bit" :) +1. Especially code movement and refactoring, e.g. factoring out tdp_mmu_clear_spte_bits_atomic() would ideally be in a standalone patch that's dead simple to review.
