Subject: __HAVE_CHOOSEPROC and microtime() optimisations
To: None <tech-kern@netbsd.org>
From: David Laight <david@l8s.co.uk>
List: tech-kern
Date: 09/23/2002 14:42:32
I've just been merging the new __HAVE_CHOOSEPROC stuff in with
my microtime optimisations.  This goes reasonably well now.

The diff below includes a gratuituous change that replaces
p->p_rtime.tv_usec and p->p_rtime.tv_sec with a 64bit
counter p->p_rtime.  I think that is a performance gain on any
system where the multiply by 1000000 isn't stupidly expensive.

Updating p->p_rtime even if we immediately switch to the same
process is probably a gain because:
1) I doubt we do null process switches that often
2) It frees some registers that would have to hold the local
   variables - indeed 'u' and 's' probably get dumped on the stack
   before being saved in the old 'p'.

I also think there is a bug in the new code because the h/w
performance counters are now running while idle...

(Oh - I haven't finished my merge yet - so haven't tested the fix)

	David

Index: kern_synch.c
===================================================================
RCS file: /cvsroot/syssrc/sys/kern/kern_synch.c,v
retrieving revision 1.113
diff -u -r1.113 kern_synch.c
--- kern_synch.c	2002/09/22 05:36:48	1.113
+++ kern_synch.c	2002/09/23 13:26:06
@@ -794,8 +805,8 @@
 {
 	struct schedstate_percpu *spc;
 	struct rlimit *rlim;
-	long s, u;
 	struct timeval tv;
+	u_quad_t us;
 #if defined(MULTIPROCESSOR)
 	int hold_count;
 #endif
@@ -833,15 +844,20 @@
 	 * process was running.
 	 */
 	microtime(&tv);
-	u = p->p_rtime.tv_usec + (tv.tv_usec - spc->spc_runtime.tv_usec);
-	s = p->p_rtime.tv_sec + (tv.tv_sec - spc->spc_runtime.tv_sec);
-	if (u < 0) {
-		u += 1000000;
-		s--;
-	} else if (u >= 1000000) {
-		u -= 1000000;
-		s++;
-	}
+	us = (tv.tv_usec - spc->spc_runtime.tv_usec)
+			+ (tv.tv_sec - spc->spc_runtime.tv_sec) * 1000000ll;
+	if (us & (1ull<<63)) {
+		/* something is terribly wrong.... */
+		printf("mi_switch: -ve interval was %ld.%.6ld is %ld.%.6ld\n",
+			spc->spc_runtime.tv_sec, spc->spc_runtime.tv_usec,
+			tv.tv_sec, tv.tv_usec );
+		us = 0;
+	}
+	us += p->p_rtime;
+	/* save current process execution time */
+	p->p_rtime = us;
+	/* and new epoch in case we reschedule something immediately */
+	spc->spc_runtime = tv;
 
 	/*
 	 * Check if the process exceeds its cpu resource allocation.
@@ -849,12 +865,14 @@
 	 * than 10 minutes, reduce priority to give others a chance.
 	 */
 	rlim = &p->p_rlimit[RLIMIT_CPU];
-	if (s >= rlim->rlim_cur) {
+	/* XXX keep scaled rlim_cur (and autonice) */
+	if (rlim->rlim_cur != RLIM_INFINITY
+	    && us >= rlim->rlim_cur * 1000000ull) {
 		/*
 		 * XXXSMP: we're inside the scheduler lock perimeter;
 		 * use sched_psignal.
 		 */
-		if (s >= rlim->rlim_max)
+		if (us >= rlim->rlim_max * 1000000ull)
 			sched_psignal(p, SIGKILL);
 		else {
 			sched_psignal(p, SIGXCPU);
@@ -862,7 +880,8 @@
 				rlim->rlim_cur += 5;
 		}
 	}
-	if (autonicetime && s > autonicetime && p->p_ucred->cr_uid &&
+	if (autonicetime && us > autonicetime * 1000000ull &&
+	    p->p_ucred->cr_uid &&
 	    p->p_nice == NZERO) {
 		p->p_nice = autoniceval + NZERO;
 		resetpriority(p);
@@ -903,13 +922,6 @@
 
 #endif /* __HAVE_CHOOSEPROC */
 
-	/*
-	 * We won't be short-circuiting our path out of here, so
-	 * update the outgoing process CPU usage.
-	 */
-	p->p_rtime.tv_usec = u;
-	p->p_rtime.tv_sec = s;
-
 #if defined(__HAVE_CHOOSEPROC)
 
 	/*
@@ -919,6 +931,9 @@
 	 */
 	if (newp == NULL)
 		newp = chooseproc();
+		/* we idled - get new execution epoch */
+		microtime(&p->p_cpu->ci_schedstate.spc_runtime);
+	}
 
 	/*
 	 * Check if we're switching to ourself.  If we're not, then
@@ -958,6 +973,9 @@
 		if (PMC_ENABLED(p))
 			pmc_restore_context(p);
 #endif
+#if !defined(__HAVE_CHOOSEPROC)
+		microtime(&p->p_cpu->ci_schedstate.spc_runtime);
+#endif
 	}
 
 	/*
@@ -967,7 +985,6 @@
 	 */
 	KDASSERT(p->p_cpu != NULL);
 	KDASSERT(p->p_cpu == curcpu());
-	microtime(&p->p_cpu->ci_schedstate.spc_runtime);
 
 #if defined(__HAVE_CHOOSEPROC)
 out:
-- 
David Laight: david@l8s.co.uk