From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gregory Haskins Subject: Re: [PATCH 4/4] Convert irq notifiers lists to RCU locking. Date: Mon, 13 Jul 2009 09:26:21 -0400 Message-ID: <4A5B35FD.9090208@gmail.com> References: <1247400233-24243-1-git-send-email-gleb@redhat.com> <1247400233-24243-5-git-send-email-gleb@redhat.com> <20090713130256.GC10402@redhat.com> <20090713131128.GI28046@redhat.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig0EF1A4A97A1310F388AF7F74" Cc: "Michael S. Tsirkin" , avi@redhat.com, "kvm@vger.kernel.org" , paulmck@linux.vnet.ibm.com To: Gleb Natapov Return-path: Received: from mail-pz0-f197.google.com ([209.85.222.197]:60504 "EHLO mail-pz0-f197.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751751AbZGMN0a (ORCPT ); Mon, 13 Jul 2009 09:26:30 -0400 Received: by pzk35 with SMTP id 35so844702pzk.33 for ; Mon, 13 Jul 2009 06:26:29 -0700 (PDT) In-Reply-To: <20090713131128.GI28046@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig0EF1A4A97A1310F388AF7F74 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Gleb Natapov wrote: > On Mon, Jul 13, 2009 at 04:02:56PM +0300, Michael S. Tsirkin wrote: > =20 >> On Sun, Jul 12, 2009 at 03:03:53PM +0300, Gleb Natapov wrote: >> =20 >>> Use RCU locking for mask/ack notifiers lists. >>> >>> Signed-off-by: Gleb Natapov >>> --- >>> virt/kvm/irq_comm.c | 20 +++++++++++--------- >>> 1 files changed, 11 insertions(+), 9 deletions(-) >>> >>> diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c >>> index 5dde1ef..ba3a115 100644 >>> --- a/virt/kvm/irq_comm.c >>> +++ b/virt/kvm/irq_comm.c >>> @@ -179,18 +179,18 @@ void kvm_notify_acked_irq(struct kvm *kvm, unsi= gned irqchip, unsigned pin) >>> break; >>> } >>> } >>> - rcu_read_unlock(); >>> =20 >>> - hlist_for_each_entry(kian, n, &kvm->irq_ack_notifier_list, link) >>> + hlist_for_each_entry_rcu(kian, n, &kvm->irq_ack_notifier_list, link= ) >>> if (kian->gsi =3D=3D gsi) >>> kian->irq_acked(kian); >>> + rcu_read_unlock(); >>> } >>> =20 >>> void kvm_register_irq_ack_notifier(struct kvm *kvm, >>> struct kvm_irq_ack_notifier *kian) >>> { >>> mutex_lock(&kvm->irq_lock); >>> - hlist_add_head(&kian->link, &kvm->irq_ack_notifier_list); >>> + hlist_add_head_rcu(&kian->link, &kvm->irq_ack_notifier_list); >>> mutex_unlock(&kvm->irq_lock); >>> } >>> =20 >>> @@ -198,8 +198,9 @@ void kvm_unregister_irq_ack_notifier(struct kvm *= kvm, >>> struct kvm_irq_ack_notifier *kian) >>> { >>> mutex_lock(&kvm->irq_lock); >>> - hlist_del_init(&kian->link); >>> + hlist_del_init_rcu(&kian->link); >>> mutex_unlock(&kvm->irq_lock); >>> + synchronize_rcu(); >>> } >>> =20 >> This is done under kvm->lock still, which means the lock might be held= >> potentially for a very long time. Can synchronize_rcu be moved out of >> this lock? >> >> =20 > Only if kvm_free_assigned_device() will be moved out of this lock. > Device de-assignment is not very frequent event though. How long do you= > think it may be held? KVM RCU read sections are very brief. > =20 Note that the delay imposed by the barrier is not only related to the length of the critical section. The barrier blocks until the next grace period, and depending on the type of RCU you are using and your config options, this could be multiple milliseconds. I am not saying that this is definitely a problem for your design. I am just pointing out that the length of the KVM-RCU read section is only one of several factors that influence the ultimate duration of your kvm->lock CS. IIUC, in an ideally designed subsystem, the read-side CS will be dwarfed by the length of the grace, so the grace (and barriers against it) are really the critical factor of this type of design. I am CC'ing Paul in case I am saying something dumb/untrue. ;) Kind Regards, -Greg > =20 >>> int kvm_request_irq_source_id(struct kvm *kvm) >>> @@ -246,7 +247,7 @@ void kvm_register_irq_mask_notifier(struct kvm *k= vm, int irq, >>> { >>> mutex_lock(&kvm->irq_lock); >>> kimn->irq =3D irq; >>> - hlist_add_head(&kimn->link, &kvm->mask_notifier_list); >>> + hlist_add_head_rcu(&kimn->link, &kvm->mask_notifier_list); >>> mutex_unlock(&kvm->irq_lock); >>> } >>> =20 >>> @@ -254,8 +255,9 @@ void kvm_unregister_irq_mask_notifier(struct kvm = *kvm, int irq, >>> struct kvm_irq_mask_notifier *kimn) >>> { >>> mutex_lock(&kvm->irq_lock); >>> - hlist_del(&kimn->link); >>> + hlist_del_rcu(&kimn->link); >>> mutex_unlock(&kvm->irq_lock); >>> + synchronize_rcu(); >>> } >>> =20 >>> void kvm_fire_mask_notifiers(struct kvm *kvm, int irq, bool mask) >>> @@ -263,11 +265,11 @@ void kvm_fire_mask_notifiers(struct kvm *kvm, i= nt irq, bool mask) >>> struct kvm_irq_mask_notifier *kimn; >>> struct hlist_node *n; >>> =20 >>> - WARN_ON(!mutex_is_locked(&kvm->irq_lock)); >>> - >>> - hlist_for_each_entry(kimn, n, &kvm->mask_notifier_list, link) >>> + rcu_read_lock(); >>> + hlist_for_each_entry_rcu(kimn, n, &kvm->mask_notifier_list, link) >>> if (kimn->irq =3D=3D irq) >>> kimn->func(kimn, mask); >>> + rcu_read_unlock(); >>> } >>> =20 >>> void kvm_free_irq_routing(struct kvm *kvm) >>> --=20 >>> 1.6.2.1 >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe kvm" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> =20 > > -- > Gleb. > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > =20 --------------enig0EF1A4A97A1310F388AF7F74 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 iEYEARECAAYFAkpbNf0ACgkQP5K2CMvXmqHyfwCdFT66YJXzqTlLY6WNK3/iQidV 84IAnj8fLFczqVa04N3KY/kxU7EALGkP =AZGx -----END PGP SIGNATURE----- --------------enig0EF1A4A97A1310F388AF7F74--