Subject: Re: scheduler_wait_hook
To: None <thorpej@shagadelic.org>
From: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
List: tech-kern
Date: 12/18/2005 15:00:57
--NextPart-20051218145509-0853400
Content-Type: Text/Plain; charset=us-ascii

> there are several choices.
> 
> 0. keep it as-is.  (IMO broken)
> 1. remove the feedback completely.
> 2. do the feedback strictly.
>    (keep how much p_estcpu is inherited from the parent.
>    and decay it appropriately)
> 3. do the feedback in a compromised way.
>    (eg. my proposed patch)
> 4. any other good idea?

more than 2 months passed now.  i'd like to make a progress.

as there seems to be some objections/concerns against #3,
i made a patch for #2. (attached)
i'll commit this unless anyone objects.

YAMAMOTO Takashi

--NextPart-20051218145509-0853400
Content-Type: Text/Plain; charset=us-ascii
Content-Disposition: attachment; filename="a.diff"

Index: sys/proc.h
===================================================================
--- sys/proc.h	(revision 1464)
+++ sys/proc.h	(working copy)
@@ -202,6 +202,8 @@ struct proc {
 
 	/* scheduling */
 	fixpt_t		p_estcpu;	/* Time averaged value of p_cpticks XXX belongs in p_startcopy section */
+	fixpt_t		p_estcpu_inherited;
+	unsigned int	p_forktime;
 	int		p_cpticks;	/* Ticks of CPU time */
 	fixpt_t		p_pctcpu;	/* %cpu for this process during p_swtime */
 
Index: kern/kern_synch.c
===================================================================
--- kern/kern_synch.c	(revision 1464)
+++ kern/kern_synch.c	(working copy)
@@ -140,9 +140,9 @@ __inline void sa_awaken(struct lwp *);
 __inline void awaken(struct lwp *);
 
 struct callout schedcpu_ch = CALLOUT_INITIALIZER_SETFUNC(schedcpu, NULL);
+static unsigned int schedcpu_ticks;
 
 
-
 /*
  * Force switch among equal priority processes every 100ms.
  * Called from hardclock every hz/10 == rrticks hardclock ticks.
@@ -262,6 +262,29 @@ decay_cpu(fixpt_t loadfac, fixpt_t estcp
 	return (uint64_t)estcpu * loadfac / (loadfac + FSCALE);
 }
 
+/*
+ * For all load averages >= 1 and max p_estcpu of (255 << ESTCPU_SHIFT),
+ * sleeping for at least seven times the loadfactor will decay p_estcpu to
+ * less than (1 << ESTCPU_SHIFT).
+ *
+ * note that our ESTCPU_MAX is actually much smaller than (255 << ESTCPU_SHIFT).
+ */
+static fixpt_t
+decay_cpu_batch(fixpt_t loadfac, fixpt_t estcpu, unsigned int n)
+{
+
+	if ((n << FSHIFT) >= 7 * loadfac) {
+		return 0;
+	}
+
+	while (estcpu != 0 && n > 1) {
+		estcpu = decay_cpu(loadfac, estcpu);
+		n--;
+	}
+
+	return estcpu;
+}
+
 /* decay 95% of `p_pctcpu' in 60 seconds; see CCPU_SHIFT before changing */
 fixpt_t	ccpu = 0.95122942450071400909 * FSCALE;		/* exp(-1/20) */
 
@@ -292,6 +315,8 @@ schedcpu(void *arg)
 	int s, minslp;
 	int clkhz;
 
+	schedcpu_ticks++;
+
 	proclist_lock_read();
 	PROCLIST_FOREACH(p, &allproc) {
 		/*
@@ -359,32 +384,21 @@ schedcpu(void *arg)
 
 /*
  * Recalculate the priority of a process after it has slept for a while.
- * For all load averages >= 1 and max p_estcpu of (255 << ESTCPU_SHIFT),
- * sleeping for at least eight times the loadfactor will decay p_estcpu to
- * less than (1 << ESTCPU_SHIFT).
- *
- * note that our ESTCPU_MAX is actually much smaller than (255 << ESTCPU_SHIFT).
  */
 void
 updatepri(struct lwp *l)
 {
 	struct proc *p = l->l_proc;
-	fixpt_t newcpu;
 	fixpt_t loadfac;
 
 	SCHED_ASSERT_LOCKED();
+	KASSERT(l->l_slptime > 1);
 
-	newcpu = p->p_estcpu;
 	loadfac = loadfactor(averunnable.ldavg[0]);
 
-	if ((l->l_slptime << FSHIFT) >= 8 * loadfac)
-		p->p_estcpu = 0; /* XXX NJWLWP */
-	else {
-		l->l_slptime--;	/* the first time was done in schedcpu */
-		while (newcpu && --l->l_slptime)
-			newcpu = decay_cpu(loadfac, newcpu);
-		p->p_estcpu = newcpu;
-	}
+	l->l_slptime--; /* the first time was done in schedcpu */
+	/* XXX NJWLWP */
+	p->p_estcpu = decay_cpu_batch(loadfac, p->p_estcpu, l->l_slptime);
 	resetpriority(l);
 }
 
@@ -1240,7 +1254,8 @@ void
 scheduler_fork_hook(struct proc *parent, struct proc *child)
 {
 
-	child->p_estcpu = parent->p_estcpu;
+	child->p_estcpu = child->p_estcpu_inherited = parent->p_estcpu;
+	child->p_forktime = schedcpu_ticks;
 }
 
 /*
@@ -1251,9 +1266,17 @@ scheduler_fork_hook(struct proc *parent,
 void
 scheduler_wait_hook(struct proc *parent, struct proc *child)
 {
+	fixpt_t loadfac = loadfactor(averunnable.ldavg[0]);
+	fixpt_t estcpu;
 
 	/* XXX Only if parent != init?? */
-	parent->p_estcpu = ESTCPULIM(parent->p_estcpu + child->p_estcpu);
+
+	estcpu = decay_cpu_batch(loadfac, child->p_estcpu_inherited,
+	    schedcpu_ticks - child->p_forktime);
+	if (child->p_estcpu > estcpu) {
+		parent->p_estcpu =
+		    ESTCPULIM(parent->p_estcpu + child->p_estcpu - estcpu);
+	}
 }
 
 /*

--NextPart-20051218145509-0853400--