[PATCH] only set scheduler timer for non-idle CPU

[PATCH] only set scheduler timer for non-idle CPU

[Yu, Ke]

[-- Attachment #1: Type: text/plain, Size: 722 bytes --]

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) )
     {

[-- Attachment #2: sched.patch --]
[-- Type: application/octet-stream, Size: 750 bytes --]

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) )
     {

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
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

[Tian, Kevin]

[-- Attachment #1: Type: text/plain, Size: 946 bytes --]

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) )
>     {
>

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
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

[Thomas Pfeuffer]

[-- Attachment #1.1: Type: text/plain, Size: 1157 bytes --]

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
>  
>


[-- Attachment #1.2: Type: text/html, Size: 1983 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
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

[Keir Fraser]

[-- Attachment #1.1: Type: text/plain, Size: 753 bytes --]

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


[-- Attachment #1.2: Type: text/html, Size: 1513 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
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

[Keir Fraser]

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

RE: [PATCH] only set scheduler timer for non-idle CPU

[Yu, Ke]

>-----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