Mailing List Archive

[PATCH] only set scheduler timer for non-idle CPU
It is not necessary to set scheduler timer for idle CPU. so this patch add conditional check for idle CPU.

This patch remove the last idle periodic timer in xen, thus enhance the idle average C state residency from two-digits ms to three-digit ms.

Signed-off-by: Yu Ke <ke.yu@intel.com>
Tian Kevin <kevin.tian@intel.com>

diff -r e4bfa70d587c xen/common/schedule.c
--- a/xen/common/schedule.c
+++ b/xen/common/schedule.c
@@ -819,7 +819,10 @@ static void schedule(void)

sd->curr = next;

- set_timer(&sd->s_timer, now + r_time);
+ if ( !is_idle_vcpu(next) )
+ {
+ set_timer(&sd->s_timer, now + r_time);
+ }

if ( unlikely(prev == next) )
{
RE: [PATCH] only set scheduler timer for non-idle CPU [ In reply to ]
Keir, how's your thought on below change? It makes sense even
not in power context. Ask here in case you didn't not it :-)

Thanks
Kevin

>From: Yu, Ke
>Sent: 2009Äê3ÔÂ31ÈÕ 11:14
>
>It is not necessary to set scheduler timer for idle CPU. so
>this patch add conditional check for idle CPU.
>
>This patch remove the last idle periodic timer in xen, thus
>enhance the idle average C state residency from two-digits ms
>to three-digit ms.
>
>Signed-off-by: Yu Ke <ke.yu@intel.com>
> Tian Kevin <kevin.tian@intel.com>
>
>diff -r e4bfa70d587c xen/common/schedule.c
>--- a/xen/common/schedule.c
>+++ b/xen/common/schedule.c
>@@ -819,7 +819,10 @@ static void schedule(void)
>
> sd->curr = next;
>
>- set_timer(&sd->s_timer, now + r_time);
>+ if ( !is_idle_vcpu(next) )
>+ {
>+ set_timer(&sd->s_timer, now + r_time);
>+ }
>
> if ( unlikely(prev == next) )
> {
>
Re: [PATCH] only set scheduler timer for non-idle CPU [ In reply to ]
Hello Ke,

>It is not necessary to set scheduler timer for idle CPU. so this patch add conditional check for idle CPU.
>
>
I think your patch is not good in case sedf-scheduler is used. If idle
VCPU is the current "running" VCPU, the scheduler timer is set to the
next "period begin" of the first VCPU in the wait queue.
Your patch prevents sedf from taking the VCPUs waiting for their next
period into the runnable queue again.

Best regards,

Thomas

>Signed-off-by: Yu Ke <ke.yu@intel.com>
> Tian Kevin <kevin.tian@intel.com>
>
>diff -r e4bfa70d587c xen/common/schedule.c
>--- a/xen/common/schedule.c
>+++ b/xen/common/schedule.c
>@@ -819,7 +819,10 @@ static void schedule(void)
>
> sd->curr = next;
>
>- set_timer(&sd->s_timer, now + r_time);
>+ if ( !is_idle_vcpu(next) )
>+ {
>+ set_timer(&sd->s_timer, now + r_time);
>+ }
>
> if ( unlikely(prev == next) )
> {
>
>
>------------------------------------------------------------------------
>
>_______________________________________________
>Xen-devel mailing list
>Xen-devel@lists.xensource.com
>http://lists.xensource.com/xen-devel
>
>
Re: [PATCH] only set scheduler timer for non-idle CPU [ In reply to ]
On 02/04/2009 13:48, "Thomas Pfeuffer" <thomas.pfeuffer@mytum.de> wrote:

> Hello Ke,
>>
>> It is not necessary to set scheduler timer for idle CPU. so this patch add
>> conditional check for idle CPU.
>>
> I think your patch is not good in case sedf-scheduler is used. If idle VCPU is
> the current "running" VCPU, the scheduler timer is set to the next "period
> begin" of the first VCPU in the wait queue.
> Your patch prevents sedf from taking the VCPUs waiting for their next period
> into the runnable queue again.

It¹s probably cleaner always to respect the specific scheduler¹s scheduling
quantum in schedule.c, but then have sched_credit.c return a very large
quantum (large as possible) for the idle VCPU.

-- Keir
Re: [PATCH] only set scheduler timer for non-idle CPU [ In reply to ]
On 02/04/2009 14:06, "Keir Fraser" <keir.fraser@eu.citrix.com> wrote:

>> I think your patch is not good in case sedf-scheduler is used. If idle VCPU
>> is the current "running" VCPU, the scheduler timer is set to the next "period
>> begin" of the first VCPU in the wait queue.
>> Your patch prevents sedf from taking the VCPUs waiting for their next period
>> into the runnable queue again.
>
> It¹s probably cleaner always to respect the specific scheduler¹s scheduling
> quantum in schedule.c, but then have sched_credit.c return a very large
> quantum (large as possible) for the idle VCPU.

I checked in something to this effect as c/s 19500.

-- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
RE: [PATCH] only set scheduler timer for non-idle CPU [ In reply to ]
>-----Original Message-----
>From: Keir Fraser [mailto:keir.fraser@eu.citrix.com]
>Sent: Thursday, April 02, 2009 9:19 PM
>To: Keir Fraser; Thomas Pfeuffer; Yu, Ke; xen-devel@lists.xensource.com
>Subject: Re: [Xen-devel] [PATCH] only set scheduler timer for non-idle CPU
>
>On 02/04/2009 14:06, "Keir Fraser" <keir.fraser@eu.citrix.com> wrote:
>
>>> I think your patch is not good in case sedf-scheduler is used. If idle VCPU
>>> is the current "running" VCPU, the scheduler timer is set to the next "period
>>> begin" of the first VCPU in the wait queue.
>>> Your patch prevents sedf from taking the VCPUs waiting for their next period
>>> into the runnable queue again.

I did not look into the much detail of the sedf-scheduler when making this patch. thanks for pointing it out, Thomas.

>>
>> It¹s probably cleaner always to respect the specific scheduler¹s scheduling
>> quantum in schedule.c, but then have sched_credit.c return a very large
>> quantum (large as possible) for the idle VCPU.
>
>I checked in something to this effect as c/s 19500.
>
> -- Keir
>

This change looks good to me. Thanks.

Best Regards
Ke

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