Re: [Xen-devel] [PATCH v2 01/10] xen: credit1: return the 'time remaining to the limit' as next timeslice.

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 30/09/16 03:53, Dario Faggioli wrote:
> If vcpu x has run for 200us, and sched_ratelimit_us is
> 1000us, continue running x _but_ return 1000us-200us as
> the next time slice. This way, next scheduling point will
> happen in 800us, i.e., exactly at the point when x crosses
> the threshold, and can be descheduled (if appropriate).
> 
> Right now (without this patch), we're always returning
> sched_ratelimit_us (1000us, in the example above), which
> means we're (potentially) allowing x to run more than
> it should have been able to.
> 
> Note that, however, in order to avoid setting timers to very
> short intervals, which is part of the purpose of rate limiting,
> we never use a time slice smaller than a well defined threshold.
> Such threshold (CSCHED_MIN_TIMER defined in this patch) is, in
> general independent from rate limiting, but it looks a good idea
> to set it to the minimum possible ratelimiting value.
> 
> Signed-off-by: Dario Faggioli <dario.faggioli@xxxxxxxxxx>

Reviewed-by: George Dunlap <george.dunlap@xxxxxxxxxx>

> ---
> Cc: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
> ---
> Changes from v1:
>  * introduce CSCHED_MIN_TIMER, as agreed during review.
> ---
>  xen/common/sched_credit.c |   12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
> index b63efac..4d84b5f 100644
> --- a/xen/common/sched_credit.c
> +++ b/xen/common/sched_credit.c
> @@ -51,6 +51,8 @@
>  /* Default timeslice: 30ms */
>  #define CSCHED_DEFAULT_TSLICE_MS    30
>  #define CSCHED_CREDITS_PER_MSEC     10
> +/* Never set a timer shorter than this value. */
> +#define CSCHED_MIN_TIMER            XEN_SYSCTL_SCHED_RATELIMIT_MIN
>  
>  
>  /*
> @@ -1811,7 +1813,15 @@ csched_schedule(
>          snext = scurr;
>          snext->start_time += now;
>          perfc_incr(delay_ms);
> -        tslice = MICROSECS(prv->ratelimit_us);
> +        /*
> +         * Next timeslice must last just until we'll have executed for
> +         * ratelimit_us. However, to avoid setting a really short timer, 
> which
> +         * will most likely be inaccurate and counterproductive, we never go
> +         * below CSCHED_MIN_TIMER.
> +         */
> +        tslice = MICROSECS(prv->ratelimit_us) - runtime;
> +        if ( unlikely(runtime < CSCHED_MIN_TIMER) )
> +            tslice = CSCHED_MIN_TIMER;
>          ret.migrated = 0;
>          goto out;
>      }
> 


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