From owner-freebsd-arch@FreeBSD.ORG Mon Sep 27 17:20:30 2004 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 8407216A4CF for ; Mon, 27 Sep 2004 17:20:30 +0000 (GMT) Received: from duchess.speedfactory.net (duchess.speedfactory.net [66.23.201.84]) by mx1.FreeBSD.org (Postfix) with SMTP id B3DCC43D3F for ; Mon, 27 Sep 2004 17:20:29 +0000 (GMT) (envelope-from ups@tree.com) Received: (qmail 18258 invoked by uid 89); 27 Sep 2004 17:20:28 -0000 Received: from duchess.speedfactory.net (66.23.201.84) by duchess.speedfactory.net with SMTP; 27 Sep 2004 17:20:28 -0000 Received: (qmail 18241 invoked by uid 89); 27 Sep 2004 17:20:28 -0000 Received: from unknown (HELO palm.tree.com) (66.23.216.49) by duchess.speedfactory.net with SMTP; 27 Sep 2004 17:20:28 -0000 Received: from [127.0.0.1] (localhost.tree.com [127.0.0.1]) by palm.tree.com (8.12.10/8.12.10) with ESMTP id i8RHKQmt003500; Mon, 27 Sep 2004 13:20:27 -0400 (EDT) (envelope-from ups@tree.com) From: Stephan Uphoff To: John Baldwin In-Reply-To: <200409271016.13345.jhb@FreeBSD.org> References: <1096133353.53798.17613.camel@palm.tree.com> <200409271016.13345.jhb@FreeBSD.org> Content-Type: text/plain Message-Id: <1096305626.95152.163.camel@palm.tree.com> Mime-Version: 1.0 X-Mailer: Ximian Evolution 1.4.6 Date: Mon, 27 Sep 2004 13:20:26 -0400 Content-Transfer-Encoding: 7bit cc: Julian Elischer cc: "freebsd-arch@freebsd.org" Subject: Re: sched_userret priority adjustment patch for sched_4bsd X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 27 Sep 2004 17:20:30 -0000 On Mon, 2004-09-27 at 10:16, John Baldwin wrote: > On Saturday 25 September 2004 01:29 pm, Stephan Uphoff wrote: > > When a thread is about to return to user space it resets its priority to > > the user level priority. > > However after lowering the permission its priority it needs to check if > > its priority is still better than all other runable threads. > > This is currently not implemented. > > Without the check the thread can block kernel or user threads with > > better priority until a switch is forced by by an interrupt. > > > > The attached patch checks the relevant runqueues and threads without > > slots in the same ksegrp and forces a thread switch if the currently > > running thread is no longer the best thread to run after it changed its > > priority. > > > > The patch should improve interactive response under heavy load somewhat. > > It needs a lot of testing. > > Perhaps the better fix is to teach the schedulers to set TDF_NEEDRESCHED based > on on a comparison against user_pri rather than td_priority inside of > sched_add()? Having the flag set by sched_add() is supposed to make this > sort of check unnecessary. Even 4.x has the same bug I think as a process > can make another process runnable after it's priority has been boosted by a > tsleep() and need_resched() is only called based on a comparison of p_pri. > Ah, 4.x doesn't have the bug because it caches the priority of curproc when > it enters the kernel and compares against that. Thus, I think the correct > fix is more like this: > > Index: sched_4bsd.c > =================================================================== > RCS file: /usr/cvs/src/sys/kern/sched_4bsd.c,v > retrieving revision 1.63 > diff -u -r1.63 sched_4bsd.c > --- sched_4bsd.c 11 Sep 2004 10:07:22 -0000 1.63 > +++ sched_4bsd.c 27 Sep 2004 14:12:03 -0000 > @@ -272,7 +272,7 @@ > { > > mtx_assert(&sched_lock, MA_OWNED); > - if (td->td_priority < curthread->td_priority) > + if (td->td_priority < curthread->td_ksegrp->kg_user_pri) > curthread->td_flags |= TDF_NEEDRESCHED; > } > > Index: sched_ule.c > =================================================================== > RCS file: /usr/cvs/src/sys/kern/sched_ule.c,v > retrieving revision 1.129 > diff -u -r1.129 sched_ule.c > --- sched_ule.c 11 Sep 2004 10:07:22 -0000 1.129 > +++ sched_ule.c 27 Sep 2004 14:13:01 -0000 > @@ -723,7 +723,7 @@ > */ > pcpu = pcpu_find(cpu); > td = pcpu->pc_curthread; > - if (ke->ke_thread->td_priority < td->td_priority || > + if (ke->ke_thread->td_priority < td->td_ksegrp->kg_user_pri || > td == pcpu->pc_idlethread) { > td->td_flags |= TDF_NEEDRESCHED; > ipi_selected(1 << cpu, IPI_AST); > > An even better fix might be to fix td_base_pri by having it be set on kernel > entry similar to how 4.x sets curpriority. The above fix should be > sufficient for now, however. I don't think that this is enough since TDF_NEEDRESCHED is thread specific and not cpu specific. However the thread marked with TDF_NEEDRESCHED might not be the next thread leaving the kernel. ( Can't really talk about ULE since I am trying to avoid looking at another shiny irresistible time sink this week ;-) I think we agree that that td_priority should be set to td_base_pri on kernel entry. Since td_base_pri is changed by sleep and condvar functions it should also be reset on kernel entry. (Probably from a new ksegrp field). Condvar waits should currently non cause the base priority to change to the current priority of the thread - otherwise td_base_pri could get stuck at a really bad user priority. ( td->td_base_pri might end up being worse than td->td_ksegrp->kg_user_pri when the ksegrp priority improves) Stephan