Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1464288 > unrolled thread

[PATCH v3] sched/cputime: Resync steal time when guest & host lose sync

Back to article view | Back to linux.kernel

Contents

  [PATCH v3] sched/cputime: Resync steal time when guest & host lose sync Wanpeng Li <kernellwp@gmail.com> - 2016-08-17 04:30 +0200
    [tip:sched/core] sched/cputime: Resync steal time when guest & host  lose sync tip-bot for Wanpeng Li <tipbot@zytor.com> - 2016-08-18 13:00 +0200
      Re: [tip:sched/core] sched/cputime: Resync steal time when guest &  host lose sync Frederic Weisbecker <fweisbec@gmail.com> - 2016-08-18 14:50 +0200

#1464288 — [PATCH v3] sched/cputime: Resync steal time when guest & host lose sync

From: Wanpeng Li <wanpeng.li@hotmail.com>

Commit:

  	57430218317e ("sched/cputime: Count actually elapsed irq & softirq time")

... triggered a regression:

| An i5 laptop, 4 pCPUs, 4vCPUs for one full dynticks guest, there are four
| cpu hog processes(for loop) running in the guest, I hot-unplug the pCPUs 
| on host one by one until there is only one left, then observe the top in 
| guest, there are 100% st for cpu0(housekeeping), and 75% st for other cpus
| (nohz full mode). However, w/o this commit, 75% for all the four cpus.

When a guest is interrupted for a longer amount of time, missed clock ticks 
are not redelivered later. Because of that, we should not limit the amount 
of steal time accounted to the amount of time that the calling functions 
think have passed.

However, the interval returned by account_other_time() is NOT rounded down 
to the nearest jiffy, while the base interval in get_vtime_delta() it is 
subtracted from is, so the max cputime limit is required to avoid underflow.

This patch fix the regression by limiting the account_other_time() from 
get_vtime_delta() to avoid underflow, and let other three call sites
(account_other_time() and steal_account_process_time()) account however 
much steal time the host told us elapsed. 

Suggested-by: Rik van Riel <riel@redhat.com> 
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krcmar <rkrcmar@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
v2 -> v3:
 * update code comments
v1 -> v2:
 * add code comments and update the changelog

 kernel/sched/cputime.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
index 9858266..2b9e5e5 100644
--- a/kernel/sched/cputime.c
+++ b/kernel/sched/cputime.c
@@ -263,6 +263,11 @@ void account_idle_time(cputime_t cputime)
 		cpustat[CPUTIME_IDLE] += (__force u64) cputime;
 }
 
+/*
+ * When a guest is interrupted for a longer amount of time, missed clock
+ * ticks are not redelivered later. Due to that, this function may on
+ * occasion account more time than the calling functions think elapsed.
+ */
 static __always_inline cputime_t steal_account_process_time(cputime_t maxtime)
 {
 #ifdef CONFIG_PARAVIRT
@@ -371,7 +376,7 @@ static void irqtime_account_process_tick(struct task_struct *p, int user_tick,
 	 * idle, or potentially user or system time. Due to rounding,
 	 * other time can exceed ticks occasionally.
 	 */
-	other = account_other_time(cputime);
+	other = account_other_time(ULONG_MAX);
 	if (other >= cputime)
 		return;
 	cputime -= other;
@@ -486,7 +491,7 @@ void account_process_tick(struct task_struct *p, int user_tick)
 	}
 
 	cputime = cputime_one_jiffy;
-	steal = steal_account_process_time(cputime);
+	steal = steal_account_process_time(ULONG_MAX);
 
 	if (steal >= cputime)
 		return;
@@ -516,7 +521,7 @@ void account_idle_ticks(unsigned long ticks)
 	}
 
 	cputime = jiffies_to_cputime(ticks);
-	steal = steal_account_process_time(cputime);
+	steal = steal_account_process_time(ULONG_MAX);
 
 	if (steal >= cputime)
 		return;
@@ -694,6 +699,13 @@ static cputime_t get_vtime_delta(struct task_struct *tsk)
 	unsigned long now = READ_ONCE(jiffies);
 	cputime_t delta, other;
 
+	/*
+	 * Unlike tick based timing, vtime based timing never has lost
+	 * ticks, and no need for steal time accounting to make up for
+	 * lost ticks. Vtime accounts a rounded version of actual
+	 * elapsed time. Limit account_other_time to prevent rounding
+	 * errors from causing elapsed vtime to go negative.
+	 */
 	delta = jiffies_to_cputime(now - tsk->vtime_snap);
 	other = account_other_time(delta);
 	WARN_ON_ONCE(tsk->vtime_snap_whence == VTIME_INACTIVE);
-- 
1.9.1

[toc] | [next] | [standalone]

#1465145 — [tip:sched/core] sched/cputime: Resync steal time when guest & host lose sync

Commit-ID:  03cbc732639ddcad15218c4b2046d255851ff1e3
Gitweb:     http://git.kernel.org/tip/03cbc732639ddcad15218c4b2046d255851ff1e3
Author:     Wanpeng Li <wanpeng.li@hotmail.com>
AuthorDate: Wed, 17 Aug 2016 10:05:46 +0800
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 18 Aug 2016 11:19:48 +0200

sched/cputime: Resync steal time when guest & host lose sync

Commit:

  57430218317e ("sched/cputime: Count actually elapsed irq & softirq time")

... fixed a bug but also triggered a regression:

On an i5 laptop, 4 pCPUs, 4vCPUs for one full dynticks guest, there are four
CPU hog processes(for loop) running in the guest, I hot-unplug the pCPUs
on host one by one until there is only one left, then observe CPU utilization
via 'top' in the guest, it shows:

  100% st for cpu0(housekeeping)
   75% st for other CPUs (nohz full mode)

However, w/o this commit it shows the correct 75% for all four CPUs.

When a guest is interrupted for a longer amount of time, missed clock ticks
are not redelivered later. Because of that, we should not limit the amount
of steal time accounted to the amount of time that the calling functions
think have passed.

However, the interval returned by account_other_time() is NOT rounded down
to the nearest jiffy, while the base interval in get_vtime_delta() it is
subtracted from is, so the max cputime limit is required to avoid underflow.

This patch fixes the regression by limiting the account_other_time() from
get_vtime_delta() to avoid underflow, and lets the other three call sites
(in account_other_time() and steal_account_process_time()) account however
much steal time the host told us elapsed.

Suggested-by: Rik van Riel <riel@redhat.com>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
Reviewed-by: Rik van Riel <riel@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Radim Krcmar <rkrcmar@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: kvm@vger.kernel.org
Link: http://lkml.kernel.org/r/1471399546-4069-1-git-send-email-wanpeng.li@hotmail.com
[ Improved the changelog. ]
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/sched/cputime.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
index 2ee83b2..a846cf8 100644
--- a/kernel/sched/cputime.c
+++ b/kernel/sched/cputime.c
@@ -263,6 +263,11 @@ void account_idle_time(cputime_t cputime)
 		cpustat[CPUTIME_IDLE] += (__force u64) cputime;
 }
 
+/*
+ * When a guest is interrupted for a longer amount of time, missed clock
+ * ticks are not redelivered later. Due to that, this function may on
+ * occasion account more time than the calling functions think elapsed.
+ */
 static __always_inline cputime_t steal_account_process_time(cputime_t maxtime)
 {
 #ifdef CONFIG_PARAVIRT
@@ -371,7 +376,7 @@ static void irqtime_account_process_tick(struct task_struct *p, int user_tick,
 	 * idle, or potentially user or system time. Due to rounding,
 	 * other time can exceed ticks occasionally.
 	 */
-	other = account_other_time(cputime);
+	other = account_other_time(ULONG_MAX);
 	if (other >= cputime)
 		return;
 	cputime -= other;
@@ -486,7 +491,7 @@ void account_process_tick(struct task_struct *p, int user_tick)
 	}
 
 	cputime = cputime_one_jiffy;
-	steal = steal_account_process_time(cputime);
+	steal = steal_account_process_time(ULONG_MAX);
 
 	if (steal >= cputime)
 		return;
@@ -516,7 +521,7 @@ void account_idle_ticks(unsigned long ticks)
 	}
 
 	cputime = jiffies_to_cputime(ticks);
-	steal = steal_account_process_time(cputime);
+	steal = steal_account_process_time(ULONG_MAX);
 
 	if (steal >= cputime)
 		return;
@@ -699,6 +704,13 @@ static cputime_t get_vtime_delta(struct task_struct *tsk)
 	unsigned long now = READ_ONCE(jiffies);
 	cputime_t delta, other;
 
+	/*
+	 * Unlike tick based timing, vtime based timing never has lost
+	 * ticks, and no need for steal time accounting to make up for
+	 * lost ticks. Vtime accounts a rounded version of actual
+	 * elapsed time. Limit account_other_time to prevent rounding
+	 * errors from causing elapsed vtime to go negative.
+	 */
 	delta = jiffies_to_cputime(now - tsk->vtime_snap);
 	other = account_other_time(delta);
 	WARN_ON_ONCE(tsk->vtime_snap_whence == VTIME_INACTIVE);

[toc] | [prev] | [next] | [standalone]

#1465229 — Re: [tip:sched/core] sched/cputime: Resync steal time when guest & host lose sync

On Thu, Aug 18, 2016 at 03:55:39AM -0700, tip-bot for Wanpeng Li wrote:
> Commit-ID:  03cbc732639ddcad15218c4b2046d255851ff1e3
> Gitweb:     http://git.kernel.org/tip/03cbc732639ddcad15218c4b2046d255851ff1e3
> Author:     Wanpeng Li <wanpeng.li@hotmail.com>
> AuthorDate: Wed, 17 Aug 2016 10:05:46 +0800
> Committer:  Ingo Molnar <mingo@kernel.org>
> CommitDate: Thu, 18 Aug 2016 11:19:48 +0200
> 
> sched/cputime: Resync steal time when guest & host lose sync
> 
> Commit:
> 
>   57430218317e ("sched/cputime: Count actually elapsed irq & softirq time")
> 
> ... fixed a bug but also triggered a regression:
> 
> On an i5 laptop, 4 pCPUs, 4vCPUs for one full dynticks guest, there are four
> CPU hog processes(for loop) running in the guest, I hot-unplug the pCPUs
> on host one by one until there is only one left, then observe CPU utilization
> via 'top' in the guest, it shows:
> 
>   100% st for cpu0(housekeeping)
>    75% st for other CPUs (nohz full mode)
> 
> However, w/o this commit it shows the correct 75% for all four CPUs.
> 
> When a guest is interrupted for a longer amount of time, missed clock ticks
> are not redelivered later. Because of that, we should not limit the amount
> of steal time accounted to the amount of time that the calling functions
> think have passed.
> 
> However, the interval returned by account_other_time() is NOT rounded down
> to the nearest jiffy, while the base interval in get_vtime_delta() it is
> subtracted from is, so the max cputime limit is required to avoid underflow.
> 
> This patch fixes the regression by limiting the account_other_time() from
> get_vtime_delta() to avoid underflow, and lets the other three call sites
> (in account_other_time() and steal_account_process_time()) account however
> much steal time the host told us elapsed.
> 
> Suggested-by: Rik van Riel <riel@redhat.com>
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> Reviewed-by: Rik van Riel <riel@redhat.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>

ACK, thanks Wanpeng Li!

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web