From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [PATCH v2 for Xen 4.6 3/4] libxl: enabling XL to set per-VCPU parameters of a domain for RTDS scheduler Date: Mon, 8 Jun 2015 17:56:24 +0200 Message-ID: <1433778984.2403.27.camel@citrix.com> References: <1432598984-20914-1-git-send-email-chong.li@wustl.edu> <1433504253.7108.231.camel@citrix.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============5325700995936822196==" Return-path: In-Reply-To: <1433504253.7108.231.camel@citrix.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: Chong Li , wei.liu2@citrix.com, Sisu Xi , george.dunlap@eu.citrix.com, ian.jackson@eu.citrix.com, xen-devel@lists.xen.org, mengxu@cis.upenn.edu, Chong Li , dgolomb@seas.upenn.edu List-Id: xen-devel@lists.xenproject.org --===============5325700995936822196== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-QPeddTkzNJTL57TUCzM3" --=-QPeddTkzNJTL57TUCzM3 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, 2015-06-05 at 12:37 +0100, Ian Campbell wrote: > On Mon, 2015-05-25 at 19:09 -0500, Chong Li wrote: > > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl > > index 117b61d..d28d274 100644 > > --- a/tools/libxl/libxl_types.idl > > +++ b/tools/libxl/libxl_types.idl > > @@ -347,6 +347,17 @@ libxl_domain_restore_params =3D Struct("domain_res= tore_params", [ > > ("checkpointed_stream", integer), > > ]) > > =20 > > +libxl_rtds_vcpu =3D Struct("rtds_vcpu",[ > > + ("period", uint32, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_PE= RIOD_DEFAULT'}), > > + ("budget", uint32, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_BU= DGET_DEFAULT'}), > > + ("vcpuid", integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_= VCPU_INDEX_DEFAULT'}), > > + ]) > > + > > +libxl_vcpu_sched_params =3D Struct("vcpu_sched_params",[ > > + ("sched", libxl_scheduler), > > + ("vcpus", Array(libxl_rtds_vcpu, "num_vcpus")), >=20 > How will we handle a second scheduler with per-vcpu parameters being > added in the future without breaking the API? >=20 Yeah, this is an issue here, as it is at the hypercall level. In my own review of this patch, I said "do something similar to the suggested hypervisor interface". Let me try to be a bit more specific. Ideally, I'd like this to look sort of as follows (let's call this 'option 0'): libxl_sched_params =3D Struct("sched_params",[ ("weight", integer, {'init_val': 'LIBXL_PARAM_WEIGHT_DEFAULT'}), ("cap", integer, {'init_val': 'LIBXL_PARAM_CAP_DEFAULT'}), ("period", integer, {'init_val': 'LIBXL_PARAM_PERIOD_DEFAULT'}), ("slice", integer, {'init_val': 'LIBXL_PARAM_SLICE_DEFAULT'}), ("latency", integer, {'init_val': 'LIBXL_PARAM_LATENCY_DEFAULT'}), ("extratime", integer, {'init_val': 'LIBXL_PARAM_EXTRATIME_DEFAULT'}= ), ("budget", integer, {'init_val': 'LIBXL_PARAM_BUDGET_DEFAULT'}), ]) libxl_vcpu_sched_params =3D Struct("vcpu_sched_params",[ ("sched", libxl_scheduler), ("vcpus", Array(libxl_sched_params, "num_vcpus")), ]) libxl_domain_sched_params =3D Struct("domain_sched_params",[ ("sched", libxl_scheduler), ("schedparams", libxl_sched_params), ]) But this would mean breaking the API too much, I guess (see libxl_domain_sched_params being completely redefined, the LIBXL_*_PARAMS constants being changed, etc.) So, I think I'd be fine with something like this (let's call this 'option 1'): libxl_domain_sched_params =3D Struct("domain_sched_params",[ //left right = as it is now!! ("sched", libxl_scheduler), ("weight", integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_WEIGHT= _DEFAULT'}), ("cap", integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_CAP_DE= FAULT'}), ("period", integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_PERIOD= _DEFAULT'}), ("slice", integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_SLICE_= DEFAULT'}), ("latency", integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_LATENC= Y_DEFAULT'}), ("extratime", integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_EXTRAT= IME_DEFAULT'}), ("budget", integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_BUDGET= _DEFAULT'}), ]) libxl_vcpu_sched_params =3D Struct("vcpu_sched_params",[ ("vcpus", Array(libxl_domain_sched_params, "num_vcpus")), ]) The mix of domain and vcpu in the type (and hence) function names may not appear ideal, but it's certainly not meaningless nor incorrect: these are scheduling parameters for a domain (as opposed to parameters global to a scheduler, like the ones in libxl_sched_credit_params), and are (now) applied at a per-vcpu level for such domain. There is some amount of redundancy, as the sched field, of libxl_scheduler type, is repeated for all vcpus, but it's not possible to use different schedulers for different vcpus, so it'll have to always be the same, or just be unused, which is indeed rather ugly an API. Alternatevily we can just add the per-vcpu stuff (as in 'option 0'), for all schedulers, and really leave libxl_domain_sched_params alone (let's call this 'option 2'): libxl_sched_params =3D Struct("sched_params",[ ("weight", integer, {'init_val': 'LIBXL_PARAM_WEIGHT_DEFAULT'}), ("cap", integer, {'init_val': 'LIBXL_PARAM_CAP_DEFAULT'}), ("period", integer, {'init_val': 'LIBXL_PARAM_PERIOD_DEFAULT'}), ("slice", integer, {'init_val': 'LIBXL_PARAM_SLICE_DEFAULT'}), ("latency", integer, {'init_val': 'LIBXL_PARAM_LATENCY_DEFAULT'}), ("extratime", integer, {'init_val': 'LIBXL_PARAM_EXTRATIME_DEFAULT'}= ), ("budget", integer, {'init_val': 'LIBXL_PARAM_BUDGET_DEFAULT'}), ]) libxl_vcpu_sched_params =3D Struct("vcpu_sched_params",[ ("sched", libxl_scheduler), ("vcpus", Array(libxl_sched_params, "num_vcpus")), ]) In this case the redundancy comes from having libxl_domain_sched_params and libxl_sched_params looking a lot similar, but not shared code in declaring them. Maybe we can also consider putting an union inside libl_domain_sched_params... but again, quite a severe breakage, and I wouldn't be sure about how to 'key it'... So, Thoughts? What do you think the best way forward could be? Regards, Dario --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-QPeddTkzNJTL57TUCzM3 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 iEYEABECAAYFAlV1uygACgkQk4XaBE3IOsTHRACgmduWuyynr46pvpB2aCVR67JR zxUAmwbtNXI8TGiXMMbrj7r9/cxaqFSS =lS22 -----END PGP SIGNATURE----- --=-QPeddTkzNJTL57TUCzM3-- --===============5325700995936822196== 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 --===============5325700995936822196==--