Subject: fix process utime and stime values
To: None <tech-kern@netbsd.org>
From: David Laight <david@l8s.co.uk>
List: tech-kern
Date: 03/03/2003 23:30:41
The utime and stime (and hence ctime) values (eg as reported by ps)
are innacurate for 3^Hseveral reasons:

1) when a process exits the time for the current 'timeslice' is ignored
   because calcru() is called after the lwp state has been changed.

2) if a process isn't interrupted by a timer tick calcru() discards
   the execution time.

3) sys_sysctl reports the values from p_stats->p_ru.ru_{u,s}time
   but these are only updated if a process asks for its child times.

4) the child times reported by sysctl are the sum of the system and
   user values - but the usecs can wrap

5) calcru() doesn't allow for more than one lwp being active.

The patch below fixes these, I'll apply it later in the week unless
someone objects.

	David

Index: kern/kern_exit.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_exit.c,v
retrieving revision 1.112
diff -u -p -r1.112 kern_exit.c
--- kern/kern_exit.c	2003/02/22 01:00:14	1.112
+++ kern/kern_exit.c	2003/03/03 22:49:27
@@ -276,6 +276,17 @@ exit1(struct lwp *l, int rv)
 	l->l_stat = SDEAD;
 
 	/*
+	 * Save exit status and final rusage info, adding in child rusage
+	 * info and self times.
+	 * In order to pick up the time for the current execution, we must
+	 * do this before unlinking the lwp from l_list.
+	 */
+	p->p_xstat = rv;
+	*p->p_ru = p->p_stats->p_ru;
+	calcru(p, &p->p_ru->ru_utime, &p->p_ru->ru_stime, NULL);
+	ruadd(p->p_ru, &p->p_stats->p_cru);
+
+	/*
 	 * Remove proc from pidhash chain so looking it up won't
 	 * work.  Move it from allproc to zombproc, but do not yet
 	 * wake up the reaper.  We will put the proc on the
@@ -340,15 +351,6 @@ exit1(struct lwp *l, int rv)
 
 		proclist_unlock_read();
 	}
-
-	/*
-	 * Save exit status and final rusage info, adding in child rusage
-	 * info and self times.
-	 */
-	p->p_xstat = rv;
-	*p->p_ru = p->p_stats->p_ru;
-	calcru(p, &p->p_ru->ru_utime, &p->p_ru->ru_stime, NULL);
-	ruadd(p->p_ru, &p->p_stats->p_cru);
 
 	/*
 	 * Notify interested parties of our demise.
Index: kern/kern_resource.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_resource.c,v
retrieving revision 1.68
diff -u -p -r1.68 kern_resource.c
--- kern/kern_resource.c	2003/01/18 10:06:29	1.68
+++ kern/kern_resource.c	2003/03/03 22:49:29
@@ -390,7 +390,7 @@ calcru(p, up, sp, ip)
 	struct timeval *ip;
 {
 	u_quad_t u, st, ut, it, tot;
-	long sec, usec;
+	unsigned long sec, usec;
 	int s;
 	struct timeval tv;
 	struct lwp *l;
@@ -401,15 +401,6 @@ calcru(p, up, sp, ip)
 	it = p->p_iticks;
 	splx(s);
 
-	tot = st + ut + it;
-	if (tot == 0) {
-		up->tv_sec = up->tv_usec = 0;
-		sp->tv_sec = sp->tv_usec = 0;
-		if (ip != NULL)
-			ip->tv_sec = ip->tv_usec = 0;
-		return;
-	}
-
 	sec = p->p_rtime.tv_sec;
 	usec = p->p_rtime.tv_usec;
 	for (l = LIST_FIRST(&p->p_lwps); l != NULL; 
@@ -430,10 +421,28 @@ calcru(p, up, sp, ip)
 			microtime(&tv);
 			sec += tv.tv_sec - spc->spc_runtime.tv_sec;
 			usec += tv.tv_usec - spc->spc_runtime.tv_usec;
-			
-			break;
 		}
 	}
+
+	tot = st + ut + it;
+	if (tot == 0) {
+		/* No ticks, so can't use to share time out, split 50-50 */
+		if (usec > 1000000 && (sec & 1)) {
+			usec -= 1000000;
+			sec++;
+		} else {
+			if (sec & 1) {
+				usec += 1000000;
+				sec--;
+			}
+		}
+		up->tv_sec = sp->tv_sec = sec / 2;
+		up->tv_usec = sp->tv_usec = usec / 2;
+		if (ip != NULL)
+			ip->tv_sec = ip->tv_usec = 0;
+		return;
+	}
+
 	u = (u_quad_t) sec * 1000000 + usec;
 	st = (u * st) / tot;
 	sp->tv_sec = st / 1000000;
Index: kern/kern_sysctl.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_sysctl.c,v
retrieving revision 1.129
diff -u -p -r1.129 kern_sysctl.c
--- kern/kern_sysctl.c	2003/03/01 23:48:44	1.129
+++ kern/kern_sysctl.c	2003/03/03 22:49:36
@@ -1794,6 +1794,8 @@ fill_kproc2(struct proc *p, struct kinfo
 {
 	struct tty *tp;
 	struct lwp *l;
+	struct timeval ut, st;
+
 	memset(ki, 0, sizeof(*ki));
 
 	ki->p_paddr = PTRTOINT64(p);
@@ -1928,10 +1930,11 @@ fill_kproc2(struct proc *p, struct kinfo
 		ki->p_ustart_sec = p->p_stats->p_start.tv_sec;
 		ki->p_ustart_usec = p->p_stats->p_start.tv_usec;
 
-		ki->p_uutime_sec = p->p_stats->p_ru.ru_utime.tv_sec;
-		ki->p_uutime_usec = p->p_stats->p_ru.ru_utime.tv_usec;
-		ki->p_ustime_sec = p->p_stats->p_ru.ru_stime.tv_sec;
-		ki->p_ustime_usec = p->p_stats->p_ru.ru_stime.tv_usec;
+		calcru(p, &ut, &st, 0);
+		ki->p_uutime_sec = ut.tv_sec;
+		ki->p_uutime_usec = ut.tv_usec;
+		ki->p_ustime_sec = st.tv_sec;
+		ki->p_ustime_usec = st.tv_usec;
 
 		ki->p_uru_maxrss = p->p_stats->p_ru.ru_maxrss;
 		ki->p_uru_ixrss = p->p_stats->p_ru.ru_ixrss;
@@ -1948,10 +1951,10 @@ fill_kproc2(struct proc *p, struct kinfo
 		ki->p_uru_nvcsw = p->p_stats->p_ru.ru_nvcsw;
 		ki->p_uru_nivcsw = p->p_stats->p_ru.ru_nivcsw;
 
-		ki->p_uctime_sec = p->p_stats->p_cru.ru_utime.tv_sec +
-		    p->p_stats->p_cru.ru_stime.tv_sec;
-		ki->p_uctime_usec = p->p_stats->p_cru.ru_utime.tv_usec +
-		    p->p_stats->p_cru.ru_stime.tv_usec;
+		timeradd(&p->p_stats->p_cru.ru_utime,
+			 &p->p_stats->p_cru.ru_stime, &ut);
+		ki->p_uctime_sec = ut.tv_sec;
+		ki->p_uctime_usec = ut.tv_usec;
 	}
 #ifdef MULTIPROCESSOR
 	if (l && l->l_cpu != NULL)

-- 
David Laight: david@l8s.co.uk