From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dario Faggioli Subject: Re: [RFC PATCH v2 0/7] Repurpose SEDF Scheduler for Real-time Use Date: Fri, 11 Jul 2014 07:01:42 +0200 Message-ID: <1405054902.29306.224.camel@Solace> References: <1404939348-4926-1-git-send-email-josh.whitehead@dornerworks.com> <1405003438.26155.12.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============6148182818185251761==" Return-path: In-Reply-To: <1405003438.26155.12.camel@kazak.uk.xensource.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: Stefano Stabellini , George Dunlap , Ian Jackson , Robert VanVossen , Xen-devel , Nathan Studer , Josh Whitehead List-Id: xen-devel@lists.xenproject.org --===============6148182818185251761== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="=-4O10/sDt3GbTZRkuPHuc" --=-4O10/sDt3GbTZRkuPHuc Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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. >=20 > 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. >=20 *** Short (well, sort of!) Answer *** Personally, I would prefer to keep SEDF alive at least for a few versions (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=20 parameters Not ideal. :-/ [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! :-) 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=20 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_, 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 --=20 <> (Raistlin Majere) ----------------------------------------------------------------- Dario Faggioli, Ph.D, http://about.me/dario.faggioli Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK) --=-4O10/sDt3GbTZRkuPHuc 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 iEYEABECAAYFAlO/b7YACgkQk4XaBE3IOsRJ3QCgocH4mnypf5xB5a9kwVyeI3kb 2Y8AoKTn6oxpWUr7UgRFXp7GRpFmsx+b =eJld -----END PGP SIGNATURE----- --=-4O10/sDt3GbTZRkuPHuc-- --===============6148182818185251761== 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 --===============6148182818185251761==--