From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH v1 3/4] libxl: add rt scheduler Date: Fri, 5 Sep 2014 19:41:44 +0200 Message-ID: <1409938904.16667.9.camel@Solace.lan> References: <1408921125-21470-1-git-send-email-mengxu@cis.upenn.edu> <1408921125-21470-4-git-send-email-mengxu@cis.upenn.edu> <1409912467.2673.288.camel@Solace.lan> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============8485023490274834404==" Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Meng Xu Cc: Ian Campbell , Sisu Xi , Stefano Stabellini , George Dunlap , Ian Jackson , "xen-devel@lists.xen.org" , Meng Xu , Jan Beulich , Chao Wang , Chong Li , Dagaen Golomb List-Id: xen-devel@lists.xenproject.org --===============8485023490274834404== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-I0ViGDb8USWE7BQ7URcj" --=-I0ViGDb8USWE7BQ7URcj Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On ven, 2014-09-05 at 11:45 -0400, Meng Xu wrote: > 2014-09-05 6:21 GMT-04:00 Dario Faggioli : > > On dom, 2014-08-24 at 18:58 -0400, Meng Xu wrote: > >> --- a/tools/libxl/libxl.c > >> +++ b/tools/libxl/libxl.c > >> @@ -5154,6 +5154,139 @@ static int sched_sedf_domain_set(libxl__gc *gc= , uint32_t domid, > >> return 0; > >> } > >> > >> +static int sched_rt_domain_get(libxl__gc *gc, uint32_t domid, > >> + libxl_domain_sched_params *scinfo) > >> +{ > >> + struct xen_domctl_sched_rt_params* sdom; > >> + uint16_t num_vcpus; > >> + int rc, i; > >> + > >> + rc =3D xc_sched_rt_domain_get_num_vcpus(CTX->xch, domid, &num_vcp= us); > >> + if (rc !=3D 0) { > >> + LOGE(ERROR, "getting num_vcpus of domain sched rt"); > >> + return ERROR_FAIL; > >> + } > >> > > As George pointed out already, you can get the num_vcpus via > > xc_domain_getinfo(). > > > > I agree with George's review about the rest of this function > > (appropriately updated to take what we decided about the interface into > > account :-) ). > > > >> +#define SCHED_RT_VCPU_PERIOD_MAX 31536000000000 /* one year in mic= rosecond*/ > >> +#define SCHED_RT_VCPU_BUDGET_MAX SCHED_RT_VCPU_PERIOD_MAX > >> + > > This may be me not remembering correctly the outcome of a preceding > > discussion... did we say we were going with _RT_ or with something like > > _RTDS_ ? > > > > ISTR the latter... >=20 > So I also need to change all RT to RT_DS in the tool stack, except > keeping the command 'xl sched-rt'? If so, I will do that. >=20 As I said in another email, the same reasons that called for renaming inside Xen, applies to the other layers, I think. I'm not sure about `xl sched-rt', but yes, I think that would be fine like that. For one, xl interface/commands are not set in stone. Moreover, when/if we'll have more policies, I think it could be fine to add a new 'policy' parameter to `xl sched-rt', rather than adding another `xl sched-xxx' thing. But then again, we can decide this later. > > > > Also, macros like INT_MAX, UINT_MAX, etc., or << and ~ "tricks" are, > > IMO, preferrable to the open coding of the value. >=20 > What does open coding of the value mean? Do you mean (2^32-1) instead > of 4294967295? >=20 I mean use INT_MAX, UINT_MAX, or similar, if possible. :-) > > > > Finally, I wonder whether these would better live in some headers, > > closer to the declaration of period and budget (where their type is als= o > > visible) and, as a nice side effect of that, available to libxl callers > > as well. >=20 > I actually considered the possibility of adding it to > xen/include/public/domctl.h, but other schedulers do not have such > range macro in the domctl.h, so I'm not sure if it will cause > inconsistence? >=20 This is toolstack stuff, libxl stuff, to be more precise, so the public header I'm mentioning is something like tools/libxl/libxl*.h. > Yes. Now I didn't use any array in toolstack or kernel to set/get > domain's parameters. So we don't use (budget =3D=3D 0 && period =3D=3D 0)= to > indicate the vcpus not changed. (Whenever user set a domain's vcpu's > parameters, we set all vcpus' parameters of this domain, so we don't > need such implication for 4.5 release. :-P) The array comes "only" > when we allow users to set/get a specific vcpu's parameters and vcpus > have different periods and budgets. >=20 True. I was thinking at something different, but hey, let's see the new version with the new interface implemented and comment on that! :-) regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-I0ViGDb8USWE7BQ7URcj 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 iEYEABECAAYFAlQJ9dgACgkQk4XaBE3IOsTwGwCgmT3HGDaHHfPlXHH66bIVYBqd gM0AnAkmXBvJqln1XYP+CvSjEQYI4vRC =eg6t -----END PGP SIGNATURE----- --=-I0ViGDb8USWE7BQ7URcj-- --===============8485023490274834404== 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 --===============8485023490274834404==--