Re: [PATCH 2/3] KVM: x86/mmu: remove unnecessary "bool shared" argument from iterators

From: Sean Christopherson
Date: Fri Sep 29 2023 - 12:14:23 EST


On Thu, Sep 28, 2023, Maxim Levitsky wrote:
> У чт, 2023-09-28 у 12:29 -0400, Paolo Bonzini пише:
> > The "bool shared" argument is more or less unnecessary in the
> > for_each_*_tdp_mmu_root_yield_safe() macros. Many users check for
> > the lock before calling it; all of them either call small functions
> > that do the check, or end up calling tdp_mmu_set_spte_atomic() and
> > tdp_mmu_iter_set_spte(). Add a few assertions to make up for the
> > lost check in for_each_*_tdp_mmu_root_yield_safe(), but even this
> > is probably overkill and mostly for documentation reasons.
>
> Why not to leave the 'kvm_lockdep_assert_mmu_lock_held' but drop the shared
> argument from it? and then use lockdep_assert_held. If I am not mistaken,
> lockdep_assert_held should assert if the lock is held for read or write.

+1, I don't see any downside to asserting that mmu_lock is held when iterating.

It'll be a redundant assertion 99% of the time, but it's not like performance
matters all that much when running with lockdep enabled. And I find lockdep
assertions to be wonderful documentation.