On Thu, Sep 21, 2023, Paolo Bonzini wrote: > On 9/16/23 02:39, Sean Christopherson wrote: > > Replace the address space ID in for_each_tdp_mmu_root_yield_safe() with a > > shared (vs. exclusive) param, and have the walker iterate over all address > > spaces as all callers want to process all address spaces. Drop the @as_id > > param as well as the manual address space iteration in callers. > > > > Add the @shared param even though the two current callers pass "false" > > unconditionally, as the main reason for refactoring the walker is to > > simplify using it to zap invalid TDP MMU roots, which is done with > > mmu_lock held for read. > > > > Cc: stable@xxxxxxxxxxxxxxx > > Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx> > > You konw what, I don't really like the "bool shared" arguments anymore. Yeah, I don't like the "shared" arguments either. Never did, but they are necessary for some paths, and I don't see an obviously better solution. :-/ > For example, neither tdp_mmu_next_root nor kvm_tdp_mmu_put_root need to know > if the lock is taken for read or write; protection is achieved via RCU and > tdp_mmu_pages_lock. It's more self-documenting to remove the argument and > assert that the lock is taken. > > Likewise, the 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; and 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(), so the per-iteration checks are also overkill. Agreed. > It may be useful to a few assertions to make up for the lost check before > the first execution of the body of for_each_*_tdp_mmu_root_yield_safe(), but > even this is more for documentation reasons than to catch actual bugs. I think it's more than sufficient, arguably even better, to document which paths *require* mmu_lock be held for read vs. write, and which paths work with either. > I'll send a v2. Can we do a cleanup of the @shared arguments on top? I would like to keep the diff reasonably small to minimize the v6.1 backport.
