Re: [PATCH 1 of 3] Support of getting scheduler defaults

[Dario Faggioli <raistlin@linux.it>]

[-- Attachment #1.1: Type: text/plain, Size: 6052 bytes --]

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 */

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).

> 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);
+    }
+

> 8<---------------------------
> 
> # HG changeset patch
> # User Ian Campbell <ian.campbell@citrix.com>
> # 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 <ian.campbell@citrix.com>
> 
> 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
>  }
>
<snip>
>  
>  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.

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?

Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)


[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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