[Xen-devel] [RFC PATCH V2 07/11] x86/xen: save and restore steal clock during hibernation

[Anchal Agarwal <anchalag@xxxxxxxxxx>]

From: Munehisa Kamata <kamatam@xxxxxxxxxx>

Currently, steal time accounting code in scheduler expects steal clock
callback to provide monotonically increasing value. If the accounting
code receives a smaller value than previous one, it uses a negative
value to calculate steal time and results in incorrectly updated idle
and steal time accounting. This breaks userspace tools which read
/proc/stat.

top - 08:05:35 up  2:12,  3 users,  load average: 0.00, 0.07, 0.23
Tasks:  80 total,   1 running,  79 sleeping,   0 stopped,   0 zombie
Cpu(s):  0.0%us,  0.0%sy,  0.0%ni,30100.0%id,  0.0%wa,  0.0%hi, 0.0%si,
-1253874204672.0%st

This can actually happen when a Xen PVHVM guest gets restored from
hibernation, because such a restored guest is just a fresh domain from
Xen perspective and the time information in runstate info starts over
from scratch.

Introduce xen_save_steal_clock() which saves current steal clock values
of all present CPUs in runstate info into per-cpu variables during system
core ops suspend callbacks. Its couterpart, xen_restore_steal_clock(),
restores a boot CPU's steal clock in the system core resume callback. It
sets offset if it found the current values in runstate info are smaller
than previous ones. xen_steal_clock() is also modified to use the offset
to ensure that scheduler only sees monotonically increasing number.

For non-boot CPUs, restore after they're brought up, because runstate
info for non-boot CPUs are not active until then.

[Anchal Changelog: Merged patch xen/time: introduce 
xen_{save,restore}_steal_clock
with this one for better code readability]
Signed-off-by: Anchal Agarwal <anchalag@xxxxxxxxxx>
Signed-off-by: Munehisa Kamata <kamatam@xxxxxxxxxx>
---
 arch/x86/xen/suspend.c | 13 ++++++++++++-
 arch/x86/xen/time.c    |  3 +++
 drivers/xen/time.c     | 28 +++++++++++++++++++++++++++-
 include/xen/xen-ops.h  |  2 ++
 4 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/arch/x86/xen/suspend.c b/arch/x86/xen/suspend.c
index 784c4484100b..dae0f74f5390 100644
--- a/arch/x86/xen/suspend.c
+++ b/arch/x86/xen/suspend.c
@@ -91,12 +91,20 @@ void xen_arch_suspend(void)
 static int xen_syscore_suspend(void)
 {
        struct xen_remove_from_physmap xrfp;
-       int ret;
+       int cpu, ret;
 
        /* Xen suspend does similar stuffs in its own logic */
        if (xen_suspend_mode_is_xen_suspend())
                return 0;
 
+       for_each_present_cpu(cpu) {
+               /*
+                * Nonboot CPUs are already offline, but the last copy of
+                * runstate info is still accessible.
+                */
+               xen_save_steal_clock(cpu);
+       }
+
        xrfp.domid = DOMID_SELF;
        xrfp.gpfn = __pa(HYPERVISOR_shared_info) >> PAGE_SHIFT;
 
@@ -118,6 +126,9 @@ static void xen_syscore_resume(void)
 
        pvclock_resume();
 
+       /* Nonboot CPUs will be resumed when they're brought up */
+       xen_restore_steal_clock(smp_processor_id());
+
        gnttab_resume();
 }
 
diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
index befbdd8b17f0..8cf632dda605 100644
--- a/arch/x86/xen/time.c
+++ b/arch/x86/xen/time.c
@@ -537,6 +537,9 @@ static void xen_hvm_setup_cpu_clockevents(void)
 {
        int cpu = smp_processor_id();
        xen_setup_runstate_info(cpu);
+       if (cpu)
+               xen_restore_steal_clock(cpu);
+
        /*
         * xen_setup_timer(cpu) - snprintf is bad in atomic context. Hence
         * doing it xen_hvm_cpu_notify (which gets called by smp_init during
diff --git a/drivers/xen/time.c b/drivers/xen/time.c
index 0968859c29d0..3713d716070c 100644
--- a/drivers/xen/time.c
+++ b/drivers/xen/time.c
@@ -20,6 +20,8 @@
 
 /* runstate info updated by Xen */
 static DEFINE_PER_CPU(struct vcpu_runstate_info, xen_runstate);
+static DEFINE_PER_CPU(u64, xen_prev_steal_clock);
+static DEFINE_PER_CPU(u64, xen_steal_clock_offset);
 
 static DEFINE_PER_CPU(u64[4], old_runstate_time);
 
@@ -149,7 +151,7 @@ bool xen_vcpu_stolen(int vcpu)
        return per_cpu(xen_runstate, vcpu).state == RUNSTATE_runnable;
 }
 
-u64 xen_steal_clock(int cpu)
+static u64 __xen_steal_clock(int cpu)
 {
        struct vcpu_runstate_info state;
 
@@ -157,6 +159,30 @@ u64 xen_steal_clock(int cpu)
        return state.time[RUNSTATE_runnable] + state.time[RUNSTATE_offline];
 }
 
+u64 xen_steal_clock(int cpu)
+{
+       return __xen_steal_clock(cpu) + per_cpu(xen_steal_clock_offset, cpu);
+}
+
+void xen_save_steal_clock(int cpu)
+{
+       per_cpu(xen_prev_steal_clock, cpu) = xen_steal_clock(cpu);
+}
+
+void xen_restore_steal_clock(int cpu)
+{
+       u64 steal_clock = __xen_steal_clock(cpu);
+
+       if (per_cpu(xen_prev_steal_clock, cpu) > steal_clock) {
+               /* Need to update the offset */
+               per_cpu(xen_steal_clock_offset, cpu) =
+                       per_cpu(xen_prev_steal_clock, cpu) - steal_clock;
+       } else {
+               /* Avoid unnecessary steal clock warp */
+               per_cpu(xen_steal_clock_offset, cpu) = 0;
+       }
+}
+
 void xen_setup_runstate_info(int cpu)
 {
        struct vcpu_register_runstate_memory_area area;
diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
index 3b3992b5b0c2..12b3f4474a05 100644
--- a/include/xen/xen-ops.h
+++ b/include/xen/xen-ops.h
@@ -37,6 +37,8 @@ void xen_time_setup_guest(void);
 void xen_manage_runstate_time(int action);
 void xen_get_runstate_snapshot(struct vcpu_runstate_info *res);
 u64 xen_steal_clock(int cpu);
+void xen_save_steal_clock(int cpu);
+void xen_restore_steal_clock(int cpu);
 
 int xen_setup_shutdown_event(void);
 
-- 
2.15.3.AMZN


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel