Re: [Xen-devel] [PATCH] libxl: fix validation of scheduling parameters for sedf

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2012-06-22 at 17:16 +0100, Dario Faggioli wrote:
> 2205914617cb does its job in correcting the "wrong domain being
> considered" issue introduced by 9d1fd58ff602. Unfortunately, when
> dealing (again!) with the sedf scheduler, it is required for the
> vCPUs of a domain to have been allocated and setup already (in
> the hypervisor), when the first call to libxl_domain_sched_params_get()
> happens, and that is not true.
> 
> This fixes that by avoiding calling that function at all, as we
> only need to know which scheduler the domain is running under,
> and that is provided by libxl__domain_scheduler() which is safe
> to be called there.
> 
> While at it, also improve a bit the comments about the whole
> sedf parameter validation and mangling process.
> 
> Signed-off-by: Dario Faggioli <dario.faggioli@xxxxxxxxxx>

Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
Committed-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -80,36 +80,48 @@ static int sched_params_valid(libxl__gc 
>      int has_slice = scp->slice != LIBXL_DOMAIN_SCHED_PARAM_SLICE_DEFAULT;
>      int has_extratime =
>                  scp->extratime != LIBXL_DOMAIN_SCHED_PARAM_EXTRATIME_DEFAULT;
> -    libxl_domain_sched_params sci;
> -
> -    libxl_domain_sched_params_get(CTX, domid, &sci);
>  
>      /* The sedf scheduler needs some more consistency checking */
> -    if (sci.sched == LIBXL_SCHEDULER_SEDF) {
> +    if (libxl__domain_scheduler(gc, domid) == LIBXL_SCHEDULER_SEDF) {
>          if (has_weight && (has_period || has_slice))
>              return 0;
> +        /* If you want a real-time domain, with its own period and
> +         * slice, please, do provide both! */
>          if (has_period != has_slice)
>              return 0;
>  
>          /*
>           * Idea is, if we specify a weight, then both period and
> -         * slice has to be zero. OTOH, if we do not specify a weight,
> -         * that means we want a pure best effort domain or an actual
> -         * real-time one. In the former case, it is period that needs
> -         * to be zero, in the latter, weight should be.
> +         * slice has to be zero. OTOH, if we do specify a period and
> +         * slice, it is weight that should be zeroed. See
> +         * docs/misc/sedf_scheduler_mini-HOWTO.txt for more details
> +         * on the meaningful combinations and their meanings.
>           */
>          if (has_weight) {
>              scp->slice = 0;
>              scp->period = 0;
>          }
>          else if (!has_period) {
> +            /* No weight nor slice/period means best effort. Parameters needs
> +             * some mangling in order to properly ask for that, though. */
> +
> +            /*
> +             * Providing no weight does not make any sense if we do not allow
> +             * the domain to run in extra time. On the other hand, if we have
> +             * extra time, weight will be ignored (and zeroed) by Xen, but it
> +             * can't be zero here, or the call for setting the scheduling
> +             * parameters will fail. So, avoid the latter by setting a random
> +             * weight (namely, 1), as it will be ignored anyway.
> +             */
> +
>              /* We can setup a proper best effort domain (extra time only)
>               * iff we either already have or are asking for some extra time. 
> */
> -            scp->weight = has_extratime ? scp->extratime : sci.extratime;
> +            scp->weight = has_extratime ? scp->extratime : 1;
>              scp->period = 0;
> +        } else {
> +            /* Real-time domain: will get slice CPU time over every period */
> +            scp->weight = 0;
>          }
> -        if (has_period && has_slice)
> -            scp->weight = 0;
>      }
>  
>      return 1;



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