On 01/04/21 18:50, Ben Gardon wrote:
This is just cosmetic, but I'd prefer to keep the call to
kvm_tdp_mmu_invalidate_roots at the original place, so that it's clear
in the next patch that it's two separate parts because of the different
locking requirements.
I'm not sure exactly what you mean and I could certainly do a better
job explaining in the commit description, but it's actually quite
important that kvm_tdp_mmu_invalidate_roots at least precede
kvm_zap_obsolete_pages as kvm_zap_obsolete_pages drops the lock and
yields. If kvm_tdp_mmu_invalidate_roots doesn't go first then vCPUs
could wind up dropping their ref on an old root and then picking it up
again before the last root had a chance to drop its ref.
Explaining in the description that kvm_tdp_mmu_zap_all is being
dropped because it is no longer necessary (as opposed to being moved)
might help make that cleaner.
No, what would help is the remark you just made about
kvm_zap_obsolete_pages yielding. But that doesn't matter after 13/13
though, does it? Perhaps it's easier to just combine them.
Paolo