[Xen-devel] [PATCH] Avoid negative runstate pieces

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

Avoid negative runstate pieces

Negative runstate calculation can happen, especially
without previous two patches. But it'd be still good
to add check on negative value although system time
skew now is normally at ns level so rare case can
still happen.

Also consolidate all places to get cpu idle time.

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

diff -r 0881e7c7870b xen/arch/x86/acpi/cpu_idle.c
--- a/xen/arch/x86/acpi/cpu_idle.c      Tue Dec 09 22:21:08 2008 -0500
+++ b/xen/arch/x86/acpi/cpu_idle.c      Wed Dec 10 00:27:32 2008 -0500
@@ -751,7 +751,6 @@
 int pmstat_get_cx_stat(uint32_t cpuid, struct pm_cx_stat *stat)
 {
     const struct acpi_processor_power *power = processor_powers[cpuid];
-    struct vcpu *v = idle_vcpu[cpuid];
     uint64_t usage;
     int i;
 
@@ -765,9 +764,7 @@
 
     stat->last = power->last_state ? power->last_state->idx : 0;
     stat->nr = power->count;
-    stat->idle_time = v->runstate.time[RUNSTATE_running];
-    if ( v->is_running )
-        stat->idle_time += NOW() - v->runstate.state_entry_time;
+    stat->idle_time = get_cpu_idle_time(cpuid);
 
     for ( i = 0; i < power->count; i++ )
     {
diff -r 0881e7c7870b xen/arch/x86/platform_hypercall.c
--- a/xen/arch/x86/platform_hypercall.c Tue Dec 09 22:21:08 2008 -0500
+++ b/xen/arch/x86/platform_hypercall.c Wed Dec 10 00:27:32 2008 -0500
@@ -337,16 +337,8 @@
         for_each_cpu_mask ( cpu, cpumap )
         {
             if ( (v = idle_vcpu[cpu]) != NULL )
-            {
-                idletime = v->runstate.time[RUNSTATE_running];
-                if ( v->is_running )
-                    idletime += now - v->runstate.state_entry_time;
-            }
-            else
-            {
-                idletime = 0;
                 cpu_clear(cpu, cpumap);
-            }
+            idletime = get_cpu_idle_time(cpu);
 
             ret = -EFAULT;
             if ( copy_to_guest_offset(idletimes, cpu, &idletime, 1) )
diff -r 0881e7c7870b xen/common/schedule.c
--- a/xen/common/schedule.c     Tue Dec 09 22:21:08 2008 -0500
+++ b/xen/common/schedule.c     Wed Dec 10 00:27:32 2008 -0500
@@ -84,33 +84,51 @@
 static inline void vcpu_runstate_change(
     struct vcpu *v, int new_state, s_time_t new_entry_time)
 {
+    s_time_t delta = new_entry_time - v->runstate.state_entry_time;
     ASSERT(v->runstate.state != new_state);
     ASSERT(spin_is_locked(&per_cpu(schedule_data,v->processor).schedule_lock));
 
     trace_runstate_change(v, new_state);
 
-    v->runstate.time[v->runstate.state] +=
-        new_entry_time - v->runstate.state_entry_time;
+    if ( delta > 0 )
+        v->runstate.time[v->runstate.state] += delta;
     v->runstate.state_entry_time = new_entry_time;
     v->runstate.state = new_state;
 }
 
 void vcpu_runstate_get(struct vcpu *v, struct vcpu_runstate_info *runstate)
 {
+    s_time_t delta;
     if ( likely(v == current) )
     {
         /* Fast lock-free path. */
         memcpy(runstate, &v->runstate, sizeof(*runstate));
         ASSERT(runstate->state == RUNSTATE_running);
-        runstate->time[RUNSTATE_running] += NOW() - runstate->state_entry_time;
+        delta = NOW() - runstate->state_entry_time;
+        if (delta > 0)
+            runstate->time[RUNSTATE_running] += delta;
     }
     else
     {
         vcpu_schedule_lock_irq(v);
         memcpy(runstate, &v->runstate, sizeof(*runstate));
-        runstate->time[runstate->state] += NOW() - runstate->state_entry_time;
+        delta = NOW() - runstate->state_entry_time;
+        if (delta > 0)
+            runstate->time[runstate->state] += delta;
         vcpu_schedule_unlock_irq(v);
     }
+}
+
+uint64_t get_cpu_idle_time(unsigned int cpu)
+{
+    struct vcpu_runstate_info state = { .state = RUNSTATE_running };
+    struct vcpu *v;
+
+    if ( (v = idle_vcpu[cpu]) == NULL )
+        return 0;
+
+    vcpu_runstate_get(v, &state);
+    return state.time[RUNSTATE_running];
 }
 
 int sched_init_vcpu(struct vcpu *v, unsigned int processor) 
diff -r 0881e7c7870b xen/common/sysctl.c
--- a/xen/common/sysctl.c       Tue Dec 09 22:21:08 2008 -0500
+++ b/xen/common/sysctl.c       Wed Dec 10 00:27:32 2008 -0500
@@ -166,7 +166,6 @@
     {
         uint32_t i, nr_cpus;
         struct xen_sysctl_cpuinfo cpuinfo;
-        struct vcpu *v;
 
         nr_cpus = min_t(uint32_t, op->u.getcpuinfo.max_cpus, NR_CPUS);
 
@@ -176,13 +175,7 @@
 
         for ( i = 0; i < nr_cpus; i++ )
         {
-            /* Assume no holes in idle-vcpu map. */
-            if ( (v = idle_vcpu[i]) == NULL )
-                break;
-
-            cpuinfo.idletime = v->runstate.time[RUNSTATE_running];
-            if ( v->is_running )
-                cpuinfo.idletime += NOW() - v->runstate.state_entry_time;
+            cpuinfo.idletime = get_cpu_idle_time(i);
 
             ret = -EFAULT;
             if ( copy_to_guest_offset(op->u.getcpuinfo.info, i, &cpuinfo, 1) )
diff -r 0881e7c7870b xen/drivers/cpufreq/cpufreq_ondemand.c
--- a/xen/drivers/cpufreq/cpufreq_ondemand.c    Tue Dec 09 22:21:08 2008 -0500
+++ b/xen/drivers/cpufreq/cpufreq_ondemand.c    Wed Dec 10 00:27:32 2008 -0500
@@ -51,21 +51,6 @@
 };
 
 static struct timer dbs_timer[NR_CPUS];
-
-uint64_t get_cpu_idle_time(unsigned int cpu)
-{
-    uint64_t idle_ns;
-    struct vcpu *v;
-
-    if ((v = idle_vcpu[cpu]) == NULL)
-        return 0;
-
-    idle_ns = v->runstate.time[RUNSTATE_running];
-    if (v->is_running)
-        idle_ns += NOW() - v->runstate.state_entry_time;
-
-    return idle_ns;
-}
 
 static void dbs_check_cpu(struct cpu_dbs_info_s *this_dbs_info)
 {
diff -r 0881e7c7870b xen/include/xen/sched.h
--- a/xen/include/xen/sched.h   Tue Dec 09 22:21:08 2008 -0500
+++ b/xen/include/xen/sched.h   Wed Dec 10 00:27:32 2008 -0500
@@ -538,6 +538,7 @@
 void vcpu_unlock_affinity(struct vcpu *v, cpumask_t *affinity);
 
 void vcpu_runstate_get(struct vcpu *v, struct vcpu_runstate_info *runstate);
+uint64_t get_cpu_idle_time(unsigned int cpu);
 
 #define IS_PRIV(_d) ((_d)->is_privileged)
 #define IS_PRIV_FOR(_d, _t) (IS_PRIV(_d) || ((_d)->target && (_d)->target == 
(_t)))

Attachment: monotonic_runstate.patch
Description: monotonic_runstate.patch

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