From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: credit2 question Date: Thu, 24 Jan 2013 11:10:15 +0000 Message-ID: <51011697.3030900@eu.citrix.com> References: <5100F37702000078000B8FC2@nat28.tlf.novell.com> <5101039E.4060504@eu.citrix.com> <5101159F02000078000B90DC@nat28.tlf.novell.com> <510107E7.7050701@eu.citrix.com> <51011E2802000078000B910E@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: <51011E2802000078000B910E@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 24/01/13 10:42, Jan Beulich wrote: >>>> On 24.01.13 at 11:07, George Dunlap wrote: >> On 24/01/13 10:06, Jan Beulich wrote: >>>>>> On 24.01.13 at 10:49, George Dunlap 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