From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 1/2] KVM: mmu_notifier: Flush TLBs before releasing mmu_lock Date: Wed, 15 Feb 2012 11:47:11 +0200 Message-ID: <4F3B7F1F.40402@redhat.com> References: <20120210152831.6ac3ac87.yoshikawa.takuya@oss.ntt.co.jp> <20120214171044.GK9440@redhat.com> <20120214172947.GA22362@amt.cnet> <20120214185356.GQ9440@redhat.com> <20120214194342.GA24117@amt.cnet> <4F3B7867.7000807@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Andrea Arcangeli , Takuya Yoshikawa , kvm@vger.kernel.org, xiaoguangrong@linux.vnet.ibm.com To: Marcelo Tosatti Return-path: Received: from mx1.redhat.com ([209.132.183.28]:46959 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1761447Ab2BOJrR (ORCPT ); Wed, 15 Feb 2012 04:47:17 -0500 In-Reply-To: <4F3B7867.7000807@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 02/15/2012 11:18 AM, Avi Kivity wrote: > On 02/14/2012 09:43 PM, Marcelo Tosatti wrote: > > Also it should not be necessary for these flushes to be inside mmu_lock > > on EPT/NPT case (since there is no write protection there). > > We do write protect with TDP, if nested virt is active. The question is > whether we have indirect pages or not, not whether TDP is active or not > (even without TDP, if you don't enable paging in the guest, you don't > have to write protect). > > > But it would > > be awkward to differentiate the unlock position based on EPT/NPT. > > > > I would really like to move the IPI back out of the lock. > > How about something like a sequence lock: > > > spin_lock(mmu_lock) > need_flush = write_protect_stuff(); > atomic_add(kvm->want_flush_counter, need_flush); > spin_unlock(mmu_lock); > > while ((done = atomic_read(kvm->done_flush_counter)) < (want = > atomic_read(kvm->want_flush_counter)) { > kvm_make_request(flush) > atomic_cmpxchg(kvm->done_flush_counter, done, want) > } > > This (or maybe a corrected and optimized version) ensures that any > need_flush cannot pass the while () barrier, no matter which thread > encounters it first. However it violates the "do not invent new locking > techniques" commandment. Can we map it to some existing method? There is no need to advance 'want' in the loop. So we could do /* must call with mmu_lock held */ void kvm_mmu_defer_remote_flush(kvm, need_flush) { if (need_flush) ++kvm->flush_counter.want; } /* may call without mmu_lock */ void kvm_mmu_commit_remote_flush(kvm) { want = ACCESS_ONCE(kvm->flush_counter.want) while ((done = atomic_read(kvm->flush_counter.done) < want) { kvm_make_request(flush) atomic_cmpxchg(kvm->flush_counter.done, done, want) } } -- error compiling committee.c: too many arguments to function