(The same bug exists in 5.x if you use the 4BSD scheduler).
The code that causes the parent to inherit the child's p_estcpu when
the child exits is broken. It was adding the parent's and child's
p_estcpu together.
However, the child process is initialized with the parent's estcpu,
so simply adding it back in to the parent will cause the parent's
p_estcpu to blow up after only a few fork/exec's.
A better solution is to average the two, taking care to handle the
case where the child might have received one tick. This way if a
parent is exec'ing a lot of children, such as when you do a
'make',
p_estcpu will ramp-up just slightly slower then a normal cpu-bound
process would, which is exactly what we want. After all, there should
be no difference in the treatment of a single cpu-bound program verses
the treatment of a cpu-bound make which fork/exec's a lot, yet we do not
want to penalize an interactive shell that just ran a (single)
long-running cpu-bound program too much.
NOTE: this patch is against DragonFly, you may have to patch 4.x
manually. It is not a complex patch.
-Matt
Matthew Dillon
<dillon@backplane.com>
Index: kern_exit.c
==================================================================RCS file:
/cvs/src/sys/kern/kern_exit.c,v
retrieving revision 1.30
retrieving revision 1.31
diff -u -r1.30 -r1.31
--- kern_exit.c 18 Jan 2004 12:29:49 -0000 1.30
+++ kern_exit.c 20 Mar 2004 19:16:24 -0000 1.31
@@ -37,7 +37,7 @@
*
* @(#)kern_exit.c 8.7 (Berkeley) 2/12/94
* $FreeBSD: src/sys/kern/kern_exit.c,v 1.92.2.11 2003/01/13 22:51:16 dillon
Exp $
- * $DragonFly: src/sys/kern/kern_exit.c,v 1.30 2004/01/18 12:29:49 dillon Exp $
+ * $DragonFly: src/sys/kern/kern_exit.c,v 1.31 2004/03/20 19:16:24 dillon Exp $
*/
#include "opt_compat.h"
@@ -468,10 +468,24 @@
}
lwkt_wait_free(p->p_thread);
- /* charge childs scheduling cpu usage to parent */
+ /*
+ * Charge the parent for the child's estimated cpu
+ * on exit to account for cpu-bound scripts that
+ * fork/exec a lot, and heavy shell use. Note that
+ * the child inherited our estcpu when it forked, so
+ * we have to undo that here. We have to add 1 to
+ * the average to deal with the case where programs
+ * take less then 1/10 second (ESTCPUFREQ) to run.
+ * Also note that we do not want to add any slop
+ * charges here since a program can fork/exec hundreds
+ * of processes a second and slop would blow p_estcpu
+ * up beyond all proportion.
+ */
if (curproc->p_pid != 1) {
- curproc->p_estcpu - ESTCPULIM(curproc->p_estcpu +
p->p_estcpu);
+ if (p->p_estcpu > curproc->p_estcpu) {
+ curproc->p_estcpu = ESTCPULIM(
+ (p->p_estcpu + curproc->p_estcpu + 1) / 2);
+ }
}
/* Take care of our return values. */
Matthew Dillon
2004-Mar-20 15:00 UTC
Urk, I take it back (was Re: Bug in p_estcpu handling on process exit in FBsd-4.x)
: The code that causes the parent to inherit the child's p_estcpu when
: the child exits is broken. It was adding the parent's and child's
: p_estcpu together.
:
: However, the child process is initialized with the parent's estcpu,
: so simply adding it back in to the parent will cause the parent's
: p_estcpu to blow up after only a few fork/exec's.
:...
Oops. I take it back. It's a bug, but my bug fix is not right, you
don't want to apply that patch :-(
It turns out to be a somewhat more complex problem. I'm going to have
to experiment a bit more to find the right solution.
-Matt