From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH RESEND 09/12] libxc: numa-sched: enable getting/specifying per-vcpu node-affinity Date: Tue, 12 Nov 2013 19:40:24 +0100 Message-ID: <1384281624.16918.26.camel@Solace> References: <20131105142844.30446.78671.stgit@Solace> <20131105143542.30446.5398.stgit@Solace> <20131112160144.GB11354@phenom.dumpdata.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0166521820040617495==" Return-path: In-Reply-To: <20131112160144.GB11354@phenom.dumpdata.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: Konrad Rzeszutek Wilk Cc: Marcus Granado , Keir Fraser , Ian Campbell , Li Yechen , George Dunlap , Andrew Cooper , Juergen Gross , Ian Jackson , xen-devel@lists.xen.org, Jan Beulich , Justin Weaver , Daniel De Graaf , Matt Wilson , Elena Ufimtseva List-Id: xen-devel@lists.xenproject.org --===============0166521820040617495== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-bqDekdvmGQLCFZ6vykjK" --=-bqDekdvmGQLCFZ6vykjK Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On mar, 2013-11-12 at 11:01 -0500, Konrad Rzeszutek Wilk wrote: > On Tue, Nov 05, 2013 at 03:35:42PM +0100, Dario Faggioli wrote: > > by providing the proper get/set interfaces and wiring them > > to the new domctl-s from the previous commit. >=20 > s/previous commit// >=20 Ok. > > diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c > > +int xc_vcpu_setnodeaffinity(xc_interface *xch, > > + uint32_t domid, > > + int vcpu, > > + xc_nodemap_t nodemap) > > +{ > > + DECLARE_DOMCTL; > > + DECLARE_HYPERCALL_BUFFER(uint8_t, local); > > + int ret =3D -1; >=20 > Ewww.. Could we just use regular -Exx for new xc_* calls? >=20 Mmm... As George said already, that is the most common behavior in libxc... If we want something different, we should really write it down somewhere (or has that happened already and I'm missing it?). Also, yes, this does return -1 in cases where I don't have an error code provided by the call that is actually failing, and yes, I totally rely on it to set errno properly. In the main operation implemented here, I just return the output of do_domctl(), which, again, may be -1, may be some err-code, but it really looks like it is what every other function there does, and, TBH, it also seems the very most sane thing to do to me. > > +int xc_vcpu_getnodeaffinity(xc_interface *xch, > > + uint32_t domid, > > + int vcpu, > > + xc_nodemap_t nodemap) > > +{ > > + DECLARE_DOMCTL; > > + DECLARE_HYPERCALL_BUFFER(uint8_t, local); > > + int ret =3D -1; > > + int nodesize; > > + > > + nodesize =3D xc_get_nodemap_size(xch); > > + if (!nodesize) > > + { > > + PERROR("Could not get number of nodes"); > > + goto out; > > + } > > + > > + local =3D xc_hypercall_buffer_alloc(xch, local, nodesize); > > + if (local =3D=3D NULL) > > + { > > + PERROR("Could not allocate memory for getvcpunodeaffinity domc= tl hypercall"); > > + goto out; > > + } > > + > > + domctl.cmd =3D XEN_DOMCTL_getvcpunodeaffinity; > > + domctl.domain =3D (domid_t)domid; > > + domctl.u.vcpuaffinity.vcpu =3D vcpu; > > + > > + set_xen_guest_handle(domctl.u.vcpuaffinity.map.bitmap, local); > > + domctl.u.vcpuaffinity.map.nr_bits =3D nodesize * 8; >=20 > Could the 8 be replaced by a sizeof? > I guess it could... What was it that you had in mind in particular? Personally, I consider the names 'bitmap' and 'nr_bits' talking enough to feel comfortable with the 8... To the point that I think the following would be even less readable: domctl.u.vcpuaffinity.map.nr_bits =3D nodesize *=20 sizeof(domctl.u.vcpuaffinity.cpumap.bitmap); But, of course, I'd do it that way if a maintainer asks for that. > > diff --git a/tools/libxc/xenctrl.h b/tools/libxc/xenctrl.h > > index 8cf3f3b..208fa2c 100644 > > --- a/tools/libxc/xenctrl.h > > +++ b/tools/libxc/xenctrl.h > > @@ -551,6 +551,25 @@ int xc_domain_node_getaffinity(xc_interface *xch, > > uint32_t domind, > > xc_nodemap_t nodemap); > > =20 > > +/** > > + * These functions set and retrieves the NUMA node-affinity > > + * of a specific vcpu. > > + * > > + * @parm xch a handle to an open hypervisor interface. > > + * @parm domid the domain id one is interested in. > > + * @parm vcpu the vcpu one wants to set/get the affinity of. > > + * @parm nodemap the map of the affine nodes. > > + * @return 0 on success, -1 on failure. >=20 > and something in errno? > Right. I'll add a mention to that in the comment. Thanks and Regards, Dario --=20 <<This happens because I choose it to happen!>> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-bqDekdvmGQLCFZ6vykjK 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) iEYEABECAAYFAlKCdhgACgkQk4XaBE3IOsRMLACeJsav92XXKdECvc6Ncdk6piLU fOIAniJGcrdO457oZnA09zBknMehQj7n =WO0o -----END PGP SIGNATURE----- --=-bqDekdvmGQLCFZ6vykjK-- --===============0166521820040617495== 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 --===============0166521820040617495==--