From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH 1 of 3] Support of getting scheduler defaults Date: Wed, 23 May 2012 08:41:07 +0100 Message-ID: <1337758867.3991.6.camel@dagon.hellion.org.uk> References: <56c50b3f6cc3eb1de8b8.1337678212@nehalem1> <1337689344.10118.92.camel@zakaz.uk.xensource.com> <4FBB86AE.7010908@ts.fujitsu.com> <1337689975.10118.102.camel@zakaz.uk.xensource.com> <4FBB8D92.8050800@ts.fujitsu.com> <1337694056.10118.128.camel@zakaz.uk.xensource.com> <1337698766.10118.139.camel@zakaz.uk.xensource.com> <1337730401.27368.38.camel@Solace> <4FBC76E3.5020602@ts.fujitsu.com> <1337757720.27368.58.camel@Solace> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1337757720.27368.58.camel@Solace> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Dario Faggioli Cc: Juergen Gross , "xen-devel@lists.xensource.com" , George Dunlap List-Id: xen-devel@lists.xenproject.org On Wed, 2012-05-23 at 08:22 +0100, Dario Faggioli wrote: > On Wed, 2012-05-23 at 07:34 +0200, Juergen Gross wrote: > > > #define PERIOD_MAX MILLISECS(10000) /* 10s */ > > > #define PERIOD_MIN (MICROSECS(10)) /* 10us */ > > > #define SLICE_MIN (MICROSECS(5)) /* 5us */ > > > > I think this should remain in the hypervisor only. > > > Me too. Agreed. > > > Also, extratime is a flag, so I think 0 and 1 are both meaningful > > > values, maybe we can go for -1 as for cap (I'll try and let you know). > > > > Why not -1 for all values? > > > Would work, I guess, unless there's some collision with big weights > represented on short unsigned value (not sure it's like that, and I've > always been bad at this kind of math! :-P). In general in libxl where we can use 0 for the default we do so. It's not a strong preference though and it's mainly historical from before the IDL supported init_val. > > >> I'm pretty sure that libxl__sched_set_params needs to get the correct > > >> scheduler for the particular domain, but I've no idea how to get that... > > >> > > > Again, I was thinking something like what Juergen did here could help > > > (go getting the scheduler of the cpupool the domain belongs to)... O am > > > I misunderstanding the issue? > > > > > > + poolinfo = libxl_list_cpupool(ctx,&n_pools); > > > + if (!poolinfo) > > > + return ERROR_NOMEM; > > > + > > > + ret = ERROR_INVAL; > > > + for (p = 0; p< n_pools; p++) { > > > + if (poolinfo[p].poolid == poolid) { > > > + scparams->sched = poolinfo[p].sched; > > > + ret = 0; > > > + } > > > + libxl_cpupoolinfo_dispose(poolinfo + p); > > > + } > > > + > > > > This was exactly the purpose of the sniplet. > > > Good to hear that. :-) Great, I'll make a suitably named function out of it. > > > # xl create vm1.cfg > > > Parsing config from vm1.cfg > > > libxl: error: libxl.c:3417:libxl_sched_sedf_domain_set: setting domain sched sedf: Invalid argument > > > > > > And I'm getting the above independently on what I put in the config file > > > (valid params, no params at all, etc.). I also can't change the > > > scheduling parameters of a domain on-line anymore: > > > > > > # xl sched-sedf -d 3 -p 100 -s 50 > > > libxl: error: libxl.c:3417:libxl_sched_sedf_domain_set: setting domain sched sedf: Invalid argument > > > libxl_sched_sedf_domain_set failed. > > > > > > I'll try digging a bit more into this ASAP. > > > > It's easy: Ian repeated an error he (and I) corrected elsewhere: > > He's always setting period, regardless of the changed parameter... > > > Not sure, I think Ian's patch fixes that: I thought so to! > --- a/tools/libxl/xl_cmdimpl.c Tue May 22 14:19:07 2012 +0100 > +++ b/tools/libxl/xl_cmdimpl.c Tue May 22 15:58:47 2012 +0100 > @@ -627,23 +627,20 @@ static void parse_config_data(const char > > libxl_domain_build_info_init_type(b_info, c_info->type); > > - /* the following is the actual config parsing with overriding values in the structures */ > + /* the following is the actual config parsing with overriding > + * values in the structures */ > if (!xlu_cfg_get_long (config, "cpu_weight", &l, 0)) > b_info->sched_params.weight = l; > if (!xlu_cfg_get_long (config, "cap", &l, 0)) > b_info->sched_params.cap = l; > - if (!xlu_cfg_get_long (config, "tslice_ms", &l, 0)) > - b_info->sched_params.tslice_ms = l; > - if (!xlu_cfg_get_long (config, "ratelimit_us", &l, 0)) > - b_info->sched_params.ratelimit_us = l; > if (!xlu_cfg_get_long (config, "period", &l, 0)) > b_info->sched_params.period = l; > if (!xlu_cfg_get_long (config, "slice", &l, 0)) > - b_info->sched_params.period = l; > + b_info->sched_params.slice = l; > if (!xlu_cfg_get_long (config, "latency", &l, 0)) > - b_info->sched_params.period = l; > + b_info->sched_params.latency = l; > if (!xlu_cfg_get_long (config, "extratime", &l, 0)) > - b_info->sched_params.period = l; > + b_info->sched_params.extratime = l; > > > So it has to be something else... > > Thanks and Regards, > Dario >