From mboxrd@z Thu Jan 1 00:00:00 1970 From: Juergen Gross Subject: Re: [PATCH 1 of 3] Support of getting scheduler defaults Date: Wed, 23 May 2012 07:34:27 +0200 Message-ID: <4FBC76E3.5020602@ts.fujitsu.com> 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> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1337730401.27368.38.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: George Dunlap , "xen-devel@lists.xensource.com" , Ian Campbell List-Id: xen-devel@lists.xenproject.org On 05/23/2012 01:46 AM, Dario Faggioli wrote: > On Tue, 2012-05-22 at 15:59 +0100, Ian Campbell wrote: >> Like the below. Lightly tested with the credit scheduler. >> >> I think CAP is the only one for which 0 is a real valid value, but I'm >> not sure (especially with the SEDF ones which didn't have existing >> limits checks in the set function I could crib from...). >> > Yep, that's because they're mostly time values. xen/common/sched_sedf.c > hosts some ranges for some of them, but I'm not sure we want to > propagate those: > > #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. > 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? >> 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. >> 8<--------------------------- >> >> # HG changeset patch >> # User Ian Campbell >> # Date 1337698727 -3600 >> # Node ID 355030f95eb313605a0e43aa7328e731b28a28b3 >> # Parent 426bbf58cea4559464b6e5d3ff0f65324a5f5926 >> libxl: make it possible to explicitly specify default sched params >> >> To do so we define a descriminating value which is never a valid real value for >> each parameter. >> >> While there: >> >> - remove tslice_ms and ratelimit_us from libxl_sched_params and from the xl >> domain config parser. These are global scheduler properties, not per-domain >> ones (and were never read in any case). >> - removed libxl_sched_*_domain in favour of libxl_sched_params. >> - rename libxl_sched_params to libxl_sched_domain_params for clarity. >> - use this new functionality for the various xl commands which set sched >> parameters, which saves an explicit read-modify-write in xl. >> - removed call of xc_domain_getinfolist from a few functions which weren't >> actually using the result (looks like a cut and paste error) >> - fix xl which was setting period for a variety of different config keys. >> >> Signed-off-by: Ian Campbell >> >> diff -r 426bbf58cea4 -r 355030f95eb3 tools/libxl/libxl.c >> --- a/tools/libxl/libxl.c Tue May 22 14:19:07 2012 +0100 >> +++ b/tools/libxl/libxl.c Tue May 22 15:58:47 2012 +0100 >> @@ -3168,19 +3168,19 @@ libxl_scheduler libxl_get_scheduler(libx >> } >> > >> >> int libxl_sched_sedf_domain_set(libxl_ctx *ctx, uint32_t domid, >> - libxl_sched_sedf_domain *scinfo) >> + libxl_sched_domain_params *scinfo) >> { >> - xc_domaininfo_t domaininfo; >> - int rc; >> - >> - rc = xc_domain_getinfolist(ctx->xch, domid, 1,&domaininfo); >> - if (rc< 0) { >> - LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain info list"); >> + uint64_t period; >> + uint64_t slice; >> + uint64_t latency; >> + uint16_t extratime; >> + uint16_t weight; >> + >> + int ret; >> + >> + ret = xc_sedf_domain_get(ctx->xch, domid,&period,&slice,&latency, >> +&extratime,&weight); >> + if (ret != 0) { >> + LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain sched sedf"); >> return ERROR_FAIL; >> } >> - if (rc != 1 || domaininfo.domain != domid) >> - return ERROR_INVAL; >> - >> - >> - rc = xc_sedf_domain_set(ctx->xch, domid, scinfo->period * 1000000, >> - scinfo->slice * 1000000, scinfo->latency * 1000000, >> - scinfo->extratime, scinfo->weight); >> - if ( rc< 0 ) { >> + >> + if (scinfo->period != LIBXL_SCHED_DOMAIN_PARAM_PERIOD_DEFAULT) >> + period = scinfo->period * 1000000; >> + if (scinfo->slice != LIBXL_SCHED_DOMAIN_PARAM_SLICE_DEFAULT) >> + period = scinfo->slice * 1000000; >> + if (scinfo->latency != LIBXL_SCHED_DOMAIN_PARAM_LATENCY_DEFAULT) >> + period = scinfo->latency * 1000000; >> + if (scinfo->extratime != LIBXL_SCHED_DOMAIN_PARAM_EXTRATIME_DEFAULT) >> + period = scinfo->extratime; >> + if (scinfo->weight != LIBXL_SCHED_DOMAIN_PARAM_WEIGHT_DEFAULT) >> + period = scinfo->weight; >> + >> + ret = xc_sedf_domain_set(ctx->xch, domid, period, slice, latency, >> + extratime, weight); >> + if ( ret< 0 ) { >> LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "setting domain sched sedf"); >> return ERROR_FAIL; >> } >> > # 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... > On more thing, are we ok with the _set command failing and he domain > being created anyway? Or perhaps the whole process should just abort? Perhaps a warning might be okay. Juergen -- Juergen Gross Principal Developer Operating Systems PDG ES&S SWE OS6 Telephone: +49 (0) 89 3222 2967 Fujitsu Technology Solutions e-mail: juergen.gross@ts.fujitsu.com Domagkstr. 28 Internet: ts.fujitsu.com D-80807 Muenchen Company details: ts.fujitsu.com/imprint.html