Skip to contentSkip to conversation

Xen support for the ondemand governor [1/2] (hypervisor code)

PATCH In review Mark Langsdorf 3 participants 7 messages Oct 23, 2007 – Oct 24, 2007

lore
Share & tools
lore

Mark Langsdorf

author
#0 lore raw

PATCH 1/1 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 <[email protected]>
xen/arch/x86/platform_hypercall.c +9 -1
diff -r b4278beaf354 xen/arch/x86/platform_hypercall.c
@@ -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;
         }
xen/include/public/platform.h +1 -0
diff -r b4278beaf354 xen/include/public/platform.h
@@ -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;
 };
#1 lore raw
Quoted 9 lines

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 <[email protected]>

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

#2 lore raw

PATCH 1/1 Allow get idle time stats by mask

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 <[email protected]>

xen/arch/x86/platform_hypercall.c +14 -7
diff -r 17f3e4f7bbe6 xen/arch/x86/platform_hypercall.c
@@ -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;
xen/include/public/platform.h +5 -4
diff -r 17f3e4f7bbe6 xen/include/public/platform.h
@@ -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);
Quoted 28 lines

From: Tian, Kevin
Sent: 2007年10月24日 11:08
To: Mark Langsdorf; [email protected]
Subject: RE: [Xen-devel] [PATCH][cpufreq] Xen support for the ondemandgovernor [1/2] (hypervisor code)

Quoted 9 lines

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 <[email protected]>

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.

Quoted 7 lines

Thanks,
Kevin

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

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 <[email protected]>

xen/arch/x86/platform_hypercall.c +14 -7
diff -r 17f3e4f7bbe6 xen/arch/x86/platform_hypercall.c
@@ -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;
xen/include/public/platform.h +5 -4
diff -r 17f3e4f7bbe6 xen/include/public/platform.h
@@ -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);

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#3 lore raw
In reply to Tian, Kevin
Quoted 12 lines · Kevin
Quoted 6 lines

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 <[email protected]>

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

Quoted 6 lines

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

#4 lore raw
In reply to Mark Langsdorf

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

Quoted 66 lines · Mark Langsdorf

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 <[email protected]>

diff -r b4278beaf354 xen/arch/x86/platform_hypercall.c

xen/arch/x86/platform_hypercall.c Thu Oct 18 16:07:53 2007 -0500 renamed from xen/arch/x86/platform_hypercall.c Wed Oct 17 13:12:03 2007 +0100 +9 -1
@@ -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

xen/include/public/platform.h Thu Oct 18 16:07:53 2007 -0500 renamed from xen/include/public/platform.h Wed Oct 17 13:12:03 2007 +0100 +1 -0
@@ -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
[email protected]
http://lists.xensource.com/xen-devel

author
#5 lore raw
Quoted 4 lines

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

#6 lore raw
Quoted 5 lines · Mark
Quoted 4 lines

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.

Quoted 8 lines

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