From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH] xl: introduce specific VCPU to PCPU mapping in config file Date: Mon, 14 May 2012 19:03:37 +0200 Message-ID: <1337015017.28326.50.camel@Solace> References: <20943633a63e0691272b.1336778417@Solace> <1336983554.31817.8.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2142793801508101071==" Return-path: In-Reply-To: <1336983554.31817.8.camel@zakaz.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 Campbell Cc: Ian Jackson , "xen-devel@lists.xen.org" List-Id: xen-devel@lists.xenproject.org --===============2142793801508101071== Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature"; boundary="=-gbDDJl4CzWVCEcdeyeIW" --=-gbDDJl4CzWVCEcdeyeIW Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, 2012-05-14 at 09:19 +0100, Ian Campbell wrote: > So effectively we create the domain pinned to the right nodes and then > rebind all the CPUS later to be mapped to specific phys-processors?=20 > Yep. Although that map might be considered a build info, it seems more a low toolstack level thing to me. Besides, we already have a cpumap. Besides, it's a fu*$ing array of ints as big as the number of vcpus... And fo all these reasons I wanted to avoid putting it in build_info as bad as hell! :-) > This > means that the memory which is allocated is from the correct nodes, even > though we appear to do the pinning later. Clever. >=20 Thanks. All the above being true, I also didn't want to miss the memory/node affinity side effect having the correct info in b_info->cpumap brings. > > + /* If single vcpu to pcpu mapping was requested, honour it */ > > + if (vcpu_to_pcpu) { >=20 > This is always allocated above, isn't it? I'm concerned that this might > break the non-1-1 case. >=20 Not really, it's only allocated in case the correct `cpus=3D` specifier is found (that being `cpus=3D[2, 3]`). > > + libxl_cpumap vcpu_cpumap; > > + > > + libxl_cpumap_alloc(ctx, &vcpu_cpumap); > > + for (i =3D 0; i < d_config.b_info.max_vcpus; i++) { > > + > > + if (vcpu_to_pcpu[i] !=3D -1) { > > + libxl_cpumap_set_none(&vcpu_cpumap); > > + libxl_cpumap_set(&vcpu_cpumap, vcpu_to_pcpu[i]); > > + } else { > > + libxl_cpumap_set_any(&vcpu_cpumap); > > + } > > + if (libxl_set_vcpuaffinity(ctx, domid, i, &vcpu_cpumap)) { > > + fprintf(stderr, "setting affinity failed on vcpu `%d'.= \n", i); > > + libxl_cpumap_dispose(&vcpu_cpumap); > > + free(vcpu_to_pcpu); > > + ret =3D ERROR_FAIL; > > + goto error_out; > > + } > > + } > > + libxl_cpumap_dispose(&vcpu_cpumap); > > + free(vcpu_to_pcpu); >=20 > vpuc_to_pcpu =3D NULL, in case you go back around again... >=20 Yep. This is needed, thanks! > For that reason it might be preferable to put vcpu_to_pcpu struct > dom_info and pass that to parse_config -- I think Goncalo is doing > something similar for the vncviewer option. >=20 Adding the proper =3DNULL above makes it work for all the cases, namely config-update, save/restore and migration. Nevertheless, I looked into moving the array in `struct domain_create', but it doesn't fit there, at least according to my personal taste. It requires adding a parameter to parse_config_data() that, in many cases, would need to be created on purpose with only that field as the meaningful one (or just be NULL), resulting in something really counter intuitive and hard to read and maintain (again, according to my personal taste :-)). Also, this is somehow different from "autoconnect" or "vncviewer", which fits perfectly there, as they're parameters to a specific create-ish command resulting in some actual action (i.e., popping the console or the VNC window up!). So, I'm resending with the array still as a global variable, but of course I'm open to rework the patch again if you and/or the other maintainers want it the other way around (within `struct domain_create'). Thanks a lot for looking into this. Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-gbDDJl4CzWVCEcdeyeIW 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.12 (GNU/Linux) iEYEABECAAYFAk+xOuoACgkQk4XaBE3IOsStMwCcCZYqptMd9NELQ3snKN/dByVM A5gAn1a9NQqF1oZXfS1h+TZqstgUq+gW =htzd -----END PGP SIGNATURE----- --=-gbDDJl4CzWVCEcdeyeIW-- --===============2142793801508101071== 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 --===============2142793801508101071==--