From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH V5 03/32] xl / libxl: push VCPU affinity pinning down to libxl Date: Thu, 15 May 2014 17:31:02 +0200 Message-ID: <1400167862.5960.3.camel@Solace> References: <1400018054-26038-1-git-send-email-wei.liu2@citrix.com> <1400018054-26038-4-git-send-email-wei.liu2@citrix.com> <1400115553.20215.9.camel@Solace> <20140515092452.GA578@zion.uk.xensource.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0995638260029606598==" Return-path: In-Reply-To: <20140515092452.GA578@zion.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: Wei Liu Cc: ian.jackson@eu.citrix.com, ian.campbell@citrix.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org --===============0995638260029606598== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-SaS+WtE3ajETRnRZWA1x" --=-SaS+WtE3ajETRnRZWA1x Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On gio, 2014-05-15 at 10:24 +0100, Wei Liu wrote: > > One question: I've skimmed through the rest of the series, and it looks > > like you are not doing anything particular with the other maps, already > > existing in b_info (cpumap and nodemap) yet, is that right? > >=20 >=20 > Correct. I found out it's not necessary to touch them. >=20 Great! I looked at the code and gave it a quick try and came to similar conclusions. > > > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c > > > index 661999c..b818815 100644 > > > --- a/tools/libxl/libxl_dom.c > > > +++ b/tools/libxl/libxl_dom.c > > > @@ -263,6 +263,54 @@ int libxl__build_pre(libxl__gc *gc, uint32_t dom= id, > > > libxl_domain_set_nodeaffinity(ctx, domid, &info->nodemap); > > > libxl_set_vcpuaffinity_all(ctx, domid, info->max_vcpus, &info->c= pumap); > > > =20 > > > + /* If we have vcpu affinity list, pin vcpu to pcpu. */ > > > + if (d_config->b_info.vcpu_affinity) { > > > + int i; > > > + libxl_bitmap vcpu_cpumap; > > > + int *vcpu_to_pcpu, sz =3D sizeof(int) * d_config->b_info.max= _vcpus; > > > + > > > + vcpu_to_pcpu =3D libxl__zalloc(gc, sz); > > > + memset(vcpu_to_pcpu, -1, sz); > > > + > > > + for (i =3D 0; i < d_config->b_info.max_vcpus; i++) { > > > + libxl_key_value_list kvl =3D d_config->b_info.vcpu_affin= ity; > > > + const char *key, *val; > > > + int k, v; > > > + > > > + key =3D kvl[i * 2]; > > > + if (!key) > > > + break; > > > + val =3D kvl[i * 2 + 1]; > > > + > > > + k =3D atoi(key); > > > + v =3D atoi(val); > > > + vcpu_to_pcpu[k] =3D v; > > > + } > > > + > > > + rc =3D libxl_cpu_bitmap_alloc(ctx, &vcpu_cpumap, 0); > > > + if (rc) { > > > + libxl_bitmap_dispose(&vcpu_cpumap); > > > + return ERROR_FAIL; > > > + } > > > + > > Perhaps you can move this up, i.e., try the allocation first so that, i= f > > it fails, you can bail before getting into decoding the key/value list? > >=20 >=20 > That's more or less a personal taste... >=20 Indeed it is. Regards, Dario > > Also, if libxl_cpu_bitmap_alloc() fails, do you really have to call > > libxl_bitmap_dispose()? I thought not. > >=20 >=20 > The paradigm of using libxl_type is to always call dispose.=20 >=20 Alright, according to that same usage paradigm, aren't you missing an libxl_bitmap_init(&vcpu_cpumap) then? Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-SaS+WtE3ajETRnRZWA1x 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.0.22 (GNU/Linux) iEYEABECAAYFAlN03bYACgkQk4XaBE3IOsR3IwCglp+dOeiWEE14DlSqN7jOxg7V DeoAnAmxNIbUyJ76fuzd2mAZZUB53K7g =pHJJ -----END PGP SIGNATURE----- --=-SaS+WtE3ajETRnRZWA1x-- --===============0995638260029606598== 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 --===============0995638260029606598==--