Re: [Xen-devel] [PATCH] credit: track residual from divisions done during accounting

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 26.02.13 at 12:26, George Dunlap <george.dunlap@xxxxxxxxxxxxx> wrote:
> On 02/25/2013 11:30 AM, Jan Beulich wrote:
>>>>> On 25.02.13 at 12:12, David Vrabel <david.vrabel@xxxxxxxxxx> wrote:
>>> On 25/02/13 09:29, Jan Beulich wrote:
>>>>>>> On 22.02.13 at 18:26, Dario Faggioli <dario.faggioli@xxxxxxxxxx> wrote:
>>>>> On Mon, 2013-02-18 at 12:37 +0000, Jan Beulich wrote:
>>>>>> --- a/xen/common/sched_credit.c
>>>>>> +++ b/xen/common/sched_credit.c
>>>>>>
>>>>>> @@ -242,6 +244,7 @@ __runq_remove(struct csched_vcpu *svc)
>>>>>>   static void burn_credits(struct csched_vcpu *svc, s_time_t now)
>>>>>>   {
>>>>>>       s_time_t delta;
>>>>>> +    uint64_t val;
>>>>>>       unsigned int credits;
>>>>>>
>>>>>>       /* Assert svc is current */
>>>>>> @@ -250,7 +253,10 @@ static void burn_credits(struct csched_v
>>>>>>       if ( (delta = now - svc->start_time) <= 0 )
>>>>>>           return;
>>>>>>
>>>>>> -    credits = (delta*CSCHED_CREDITS_PER_MSEC + MILLISECS(1)/2) /
>>> MILLISECS(1);
>>>>>> +    val = delta * CSCHED_CREDITS_PER_MSEC + svc->residual;
>>>>>> +    svc->residual = do_div(val, MILLISECS(1));
>>>>>> +    credits = val;
>>>>>> +    ASSERT(credits == val);
>>>>>
>>>>> I may be missing something, but how can the assert ever be false, given
>>>>> the assignment right before it?
>>>>
>>>> val being wider than credit, this checks that there was no truncation.
>>>
>>> ASSERT(val <= UINT_MAX);
>>>
>>> Would be clearer.
>>
>> A matter of taste perhaps...
> 
> I have a taste for coders having to keep as little state in their head 
> as possible. :-)  Comparing to UINT_MAX prompts the coder specifically 
> to think about the size of the variables.

Okay, assuming this is the only thing you dislike, I'll change it then
and re-submit.

But for the record - using UINT_MAX here will get things out of
sync the moment the type of "credits" changes, whereas with
the way I had coded it this would be taken care of implicitly.

Jan


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