Problems with kvm steal time accounting.

Message ID 1434062535-28185-1-git-send-email-john.stultz@linaro.org
State New
Headers show

Commit Message

John Stultz June 11, 2015, 10:42 p.m.
I recently noticed that running the selftest/timers/posix_timers
test after a suspend/resume cycle in my VM environment was
hanging, because the ITIMER_VIRTUAL signal wasn't firing.

More specifically, it seemed like after a suspend, process utime
wasn't being updated unless the process was running on cpu 0.

I chased this down a bit, and it seems the problem is that
on resume, the paravirt_steal_clock() is reset to zero on all
cpus other then cpu0. This causes the rq->prev_steal_time
value to be set to a negative value, which mucks up the
rest of the time accounting.

I'm not sure if this is due to a bug in kvm or what.
I've not chased back how long this bug has been around,
but it seemed to crop up in at least 3.16 when I tried
it.

The following hack works around this, but I suspect
the steal-clock offset needs to be saved on suspend
and re-added on resume, or the prev_steal_time values
need to be reset, so that we can be robust in the
face of this sort of behavior.

Thoughts or other ideas?

thanks
-john

Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Gleb Natapov <gleb@kernel.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Marcelo Tosatti <mtosatti@redhat.com>
---
 kernel/sched/cputime.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Patch

diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
index 8394b1e..439d880 100644
--- a/kernel/sched/cputime.c
+++ b/kernel/sched/cputime.c
@@ -258,12 +258,17 @@  static __always_inline bool steal_account_process_tick(void)
 {
 #ifdef CONFIG_PARAVIRT
 	if (static_key_false(&paravirt_steal_enabled)) {
-		u64 steal;
+		u64 steal, prev;
 		cputime_t steal_ct;
 
 		steal = paravirt_steal_clock(smp_processor_id());
-		steal -= this_rq()->prev_steal_time;
-
+		prev = this_rq()->prev_steal_time;
+		if (steal >= prev)
+			steal -= prev;
+		else {
+			printk("WARNING: %lld - %lld is negative!!\n", steal, prev);
+			this_rq()->prev_steal_time = steal;
+		}
 		/*
 		 * cputime_t may be less precise than nsecs (eg: if it's
 		 * based on jiffies). Lets cast the result to cputime