Date: Wed, 6 Mar 2013 09:17:13 -0500 From: John Baldwin <jhb@freebsd.org> To: Ian Lepore <ian@freebsd.org> Cc: freebsd-hackers@freebsd.org Subject: Re: rtprio_thread trouble Message-ID: <201303060917.13642.jhb@freebsd.org> In-Reply-To: <1362081556.1195.62.camel@revolution.hippie.lan> References: <1361559960.1185.81.camel@revolution.hippie.lan> <201302261529.24862.jhb@freebsd.org> <1362081556.1195.62.camel@revolution.hippie.lan>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thursday, February 28, 2013 2:59:16 pm Ian Lepore wrote: > On Tue, 2013-02-26 at 15:29 -0500, John Baldwin wrote: > > On Friday, February 22, 2013 2:06:00 pm Ian Lepore wrote: > > > I ran into some trouble with rtprio_thread() today. I have a worker > > > thread that I want to run at idle priority most of the time, but if it > > > falls too far behind I'd like to bump it back up to regular timeshare > > > priority until it catches up. In a worst case, the system is > > > continuously busy and something scheduled at idle priority is never > > > going to run (for some definition of 'never'). > > > > > > What I found is that in this worst case, even after my main thread has > > > used rtprio_thread() to change the worker thread back to > > > RTP_PRIO_NORMAL, the worker thread never gets scheduled. This is with > > > the 4BSD scheduler but it appears that the same would be the case with > > > ULE, based on code inspection. I find that this fixes it for 4BSD, and > > > I think the same would be true for ULE... > > > > > > --- a/sys/kern/sched_4bsd.c Wed Feb 13 12:54:36 2013 -0700 > > > +++ b/sys/kern/sched_4bsd.c Fri Feb 22 11:55:35 2013 -0700 > > > @@ -881,6 +881,9 @@ sched_user_prio(struct thread *td, u_cha > > > return; > > > oldprio = td->td_user_pri; > > > td->td_user_pri = prio; > > > + if (td->td_flags & TDF_BORROWING && td->td_priority <= prio) > > > + return; > > > + sched_priority(td, prio); > > > } > > > > > > void > > > > > > But I'm not sure if this would have any negative side effects, > > > especially since in the ULE case there's a comment on this function that > > > specifically notes that it changes the the user priority without > > > changing the current priority (but it doesn't say why that matters). > > > > > > Is this a reasonable way to fix this problem, or is there a better way? > > > > This will lose the "priority boost" afforded to interactive threads when they > > sleep in the kernel in the 4BSD scheduler. You aren't supposed to drop the > > user priority to loose this boost until userret(). You could perhaps try > > only altering the priority if the new user pri is lower than your current > > priority (and then you don't have to check TDF_BORROWING I believe): > > > > if (prio < td->td_priority) > > sched_priority(td, prio); > > > > That's just the sort of insight I was looking for, thanks. That made me > look at the code more and think harder about the problem I'm trying to > solve, and I concluded that doing it within the scheduler is all wrong. > > That led me to look elsewhere, and I discovered the change you made in > r228207, which does almost what I want, but your change does it only for > realtime priorities, and I need a similar effect for idle priorities. > What I came up with is a bit different than yours (attached below) and > I'd like your thoughts on it. > > I start with the same test as yours: if sched_user_prio() didn't > actually change the user priority (due to borrowing), do nothing. Then > mine differs: call sched_prio() to effect the change only if either the > old or the new priority class is not timeshare. > > My reasoning for the second half of the test is that if it's a change in > timeshare priority then the scheduler is going to adjust that priority > in a way that completely wipes out the requested change anyway, so > what's the point? (If that's not true, then allowing a thread to change > its own timeshare priority would subvert the scheduler's adjustments and > let a cpu-bound thread monopolize the cpu; if allowed at all, that > should require priveleges.) > > On the other hand, if either the old or new priority class is not > timeshare, then the scheduler doesn't make automatic adjustments, so we > should honor the request and make the priority change right away. The > reason the old class gets caught up in this is the very reason I'm > wanting to make a change: when thread A changes the priority of its > child thread B from idle back to timeshare, thread B never actually gets > moved to a timeshare-range run queue unless there are some idle cycles > available to allow it to first get scheduled again as an idle thread. > > Finally, my change doesn't consider the td == curthread situation at > all, because I don't see how that's germane. This is the thing I'm > least sure of -- I don't at all understand why the old code (even before > your changes) had that test. The old code had that flagged as "XXX > dubious" (a comment a bit too cryptic to be useful). I think your change is correct. One style nit: please sort the order of variables (oldclass comes before oldpri). -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201303060917.13642.jhb>