From mboxrd@z Thu Jan  1 00:00:00 1970
From: Takuya Yoshikawa <takuya.yoshikawa@gmail.com>
Subject: Re: [PATCH 1/1 v2] KVM: Reduce mmu_lock contention during dirty
 logging by cond_resched()
Date: Sun, 29 Apr 2012 21:17:59 +0900
Message-ID: <20120429211759.0265df80d7193048344ff35b@gmail.com>
References: <20120428190544.7dc2bfd281054c1fcac5a14e@gmail.com>
	<20120428190754.5c27325da5663e5d8c9b9be1@gmail.com>
	<4F9D25A2.7020303@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Cc: mtosatti@redhat.com, kvm@vger.kernel.org,
	yoshikawa.takuya@oss.ntt.co.jp
To: Avi Kivity <avi@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mail-pz0-f51.google.com ([209.85.210.51]:33278 "EHLO
	mail-pz0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1752348Ab2D2MSF (ORCPT <rfc822;kvm@vger.kernel.org>);
	Sun, 29 Apr 2012 08:18:05 -0400
Received: by dadz8 with SMTP id z8so2931612dad.10
        for <kvm@vger.kernel.org>; Sun, 29 Apr 2012 05:18:04 -0700 (PDT)
In-Reply-To: <4F9D25A2.7020303@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

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