Re: [PATCH 7/11] KVM: Remove redundant check in the kvm_get_dirty_log_protect()
From: Sean Christopherson <hidden>
Date: 2019-01-04 15:50:48
Also in:
kvm, kvmarm, linuxppc-dev
On Fri, Jan 04, 2019 at 04:54:01PM +0800, lantianyu1986@gmail.com wrote:
quoted hunk ↗ jump to hunk
From: Lan Tianyu <redacted> The dirty bits have already been checked in the previous check of "dirty_bitmap" and mask must be non-zero value at this point. Signed-off-by: Lan Tianyu <redacted> --- virt/kvm/kvm_main.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index cf7cc0554094..e75dbb15fd09 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c@@ -1206,11 +1206,9 @@ int kvm_get_dirty_log_protect(struct kvm *kvm, mask = xchg(&dirty_bitmap[i], 0); dirty_bitmap_buffer[i] = mask; - if (mask) { - offset = i * BITS_PER_LONG; - kvm_arch_mmu_enable_log_dirty_pt_masked(kvm, memslot, - offset, mask); - } + offset = i * BITS_PER_LONG; + kvm_arch_mmu_enable_log_dirty_pt_masked(kvm, memslot, + offset, mask);
Hmm, the check against mask was explicitly added by commit 58d2930f4ee3
("KVM: Eliminate extra function calls in kvm_get_dirty_log_protect()").
AFAIK KVM only *sets* bits in dirty_bitmap without holding slots_lock
and/or mmu_lock, so I agree that checking mask is redundant, but it'd be
nice to elaborate a bit more in the changelog.
At the very least this needs a Fixes tag for the aforementioned commit.
Tangentially related, does mmu_lock actually need to be held while we
walk dirty_bitmap in kvm_{clear,get}_dirty_log_protect()? The bitmap
itself is protected by slots_lock (a lockdep assertion would be nice
too), e.g. can we grab the lock iff dirty_bitmap[i] != 0?
} spin_unlock(&kvm->mmu_lock); } -- 2.14.4
_______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel