Re: [RFC PATCH 3/4] Updated comments/variables to reflect cbs, fixed formatting and confusing comments/variables

[George Dunlap <george.dunlap@eu.citrix.com>]

On 06/17/2014 05:11 PM, Dario Faggioli wrote:
> On lun, 2014-06-16 at 16:29 +0100, George Dunlap wrote:
>> On 06/16/2014 10:33 AM, Jan Beulich wrote:
>>>>>> On 13.06.14 at 21:58, <josh.whitehead@dornerworks.com> wrote:
>>>> --- a/xen/include/public/trace.h
>>>> +++ b/xen/include/public/trace.h
>>>> @@ -75,7 +75,7 @@
>>>>    /* Per-scheduler IDs, to identify scheduler specific events */
>>>>    #define TRC_SCHED_CSCHED   0
>>>>    #define TRC_SCHED_CSCHED2  1
>>>> -#define TRC_SCHED_SEDF     2
>>>> +#define TRC_SCHED_CBS      2
>>> While the change to domctl.h is fine, I'm not sure we can allow simple
>>> renaming elsewhere in the public headers (i.e. the old name may need
>>> to remain there, guarded with a __XEN_INTERFACE_VERSION__
>>> conditional).
>> I think the tracing stuff is fine too -- we've always considered that
>> non-stable (and have made incompatible changes across versions).
>>
>> But the libxl interfaces *do* need to have something sensible done with
>> them.
>>
>> Given that,  I think it would probably be better to make this patch series:
>>
>> 1/N: Add sched_cbs.c to Xen
>> 2/N: Add cbs to toolstack
>> 3/N: Remove sedf scheduler (with appropriate backwards-compatibility bits)
>>
>> I think that would make it a bit easier to review as well.
>>
> As far as this patch is concerned, I agree with George.
>
> However... Is removing SEDF an option? Is radically changing, if not
> it's behavior (as it's known to be pretty broken), the expectations of
> an user, e.g., of an old application being compiled with a new version
> of xen+libxl an option?
>
> If yes, what's the process to do that?
> Personally, I'm all for having a really working real-time scheduling
> solution, and you all know that. :-) However, especially considering
> Josh's and Robbie's series, I think I would not remove or rename SEDF, I
> rather "just" amend the implementation.
>
> In future, it would be interesting to introduce more advanced real-time
> scheduling features an capabilities, like the ones coming from RT-Xen
> (and the RT-Xen guys are working on doing that), but I think that can be
> done step-by-step, and without any massive renaming or removal.
>
> So, I'm asking, mostly to George, about the overall scheduling aspects
> and implications, and to the tools maintainer, as that's where API
> stability is to be enforced: should this be a concern? In what sense API
> stability applies here? Can we, for example, start to ignore one or more
> SEDF scheduling parameters?

Well the primary thing is that programs using the old interface still 
need to compile.  This could be done with the libxl interface version 
#ifdef-ery.

One of the things that makes this handy is that the only way a user can 
*set* SEDF via libxl is when they create a cpupool; all the other uses 
are just recognizing SEDF if it's actually the scheduler already.

Re the functionality: for one, if nobody is actually using it, we don't 
need to keep it around.  One advantage of adding a CBS scheduler and 
just removing the SEDF scheduler is that it should make it pretty clear 
who those people are, and make putting it back pretty easy.

If someone is using it -- I think asking them to switch to the 
(presumably) better CBS scheduler makes sense, since it accomplishes 
much the same thing.

On the whole, I'm inclined to say we should just go ahead and change the 
SEDF interface in place rather than introducing a new scheduler.

The question then comes with the parameters: whether it's better to 
attempt to keep working the best you can, or whether it's better to 
alert the admin to the fact that the scheduler has changed significantly 
and it's time to re-tweak the system.

I'm inclined to say that we should:
* Leave the name and number as sedf
* Remove parameters no longer being used, gated with the LIBXL_API_VERSION.
* For all schedulers, if parameters that are not used are set to 
anything other than LIBXL_DOMAIN_SCHED_PARAM_{FOO}_DEFAULT, 
libxl_domain_set_sched_params() should return an error.

Alternately, we could have libxl throw a warning that we're ignoring a 
parameter in most cases; and in the case of no longer used parameters, 
warn specifically that it's no longer implemented.  We could maybe also 
point them to some documentation, and in the documentation say to 
contact the xen-devel list if you're still using it.

If people complain, then we can try to accomodate them.  But at very 
least we get an idea if anyone actually cares. :-)

  -George