From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 2/3] KVM: Centralize slots_lock aquisition during KVM_CREATE_IRQCHIP Date: Thu, 16 Dec 2010 11:11:59 +0200 Message-ID: <4D09D7DF.3010209@redhat.com> References: <20101216013917.a8a8bab9.takuya.yoshikawa@gmail.com> <20101216014326.9210211a.takuya.yoshikawa@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: mtosatti@redhat.com, kvm@vger.kernel.org, yoshikawa.takuya@oss.ntt.co.jp, kvm-ia64@vger.kernel.org To: Takuya Yoshikawa Return-path: Received: from mx1.redhat.com ([209.132.183.28]:64793 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752801Ab0LPJMH (ORCPT ); Thu, 16 Dec 2010 04:12:07 -0500 In-Reply-To: <20101216014326.9210211a.takuya.yoshikawa@gmail.com> Sender: kvm-owner@vger.kernel.org List-ID: On 12/15/2010 06:43 PM, Takuya Yoshikawa wrote: > From: Takuya Yoshikawa > > Move slots_lock aquisition from kvm_ioapic_init() and kvm_create_pic() > to their caller. > > As a result, x86's KVM_CREATE_IRQCHIP is now covered by a unified slots_lock > section, including kvm_setup_default_irq_routing(). > > I'm not sure about this... > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -3300,6 +3300,7 @@ long kvm_arch_vm_ioctl(struct file *filp, > struct kvm_pic *vpic; > > mutex_lock(&kvm->lock); > + mutex_lock(&kvm->slots_lock); here, the reader might wonder why we take slots_lock, since we aren't manipulating anything covered by the lock directly. > r = -EEXIST; > if (kvm->arch.vpic) > goto create_irqchip_unlock; > @@ -3308,10 +3309,8 @@ long kvm_arch_vm_ioctl(struct file *filp, > if (vpic) { > r = kvm_ioapic_init(kvm); > if (r) { > - mutex_lock(&kvm->slots_lock); and here, the reader might wonder why we don't take slots_lock, which protects io_bus. Maybe we ought to move slots_lock acquisition to kvm_io_bus_register() and friends. -- error compiling committee.c: too many arguments to function