linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] sched/cputime: add steal time support to full dynticks CPU time accounting
@ 2016-05-10  5:34 Wanpeng Li
  2016-05-18  4:04 ` Rik van Riel
  2016-05-18  7:10 ` Rik van Riel
  0 siblings, 2 replies; 4+ messages in thread
From: Wanpeng Li @ 2016-05-10  5:34 UTC (permalink / raw)
  To: linux-kernel, kvm
  Cc: Wanpeng Li, Ingo Molnar, Peter Zijlstra (Intel),
	Rik van Riel, Thomas Gleixner, Frederic Weisbecker,
	Paolo Bonzini, Radim

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

This patch adds steal guest time support to full dynticks CPU time 
accounting. After commit ff9a9b4c(sched, time: Switch VIRT_CPU_ACCOUNTING_GEN 
to jiffy granularity), time is jiffy based sampling even if it's 
still listened to ring boundaries, so steal_account_process_tick() 
is reused to account how much 'ticks' are steal time after the 
last accumulation. 

Suggested-by: Rik van Riel <riel@redhat.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Rik van Riel <riel@redhat.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim <rkrcmar@redhat.com>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
---
 kernel/sched/cputime.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
index 75f98c5..b96bd8f 100644
--- a/kernel/sched/cputime.c
+++ b/kernel/sched/cputime.c
@@ -257,7 +257,7 @@ void account_idle_time(cputime_t cputime)
 		cpustat[CPUTIME_IDLE] += (__force u64) cputime;
 }
 
-static __always_inline bool steal_account_process_tick(void)
+static __always_inline unsigned long steal_account_process_tick(void)
 {
 #ifdef CONFIG_PARAVIRT
 	if (static_key_false(&paravirt_steal_enabled)) {
@@ -279,7 +279,7 @@ static __always_inline bool steal_account_process_tick(void)
 		return steal_jiffies;
 	}
 #endif
-	return false;
+	return 0;
 }
 
 /*
@@ -691,8 +691,11 @@ static cputime_t get_vtime_delta(struct task_struct *tsk)
 
 static void __vtime_account_system(struct task_struct *tsk)
 {
+	unsigned long steal_time = steal_account_process_tick();
 	cputime_t delta_cpu = get_vtime_delta(tsk);
 
+	delta_cpu = steal_time ? (delta_cpu -
+		jiffies_to_cputime(steal_time)) : delta_cpu;
 	account_system_time(tsk, irq_count(), delta_cpu, cputime_to_scaled(delta_cpu));
 }
 
@@ -723,7 +726,11 @@ void vtime_account_user(struct task_struct *tsk)
 	write_seqcount_begin(&tsk->vtime_seqcount);
 	tsk->vtime_snap_whence = VTIME_SYS;
 	if (vtime_delta(tsk)) {
+		unsigned long steal_time = steal_account_process_tick();
 		delta_cpu = get_vtime_delta(tsk);
+
+		delta_cpu = steal_time ? (delta_cpu -
+			jiffies_to_cputime(steal_time)) : delta_cpu;
 		account_user_time(tsk, delta_cpu, cputime_to_scaled(delta_cpu));
 	}
 	write_seqcount_end(&tsk->vtime_seqcount);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] sched/cputime: add steal time support to full dynticks CPU time accounting
  2016-05-10  5:34 [PATCH] sched/cputime: add steal time support to full dynticks CPU time accounting Wanpeng Li
@ 2016-05-18  4:04 ` Rik van Riel
  2016-05-18  7:10 ` Rik van Riel
  1 sibling, 0 replies; 4+ messages in thread
From: Rik van Riel @ 2016-05-18  4:04 UTC (permalink / raw)
  To: Wanpeng Li, linux-kernel, kvm
  Cc: Wanpeng Li, Ingo Molnar, Peter Zijlstra (Intel),
	Thomas Gleixner, Frederic Weisbecker, Paolo Bonzini, Radim

[-- Attachment #1: Type: text/plain, Size: 978 bytes --]

On Tue, 2016-05-10 at 13:34 +0800, Wanpeng Li wrote:
> From: Wanpeng Li <wanpeng.li@hotmail.com>
> 
> This patch adds steal guest time support to full dynticks CPU time 
> accounting. After commit ff9a9b4c(sched, time: Switch
> VIRT_CPU_ACCOUNTING_GEN 
> to jiffy granularity), time is jiffy based sampling even if it's 
> still listened to ring boundaries, so steal_account_process_tick() 
> is reused to account how much 'ticks' are steal time after the 
> last accumulation. 
> 
> Suggested-by: Rik van Riel <riel@redhat.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
> Cc: Rik van Riel <riel@redhat.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim <rkrcmar@redhat.com>
> Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
> 

Acked-by: Rik van Riel <riel@redhat.com>

-- 
All Rights Reversed.


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] sched/cputime: add steal time support to full dynticks CPU time accounting
  2016-05-10  5:34 [PATCH] sched/cputime: add steal time support to full dynticks CPU time accounting Wanpeng Li
  2016-05-18  4:04 ` Rik van Riel
@ 2016-05-18  7:10 ` Rik van Riel
  2016-05-18  7:45   ` Wanpeng Li
  1 sibling, 1 reply; 4+ messages in thread
From: Rik van Riel @ 2016-05-18  7:10 UTC (permalink / raw)
  To: Wanpeng Li, linux-kernel, kvm
  Cc: Wanpeng Li, Ingo Molnar, Peter Zijlstra (Intel),
	Thomas Gleixner, Frederic Weisbecker, Paolo Bonzini, Radim

[-- Attachment #1: Type: text/plain, Size: 1070 bytes --]

On Tue, 2016-05-10 at 13:34 +0800, Wanpeng Li wrote:
> 
> +++ b/kernel/sched/cputime.c
> 
> @@ -691,8 +691,11 @@ static cputime_t get_vtime_delta(struct
> task_struct *tsk)
>  
>  static void __vtime_account_system(struct task_struct *tsk)
>  {
> +	unsigned long steal_time = steal_account_process_tick();
>  	cputime_t delta_cpu = get_vtime_delta(tsk);
>  
> +	delta_cpu = steal_time ? (delta_cpu -
> +		jiffies_to_cputime(steal_time)) : delta_cpu;
>  	account_system_time(tsk, irq_count(), delta_cpu,
> cputime_to_scaled(delta_cpu));
>  }
>  

Sorry to have to go back on my previous email, but
this is now a NAK

The above code can end up passing a negative number
to account_system_time(), which in turn can cause a
divide by zero in scale_stime()

The code needs to ensure that if all the time that
passed was accounted as steal time (which could be
more jiffies than expected, due to remaining partial
jiffies in steal_account_process_tick), the function
does not call account_system_time().

-- 
All Rights Reversed.


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] sched/cputime: add steal time support to full dynticks CPU time accounting
  2016-05-18  7:10 ` Rik van Riel
@ 2016-05-18  7:45   ` Wanpeng Li
  0 siblings, 0 replies; 4+ messages in thread
From: Wanpeng Li @ 2016-05-18  7:45 UTC (permalink / raw)
  To: Rik van Riel
  Cc: linux-kernel, kvm, Wanpeng Li, Ingo Molnar,
	Peter Zijlstra (Intel),
	Thomas Gleixner, Frederic Weisbecker, Paolo Bonzini, Radim

2016-05-18 15:10 GMT+08:00 Rik van Riel <riel@redhat.com>:
> On Tue, 2016-05-10 at 13:34 +0800, Wanpeng Li wrote:
>>
>> +++ b/kernel/sched/cputime.c
>>
>> @@ -691,8 +691,11 @@ static cputime_t get_vtime_delta(struct
>> task_struct *tsk)
>>
>>  static void __vtime_account_system(struct task_struct *tsk)
>>  {
>> +     unsigned long steal_time = steal_account_process_tick();
>>       cputime_t delta_cpu = get_vtime_delta(tsk);
>>
>> +     delta_cpu = steal_time ? (delta_cpu -
>> +             jiffies_to_cputime(steal_time)) : delta_cpu;
>>       account_system_time(tsk, irq_count(), delta_cpu,
>> cputime_to_scaled(delta_cpu));
>>  }
>>
>
> Sorry to have to go back on my previous email, but
> this is now a NAK
>
> The above code can end up passing a negative number
> to account_system_time(), which in turn can cause a
> divide by zero in scale_stime()
>
> The code needs to ensure that if all the time that
> passed was accounted as steal time (which could be
> more jiffies than expected, due to remaining partial
> jiffies in steal_account_process_tick), the function
> does not call account_system_time().

Yeah, I will fix it in next version, thank Rik very much for debugging
with me to figure out the root cause of divide zero.

Regards,
Wanpeng Li

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-05-18  7:45 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-10  5:34 [PATCH] sched/cputime: add steal time support to full dynticks CPU time accounting Wanpeng Li
2016-05-18  4:04 ` Rik van Riel
2016-05-18  7:10 ` Rik van Riel
2016-05-18  7:45   ` Wanpeng Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).