From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: Re: [RFC] KVM: x86: conditionally acquire/release slots_lock on entry/exit Date: Thu, 27 Aug 2009 19:59:40 -0300 Message-ID: <20090827225940.GA13571@amt.cnet> References: <20090827012000.762063112@localhost.localdomain> <20090827155450.GA6312@amt.cnet> <4A96B404.5010500@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm@vger.kernel.org, Gleb Natapov To: Avi Kivity Return-path: Received: from mx1.redhat.com ([209.132.183.28]:14343 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751142AbZH0W7x (ORCPT ); Thu, 27 Aug 2009 18:59:53 -0400 Received: from int-mx04.intmail.prod.int.phx2.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.17]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id n7RMxtFh009835 for ; Thu, 27 Aug 2009 18:59:55 -0400 Content-Disposition: inline In-Reply-To: <4A96B404.5010500@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Thu, Aug 27, 2009 at 07:27:48PM +0300, Avi Kivity wrote: > On 08/27/2009 06:54 PM, Marcelo Tosatti wrote: >> perf report shows heavy overhead from down/up of slots_lock. >> >> Attempted to remove slots_lock by having vcpus stop on a synchronization >> point, but this introduced further complexity (a vcpu can be scheduled >> out before reaching the synchronization point, and can sched back in at >> points which are slots_lock protected, etc). >> >> This patch changes vcpu_enter_guest to conditionally release/acquire >> slots_lock in case a vcpu state bit is set. >> >> vmexit performance improves by 5-10% on UP guest. >> > > Sorry, it looks pretty complex. Why? > Have you considered using srcu? It seems to me down/up_read() can > be replaced by srcu_read_lock/unlock(), and after proper conversion > of memslots and io_bus to rcu_assign_pointer(), we can just add > synchronize_srcu() immediately after changing stuff (of course > mmu_lock still needs to be held when updating slots). I don't see RCU as being suitable because in certain operations you want to stop writers (on behalf of vcpus), do something, and let them continue afterwards. The dirty log, for example. Or any operation that wants to modify lockless vcpu specific data. Also, synchronize_srcu() is limited to preemptible sections. io_bus could use RCU, but I think being able to stop vcpus is also a different requirement. Do you have any suggestion on how to do it in a better way?