From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH 1/2] xen: fix a (latent) cpupool-related race during domain destroy Date: Thu, 14 Jul 2016 16:54:53 +0200 Message-ID: <1468508093.13039.55.camel@citrix.com> References: <146847830772.25458.3001435525572823491.stgit@Solace.fritz.box> <146847849719.25458.17913062138178199099.stgit@Solace.fritz.box> <37bfb624-111a-32ed-ebfa-1f0be387dba0@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============7378275978309037380==" Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bNi2c-0002wW-1a for xen-devel@lists.xenproject.org; Thu, 14 Jul 2016 14:55:06 +0000 In-Reply-To: <37bfb624-111a-32ed-ebfa-1f0be387dba0@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Andrew Cooper , xen-devel@lists.xenproject.org Cc: Juergen Gross , George Dunlap , Jan Beulich List-Id: xen-devel@lists.xenproject.org --===============7378275978309037380== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="=-paMG0e20EoMWjYLxEsd8" --=-paMG0e20EoMWjYLxEsd8 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, 2016-07-14 at 10:37 +0100, Andrew Cooper wrote: > On 14/07/16 07:41, Dario Faggioli wrote: > >=20 > > So, during domain destruction, we do: > > =C2=A0cpupool_rm_domain()=C2=A0=C2=A0=C2=A0=C2=A0[ in domain_destroy() = ] > > =C2=A0sched_destroy_domain() [ in complete_domain_destroy() ] > >=20 > > Therefore, there's a window during which, from the > > scheduler's point of view, a domain is still there, but > > without it being part of any cpupool. > >=20 > > [...] > > > > On the other hand, cpupool_rm_domain() "only" does > > cpupool related bookkeeping, and there's no harm > > postponing it a little bit. > >=20 > > Finally, considering that, during domain initialization, > > we do: > > =C2=A0cpupool_add_domain() > > =C2=A0sched_init_domain() > >=20 > > It looks like it makes much more sense for the domain > > destroy path to look like the opposite of it, i.e.: > > =C2=A0sched_destroy_domain() > > =C2=A0cpupool_rm_domain() > >=20 > > This patch does that, and it's considered worth, as it > > fixes a bug, even if only a latent one. > >=20 > > Signed-off-by: Dario Faggioli > As the cpupool bookkeeping is very closely related to the scheduler > bookkeeping, how about having the sched_*_domain() functions involve > the > cpupool_*_domain() functions? >=20 That's certainly a good point. At minimum, I certainly can (and probably should have :-P) put a couple of ASSERT()-s in place. However, both cpupool_add_domain() and cpupool_rm_domain() are called only once, and I guess I can make them go into sched_init_domain() and sched_destroy_domain(), respectively. Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-paMG0e20EoMWjYLxEsd8 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 iQIcBAABCAAGBQJXh6e9AAoJEBZCeImluHPuTMYQAJWhsACX1FvvM1G9plQo5tcQ USObP6aMJ+kkk2iDCh1gGR9FpDC0KMxOBwOyYAClFRz5jbgzIZ8AXKuFSuhWWYgw Uklv8ZxTzOTYStD9gYNQhKaYCpDq+yIA2v3ZHiHxH791XB3ELZUEUwF/nvvX8w11 5OI6k3XdPxnZ4HE3OK+bzBAiOywrrym1Wtci7VE9lmZPtDZzoZseIQ//gyYYXCHS jqndKSFaEvRknr/6E3lFpk2xLLe3zxL55MQ1czTWeZte/tvBGRmp3YorzMEwXlpZ h+2jK2ZXEgdKZ7QzkPjOL73fW9a6HHsKR/99n6KwtRhPqdeRo5llvPDkCByTJnrA 7zmGxJeRiV5CVlXONEHEVEPOnjANbZF2qPRNzhzGkYPKCgHqXhnWQHua7HZN9gh6 M3IuhR3qgpLAuAucWAtMBH+1EfXp6kja0yuOOiPc0taNZx0nylMHqyvM1cwY9XIj qo/EFNU/zLB5GmNpvb3WG/3ORv/EcSPF/I9qwCWCddXJ00+sRxDnHQ3idMmD1lYb LXHDW82Iv9xCY2nADmyTiQQSOwc+h/8KFhTmUHIXkn7RNAfE9Ik8KhaS7RZn4IYv ZO2Qpbf/xNdj65GJ6syEuL8pz3nAYz7Fwb/U2WZuOmahXPmvHkTx/GWw7mfMbDxh vGvYhJ9xJMdSVLDUf8Vh =j0mM -----END PGP SIGNATURE----- --=-paMG0e20EoMWjYLxEsd8-- --===============7378275978309037380== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwczovL2xpc3RzLnhlbi5v cmcveGVuLWRldmVsCg== --===============7378275978309037380==--