Re: [PATCH RFC v1 2/3] libxl: enable per-VCPU work conserving flag for RTDS

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

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

On Fri, 2017-08-04 at 13:10 +0100, Wei Liu wrote:
> On Fri, Aug 04, 2017 at 10:13:18AM +0200, Dario Faggioli wrote:
> > On Thu, 2017-08-03 at 17:39 -0400, Meng Xu wrote:
> > > 
> > *HOWEVER*, in this case, we do have that 'extratime' field already,
> > as
> > a leftover from SEDF, which is there taking space and cluttering
> > the
> > interface, so why don't make good use of it. Especially considering
> > it
> > was used for _exactly_ the same thing, and with _exactly_ the same
> > meaning, and even for a very similar (i.e., SEDF was also real-
> > time)
> > kind of scheduler.
> 
> Correct me if I'm wrong:
> 
> 1. extratime is ever only used in SEDF
> 2. SEDF is removed
> 
> That means we do have extratime to use in all other schedulers.
> 
I'm not sure what you mean with this last line.

IAC, this is how our the related data structures looks like, right now:

libxl_sched_params = Struct("sched_params",[
    ("vcpuid",       integer, {'init_val': 'LIBXL_SCHED_PARAM_VCPU_INDEX_DEFAULT'}),
    ("weight",       integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_WEIGHT_DEFAULT'}),
    ("cap",          integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_CAP_DEFAULT'}),
    ("period",       integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_PERIOD_DEFAULT'}),
    ("extratime",    integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT'}),
    ("budget",       integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_BUDGET_DEFAULT'}),
    ])

The extratime field is there. Any scheduler can use it, if it wants
(and in the way it wants). Currently, no one of them does that.

libxl_domain_sched_params = Struct("domain_sched_params",[
    ("sched",        libxl_scheduler),
    ("weight",       integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_WEIGHT_DEFAULT'}),
    ("cap",          integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_CAP_DEFAULT'}),
    ("period",       integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_PERIOD_DEFAULT'}),
    ("budget",       integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_BUDGET_DEFAULT'}),

    # The following three parameters ('slice', 'latency' and 'extratime') are deprecated,
    # and will have no effect if used, since the SEDF scheduler has been removed.
    # Note that 'period' was an SDF parameter too, but it is still effective as it is
    # now used (together with 'budget') by the RTDS scheduler.
    ("slice",        integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_SLICE_DEFAULT'}),
    ("latency",      integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_LATENCY_DEFAULT'}),
    ("extratime",    integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT'}),
    ])

Same here. 'slice', 'latency' and 'extratime' are there because we
deprecate, but don't remove stuff. They're not used in any way. [*]

If, at some point, I'd decide to develop a feature for, say Credit2,
that controll the latency (whatever that would mean, it's just an
example! :-D) of domains, I think I'll use this 'latency' field, for
its interface, instead of adding some other stuff.

> However, please consider the possibility of reintroducing SEDF in the
> future. Suppose that would happen, does extratime still has the same
> semantics?
> 
Well, I guess yes. But how does this matter? Each scheduler can, if it
wants, use all these parameters in the way it actuallly prefers. So,
the fact that RTDS will be using 'extratime' for letting vCPUs execute
past their own real-time reservation, does not prevent the reintroduced
SEDF --nor any other already existing or new scheduler-- to also use
it, for similar (or maybe even not so similar) purposes.

Or am I missing something?

> > IAC, final say is Wei's and Ian's, and although I do have a
> > preference,
> > which I voiced, I'm totally fine with whichever between the two
> > approaches they advise us to take. :-)
> > 
> 
> I will leave this to you two to decide. I don't think I have an
> opinion
> here as long as the things I mentioned above are taken into
> consideration.
>
Ok, thanks. :-)

My preference remains reusing extratime. It's there, it fit the
purpose, I don't see why introducing new fields.

Regards,
Dario

[*] The reason why extratime is in both libxl_sched_params and
libxl_domain_sched_params, while slice and latency (all three of them
were SEDF only parameters) are only in the latter is not really clear
to me right now. libxl_domain_sched_params was there before, and so I
understand why everything is there.

I don't remember why, when adding libxl_sched_params, we decided to add
extratime, as no one was using it... It's quite possible that we did
that, because we knew we could use it in RTDS in future, for this very
purpose! :-)
-- 
<<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: 819 bytes --]

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

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