@@ -2297,7 +2297,6 @@ static int kvm_clear_dirty_log_protect(struct kvm *kvm,
if (copy_from_user(dirty_bitmap_buffer, log->dirty_bitmap, n))
return -EFAULT;
- KVM_MMU_LOCK(kvm);
for (offset = log->first_page, i = offset / BITS_PER_LONG,
n = DIV_ROUND_UP(log->num_pages, BITS_PER_LONG); n--;
i++, offset += BITS_PER_LONG) {
@@ -2316,11 +2315,12 @@ static int kvm_clear_dirty_log_protect(struct kvm *kvm,
*/
if (mask) {
flush = true;
+ KVM_MMU_LOCK(kvm);
kvm_arch_mmu_enable_log_dirty_pt_masked(kvm, memslot,
offset, mask);
+ KVM_MMU_UNLOCK(kvm);
}
}
- KVM_MMU_UNLOCK(kvm);
if (flush)
kvm_flush_remote_tlbs_memslot(kvm, memslot);
Drop and reacquire the mmu_lock during CLEAR_DIRTY_LOG to avoid blocking other threads from acquiring the mmu_lock (e.g. vCPUs taking page faults). It should be safe to drop and reacquire the mmu_lock from a correctness standpoint. slots_lock already ensures that only one thread in KVM is processing a GET/CLEAR_DIRTY_LOG ioctl. And KVM already has to be prepared to handle vCPUs updating the dirty log concurrent with CLEAR_DIRTY_LOG, hence the atomic_long_fetch_andnot(). So holding the mmu_lock across loop iterations is entirely unnecessary. It only needs to be acquired when calling in the arch-specific code to modify page tables. This change eliminates drops in guest performance during the live migration of a 160 vCPU VM that we've observed while userspace is issuing CLEAR ioctls (tested with 1GiB and 8GiB CLEARs). Userspace could issue finer-grained CLEAR ioctls, which would also reduce contention on the mmu_lock, but doing so will increase the rate of remote TLB flushing, so there is a limit. And there's really no reason to punt this problem to userspace. KVM can just drop and reacquire the lock more frequently to avoid holding it for too long. Signed-off-by: David Matlack <dmatlack@google.com> --- virt/kvm/kvm_main.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)