From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH 3/3] VMX: Remove the vcpu from the per-cpu blocking list after domain termination Date: Mon, 23 May 2016 16:45:38 +0200 Message-ID: <1464014738.21930.60.camel@citrix.com> References: <1463734431-22353-1-git-send-email-feng.wu@intel.com> <1463734431-22353-4-git-send-email-feng.wu@intel.com> <1464006639.21930.51.camel@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1253839638797518609==" Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: "Wu, Feng" , "xen-devel@lists.xen.org" Cc: "Tian, Kevin" , "keir@xen.org" , "george.dunlap@eu.citrix.com" , "andrew.cooper3@citrix.com" , "jbeulich@suse.com" List-Id: xen-devel@lists.xenproject.org --===============1253839638797518609== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="=-Z6hDMe46QiM7Ux7ja9gv" --=-Z6hDMe46QiM7Ux7ja9gv Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, 2016-05-23 at 13:32 +0000, Wu, Feng wrote: >=C2=A0 > > > --- a/xen/arch/x86/hvm/vmx/vmx.c > > > +++ b/xen/arch/x86/hvm/vmx/vmx.c > > > @@ -248,6 +248,36 @@ void vmx_pi_hooks_deassign(struct domain *d) > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0d->arch.hvm_domain.vmx.pi_switch_to =3D= NULL; > > > =C2=A0} > > >=20 > > > +static void vmx_pi_blocking_list_cleanup(struct domain *d) > > > +{ > > > +=C2=A0=C2=A0=C2=A0=C2=A0unsigned int cpu; > > > + > > > +=C2=A0=C2=A0=C2=A0=C2=A0for_each_online_cpu ( cpu ) > > > +=C2=A0=C2=A0=C2=A0=C2=A0{ > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0struct vcpu *v; > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0unsigned long flags; > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0struct arch_vmx_stru= ct *vmx, *tmp; > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0spinlock_t *lock =3D= &per_cpu(vmx_pi_blocking, cpu).lock; > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0struct list_head *bl= ocked_vcpus =3D > > > &per_cpu(vmx_pi_blocking, > > > cpu).list; > > > + > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0spin_lock_irqsave(lo= ck, flags); > > > + > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0list_for_each_entry_= safe(vmx, tmp, blocked_vcpus, > > > pi_blocking.list) > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0{ > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0v =3D container_of(vmx, struct vcpu, arch.hvm_vmx); > > > + > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0if (v->domain =3D=3D d) > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0{ > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0list_del(&vmx->pi_blocking.list); > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0ASSERT(vmx->pi_blocking.lock =3D=3D lock); > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0vmx->pi_blocking.lock =3D NULL; > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0} > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0} > > > + > > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0spin_unlock_irqresto= re(lock, flags); > > > +=C2=A0=C2=A0=C2=A0=C2=A0} > > >=20 > > So, I'm probably missing something very ver basic, but I don't see > > what's the reason why we need this loop... can't we arrange for > > checking > >=20 > > =C2=A0list_empty(&v->arch.hvm_vmx.pi_blocking.list) > Yes, I also cannot find the reason why can't we use this good > suggestion, Except we need use list_del_init() instead of > list_del() in the current code.=20 > Yes, I saw that, and it's well worth doing that, to get rid of the loop. :-) > Or we can just check whether > ' vmx->pi_blocking.lock ' is NULL?=20 > I guess that will work as well. Still, if it were me doing this, I'd go for the list_del_init()/list_empty() approach. > I total don't know why I > missed it! :) >=20 :-) Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-Z6hDMe46QiM7Ux7ja9gv Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAABCAAGBQJXQxeSAAoJEBZCeImluHPuIFQQAOl33y/pMnqjKzrju6sQigH1 Mmphp7xXxDm3QY6FTVd1rrH1pD33vyc9VI5O9Cw200TVq5uYQm2aKizG4Hi2Ee16 Dd/2MspuzAWT293lS7PTCn8XnoIGsn9Z1IpEKsHMHa+e7hguIHL9122Ty6COboSy BwRHV/k5ehcpIm+mOESi5JV7jBH8DTAdRzLgEWMalVWC3kPxRGoYT5xLIPS5fI2Y ccw2MuEkQL1ZQqhGNVdeKDM9wws7h2mXZex7SeNj6VGGT2VOoNKc/+keHJqBBpPG 5c61nXaz8VgpPaK6KVL3/305ZFJ2BH0O0ce0csSUs+5aOONUATqMLdh9L75MeT6v m8pHHt4zxcV6JWkHvYSRpTJGnBdaE96B6uATcHpiyYhHcyHkV8zxhrfWFyUGpwRi WHRTgMpm9mvGDnoL6uyKGsUfeY/sR8CIxj0Dg9mpTgyvGIayKwlrXCyDsRX4Q1X/ uquttg+JI7pv0TAxtVT3eDeVuUE3Z9AI+qzjxP+nEDk8JrBWjTboxKMfy4Q4bvFQ DmrS+Ir8s92RMGlCo77E7OgM4pK1iVfCVkZhklw4/y7uE47qgqYJDxFZP22wNTik zZJ8NnY8DJL8c3AtzbbORCBNvIaPEo1SlKmpwMptOY/0dDCUrr/leSbWxV/MGB19 jz27kR+Widwf1MWEfaYA =FlHO -----END PGP SIGNATURE----- --=-Z6hDMe46QiM7Ux7ja9gv-- --===============1253839638797518609== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwOi8vbGlzdHMueGVuLm9y Zy94ZW4tZGV2ZWwK --===============1253839638797518609==--