[PATCH] fix scaled time accounting possible divide by zero

STALE6868d

4 messages, 2 authors, 2007-11-20 · open the first message on its own page

[PATCH] fix scaled time accounting possible divide by zero

From: Michael Neuling <hidden>
Date: 2007-11-20 03:25:41

This fixes a problem noticed by Balbir Singh

Signed-off-by: Michael Neuling <redacted>
---
Paulus: can we send this up for 2.6.24?

 arch/powerpc/kernel/time.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Index: linux-2.6-ozlabs/arch/powerpc/kernel/time.c
===================================================================
--- linux-2.6-ozlabs.orig/arch/powerpc/kernel/time.c
+++ linux-2.6-ozlabs/arch/powerpc/kernel/time.c
@@ -244,8 +244,9 @@ void account_system_vtime(struct task_st
 		/* deltascaled includes both user and system time.
 		 * Hence scale it based on the purr ratio to estimate
 		 * the system time */
-		deltascaled = deltascaled * get_paca()->system_time /
-		(get_paca()->system_time + get_paca()->user_time);
+		if (get_paca()->user_time)
+			deltascaled = deltascaled * get_paca()->system_time /
+			(get_paca()->system_time + get_paca()->user_time);
 		delta += get_paca()->system_time;
 		get_paca()->system_time = 0;
 	}

Re: [PATCH] fix scaled time accounting possible divide by zero

From: Balbir Singh <hidden>
Date: 2007-11-20 04:16:22

Michael Neuling wrote:
quoted hunk
This fixes a problem noticed by Balbir Singh

Signed-off-by: Michael Neuling <redacted>
---
Paulus: can we send this up for 2.6.24?

 arch/powerpc/kernel/time.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Index: linux-2.6-ozlabs/arch/powerpc/kernel/time.c
===================================================================
--- linux-2.6-ozlabs.orig/arch/powerpc/kernel/time.c
+++ linux-2.6-ozlabs/arch/powerpc/kernel/time.c
@@ -244,8 +244,9 @@ void account_system_vtime(struct task_st
 		/* deltascaled includes both user and system time.
 		 * Hence scale it based on the purr ratio to estimate
 		 * the system time */
-		deltascaled = deltascaled * get_paca()->system_time /
-		(get_paca()->system_time + get_paca()->user_time);
+		if (get_paca()->user_time)
+			deltascaled = deltascaled * get_paca()->system_time /
+			(get_paca()->system_time + get_paca()->user_time);
Hi, Michael,

I'd be doubly careful with scaled multiplication approach, we tried this
for CFS (please see task_utime() and task_stime() and the fixes that
went around it). We ran into problems were due to multiplication
rounding errors, we would see stime and utime go back after a period
of time.

Please see http://kerneltrap.org/mailarchive/linux-kernel/2007/10/16/344377

Our problems were made severe by the fact that sum_exec_runtime and
stime/utime accounting occured differently. stime/utime were sampled
at jiffy boundaries and hence could we incorrect. I think we need
to use rounding to ensure that ac_scaled*time never goes back
due to rounding errors.
 		delta += get_paca()->system_time;
 		get_paca()->system_time = 0;
 	}

-- 
	Warm Regards,
	Balbir Singh
	Linux Technology Center
	IBM, ISTL

[PATCH] fix scaled time accounting possible divide by zero

From: Michael Neuling <hidden>
Date: 2007-11-20 04:18:40

If we get no user time allocated since the last account_system_vtime,
the system to user time ratio estimate can end up dividing by zero.

This was causing a problem noticed by Balbir Singh.

Signed-off-by: Michael Neuling <redacted>
---
Resent as the first version had whitespace corruption.  

Paulus: can we send this up for 2.6.24?

 arch/powerpc/kernel/time.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Index: linux-2.6-ozlabs/arch/powerpc/kernel/time.c
===================================================================
--- linux-2.6-ozlabs.orig/arch/powerpc/kernel/time.c
+++ linux-2.6-ozlabs/arch/powerpc/kernel/time.c
@@ -241,8 +241,9 @@ void account_system_vtime(struct task_st
 		/* deltascaled includes both user and system time.
 		 * Hence scale it based on the purr ratio to estimate
 		 * the system time */
-		deltascaled = deltascaled * get_paca()->system_time /
-			(get_paca()->system_time + get_paca()->user_time);
+		if (get_paca()->user_time)
+			deltascaled = deltascaled * get_paca()->system_time /
+			     (get_paca()->system_time + get_paca()->user_time);
 		delta += get_paca()->system_time;
 		get_paca()->system_time = 0;
 	}

Re: [PATCH] fix scaled time accounting possible divide by zero

From: Michael Neuling <hidden>
Date: 2007-11-20 04:36:49

In message [off-list ref] you wrote:
Michael Neuling wrote:
quoted
This fixes a problem noticed by Balbir Singh

Signed-off-by: Michael Neuling <redacted>
---
Paulus: can we send this up for 2.6.24?

 arch/powerpc/kernel/time.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Index: linux-2.6-ozlabs/arch/powerpc/kernel/time.c
===================================================================
--- linux-2.6-ozlabs.orig/arch/powerpc/kernel/time.c
+++ linux-2.6-ozlabs/arch/powerpc/kernel/time.c
@@ -244,8 +244,9 @@ void account_system_vtime(struct task_st
 		/* deltascaled includes both user and system time.
 		 * Hence scale it based on the purr ratio to estimate
 		 * the system time */
-		deltascaled = deltascaled * get_paca()->system_time /
-		(get_paca()->system_time + get_paca()->user_time);
+		if (get_paca()->user_time)
+			deltascaled = deltascaled * get_paca()->system_time /
+			(get_paca()->system_time + get_paca()->user_time);
Hi, Michael,

I'd be doubly careful with scaled multiplication approach, we tried this
for CFS (please see task_utime() and task_stime() and the fixes that
went around it). We ran into problems were due to multiplication
rounding errors, we would see stime and utime go back after a period
of time.

Please see http://kerneltrap.org/mailarchive/linux-kernel/2007/10/16/344377

Our problems were made severe by the fact that sum_exec_runtime and
stime/utime accounting occured differently. stime/utime were sampled
at jiffy boundaries and hence could we incorrect. I think we need
to use rounding to ensure that ac_scaled*time never goes back
due to rounding errors.
I've not changed the math here much, just the case of user_time being
zero.

Is this related to this patch specifically, or something that's been
wrong with these patches for a while?

Mikey
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help