From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH RESEND 12/12] xl: numa-sched: enable specifying node-affinity in VM config file Date: Fri, 8 Nov 2013 10:49:55 +0100 Message-ID: <1383904195.12181.33.camel@Abyss> References: <20131105142844.30446.78671.stgit@Solace> <20131105143617.30446.53267.stgit@Solace> <21115.56695.827216.459048@mariner.uk.xensource.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2870764335355131257==" Return-path: In-Reply-To: <21115.56695.827216.459048@mariner.uk.xensource.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: Ian Jackson Cc: Marcus Granado , Keir Fraser , Ian Campbell , Li Yechen , George Dunlap , Andrew Cooper , Juergen Gross , xen-devel@lists.xen.org, Jan Beulich , Justin Weaver , Daniel De Graaf , Matt Wilson , Elena Ufimtseva List-Id: xen-devel@lists.xenproject.org --===============2870764335355131257== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-xEU6GOuLX9mStrKhjcih" --=-xEU6GOuLX9mStrKhjcih Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On gio, 2013-11-07 at 18:35 +0000, Ian Jackson wrote: > Dario Faggioli writes ("[PATCH RESEND 12/12] xl: numa-sched: enable speci= fying node-affinity in VM config file"): > > in a similar way to how it is possible to specify vcpu-affinity. > ... > > /* > > - * Check if the domain has any CPU affinity. If not, try to build > > - * up one. In case numa_place_domain() find at least a suitable > > - * candidate, it will affect info->nodemap accordingly; if it > > - * does not, it just leaves it as it is. This means (unless > > - * some weird error manifests) the subsequent call to > > - * libxl_domain_set_nodeaffinity() will do the actual placement, > > + * Check if the domain has any pinning or node-affinity and, if no= t, try > > + * to build up one. > > + * > > + * In case numa_place_domain() find at least a suitable candidate,= it will > > + * affect info->nodemap accordingly; if it does not, it just leave= s it as > > + * it is. This means (unless some weird error manifests) the subse= quent > > + * call to libxl_domain_set_nodeaffinity() will do the actual plac= ement, > > * whatever that turns out to be. > > */ > > if (libxl_defbool_val(info->numa_placement)) { > > =20 > > - if (!libxl_bitmap_is_full(&info->cpumap)) { > > + if (!libxl_bitmap_is_full(&info->cpumap) || > > + !libxl_bitmap_is_full(&info->nodemap)) { > > LOG(ERROR, "Can run NUMA placement only if no vcpu " > > - "affinity is specified"); > > + "pinning or node-affinity is specified"); >=20 > I appreciate I may be a bit late with this complaint, but surely this > approach (setting the cpumap and nodemap when they aren't > all-bits-set) means that it's not possible to explicitly set "all > cpus". >=20 The funny part is not that you're late, but to whom you're saying it! :-D :-D http://lists.xen.org/archives/html/xen-devel/2012-07/msg01077.html From: Ian Jackson Date: Mon, 23 Jul 2012 12:04:29 +0100 Dario Faggioli writes ("[PATCH 2 of 4 v6/leftover] xl: inform libxl if ther= e was a cpumap in the config file"): [...] Shouldn't this be - if (libxl_bitmap_is_full(&info->cpumap)) { + if (libxl_defbool_val(info->numa_placement)) { + if (!libxl_bitmap_is_full(&info->cpumap)) { + rc =3D ERROR_INVAL; + LOG blah blah invalid not supported + goto out; + } The story behind all this is that, back at that time, someone (and I'm sure it's you again, although I don't find the thread now) asked to introduce info->numa_placement, to have a mean for: (1) disabling NUMA placement completely; (2) help distinguishing the case where cpumap is full because we want "cpus=3Dall" from the one where it's full because we did not touched it, since being full is the default value. I remember finding it sound, and thus going for it. Perhaps we could have done it differently, but given that info->cpumap is full by default, I really don't see that many alternatives (apart from making cpumap empty by default, which may look even weirder). So, the idea here is: - if it's full, and placement is enabled, full means "do this for me", and that's what happens. - if placement is disabled, then nobody touches it, so if it was full it stay that way. So, for saying "all", the procedure is as follows: info->numa_placement =3D false; fill_bitmap(info->cpumap); > If I say in the config file "all" for cpus and "all" for nodes, this > code will override that. I don't think that can be right. >=20 And in fact, xl, when parsing the config file, turns numa_placement to false when finding a "cpus=3D" item. All the above applies to the new "nodes=3D" switch as well, of course. 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) --=-xEU6GOuLX9mStrKhjcih 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) iEYEABECAAYFAlJ8s8MACgkQk4XaBE3IOsRDKwCgob9FYouOeHr+Jx1vtbU+aIiI Je4AnAl6Bmr98AcwIfa1RQWSJTA8lQhO =Tiui -----END PGP SIGNATURE----- --=-xEU6GOuLX9mStrKhjcih-- --===============2870764335355131257== 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 --===============2870764335355131257==--