From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: Re: [PATCH v4 06/10] KVM: MMU: fast path of handling guest page fault Date: Mon, 30 Apr 2012 23:31:45 -0300 Message-ID: <20120501023145.GC10142@amt.cnet> References: <4F9776D2.7020506@linux.vnet.ibm.com> <4F9777A4.208@linux.vnet.ibm.com> <20120426234535.GA5057@amt.cnet> <4F9A3445.2060305@linux.vnet.ibm.com> <20120427145213.GB28796@amt.cnet> <20120429175004.b54d8c095a60d98c8cdbc942@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Xiao Guangrong , Avi Kivity , LKML , KVM To: Takuya Yoshikawa Return-path: Content-Disposition: inline In-Reply-To: <20120429175004.b54d8c095a60d98c8cdbc942@gmail.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On Sun, Apr 29, 2012 at 05:50:04PM +0900, Takuya Yoshikawa wrote: > On Fri, 27 Apr 2012 11:52:13 -0300 > Marcelo Tosatti wrote: > > > Yes but the objective you are aiming for is to read and write sptes > > without mmu_lock. That is, i am not talking about this patch. > > Please read carefully the two examples i gave (separated by "example)"). > > The real objective is not still clear. > > The ~10% improvement reported before was on macro benchmarks during live > migration. At least, that optimization was the initial objective. > > But at some point, the objective suddenly changed to "lock-less" without > understanding what introduced the original improvement. > > Was the problem really mmu_lock contention? > > If the path being introduced by this patch is really fast, isn't it > possible to achieve the same improvement still using mmu_lock? Right. Supposedly, mmu_lock cacheline bouncing is the problem. Hum: $ pahole -C "kvm" /tmp/kvm.ko struct kvm { spinlock_t mmu_lock; /* 0 2 */ /* XXX 6 bytes hole, try to pack */ struct mutex slots_lock; /* 8 32 */ struct mm_struct * mm; /* 40 8 */ struct kvm_memslots * memslots; /* 48 8 */ struct srcu_struct srcu; /* 56 48 */ /* --- cacheline 1 boundary (64 bytes) was 40 bytes ago --- */ u32 bsp_vcpu_id; /* 104 4 */ /* XXX 4 bytes hole, try to pack */ Oops. False sharing? > Note: During live migration, the fact that the guest gets faulted is > itself a limitation. We could easily see noticeable slowdown of a > program even if it runs only between two GET_DIRTY_LOGs. > > > > The rules for code under mmu_lock should be: > > > > 1) Spte updates under mmu lock must always be atomic and > > with locked instructions. > > 2) Spte values must be read once, and appropriate action > > must be taken when writing them back in case their value > > has changed (remote TLB flush might be required). > > Although I am not certain about what will be really needed in the > final form, if this kind of maybe-needed-overhead is going to be > added little by little, I worry about possible regression. > > Thanks, > Takuya Yes, that is a possibility.