Xen devel - Mar 2009 - [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) )
     {


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

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) )
>     {
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

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


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

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



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

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

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