Re: [Xen-devel] [RFC PATCH v2 0/7] Repurpose SEDF Scheduler for Real-time Use

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2014-07-11 at 07:01 +0200, Dario Faggioli wrote:
> On gio, 2014-07-10 at 15:43 +0100, Ian Campbell wrote:
> > On Wed, 2014-07-09 at 16:55 -0400, Josh Whitehead wrote:
> > > Finally, the amount of name changes has been significantly reduced,
> > > the most prominent being to keep the "sedf" name and not switch to a
> > > new name as was also decided through previous discussion.
> > 
> > This is unfortunate since renaming would have meant a new member of the
> > libxl_scheduler enum which would have made the API
> > deprecation/compatibility stuff much simpler -- since you could just
> > return ERROR_INVAL (or ERROR_SEDF_WENT_AWAY ;-)) whenever you saw the
> > SEDF enum value, rather than having to check each field individually.
> > 
> *** Short (well, sort of!) Answer ***
> Personally, I would prefer to keep SEDF alive at least for a few
> versions

As it currently is or somehow partially modified?

>  (warning the user about some params being ignored, etc.).
> 
> However, whatever route we take, concentrating on this series, I don't
> think the renaming+SEDF deprecation should happen until proper SMP
> support is implemented, and probably also not until support for per-VCPU
> scheduling parameters (quite important for an advanced real-time
> scheduling solution) is there. It's fine for an RFC to be incomplete, so
> I'm not complaining or saying this can't be a step in the right
> direction.
> 
> However, the biggest problem of SEDF is its poor support for SMP, and
> lack of capability for setting scheduling parameters on a per-VCPU
> basis, which this series (rightfully) does not address.
> Imagine if, after proper review and cleanups, the set of changes from
> this series goes in, and no other follow up series makes it by feature
> freeze, 4.5 will have:
>  - SCHED_SEDF deprecated, because it was lacking SMP support and support
>    for per-VCPU scheduling parameters
>  - SCHED_CBS, _lacking_ SMP support and support for per-VCPU scheduling 
>    parameters
> Not ideal. :-/

If people aren't happy with that situation then I suppose we wouldn't
accept the series until CBS was more complete.

And this seems to me to be completely orthogonal to whether the approach
is to rename or rewrite.

> [That, of course, does not mean that I won't be reviewing the series
> itself. On the contrary, I'm down to it, especially patches 1, 2 and 3.]
> 
> So, to summarize, I'm ok with whatever route we decide to take, included
> full deprecation and renaming, but let's make it worth it!
> If wanting to do that, Josh and Robert, you can well assume that the
> unnecessary parameters and the messy code currently in sched_sedf.c
> related to them can go away (either being ignored by libxl, or because
> this will be a new scheduler), and so it's fine for you to kill them in
> one of your patches. This is something I was not so sure myself, so,
> thanks for bringing the discussion up on it with this series! :-)

If we are going to deprecate sedf then we should leave it alone until
the point where it is removed, not mess around cleaning it up.

> Finally, I also apologize if my comments were not explicitly along this
> line during v1 review... it just took a bit of thinking to wrap my head
> around what the best way forward could be here. :-(
> 
> Dario
> 
> *** Long (Really Long!!) Answer ***
> The _core_ of the contribution of this series, as of now, is not a new
> scheduling policy. It is a slight (although, to me, very relevant)
> modification to SEDF internals in order to fix a few problems the
> scheduler has.
> 
> In fact, I like the fact that the subject itself is 'Repurpose SEDF
> Scheduler'. :-)
> 
> The problems SEDF has are:
>  1. it has really really really poor SMP support
>  2. it does not allow to specify scheduling parameters on a per-VCPU
>     basis, but only on a domain basis. This is fine for general purpose
>     schedulers, but can be quite important in real-time workloads
>  3. it tries to be both a real-time and a general purpose scheduler
>     which is:
>     a) very hard, and often means adding a lot of complexity for
>        nothing, since you're not going to be that good at both anyway...
>        and this is one of the cases;
>     b) not necessary any longer, now that we have good general purpose 
>        schedulers, like credit and (hopefully ready soon) credit2;
>  4. it deals with VCPU wake-ups in an hacky, unnecessarily complex and,
>     if you ask me, incorrect way.
> 
> Note that, in order to achieve 3, not only a lot of complexity (hacks!)
> were introduced inside sched_sedf.c in Xen, it is also the interface
> that contains parameters that are not useful for a real-time scheduler
> which only aims at doing its own job well, i.e.,
> _being_a_good_real-time_scheduler_. :-D
> 
> The issues are, in my opinion, sort-of in priority order, with 3 and 4
> being related and almost equally important. 4 is probably the easiest,
> as it only involves changes in xen/common/sched_sedf.c, while 3 is not
> hard to do per se, but has interface implications (i.e., exactly what
> we're discussing here).
> 
> * Just to make sure everyone understand, even if not into SEDF and RT
> * scheduling, this patch series, as it stands now, fixes 4, and tries
> * to deal with 2.
> *
> * That's why I'm saying "let's discuss renaming later".
> 
> In the long run, what we want is a good, SMP capable, real-time
> scheduler. This good SMP capable real-time scheduler won't ever need
> some of the parameters that the SEDF interface includes or, if we make
> it digest them, semantic and behavior will be different from now anyway.
> 
> As far as I can tell, this can be implemented:
>  A) modifying SEDF, i.e., actually modifying sched_sedf.c, fixing the
>     problems above
>  B) introducing something completely new (sched_rt.c ?), which leaves
>     open the question of what to do with sched_sedf.c and
>     {LIBXL,XEN}_SCHEDULER_SEDF as a scheduling policy.
> 
> If it were me doing it, I probably would have tried to modify SEDF, in a
> very similar way to what this series does, and then, probably, even in
> the long run, I'd have kept the name and deprecated (or changed the
> meaning, and warning the user about it) some of the params. And, in
> fact, Josh's work springs from a conversation we had about this a while
> back, right Josh? :-)
> 
> In fact, I don't like the option of getting rid of SEDF all together and
> all of a sudden, even given the issues it has. I think that would not be
> in line with what we've done up to now, in terms of API stability/not
> breaking things for users (see xm-->xl migr.), and I don't think it's
> necessary. Fixing it internally and warning the user that a few
> parameters are now being ignored seems a good solution to me, at least
> for a few versions.
> 
> And even if we decide that SEDF has to go, in favor of SCHED_CBS,
> SCHED_RT, SCHED_<whatever>, I personally would _NOT_ do that until such
> a new scheduler:
>  - is a good and SMP capable real-time scheduler
>  - has an interface we're happy with, and can commit to keep it stable
> 
> This series is an RFC, so it's ok for it not to be complete and, since
> it tries to fix 2 of the 4 issues I identified myself, I certainly am
> not saying it's not a step in (one of the possible) right direction. For
> sure, thanks to the discussion we've had as a consequence of this
> series, we now know that deprecating SEDF is --either sooner or later--
> an option... probably even the best option!
> 
> So go ahead with the items you have in your 'FUTURE DEVELOPMENT' section
> already, and we'll kill SEDF as soon as it will be worth it. I'll let
> you have my comments on patches 1, 2 and 3 ASAP.
> 
> Thanks and Regards,
> Dario
> 
> 



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