From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH v3 for Xen 4.6 3/4] libxl: enable per-VCPU parameter settings for RTDS scheduler Date: Tue, 7 Jul 2015 18:23:56 +0200 Message-ID: <1436286236.22672.149.camel@citrix.com> References: <1435545899-22751-1-git-send-email-chong.li@wustl.edu> <1435545899-22751-4-git-send-email-chong.li@wustl.edu> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============7319147022241071939==" Return-path: In-Reply-To: <1435545899-22751-4-git-send-email-chong.li@wustl.edu> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Chong Li Cc: Chong Li , wei.liu2@citrix.com, Sisu Xi , george.dunlap@eu.citrix.com, ian.jackson@eu.citrix.com, xen-devel@lists.xen.org, ian.campbell@eu.citrix.com, Meng Xu , dgolomb@seas.upenn.edu List-Id: xen-devel@lists.xenproject.org --===============7319147022241071939== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-hVQpgX10EcIh3b40453G" --=-hVQpgX10EcIh3b40453G Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Sun, 2015-06-28 at 21:44 -0500, Chong Li wrote: > Add libxl_vcpu_sched_params_get/set and sched_rtds_vcpu_get/set functions= to support > per-VCPU settings. >=20 > Changes on PATCH v2: >=20 > 1) New data structure (libxl_vcpu_sched_params and libxl_sched_params) to= help per-VCPU settings. >=20 > 2) sched_rtds_vcpu_get now can return a random subset of the parameters o= f the VCPUs of a specific domain. >=20 > Signed-off-by: Chong Li > Signed-off-by: Meng Xu > Signed-off-by: Sisu Xi >=20 So, apart from what IanC said already, I think this patch is mostly fine, apart from: - the changelog needing to improve, as explained already while reviewing one other patch in the series (that, in fact, applies to all of them) - comments/documentation about the new behavior of per domain scheduling API calls looks to be missing to me... - some coding style issues (see below). > +static int sched_rtds_vcpu_get(libxl__gc *gc, uint32_t domid, > + libxl_vcpu_sched_params *scinfo) > +{ > + uint16_t num_vcpus; > + int rc, i; > + xc_dominfo_t info; > + xen_domctl_schedparam_vcpu_t *vcpus; > + > + if (scinfo->num_vcpus =3D=3D 0) { > + rc =3D xc_domain_getinfo(CTX->xch, domid, 1, &info); > + if (rc < 0) { > + LOGE(ERROR, "getting domain info"); > + return ERROR_FAIL; > + } > + num_vcpus =3D info.max_vcpu_id + 1; > + } else > + num_vcpus =3D scinfo->num_vcpus; > + > + GCNEW_ARRAY(vcpus, num_vcpus); > + > + if (scinfo->num_vcpus > 0) > + for(i=3D0; i < num_vcpus; i++) spaces ("for (...") > +static int sched_rtds_vcpu_set(libxl__gc *gc, uint32_t domid, > + const libxl_vcpu_sched_params *scinfo) > +{ > + int rc; > + int i; > + uint16_t max_vcpuid; > + xc_dominfo_t info; > + xen_domctl_schedparam_vcpu_t *vcpus; > + int num_vcpus; > + > + rc =3D xc_domain_getinfo(CTX->xch, domid, 1, &info); > + if (rc < 0) { > + LOGE(ERROR, "getting domain info"); > + return ERROR_FAIL; > + } > + max_vcpuid =3D info.max_vcpu_id; > + > + if (scinfo->num_vcpus > 0) { > + num_vcpus =3D scinfo->num_vcpus; > + GCNEW_ARRAY(vcpus, num_vcpus); > + for (i =3D 0; i < num_vcpus; i++) { > + if (scinfo->vcpus[i].vcpuid < 0 || > + scinfo->vcpus[i].vcpuid > max_vcpuid) { > + LOG(ERROR, "VCPU index is out of range, " > + "valid values are within range from 0 to %d", > + max_vcpuid); > + return ERROR_INVAL; > + } > + vcpus[i].vcpuid =3D scinfo->vcpus[i].vcpuid; > + > + rc =3D sched_rtds_validate_params(gc, > + scinfo->vcpus[i].period, scinfo->vcpus[i].budget, > + &vcpus[i].s.rtds.period, &vcpus[i].s.rtds.budget); > + if (rc) > + return ERROR_INVAL; > + } > + } else { > + num_vcpus =3D max_vcpuid + 1; > + GCNEW_ARRAY(vcpus, num_vcpus); > Indentation. > --- a/tools/libxl/libxl.h > +++ b/tools/libxl/libxl.h > @@ -200,6 +200,16 @@ > #define LIBXL_HAVE_DEVICETREE_PASSTHROUGH 1 > =20 > /* > + * libxl_vcpu_sched_params is used to get/set per-vcpu params > +*/ > +#define LIBXL_HAVE_VCPU_SCHED_PARAMS 1 > + > +/* > + * libxl_sched_params is used to store per-vcpu params > +*/ > +#define LIBXL_SCHED_PARAMS 1 > + > +/* > * libxl ABI compatibility > * > * The only guarantee which libxl makes regarding ABI compatibility > @@ -1554,10 +1564,17 @@ int libxl_sched_credit_params_set(libxl_ctx *ctx,= uint32_t poolid, > #define LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT -1 > #define LIBXL_DOMAIN_SCHED_PARAM_BUDGET_DEFAULT -1 > =20 > +/* Per-VCPU parameters*/ > +#define LIBXL_DOMAIN_SCHED_PARAM_VCPU_INDEX_DEFAULT -1 > + > int libxl_domain_sched_params_get(libxl_ctx *ctx, uint32_t domid, > libxl_domain_sched_params *params); > int libxl_domain_sched_params_set(libxl_ctx *ctx, uint32_t domid, > const libxl_domain_sched_params *param= s); > +int libxl_vcpu_sched_params_get(libxl_ctx *ctx, uint32_t domid, > + libxl_vcpu_sched_params *params); > +int libxl_vcpu_sched_params_set(libxl_ctx *ctx, uint32_t domid, > + const libxl_vcpu_sched_params *params)= ; > Didn't we say that we wanted the fact that now, at least for RTDS, libxl_domain_sched_params_*() deals with default parameters to be documented in a comment somewhere? Have I missed it? I appreciate that this is happening in lower levels, and that libxl is just reporting it, but, still, it is something that I would like to find in the headers of the library, in order to be able to tell the differences between libxl_vcpu_sched_params_* and libxl_domain_sched_params_* ... Ian? Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-hVQpgX10EcIh3b40453G Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iEYEABECAAYFAlWb/RwACgkQk4XaBE3IOsQ65QCfVBmRPAR3+VSz9kKPk87mL8Db gq8AoJWNzwfEEcpSBLdfmHSt0O84CTg0 =sUF2 -----END PGP SIGNATURE----- --=-hVQpgX10EcIh3b40453G-- --===============7319147022241071939== 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 --===============7319147022241071939==--