Skip to contentSkip to conversation

fix scaled time accounting possible divide by zero

PATCH In review Michael Neuling 2 participants 4 messages Nov 20, 2007 PowerPC

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 fix scaled time accounting possible divide by zero

This fixes a problem noticed by Balbir Singh

---
Paulus: can we send this up for 2.6.24?

 arch/powerpc/kernel/time.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
arch/powerpc/kernel/time.c +3 -2
Index: 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;
 	}
#1 lore raw
Quoted 14 lines · Michael Neuling

This fixes a problem noticed by Balbir Singh

---
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
===================================================================

arch/powerpc/kernel/time.c void account_system_vtime(struct task_st · line 249

Quoted 8 lines
arch/powerpc/kernel/time.c +3 -2
@@ -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.

Quoted 3 lines
arch/powerpc/kernel/time.c +0 -0
 		delta += get_paca()->system_time;
 		get_paca()->system_time = 0;
 	}
Signature · 4 lines
	Warm Regards,
	Balbir Singh
	Linux Technology Center
	IBM, ISTL
#2 lore raw
Quoted 32 lines · In message <[email protected]> you wrote:
Quoted 14 lines · Michael Neuling

This fixes a problem noticed by Balbir Singh

---
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
===================================================================

arch/powerpc/kernel/time.c void account_system_vtime(struct task_st · line 249

Quoted 8 lines
arch/powerpc/kernel/time.c +3 -2
@@ -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.

Quoted 7 lines

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

#3 lore raw

PATCH 1/1 fix scaled time accounting possible divide by zero

In reply to Michael Neuling

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.

---
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(-)
arch/powerpc/kernel/time.c +3 -2
Index: 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;
 	}