On Sun, 29 Apr 2012 14:27:30 +0300 Avi Kivity <avi@xxxxxxxxxx> 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 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html