Re: credit2 question

[George Dunlap <george.dunlap@eu.citrix.com>]

On 24/01/13 10:42, Jan Beulich wrote:
>>>> On 24.01.13 at 11:07, George Dunlap <george.dunlap@eu.citrix.com> wrote:
>> On 24/01/13 10:06, Jan Beulich wrote:
>>>>>> On 24.01.13 at 10:49, George Dunlap <george.dunlap@eu.citrix.com> wrote:
>>>> On 24/01/13 07:40, Jan Beulich wrote:
>>>>> George,
>>>>>
>>>>> I'm getting puzzled by the second c2t() invocation in
>>>>> csched_runtime(): Why is the difference of credits being passed
>>>>> here? Doesn't that (unless svc->credit is non-positive, i.e. in all
>>>>> but unusual cases) guarantee time > ntime, and particularly
>>>>> allow for negative ntime?
>>>> Ah, right -- yes, if the other guys' credit is positive, "ntime" is
>>>> guaranteed to be lower.  Since c2t() involves integer division, it would
>>>> definiteyl be good to get rid of the extra call if we can.
>>>>
>>>> My general principle is to make the code clear and easily readable
>>>> first, and then do optimization afterwards -- in this case I just never
>>>> came back and did the optimization step.
>>> Oh, I wasn't thinking of just the optimization. It seemed wrong to
>>> me to do the subtraction there in the first place: "time" is being
>>> calculated from a plain value, so why would "ntime" be calculated
>>> from a delta?
>> Ah, right -- so the idea here was to run until snext->credit was equal
>> to svc->credit.  That's why the delta.
> Which then means that under normal circumstances you would
> always only run each vCPU for CSCHED_MIN_TIMER, which
> seems quite odd. Wouldn't it be more fair to do e.g.
>
>              if ( time > ntime )
>                  time = (time + ntime) / 2;
>
> since otherwise at the expiry of the time the two vCPU-s have
> equal credit, whereas you would generally expect a vCPU that
> just finished running to have lower credit than the next one to
> run?
>
> But as you validly said earlier, avoiding the c2t() in cases where we
> can tell up front that "time" would end up below CSCHED_MIN_TIMER
> (particularly zero or negative) would be desirable. I'd prefer to
> leave doing that to you though.

Hmm, actually doing the stuff with MIN and MAX timer is more tricky; I 
forgot that vcpus with different weights burn credit at different rates, 
so we'd have to pre-calculate t2c(MIN) and t2c(MAX) for each vcpu.  That 
sounds like a bit more code than I'm up for ATM (and more complexity 
than I'd like to add unless there's a measurable benefit, which I don't 
have time to measure ATM); but I can certainly get rid of the second c2t().

  -George