From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH v2 08/16] xen: derive NUMA node affinity from hard and soft CPU affinity Date: Thu, 14 Nov 2013 17:30:03 +0100 Message-ID: <1384446603.29902.170.camel@Abyss> References: <20131113190852.18086.5437.stgit@Solace> <20131113191214.18086.51921.stgit@Solace> <5284EA84.30506@eu.citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============7201234086797561750==" Return-path: In-Reply-To: <5284EA84.30506@eu.citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: George Dunlap Cc: Marcus Granado , Keir Fraser , Ian Campbell , Li Yechen , Andrew Cooper , Juergen Gross , Ian Jackson , xen-devel@lists.xen.org, Jan Beulich , Justin Weaver , Matt Wilson , Elena Ufimtseva List-Id: xen-devel@lists.xenproject.org --===============7201234086797561750== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-rLW4SkVL68/Co73SHlXj" --=-rLW4SkVL68/Co73SHlXj Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On gio, 2013-11-14 at 15:21 +0000, George Dunlap wrote: > > Signed-off-by: Dario Faggioli > > diff --git a/xen/common/domain.c b/xen/common/domain.c > > @@ -373,8 +379,12 @@ void domain_update_node_affinity(struct domain *d) > > =20 > > for_each_vcpu ( d, v ) > > { > > + /* Build up the mask of online pcpus we have hard affinity wit= h */ > > cpumask_and(online_affinity, v->cpu_hard_affinity, online); > > cpumask_or(cpumask, cpumask, online_affinity); > > + > > + /* As well as the mask of all pcpus we have soft affinity with= */ > > + cpumask_or(cpumask_soft, cpumask_soft, v->cpu_soft_affinity); >=20 > Is this really the most efficient way to do this? I would have thought= =20 > or'ing cpumask and cpumask_soft, and then if it's not empty, then use=20 > it; maybe use a pointer so you don't have to copy one into the other one? >=20 I'm not sure I fully get what you mean... I cannot afford neglecting online_affinity, independently from how cpumask and cpumask_soft look like, because that's what's needed to account for cpupools. Anyway, I'll think more about it and see if I can make it better. > Also, all of the above computation happens unconditionally, but is only= =20 > used if d->auto_node_affinity is true. It seems like it would be better= =20 > to move this calculation inside the conditional. >=20 That is true, and I thought it too. I didn't do this here because it seems an unrelated change (the calculation of cpumask|=3Dhard_affinity&online was outside of the if already). Anyway, again, I agree with this, so I'll either do it here or in a pre-patch. > Leaving the allocation outside might make sense to reduce the code=20 > covered by the lock -- not sure about that; I don't think this is a=20 > heavily-contended lock, is it? >=20 Not really, nor this is an hot-path, but I still think it's worth doing what you suggest. Thanks and Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-rLW4SkVL68/Co73SHlXj 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 v1.4.15 (GNU/Linux) iEYEABECAAYFAlKE+osACgkQk4XaBE3IOsRlSgCfRCPb4G+cqi/yw/9j+nRaN4G+ JukAoJ0p3O0cexK1h/El82p5AZEBjZZQ =5oKu -----END PGP SIGNATURE----- --=-rLW4SkVL68/Co73SHlXj-- --===============7201234086797561750== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============7201234086797561750==--