RE: [Xen-devel] [PATCH][cpufreq] Xen support for the ondemand governor [2/2] (linux)

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

>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.  Otherwise, use
>basically the same scheme as native Linux.
>
>Signed-off-by: Mark Langsdorf <mark.langsdorf@xxxxxxx>
>
[...]
>+#else
>+
>+#include <xen/interface/platform.h>
>+static int dbs_calc_load(struct cpu_dbs_info_s *this_dbs_info)
>+{
>+      int load = 0;
>+        struct xen_platform_op op;
>+        uint64_t idletime[NR_CPUS];
>+      uint64_t totaltime[NR_CPUS];
>+      struct cpufreq_policy *policy;
>+      unsigned int j;

Note indent.

>+
>+        op.cmd = XENPF_getidletime;
>+        op.u.getidletime.max_cpus = num_online_cpus();

For current getidletime syntax, max_cpus is the max cpu index instead 
of max active cpu numbers. How do you handle cpu-hotplug case?

>+        set_xen_guest_handle(op.u.getidletime.idletime, idletime);
>+        set_xen_guest_handle(op.u.getidletime.totaltime, totaltime);
>+        HYPERVISOR_platform_op(&op);

Check return value.

>+
>+      policy = this_dbs_info->cur_policy;
>+      for_each_cpu_mask(j, policy->cpus) {
>+              cputime64_t total_idle_ticks, total_wall_ticks;
>+              cputime64_t tmp_idle_ticks, tmp_wall_ticks;

Rename "*_ticks" since Xen returns ns-based values?

>+              struct cpu_dbs_info_s *j_dbs_info;
>+              unsigned long tmp_load;
>+
>+              j_dbs_info = &per_cpu(cpu_dbs_info, j);
>+              total_idle_ticks = idletime[j];
>+              tmp_idle_ticks = cputime64_sub(total_idle_ticks,
>+                              j_dbs_info->prev_cpu_idle);
>+              total_wall_ticks = totaltime[j];
>+              tmp_wall_ticks = cputime64_sub(total_wall_ticks,
>+                              j_dbs_info->prev_cpu_wall);
>+              if (tmp_wall_ticks == 0)
>+                      return 200;

As I commented in another mail, you don't need count on elapsed time 
on other cpus. The agent cpu runs governor and thus the sample period 
is decided by that cpu. Just like the native Linux case, only pick the 
smallest idle delta in this loop.

>+
>+              j_dbs_info->prev_cpu_wall = total_wall_ticks;
>+              j_dbs_info->prev_cpu_idle = total_idle_ticks;
>+
>+              tmp_load = (100 * (tmp_wall_ticks - tmp_idle_ticks)) /
>+                              tmp_wall_ticks;
>+              load = max(load, min(100, (int) tmp_load));

I suppose 'min' here instead of 'max'. and then init value of load is set 
to ~0...

>+      }
>+      return load;
>+}
>+#endif
[...]

Thanks,
Kevin

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