Xen devel - Oct 2007 - [PATCH][cpufreq] Xen support for the ondemand governor [1/2] (hypervisor code)

Modify the cpufreq ondemand governor so that it can get idle and
total ticks from the Xen hypervisor.  Linux and Xen have different
ideas of what an idle tick is, so the Xen values for both have to
be returned in the same platform hypercall.

Signed-off-by: Mark Langsdorf <mark.langsdorf@amd.com>

diff -r b4278beaf354 xen/arch/x86/platform_hypercall.c
--- a/xen/arch/x86/platform_hypercall.c	Wed Oct 17 13:12:03 2007 +0100
+++ b/xen/arch/x86/platform_hypercall.c	Thu Oct 18 16:07:53 2007 -0500
@@ -295,14 +295,17 @@ ret_t do_platform_op(XEN_GUEST_HANDLE(xe
     {
         uint32_t i, nr_cpus;
         uint64_t idletime;
+	uint64_t totaltime;
         struct vcpu *v;
         XEN_GUEST_HANDLE(uint64_t) idletimes;
+        XEN_GUEST_HANDLE(uint64_t) totaltimes;
 
         ret = -ENOSYS;
         if ( cpufreq_controller != FREQCTL_dom0_kernel )
             break;
 
         guest_from_compat_handle(idletimes, op->u.getidletime.idletime);
+        guest_from_compat_handle(totaltimes, op->u.getidletime.totaltime);
         nr_cpus = min_t(uint32_t, op->u.getidletime.max_cpus, NR_CPUS);
 
         for ( i = 0; i < nr_cpus; i++ )
@@ -312,11 +315,16 @@ ret_t do_platform_op(XEN_GUEST_HANDLE(xe
                 break;
 
             idletime = v->runstate.time[RUNSTATE_running];
+            totaltime = idletime + v->runstate.time[RUNSTATE_runnable] +
+                v->runstate.time[RUNSTATE_blocked] +
+                v->runstate.time[RUNSTATE_offline];
             if ( v->is_running )
                 idletime += NOW() - v->runstate.state_entry_time;
 
             ret = -EFAULT;
-            if ( copy_to_guest_offset(idletimes, i, &idletime, 1) )
+            if ( copy_to_guest_offset(idletimes, i, &idletime, 1) ) 
+                goto out;
+            if ( copy_to_guest_offset(totaltimes, i, &totaltime, 1) )
                 goto out;
         }
 
diff -r b4278beaf354 xen/include/public/platform.h
--- a/xen/include/public/platform.h	Wed Oct 17 13:12:03 2007 +0100
+++ b/xen/include/public/platform.h	Thu Oct 18 16:07:53 2007 -0500
@@ -179,6 +179,7 @@ struct xenpf_getidletime {
     /* IN variables. */
     uint32_t max_cpus;
     XEN_GUEST_HANDLE(uint64_t) idletime;
+    XEN_GUEST_HANDLE(uint64_t) totaltime;
     /* OUT variables. */
     uint32_t nr_cpus;
 };





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

>From: Mark Langsdorf
>Sent: 2007年10月24日 6:00
>
>Modify the cpufreq ondemand governor so that it can get idle and
>total ticks from the Xen hypervisor.  Linux and Xen have different
>ideas of what an idle tick is, so the Xen values for both have to
>be returned in the same platform hypercall.
>
>Signed-off-by: Mark Langsdorf <mark.langsdorf@amd.com>
I would suggest adding bit mask info into getidletime, and then only 
fetching idle stats of concerned cpus. Currently [0-max_cpus] is 
overkill when on-demand governor only takes care of one cpu (hw 
coordination) or sibling cores (sw coordination).

Also there''s no need to return total time for each concerned cpu. For 
sw coordination model, on-demand governor only runs on one cpu 
and getidletime is only called on that agent cpu which takes care of 
all the rest idle stats. Naturally elapsed cycles since last sample point 
should be same on all affected cpus and it''s useless to cal for them 
individually. You just need to stamp NOW() for the sample point.

Thanks,
Kevin

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

More specifically, here is what I meant. Build tested.

Allow dom0 to request idle stats by mask, which is required
by sw coordination model. Also NOW() is stamped for caller
to cal sample period.

Signed-off-by Kevin Tian <kevin.tian@intel.com>

diff -r 17f3e4f7bbe6 xen/arch/x86/platform_hypercall.c
--- a/xen/arch/x86/platform_hypercall.c	Mon Oct 22 17:44:07 2007 -0400
+++ b/xen/arch/x86/platform_hypercall.c	Tue Oct 23 16:53:43 2007 -0400
@@ -293,19 +293,23 @@ ret_t do_platform_op(XEN_GUEST_HANDLE(xe
 
     case XENPF_getidletime:
     {
-        uint32_t i, nr_cpus;
+        uint32_t i;
         uint64_t idletime;
         struct vcpu *v;
+        struct xenctl_cpumap ctlmap;
+        cpumask_t cpumap;
         XEN_GUEST_HANDLE(uint64_t) idletimes;
 
         ret = -ENOSYS;
         if ( cpufreq_controller != FREQCTL_dom0_kernel )
             break;
 
+        ctlmap.nr_cpus = op->u.getidletime.nr_cpus;
+        guest_from_compat_handle(ctlmap.bitmap, op->u.getidletime.bitmap);
+        xenctl_cpumap_to_cpumask(&cpumap, &ctlmap);
         guest_from_compat_handle(idletimes, op->u.getidletime.idletime);
-        nr_cpus = min_t(uint32_t, op->u.getidletime.max_cpus, NR_CPUS);
-
-        for ( i = 0; i < nr_cpus; i++ )
+
+        for_each_cpu_mask(i, cpumap)
         {
             /* Assume no holes in idle-vcpu map. */
             if ( (v = idle_vcpu[i]) == NULL )
@@ -318,9 +322,12 @@ ret_t do_platform_op(XEN_GUEST_HANDLE(xe
             ret = -EFAULT;
             if ( copy_to_guest_offset(idletimes, i, &idletime, 1) )
                 goto out;
-        }
-
-        op->u.getidletime.nr_cpus = i;
+
+            cpu_clear(i, cpumap);
+        }
+
+        op->u.getidletime.now = NOW();
+        cpumask_to_xenctl_cpumap(&ctlmap, &cpumap);
         ret = copy_to_guest(u_xenpf_op, op, 1) ? -EFAULT : 0;
     }
     break;
diff -r 17f3e4f7bbe6 xen/include/public/platform.h
--- a/xen/include/public/platform.h	Mon Oct 22 17:44:07 2007 -0400
+++ b/xen/include/public/platform.h	Tue Oct 23 16:12:22 2007 -0400
@@ -176,11 +176,12 @@ DEFINE_XEN_GUEST_HANDLE(xenpf_change_fre
 
 #define XENPF_getidletime         53
 struct xenpf_getidletime {
-    /* IN variables. */
-    uint32_t max_cpus;
+    /* IN variables */
+    XEN_GUEST_HANDLE(uint8_t) bitmap;
+    uint32_t nr_cpus;
     XEN_GUEST_HANDLE(uint64_t) idletime;
-    /* OUT variables. */
-    uint32_t nr_cpus;
+    /* OUT variables */
+    uint64_t now;
 };
 typedef struct xenpf_getidletime xenpf_getidletime_t;
 DEFINE_XEN_GUEST_HANDLE(xenpf_getidletime_t);
>From: Tian, Kevin
>Sent: 2007年10月24日 11:08
>To: Mark Langsdorf; xen-devel@lists.xensource.com
>Subject: RE: [Xen-devel] [PATCH][cpufreq] Xen support for the
>ondemandgovernor [1/2] (hypervisor code)
>
>>From: Mark Langsdorf
>>Sent: 2007年10月24日 6:00
>>
>>Modify the cpufreq ondemand governor so that it can get idle and
>>total ticks from the Xen hypervisor.  Linux and Xen have different
>>ideas of what an idle tick is, so the Xen values for both have to
>>be returned in the same platform hypercall.
>>
>>Signed-off-by: Mark Langsdorf <mark.langsdorf@amd.com>
>
>I would suggest adding bit mask info into getidletime, and then only
>fetching idle stats of concerned cpus. Currently [0-max_cpus] is
>overkill when on-demand governor only takes care of one cpu (hw
>coordination) or sibling cores (sw coordination).
>
>Also there''s no need to return total time for each concerned cpu.
For
>sw coordination model, on-demand governor only runs on one cpu
>and getidletime is only called on that agent cpu which takes care of
>all the rest idle stats. Naturally elapsed cycles since last sample point
>should be same on all affected cpus and it''s useless to cal for
them
>individually. You just need to stamp NOW() for the sample point.
>
>Thanks,
>Kevin
>
>_______________________________________________
>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

You can already work out total ''ticks'' (actually nanoseconds
since boot) via
data stored in shared_info. There''s code in arch/i386/kernel/time-xen.c
to
do so. Use that rather than modify this hypercall.

 -- Keir

On 23/10/07 23:00, "Mark Langsdorf" <mark.langsdorf@amd.com>
wrote:
> Modify the cpufreq ondemand governor so that it can get idle and
> total ticks from the Xen hypervisor.  Linux and Xen have different
> ideas of what an idle tick is, so the Xen values for both have to
> be returned in the same platform hypercall.
> 
> Signed-off-by: Mark Langsdorf <mark.langsdorf@amd.com>
> 
> diff -r b4278beaf354 xen/arch/x86/platform_hypercall.c
> --- a/xen/arch/x86/platform_hypercall.c Wed Oct 17 13:12:03 2007 +0100
> +++ b/xen/arch/x86/platform_hypercall.c Thu Oct 18 16:07:53 2007 -0500
> @@ -295,14 +295,17 @@ ret_t do_platform_op(XEN_GUEST_HANDLE(xe
>      {
>          uint32_t i, nr_cpus;
>          uint64_t idletime;
> + uint64_t totaltime;
>          struct vcpu *v;
>          XEN_GUEST_HANDLE(uint64_t) idletimes;
> +        XEN_GUEST_HANDLE(uint64_t) totaltimes;
>  
>          ret = -ENOSYS;
>          if ( cpufreq_controller != FREQCTL_dom0_kernel )
>              break;
>  
>          guest_from_compat_handle(idletimes,
op->u.getidletime.idletime);
> +        guest_from_compat_handle(totaltimes,
op->u.getidletime.totaltime);
>          nr_cpus = min_t(uint32_t, op->u.getidletime.max_cpus, NR_CPUS);
>  
>          for ( i = 0; i < nr_cpus; i++ )
> @@ -312,11 +315,16 @@ ret_t do_platform_op(XEN_GUEST_HANDLE(xe
>                  break;
>  
>              idletime = v->runstate.time[RUNSTATE_running];
> +            totaltime = idletime + v->runstate.time[RUNSTATE_runnable]
+
> +                v->runstate.time[RUNSTATE_blocked] +
> +                v->runstate.time[RUNSTATE_offline];
>              if ( v->is_running )
>                  idletime += NOW() - v->runstate.state_entry_time;
>  
>              ret = -EFAULT;
> -            if ( copy_to_guest_offset(idletimes, i, &idletime, 1) )
> +            if ( copy_to_guest_offset(idletimes, i, &idletime, 1) )
> +                goto out;
> +            if ( copy_to_guest_offset(totaltimes, i, &totaltime, 1) )
>                  goto out;
>          }
>  
> diff -r b4278beaf354 xen/include/public/platform.h
> --- a/xen/include/public/platform.h Wed Oct 17 13:12:03 2007 +0100
> +++ b/xen/include/public/platform.h Thu Oct 18 16:07:53 2007 -0500
> @@ -179,6 +179,7 @@ struct xenpf_getidletime {
>      /* IN variables. */
>      uint32_t max_cpus;
>      XEN_GUEST_HANDLE(uint64_t) idletime;
> +    XEN_GUEST_HANDLE(uint64_t) totaltime;
>      /* OUT variables. */
>      uint32_t nr_cpus;
>  };
> 
> 
> 
> 
> 
> _______________________________________________
> 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 24/10/07 04:08, "Tian, Kevin" <kevin.tian@intel.com> wrote:
>> Modify the cpufreq ondemand governor so that it can get idle and
>> total ticks from the Xen hypervisor.  Linux and Xen have different
>> ideas of what an idle tick is, so the Xen values for both have to
>> be returned in the same platform hypercall.
>> 
>> Signed-off-by: Mark Langsdorf <mark.langsdorf@amd.com>
> 
> I would suggest adding bit mask info into getidletime, and then only
> fetching idle stats of concerned cpus. Currently [0-max_cpus] is
> overkill when on-demand governor only takes care of one cpu (hw
> coordination) or sibling cores (sw coordination).
> 
> Also there''s no need to return total time for each concerned cpu.
For
> sw coordination model, on-demand governor only runs on one cpu
> and getidletime is only called on that agent cpu which takes care of
> all the rest idle stats. Naturally elapsed cycles since last sample point
> should be same on all affected cpus and it''s useless to cal for
them
> individually. You just need to stamp NOW() for the sample point.
Both good suggestions. Taking a cpumask seems a good idea. I''ll add
that
myself.

 -- Keir



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

> You can already work out total ''ticks'' (actually
nanoseconds
> since boot) via data stored in shared_info. There''s code in 
> arch/i386/kernel/time-xen.c to
> do so. Use that rather than modify this hypercall.
Excellent.  I wasn''t aware of that, and wasn''t wild
about having to modify the hypercall.

Since you were planning on modifying it to use the
cpumask, are you also going to add the shared_info
code?  If not, can you send me a pointer to the
changeset so I know what to work from for my 
modifications?

-Mark Langsdorf
Operating System Research Center
AMD



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

On 24/10/07 14:46, "Langsdorf, Mark" <mark.langsdorf@amd.com>
wrote:
>> You can already work out total ''ticks'' (actually
nanoseconds
>> since boot) via data stored in shared_info. There''s code in
>> arch/i386/kernel/time-xen.c to
>> do so. Use that rather than modify this hypercall.
> 
> Excellent.  I wasn''t aware of that, and wasn''t wild
> about having to modify the hypercall.
> 
> Since you were planning on modifying it to use the
> cpumask, are you also going to add the shared_info
> code?  If not, can you send me a pointer to the
> changeset so I know what to work from for my
> modifications?
Actually I checked in Kevin Tian''s patch (slightly modified, of course
;-).
So go take a look in xen-unstable staging. You''ll see that we do return
the
current system time, since it is easy to do and is probably convenient for
the caller. But we return it as a single value, not a per-cpu value!

 -- Keir



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