From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH v3 1/7] libxl: get rid of the SEDF scheduler Date: Tue, 7 Jul 2015 15:22:59 +0100 Message-ID: <1436278979.25646.214.camel@citrix.com> References: <20150706152620.12310.7021.stgit@Solace.station> <20150706153043.12310.43382.stgit@Solace.station> <559AA178.5030600@eu.citrix.com> <1436199470.10763.11.camel@citrix.com> <1436276932.25646.208.camel@citrix.com> <1436278521.25646.210.camel@citrix.com> <559BDFF9.8050907@eu.citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZCTmM-0003bd-EI for xen-devel@lists.xenproject.org; Tue, 07 Jul 2015 14:23:22 +0000 In-Reply-To: <559BDFF9.8050907@eu.citrix.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: George Dunlap Cc: xen-devel@lists.xenproject.org, Dario Faggioli , Wei Liu , Ian Jackson , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org On Tue, 2015-07-07 at 15:19 +0100, George Dunlap wrote: > On 07/07/2015 03:15 PM, Ian Campbell wrote: > > On Tue, 2015-07-07 at 14:48 +0100, Ian Campbell wrote: > >> On Mon, 2015-07-06 at 18:17 +0200, Dario Faggioli wrote: > >>>>> @@ -356,9 +357,13 @@ libxl_domain_sched_params = Struct("domain_sched_params",[ > >>>>> ("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'}), > >>>>> - ("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'}), > >>>>> + # 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'}), # deprecated > >>>>> + ("latency", integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_LATENCY_DEFAULT'}), # deprecated > >>>>> + ("extratime", integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT'}), # deprecated > >>>>> ("budget", integer, {'init_val': 'LIBXL_DOMAIN_SCHED_PARAM_BUDGET_DEFAULT'}), > >>>> > >>>> Since we're aiming for API compatibility rather than ABI compatibility, > >>>> is it allowable to move 'budget' up above the comment, so that it's more > >>>> obvious that it hasn't been deprecated? > >>>> > >>> It's tool's people call, I guess. My opinion is that, yes, it should be > >>> possible without any issue, and yes, I also would like the end result > >>> better. > >> > >> Yes, I think you can move it up. > >> > >> You should also add a blank line before "# The following three" and you > >> can now also drop the per line "# deprecated" since it will be visually > >> obvious which three the bigger comment refers to. > >> > >> Nit: > >>> (-25, "FEATURE_REMOVED"), # For functionallities that are no longer there > >> > >> The correct spelling would be "functionalities", but the correct meaning > >> would be "functionality that is", I'd probably also go with "that has > >> been removed". > >> > >> Unless there is some reason to resend > > > > Which of course I gave myself in the first couple of paras! > > Oh, took you to mean that you were offering to make both modifications > on check-in, and I thought "Well, that's generous of him." I guess > you're not so generous after all. ;-) Not so much generous as absent minded I'm afraid, sorry Dario...