#16 - credit: sysctl doesn't update other parameter when setting tslice_ms

Owner: George Dunlap <George.Dunlap@eu.citrix.com>

Date: Mon Sep 2 11:00:01 2013

Last Update: Mon Sep 2 11:00:01 2013

Severity: normal

Affects:

State: Closed

[ Retrieve as mbox ]


From: lwcheng@cs.hku.hk
To: xen-devel@lists.xen.org
Subject: [Xen-devel] [BUG] unfairness in Xen's credit scheduler
Date: Mon, 02 Sep 2013 17:54:00 +0800
Message-ID: <20130902175400.20281fqjz8dg7fwg@intranet.cs.hku.hk>

[ Reply to this message; Retrieve Raw Message; Archives: marc.info, gmane ]

Hi all,

Since Xen 4.2.0, users can change time slice of the scheduler at
runtime via xl command line, a very nice feature. However, it is
not *correctly* implemented.

Problem description
--------------------
say you set the 'cap' of one VM to 50 (a half core),
-when setting the time slice to be *greater* than 30ms, the VM gets
much *less* CPU cycles than its allocation
-when setting the time slice to be *smaller* than 30ms, the VM gets
much *more* CPU cycles than its allocation


Problem happens in sched_credit.c/csched_sys_cntl():
--------------------
after changing prv->tslice_ms, other parameters *should* also be
changed accordingly:

csched_sys_cntl() {
     ...
     prv->tslice_ms = params->tslice_ms;
     prv->ratelimit_us = params->ratelimit_us;
-------
     /* my patch: these parameters should also be changed */
     prv->ticks_per_tslice = CSCHED_TICKS_PER_TSLICE;
     if ( prv->tslice_ms < prv->ticks_per_tslice )
         prv->ticks_per_tslice = 1;
     prv->tick_period_us = prv->tslice_ms * 1000 / prv->ticks_per_tslice;
     prv->credits_per_tslice = CSCHED_CREDITS_PER_MSEC * prv->tslice_ms;
-------
     ...
}

Particularly, [prv->credits_per_tslice] is very important to maintain
the *fairness* of credit allocation in csched_acct().

This bug has been there since Xen-4.2.0.

Thanks,
CHENG Luwei
--
PhD student
Department of Computer Science
The University of Hong Kong
Homepage: http://www.cs.hku.hk/~lwcheng

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

From: George Dunlap <George.Dunlap@eu.citrix.com>
To: lwcheng@cs.hku.hk
Cc: "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>
Subject: Re: [Xen-devel] [BUG] unfairness in Xen's credit scheduler
Date: Mon, 2 Sep 2013 11:32:33 +0100
Message-ID: <CAFLBxZYcLbjwzXB+WnN22RgkrpWQk6rSP1Qj_nB_Ldp-Nmydzg@mail.gmail.com>

[ Reply to this message; Retrieve Raw Message; Archives: marc.info, gmane ]

On Mon, Sep 2, 2013 at 10:54 AM,  <lwcheng@cs.hku.hk> wrote:
> Hi all,
>
> Since Xen 4.2.0, users can change time slice of the scheduler at
> runtime via xl command line, a very nice feature. However, it is
> not *correctly* implemented.
>
> Problem description
> --------------------
> say you set the 'cap' of one VM to 50 (a half core),
> -when setting the time slice to be *greater* than 30ms, the VM gets
> much *less* CPU cycles than its allocation
> -when setting the time slice to be *smaller* than 30ms, the VM gets
> much *more* CPU cycles than its allocation
>
>
> Problem happens in sched_credit.c/csched_sys_cntl():
> --------------------
> after changing prv->tslice_ms, other parameters *should* also be
> changed accordingly:
>
> csched_sys_cntl() {
>     ...
>     prv->tslice_ms = params->tslice_ms;
>     prv->ratelimit_us = params->ratelimit_us;
> -------
>     /* my patch: these parameters should also be changed */
>     prv->ticks_per_tslice = CSCHED_TICKS_PER_TSLICE;
>     if ( prv->tslice_ms < prv->ticks_per_tslice )
>         prv->ticks_per_tslice = 1;
>     prv->tick_period_us = prv->tslice_ms * 1000 / prv->ticks_per_tslice;
>     prv->credits_per_tslice = CSCHED_CREDITS_PER_MSEC * prv->tslice_ms;

Oh, right -- sorry, that was my patch, and I don't know what I was thinking.

If you have time, could you make a patch which creates a function to
do this, and have both SCHEDOP_put_into and csched_init() call it?
That way we avoid duplicating the code.

Some hints for making a good patch and sending it to the list can be found here:

http://wiki.xen.org/wiki/Submitting_Xen_Patches

Thanks!
 -George

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

From: lwcheng@cs.hku.hk
To: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>
Subject: Re: [Xen-devel] [BUG] unfairness in Xen's credit scheduler
Date: Mon, 02 Sep 2013 18:38:58 +0800
Message-ID: <20130902183858.92074lpwk7tzjlkw@intranet.cs.hku.hk>

[ Reply to this message; Retrieve Raw Message; Archives: marc.info, gmane ]

Thanks for your reply, George.
I will generate the patch later on.

-Luwei

Quoting George Dunlap <George.Dunlap@eu.citrix.com>:

> On Mon, Sep 2, 2013 at 10:54 AM,  <lwcheng@cs.hku.hk> wrote:
>> Hi all,
>>
>> Since Xen 4.2.0, users can change time slice of the scheduler at
>> runtime via xl command line, a very nice feature. However, it is
>> not *correctly* implemented.
>>
>> Problem description
>> --------------------
>> say you set the 'cap' of one VM to 50 (a half core),
>> -when setting the time slice to be *greater* than 30ms, the VM gets
>> much *less* CPU cycles than its allocation
>> -when setting the time slice to be *smaller* than 30ms, the VM gets
>> much *more* CPU cycles than its allocation
>>
>>
>> Problem happens in sched_credit.c/csched_sys_cntl():
>> --------------------
>> after changing prv->tslice_ms, other parameters *should* also be
>> changed accordingly:
>>
>> csched_sys_cntl() {
>>     ...
>>     prv->tslice_ms = params->tslice_ms;
>>     prv->ratelimit_us = params->ratelimit_us;
>> -------
>>     /* my patch: these parameters should also be changed */
>>     prv->ticks_per_tslice = CSCHED_TICKS_PER_TSLICE;
>>     if ( prv->tslice_ms < prv->ticks_per_tslice )
>>         prv->ticks_per_tslice = 1;
>>     prv->tick_period_us = prv->tslice_ms * 1000 / prv->ticks_per_tslice;
>>     prv->credits_per_tslice = CSCHED_CREDITS_PER_MSEC * prv->tslice_ms;
>
> Oh, right -- sorry, that was my patch, and I don't know what I was thinking.
>
> If you have time, could you make a patch which creates a function to
> do this, and have both SCHEDOP_put_into and csched_init() call it?
> That way we avoid duplicating the code.
>
> Some hints for making a good patch and sending it to the list can be  
> found here:
>
> http://wiki.xen.org/wiki/Submitting_Xen_Patches
>
> Thanks!
>  -George
>



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

From: George Dunlap <George.Dunlap@eu.citrix.com>
To: lwcheng@cs.hku.hk
Cc: "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>
Subject: Re: [Xen-devel] [BUG] unfairness in Xen's credit scheduler
Date: Mon, 2 Sep 2013 11:57:46 +0100
Message-ID: <CAFLBxZba3SfCG-JdPGN05SLkiXguDbw4QG-mfstOVBZCe21Vug@mail.gmail.com>

[ Reply to this message; Retrieve Raw Message; Archives: marc.info, gmane ]

create ^
title it credit: sysctl doesn't update other parameter when setting tslice_ms
thanks


On Mon, Sep 2, 2013 at 10:54 AM,  <lwcheng@cs.hku.hk> wrote:
> Hi all,
>
> Since Xen 4.2.0, users can change time slice of the scheduler at
> runtime via xl command line, a very nice feature. However, it is
> not *correctly* implemented.
>
> Problem description
> --------------------
> say you set the 'cap' of one VM to 50 (a half core),
> -when setting the time slice to be *greater* than 30ms, the VM gets
> much *less* CPU cycles than its allocation
> -when setting the time slice to be *smaller* than 30ms, the VM gets
> much *more* CPU cycles than its allocation
>
>
> Problem happens in sched_credit.c/csched_sys_cntl():
> --------------------
> after changing prv->tslice_ms, other parameters *should* also be
> changed accordingly:
>
> csched_sys_cntl() {
>     ...
>     prv->tslice_ms = params->tslice_ms;
>     prv->ratelimit_us = params->ratelimit_us;
> -------
>     /* my patch: these parameters should also be changed */
>     prv->ticks_per_tslice = CSCHED_TICKS_PER_TSLICE;
>     if ( prv->tslice_ms < prv->ticks_per_tslice )
>         prv->ticks_per_tslice = 1;
>     prv->tick_period_us = prv->tslice_ms * 1000 / prv->ticks_per_tslice;
>     prv->credits_per_tslice = CSCHED_CREDITS_PER_MSEC * prv->tslice_ms;
> -------
>     ...
> }
>
> Particularly, [prv->credits_per_tslice] is very important to maintain
> the *fairness* of credit allocation in csched_acct().
>
> This bug has been there since Xen-4.2.0.
>
> Thanks,
> CHENG Luwei
> --
> PhD student
> Department of Computer Science
> The University of Hong Kong
> Homepage: http://www.cs.hku.hk/~lwcheng
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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


From: George Dunlap <george.dunlap@eu.citrix.com>
To: lwcheng@cs.hku.hk
Cc: "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>
Subject: Re: [Xen-devel] [BUG] unfairness in Xen's credit scheduler
Date: Tue, 3 Sep 2013 10:26:07 +0100
Message-ID: <5225AB2F.10006@eu.citrix.com>

[ Reply to this message; Retrieve Raw Message; Archives: marc.info, gmane ]

On 09/02/2013 12:35 PM, lwcheng@cs.hku.hk wrote:
> Hi George, you may fix this problem quickly. I am stuck with some
> research work in recent days and it will take a long time to wait for my
> patch.. thanks!

OK, no problem.  It's on my list, so it will get fixed before the 4.4 
release one way or another.

BTW, convention on this list is to reply below the thing you're replying 
to (as I'm doing now), rather than top-posting (as you have done a few 
times now).

Peace,
  -George


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

From: Wei Liu <liuw@liuw.name>
To: George Dunlap <george.dunlap@eu.citrix.com>
Cc: "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>, lwcheng@cs.hku.hk
Subject: Re: [Xen-devel] [BUG] unfairness in Xen's credit scheduler
Date: Wed, 18 Dec 2013 00:15:30 +0000
Message-ID: <CAOsiSVUZhVi+LoZoP-egS+HM7YkeQTSmzEMtrp0Das41PpL2ZQ@mail.gmail.com>

[ Reply to this message; Retrieve Raw Message; Archives: marc.info, gmane ]

On Tue, Sep 3, 2013 at 10:26 AM, George Dunlap
<george.dunlap@eu.citrix.com> wrote:
> On 09/02/2013 12:35 PM, lwcheng@cs.hku.hk wrote:
>>
>> Hi George, you may fix this problem quickly. I am stuck with some
>> research work in recent days and it will take a long time to wait for my
>> patch.. thanks!
>
>
> OK, no problem.  It's on my list, so it will get fixed before the 4.4
> release one way or another.
>
> BTW, convention on this list is to reply below the thing you're replying to
> (as I'm doing now), rather than top-posting (as you have done a few times
> now).
>
> Peace,
>  -George
>

Sort of doing a sweep of bug list.

This bug was independently reported and fixed by

Author:     Nate Studer <nate.studer@dornerworks.com>
AuthorDate: Fri Nov 15 17:38:10 2013 +0100
Commit:     Jan Beulich <jbeulich@suse.com>
CommitDate: Fri Nov 15 17:38:10 2013 +0100

    credit: Update other parameters when setting tslice_ms

    Add a utility function to update the rest of the timeslice
    accounting fields when updating the timeslice of the
    credit scheduler, so that capped CPUs behave correctly.

    Before this patch changing the timeslice to a value higher
    than the default would result in a domain not utilizing
    its full capacity and changing the timeslice to a value
    lower than the default would result in a domain exceeding
    its capacity.

    Signed-off-by: Nate Studer <nate.studer@dornerworks.com>
    Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
    Reviewed-by: George Dunlap <george.dunlap@eu.citrix.com>

Cool, no need to do anything anymore. This one can be marked closed now. :-)

Wei.

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

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

From: Ian Campbell <Ian.Campbell@citrix.com>
To: Wei Liu <liuw@liuw.name>
Cc: George Dunlap <george.dunlap@eu.citrix.com>, "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>, lwcheng@cs.hku.hk
Subject: Re: [Xen-devel] [BUG] unfairness in Xen's credit scheduler
Date: Wed, 18 Dec 2013 09:58:40 +0000
Message-ID: <1387360720.27441.85.camel@kazak.uk.xensource.com>

[ Reply to this message; Retrieve Raw Message; Archives: marc.info, gmane ]

On Wed, 2013-12-18 at 00:15 +0000, Wei Liu wrote:

> Cool, no need to do anything anymore. This one can be marked closed now. :-)

FYI anyone can close a bug. Send a mail Bcc: xen@bugs.xenproject.org as
well as To: the list (so the explanation is public) which begins:

------8<--------
close 16
thanks

Any explanation you feel is necessary/useful
------8<--------

Ian.


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

From: Wei Liu <liuw@liuw.name>
To: lwcheng@cs.hku.hk
Cc: "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>
Subject: Re: [Xen-devel] [BUG] unfairness in Xen's credit scheduler
Date: Wed, 18 Dec 2013 12:27:26 +0000
Message-ID: <CAOsiSVVAUgwB+EAHt5xo6d=DQU+cUm1+23sEFVqcChoiuK1eKg@mail.gmail.com>

[ Reply to this message; Retrieve Raw Message; Archives: marc.info, gmane ]

close 16
thanks

This bug was reported independently and fixed by:

Author:     Nate Studer <nate.studer@dornerworks.com>
AuthorDate: Fri Nov 15 17:38:10 2013 +0100
Commit:     Jan Beulich <jbeulich@suse.com>
CommitDate: Fri Nov 15 17:38:10 2013 +0100

    credit: Update other parameters when setting tslice_ms

    Add a utility function to update the rest of the timeslice
    accounting fields when updating the timeslice of the
    credit scheduler, so that capped CPUs behave correctly.

    Before this patch changing the timeslice to a value higher
    than the default would result in a domain not utilizing
    its full capacity and changing the timeslice to a value
    lower than the default would result in a domain exceeding
    its capacity.

    Signed-off-by: Nate Studer <nate.studer@dornerworks.com>
    Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com>
    Reviewed-by: George Dunlap <george.dunlap@eu.citrix.com>

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