Re: [Xen-devel] [PATCH 2/2] credit2: track residual from divisions done during accounting

[George Dunlap <George.Dunlap@xxxxxxxxxxxxx>]

On Wed, Feb 27, 2013 at 9:40 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:

>>> On 26.02.13 at 18:08, George Dunlap <george.dunlap@xxxxxxxxxxxxx> wrote:
> @@ -271,16 +272,24 @@ struct csched_dom {
>
>  /*
>   * Time-to-credit, credit-to-time.
> + *
> + * We keep track of the "residual" time to make sure that frequent short
> + * schedules still get accounted for in the end.
> + *
>   * FIXME: Do pre-calculated division?
>   */
> -static s_time_t t2c(struct csched_runqueue_data *rqd, s_time_t time, struct csched_vcpu *svc)
> +static void t2c_update(struct csched_runqueue_data *rqd, s_time_t time,
> +                          struct csched_vcpu *svc)
>  {
> -    return time * rqd->max_weight / svc->weight;
> +    uint64_t val = time * rqd->max_weight + svc->residual;
> +
> +    svc->residual = do_div(val, svc->weight);
> +    svc->credit -= val;
>  }
>
>  static s_time_t c2t(struct csched_runqueue_data *rqd, s_time_t credit, struct csched_vcpu *svc)
>  {
> -    return credit * svc->weight / rqd->max_weight;
> +    return (credit * svc->weight) / rqd->max_weight;

So you dropped the subtraction of svc->residual here - why?

And if indeed intended, the insertion of parentheses here could be
dropped?

Well for one I think the equation was wrong -- the residual is units of "time", so you should have subtracted the residual after dividing by max_weight.  (From a mathematical perspective, svc->weight / rqd->max_weight is the ratio that changes units of "credit" into units of "time; if we were using floating point ops I might put parentheses around that ratio to make it more clear that's what's going on.) 

I had originally thought that there was no real need to subtract the time off the end -- we don't do it for credit1, after all, and as long as we're adding up the residuals it will all "come out in the wash", so to speak.  But on further reflection I think it's probably not a bad idea.  I'll respin the patch with the modified equation.

 -George

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