From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH] credit: track residual from divisions done during accounting Date: Tue, 26 Feb 2013 15:00:41 +0000 Message-ID: <512CCE19.8070304@eu.citrix.com> References: <51222E8302000078000BF1F3@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <51222E8302000078000BF1F3@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel List-Id: xen-devel@lists.xenproject.org On 02/18/2013 12:37 PM, Jan Beulich wrote: > This should help with under-accounting of vCPU-s running for extremly > short periods of time, but becoming runnable again at a high frequency. > > Signed-off-by: Jan Beulich The changes to credit1 look good, and I'm fine with a patch having those (and a commented ASSERT) go in. Credit2 I'm not so happy with, because the names "t2c" and "c2t" imply (at least to me) that they are only converting, not changing anything; particularly in the way that t2c is called. At the moment everything will work fine, but it's just laying a trap for someone in the future. :-) I've got a patch in my queue dealing with this section already -- why don't you apply just the sched_credit.c part of the patch, and I'll take the credit2 part of your patch and rework it so it satisfies me. -George > > --- a/xen/common/sched_credit.c > +++ b/xen/common/sched_credit.c > @@ -19,6 +19,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -136,6 +137,7 @@ struct csched_vcpu { > struct csched_dom *sdom; > struct vcpu *vcpu; > atomic_t credit; > + unsigned int residual; > s_time_t start_time; /* When we were scheduled (used for credit) */ > uint16_t flags; > int16_t pri; > @@ -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); > atomic_sub(credits, &svc->credit); > svc->start_time += (credits * MILLISECS(1)) / CSCHED_CREDITS_PER_MSEC; > } > --- a/xen/common/sched_credit2.c > +++ b/xen/common/sched_credit2.c > @@ -21,7 +21,7 @@ > #include > #include > #include > -#include > +#include > #include > #include > #include > @@ -205,7 +205,7 @@ struct csched_runqueue_data { > > struct list_head runq; /* Ordered list of runnable vms */ > struct list_head svc; /* List of all vcpus assigned to this runqueue */ > - int max_weight; > + unsigned int max_weight; > > cpumask_t idle, /* Currently idle */ > tickled; /* Another cpu in the queue is already targeted for this one */ > @@ -244,7 +244,8 @@ struct csched_vcpu { > struct csched_dom *sdom; > struct vcpu *vcpu; > > - int weight; > + unsigned int weight; > + unsigned int residual; > > int credit; > s_time_t start_time; /* When we were scheduled (used for credit) */ > @@ -275,12 +276,15 @@ struct csched_dom { > */ > static s_time_t t2c(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); > + return 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 - svc->residual) / rqd->max_weight; > } > > /* > > >