Date: Sat, 16 Oct 2004 22:30:25 GMT From: Antoine Brodin <antoine.brodin@laposte.net> To: freebsd-bugs@FreeBSD.org Subject: Re: kern/72659: [patch] little bug in sched_ule interractivty scorer Message-ID: <200410162230.i9GMUPGh053659@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/72659; it has been noted by GNATS.
From: Antoine Brodin <antoine.brodin@laposte.net>
To: freebsd-gnats-submit@FreeBSD.org, antoine.brodin@laposte.net
Cc:
Subject: Re: kern/72659: [patch] little bug in sched_ule interractivty
scorer
Date: Sun, 17 Oct 2004 00:28:12 +0200
This is a multi-part message in MIME format.
--Multipart=_Sun__17_Oct_2004_00_28_12_+0200_slFX=AB9uvnDw5CT
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
I've identified two more bugs :
1) The computation of hzticks in sched_wakeup() is broken :
On a system with kern.hz="1000", if we leave a shell 50 minutes alone,
when we come back, hzticks = (ticks - td->td_slptime) << 10 is negative
so we lose our kg_slptime and this kg_slptime won't be positive before a
long time. The shell will stay uninteractive during this time.
A fix that solves the problem if we don't sleep INT_MAX-INT_MIN+1 ticks
or more is attached. If we sleep more, at least we don't get a negative
hzticks.
2) Threads that voluntarily sleep at ticks = 0 (typically kernel
threads that sleep during boot) are not handled the same way as others
when they're woken up because a non null td_slptime is used to
distinguish voluntarily aslept threads.
A dirty fix is attached.
Note : the difference of signification of kg_slptime in
sched_4bsd.c/kern_proc.c/vm_glue.c and sched_ule.c is quite confusing
and the use of kg_slptime in fill_kinfo_thread() in kern_proc.c and in
vm_glue.c when we use SCHED_ULE seems bogus...
--Multipart=_Sun__17_Oct_2004_00_28_12_+0200_slFX=AB9uvnDw5CT
Content-Type: text/plain;
name="sched_ule.txt"
Content-Disposition: attachment;
filename="sched_ule.txt"
Content-Transfer-Encoding: 7bit
Index: sched_ule.c
===================================================================
RCS file: /home/ncvs/src/sys/kern/sched_ule.c,v
retrieving revision 1.134
diff -u -r1.134 sched_ule.c
--- sched_ule.c 5 Oct 2004 22:14:02 -0000 1.134
+++ sched_ule.c 16 Oct 2004 20:05:28 -0000
@@ -1143,19 +1143,17 @@
int div;
if (kg->kg_runtime > kg->kg_slptime) {
- div = max(1, kg->kg_runtime / SCHED_INTERACT_HALF);
+ div = imax(1, kg->kg_runtime / SCHED_INTERACT_HALF);
return (SCHED_INTERACT_HALF +
(SCHED_INTERACT_HALF - (kg->kg_slptime / div)));
- } if (kg->kg_slptime > kg->kg_runtime) {
- div = max(1, kg->kg_slptime / SCHED_INTERACT_HALF);
+ }
+ if (kg->kg_slptime > kg->kg_runtime) {
+ div = imax(1, kg->kg_slptime / SCHED_INTERACT_HALF);
return (kg->kg_runtime / div);
}
-
- /*
- * This can happen if slptime and runtime are 0.
- */
+ if (kg->kg_runtime > 0)
+ return (SCHED_INTERACT_HALF);
return (0);
-
}
/*
@@ -1351,7 +1349,7 @@
{
mtx_assert(&sched_lock, MA_OWNED);
- td->td_slptime = ticks;
+ td->td_slptime = ticks != 0 ? ticks : ticks - 1;
td->td_base_pri = td->td_priority;
CTR2(KTR_ULE, "sleep thread %p (tick: %d)",
@@ -1372,12 +1370,12 @@
int hzticks;
kg = td->td_ksegrp;
- hzticks = (ticks - td->td_slptime) << 10;
- if (hzticks >= SCHED_SLP_RUN_MAX) {
+ hzticks = ticks - td->td_slptime;
+ if (hzticks < 0 || hzticks >= SCHED_SLP_RUN_MAX >> 10) {
kg->kg_slptime = SCHED_SLP_RUN_MAX;
kg->kg_runtime = 1;
} else {
- kg->kg_slptime += hzticks;
+ kg->kg_slptime += hzticks << 10;
sched_interact_update(kg);
}
sched_priority(kg);
--Multipart=_Sun__17_Oct_2004_00_28_12_+0200_slFX=AB9uvnDw5CT--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200410162230.i9GMUPGh053659>