From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Date: Thu, 16 Dec 2010 09:11:59 +0000 Subject: Re: [PATCH 2/3] KVM: Centralize slots_lock aquisition during KVM_CREATE_IRQCHIP Message-Id: <4D09D7DF.3010209@redhat.com> List-Id: References: <20101216014326.9210211a.takuya.yoshikawa@gmail.com> In-Reply-To: <20101216014326.9210211a.takuya.yoshikawa@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: kvm-ia64@vger.kernel.org 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