Re: [PATCH v2 for Xen 4.6 3/4] libxl: enabling XL to set per-VCPU parameters of a domain for RTDS scheduler

[Dario Faggioli <dario.faggioli@citrix.com>]

[-- Attachment #1.1: Type: text/plain, Size: 4156 bytes --]

On Wed, 2015-06-17 at 13:08 +0100, Ian Campbell wrote:
> On Tue, 2015-06-09 at 18:18 +0200, Dario Faggioli wrote:
> > On Mon, 2015-06-08 at 15:55 -0500, Chong Li wrote:
> > > On Mon, Jun 8, 2015 at 10:56 AM, Dario Faggioli
> > 
> > > > So, Thoughts? What do you think the best way forward could be?
> > > 
> > > I like option 2 more. But I think we may also need a 'vcpuid' field in
> > > libxl_sched_params.
> > > 
> > For sparse array support, yes. At which point, I would flip the names as
> > well, i.e., something like this:
> > 
> > libxl_vcpu_sched_params = Struct("vcpu_sched_params",[
> >     ("vcpuid",       integer, { xxx some init val xxx}),
> >     ("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'}),
> 
> Is a full new set of defaults really necessary? 
>
No, it's not... I was 'sketching' the various alternatives and just went
too far. :-)

> I don't think it would
> be semantically all that strange to say that an unspecified per-vcpu
> value will default to the domain default, at which point having
> LIBXL_DOMAIN_SCHED_PARAM_FOO_DEFAULT mentioned doesn't seem all that
> strange.
> 
Indeed.

> > libxl_sched_params = Struct("sched_params",[
> >     ("sched",        libxl_scheduler),
> >     ("vcpus",        Array(libxl_sched_params, "num_vcpus")),
> >     ])
> > 
> > With the possibility of naming the latter 'libxl_vcpus_sched_params',
> > which is more descriptive, but perhaps is too similar to
> > libxl_vcpu_sched_params.
> 
> I think I'd go the other way and name the thing with all the values in
> it "libxl_sched_params" and the thing with the per-vcpu array in it
> "libxl_vcpu_sched_params".
> 
Fine with me.

> > While we're here, another thing we would appreciate some feedback on is
> > what should happen to libxl_domain_sched_params_get(). This occurred to
> > my mind while reviewing patch 4 of this series. Actually, I think we've
> > discussed this before, but can't find the reference now.
> > 
> > Anyway, my view is that, for a scheduler that uses per-vcpu parameters,
> > libxl_domain_sched_params_set() should set the same parameters for all
> > the vcpus.
> > When it comes to _get(), however, I'm not sure. To match the _set()
> > case, we'd need to return the parameters of all the vcpus, but we can't,
> > because the function takes a libxl_domain_sched_params argument, which
> > just holds 1 tuple.
> 
> Wouldn't domain_get/set be manipulating the domain's default values for
> things, i.e. what a vcpu gets if nothing more specific is specified? Or
> is it too late to say that?
> 
I actually like this. A lot.

> For set overall I don't really think it matters too much if it sets
> everything (i..e all vcpus and the defaults) so long as is a documented
> effect of the API -- anyone who adds per-vcpu support would then be
> aware of this and can adjust their code accordingly.
> 
Exactly. Personally, I think I'd make per-domain set behave
consistently, and hence use it for set the default.

> For get I think saying it returns the defaults used for vcpus which
> don't have something more explicit set is perfectly fine and doesn't
> pose an upgrade wrinkle, since only those who are aware of the vcpu
> settings would care about the distinction.
> 
As said, I like this, great idea.

Chong, what do you think? Can you make this happen?

Regards,
Dario
-- 
<<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)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel