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

[George Dunlap <george.dunlap@xxxxxxxxxxxxx>]

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.
 -George

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