Xen devel - Sep 2013 - [BUG] unfairness in Xen's credit scheduler

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

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

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
>

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

Processing commands for xen@bugs.xenproject.org:
> create ^
Created new bug #16 rooted at
`<20130902175400.20281fqjz8dg7fwg@intranet.cs.hku.hk>''
Title: `Re: [Xen-devel] [BUG] unfairness in Xen''s credit
scheduler''
> title it credit: sysctl doesn''t update other parameter when
setting tslice_ms
Set title for #16 to `credit: sysctl doesn''t update other parameter
when setting tslice_ms''
> thanks
Finished processing.

Modified/created Bugs:
 - 16: http://bugs.xenproject.org/xen/bug/16 (new)

---
Xen Hypervisor Bug Tracker
See http://wiki.xen.org/wiki/Reporting_Bugs_against_Xen for information on
reporting bugs
Contact xen-bugs-owner@bugs.xenproject.org with any infrastructure issues

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