From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gregory Haskins Subject: Re: [PATCH 1/5] Protect irq_sources_bitmap by kvm->lock instead of kvm->irq_lock Date: Mon, 13 Jul 2009 10:29:02 -0400 Message-ID: <4A5B44AE.4000407@gmail.com> References: <1247476355-27284-1-git-send-email-gleb@redhat.com> <1247476355-27284-2-git-send-email-gleb@redhat.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig93F6CBEC9970AFAF58ABB3FE" Cc: kvm@vger.kernel.org, avi@redhat.com, mtosatti@redhat.com To: Gleb Natapov Return-path: Received: from mail-pz0-f197.google.com ([209.85.222.197]:64330 "EHLO mail-pz0-f197.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754083AbZGMO3L (ORCPT ); Mon, 13 Jul 2009 10:29:11 -0400 Received: by pzk35 with SMTP id 35so871803pzk.33 for ; Mon, 13 Jul 2009 07:29:10 -0700 (PDT) In-Reply-To: <1247476355-27284-2-git-send-email-gleb@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig93F6CBEC9970AFAF58ABB3FE Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Gleb Natapov wrote: > It is already protected by kvm->lock on device assignment path. Just > take the same lock in the PIT code. > > Signed-off-by: Gleb Natapov > --- > arch/x86/kvm/i8254.c | 2 ++ > virt/kvm/irq_comm.c | 8 ++++---- > 2 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/kvm/i8254.c b/arch/x86/kvm/i8254.c > index 05e00a8..e1b9016 100644 > --- a/arch/x86/kvm/i8254.c > +++ b/arch/x86/kvm/i8254.c > @@ -596,7 +596,9 @@ struct kvm_pit *kvm_create_pit(struct kvm *kvm, u32= flags) > if (!pit) > return NULL; > =20 > + mutex_lock(&kvm->lock); > pit->irq_source_id =3D kvm_request_irq_source_id(kvm); > + mutex_unlock(&kvm->lock); > if (pit->irq_source_id < 0) { > kfree(pit); > return NULL; > diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c > index 6c57e46..ce8fcd3 100644 > --- a/virt/kvm/irq_comm.c > +++ b/virt/kvm/irq_comm.c > @@ -210,7 +210,8 @@ int kvm_request_irq_source_id(struct kvm *kvm) > unsigned long *bitmap =3D &kvm->arch.irq_sources_bitmap; > int irq_source_id; > =20 > - mutex_lock(&kvm->irq_lock); > + WARN_ON(!mutex_is_locked(&kvm->lock)); > =20 Shouldn't this be fatal? (e.g. BUG_ON). I know the usage between BUG/WARN is controversial, but it seems to me that something is completely broken if you expect it to be locked and its not. Might as well fail the system, IMO. Regards, -Greg > + > irq_source_id =3D find_first_zero_bit(bitmap, > sizeof(kvm->arch.irq_sources_bitmap)); > =20 > @@ -221,7 +222,6 @@ int kvm_request_irq_source_id(struct kvm *kvm) > =20 > ASSERT(irq_source_id !=3D KVM_USERSPACE_IRQ_SOURCE_ID); > set_bit(irq_source_id, bitmap); > - mutex_unlock(&kvm->irq_lock); > =20 > return irq_source_id; > } > @@ -230,9 +230,10 @@ void kvm_free_irq_source_id(struct kvm *kvm, int i= rq_source_id) > { > int i; > =20 > + /* during vm destruction this function is called without locking */ > + WARN_ON(!mutex_is_locked(&kvm->lock) && atomic_read(&kvm->users_count= )); > ASSERT(irq_source_id !=3D KVM_USERSPACE_IRQ_SOURCE_ID); > =20 > - mutex_lock(&kvm->irq_lock); > if (irq_source_id < 0 || > irq_source_id >=3D sizeof(kvm->arch.irq_sources_bitmap)) { > printk(KERN_ERR "kvm: IRQ source ID out of range!\n"); > @@ -241,7 +242,6 @@ void kvm_free_irq_source_id(struct kvm *kvm, int ir= q_source_id) > for (i =3D 0; i < KVM_IOAPIC_NUM_PINS; i++) > clear_bit(irq_source_id, &kvm->arch.irq_states[i]); > clear_bit(irq_source_id, &kvm->arch.irq_sources_bitmap); > - mutex_unlock(&kvm->irq_lock); > } > =20 > void kvm_register_irq_mask_notifier(struct kvm *kvm, int irq, > =20 --------------enig93F6CBEC9970AFAF58ABB3FE Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG/MacGPG2 v2.0.11 (Darwin) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAkpbRK4ACgkQP5K2CMvXmqG9tQCfTJd16U3LobxRqDL8hQRbV+Ct PzQAnRgIXatZWehlnWCOPKdS4FUY3x/m =Pra/ -----END PGP SIGNATURE----- --------------enig93F6CBEC9970AFAF58ABB3FE--