From mboxrd@z Thu Jan  1 00:00:00 1970
From: George Dunlap <George.Dunlap@eu.citrix.com>
Subject: Re: [PATCH] Credit1: Tweak reset condition
Date: Mon, 9 Aug 2010 13:49:39 +0100
Message-ID: <AANLkTimE5DUokwPmaE=BaLWAWCXFkS7Py-3VMHByJbe1@mail.gmail.com>
References: <1a29b22ef2e9ade8e478.1281353913@gdunlap-desktop>
	<20100809123128.GX13291@whitby.uk.xensource.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
Return-path: <xen-devel-bounces@lists.xensource.com>
In-Reply-To: <20100809123128.GX13291@whitby.uk.xensource.com>
List-Unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xensource.com>
List-Help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-Subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
Sender: xen-devel-bounces@lists.xensource.com
Errors-To: xen-devel-bounces@lists.xensource.com
To: Tim Deegan <Tim.Deegan@citrix.com>
Cc: "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>
List-Id: xen-devel@lists.xenproject.org

If we wanted to be clever, we could rely upon the fact that this
condition only happens when credit > CSCHED_CREDITS_PER_TSLICE, which
is a positive value.

But I'm all for being defensive and using /=3D 2 instead.

 -George

On Mon, Aug 9, 2010 at 1:31 PM, Tim Deegan <Tim.Deegan@citrix.com> wrote:
> Hi,
>
> At 12:38 +0100 on 09 Aug (1281357513), George Dunlap wrote:
>> VMs that don't use their full timeslice are guaranteed to flip back
>> and forth between "active" and "inactive". =A0If we set credit to 0
>> when setting "inactive", then when the VM comes back to "active"
>> again, it will effectively be behind most other vcpus in credit.
>> This causes the credit1 to effectively discriminate *against*
>> VMs which use less than their full timeslice.
>>
>> Instead of setting credit to 0, divide it in half (shifting
>> by one bit for more efficiency).
>
> "credit" here is signed, and the compiler is allowed to use either a
> signed or an unsigned shift for >>. =A0Better to code it as /=3D2 and tru=
st
> the compiler to DTRT with a constant division.
>
> Cheers,
>
> Tim.
>
>> =A0This gets rid of some of the
>> system credit while allowing non-cpu-bound VMs to keep some priority
>> advantage.
>>
>> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
>>
>> diff -r 6f07d9ac1e7c -r 1a29b22ef2e9 xen/common/sched_credit.c
>> --- a/xen/common/sched_credit.c =A0 =A0 =A0 Thu Aug 05 14:41:14 2010 +01=
00
>> +++ b/xen/common/sched_credit.c =A0 =A0 =A0 Mon Aug 09 12:38:12 2010 +01=
00
>> @@ -1069,7 +1069,9 @@
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if ( credit > CSCHED_CREDITS_PER_TSLI=
CE )
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0{
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0__csched_vcpu_acct_stop_locke=
d(prv, svc);
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0credit =3D 0;
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* Divide credits in half, so t=
hat when it starts
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 * accounting again, it starts =
a little bit "ahead" */
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0credit >>=3D 1;
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0atomic_set(&svc->credit, cred=
it);
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0}
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
>
> --
> Tim Deegan <Tim.Deegan@citrix.com>
> Principal Software Engineer, XenServer Engineering
> Citrix Systems UK Ltd. =A0(Company #02937203, SL9 0BG)
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>