From mboxrd@z Thu Jan 1 00:00:00 1970 From: Xiao Guangrong Subject: Re: [PATCH 1/4] KVM: Add APIs for unlocked TLB flush Date: Mon, 07 May 2012 15:06:12 +0800 Message-ID: <4FA77464.4080300@linux.vnet.ibm.com> References: <1336044182-12023-1-git-send-email-avi@redhat.com> <1336044182-12023-2-git-send-email-avi@redhat.com> <4FA286BD.2020009@linux.vnet.ibm.com> <4FA291F5.4090408@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org, Marcelo Tosatti , takuya.yoshikawa@gmail.com To: Avi Kivity Return-path: Received: from e28smtp03.in.ibm.com ([122.248.162.3]:42364 "EHLO e28smtp03.in.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751839Ab2EGHOU (ORCPT ); Mon, 7 May 2012 03:14:20 -0400 Received: from /spool/local by e28smtp03.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 7 May 2012 12:36:26 +0530 Received: from d28av01.in.ibm.com (d28av01.in.ibm.com [9.184.220.63]) by d28relay02.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q4776GEq12779904 for ; Mon, 7 May 2012 12:36:17 +0530 Received: from d28av01.in.ibm.com (loopback [127.0.0.1]) by d28av01.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q47CZtxx023055 for ; Mon, 7 May 2012 18:05:56 +0530 In-Reply-To: <4FA291F5.4090408@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 05/03/2012 10:11 PM, Avi Kivity wrote: > On 05/03/2012 04:23 PM, Xiao Guangrong wrote: >> On 05/03/2012 07:22 PM, Avi Kivity wrote: >> >>> Currently we flush the TLB while holding mmu_lock. This >>> increases the lock hold time by the IPI round-trip time, increasing >>> contention, and makes dropping the lock (for latency reasons) harder. >>> >>> This patch changes TLB management to be usable locklessly, introducing >>> the following APIs: >>> >>> kvm_mark_tlb_dirty() - mark the TLB as containing stale entries >>> kvm_cond_flush_remote_tlbs() - flush the TLB if it was marked as >>> dirty >>> >>> These APIs can be used without holding mmu_lock (though if the TLB >>> became stale due to shadow page table modifications, typically it >>> will need to be called with the lock held to prevent other threads >>> from seeing the modified page tables with the TLB unmarked and unflushed)/ >>> >>> Signed-off-by: Avi Kivity >>> --- >>> Documentation/virtual/kvm/locking.txt | 14 ++++++++++++++ >>> arch/x86/kvm/paging_tmpl.h | 4 ++-- >>> include/linux/kvm_host.h | 22 +++++++++++++++++++++- >>> virt/kvm/kvm_main.c | 29 ++++++++++++++++++++--------- >>> 4 files changed, 57 insertions(+), 12 deletions(-) >>> >>> diff --git a/Documentation/virtual/kvm/locking.txt b/Documentation/virtual/kvm/locking.txt >>> index 3b4cd3b..f6c90479 100644 >>> --- a/Documentation/virtual/kvm/locking.txt >>> +++ b/Documentation/virtual/kvm/locking.txt >>> @@ -23,3 +23,17 @@ Arch: x86 >>> Protects: - kvm_arch::{last_tsc_write,last_tsc_nsec,last_tsc_offset} >>> - tsc offset in vmcb >>> Comment: 'raw' because updating the tsc offsets must not be preempted. >>> + >>> +3. TLB control >>> +-------------- >>> + >>> +The following APIs should be used for TLB control: >>> + >>> + - kvm_mark_tlb_dirty() - indicates that the TLB is out of sync wrt >>> + either guest or host page tables. >>> + - kvm_flush_remote_tlbs() - unconditionally flush the tlbs >>> + - kvm_cond_flush_remote_tlbs() - flush the TLBs if previously marked >>> + >>> +These may be used without mmu_lock, though kvm_mark_tlb_dirty() needs to be >>> +used while holding mmu_lock if it is called due to host page table changes >>> +(contrast to guest page table changes). >> >> >> In these patches, it seems that kvm_mark_tlb_dirty is always used >> under the protection of mmu-lock, yes? > > Correct. It's possible we'll find a use outside mmu_lock, but this > isn't likely. If we need call kvm_mark_tlb_dirty outside mmu-lock, just use kvm_flush_remote_tlbs instead: if (need-flush-tlb) flush = true; do something... if (flush) kvm_flush_remote_tlbs > >> If both kvm_mark_tlb_dirty and kvm_cond_flush_remote_tlbs are use >> out of mmu-lock, i think we can use kvm_flush_remote_tlbs instead. >> >> If it is so, dirtied_count/flushed_count need not be atomic. > > But we always mark with mmu_lock held. > Yes, so, we can change kvm_mark_tlb_dirty to: +static inline void kvm_mark_tlb_dirty(struct kvm *kvm) +{ + /* + * Make any changes to the page tables visible to remote flushers. + */ + smb_mb(); + kvm->tlb_state.dirtied_count++; +}