Re: [PATCH 1/1 v2] KVM: Reduce mmu_lock contention during dirty logging by cond_resched()

[Takuya Yoshikawa <takuya.yoshikawa@gmail.com>]

On Sun, 29 Apr 2012 14:27:30 +0300
Avi Kivity <avi@redhat.com> wrote:

> > +			if (need_resched() || spin_is_contended(&kvm->mmu_lock)) {
> > +				kvm_flush_remote_tlbs(kvm);
> 
> Do we really need to flush the TLB here?
> 
> Suppose we don't.  So some pages could still be written to using old TLB
> entries, but that's okay, since we're reporting those pages as dirty
> anyway.  In fact we might be saving userspace another pass at the page,
> if it won't be written afterwards.  A flush is only needed to prevent
> unlogged writes after we return the dirty bitmap.
> 
> If this reasoning is correct, we can replace the whole thing with
> cond_resched_lock().

Correct for dirty logging.

Actually, that was the reason I once introduced a rmap-write-protection race
when I first did rmap-based write protection.  I did not think about other
paths which conditionally flush TLBs.

For this patch, TLB flush is needed.

> Oh, but this might trick a later rmap_write_protect() into thinking that
> no write protection and tlb flush is needed.  So we should touch
> kvm->tlbs_dirty.

The problem was making others think that
"already protected, no need to flush."

As we discussed before, we need to add some tricks to de-couple mmu_lock and
TLB flush.

Thanks,
	Takuya