Date: Sun, 15 Feb 2004 09:44:28 +1100 (EST) From: Bruce Evans <bde@zeta.org.au> To: Nate Lawson <nate@root.org> Cc: Brian Feldman <green@FreeBSD.org> Subject: Re: cvs commit: src/sys/kern tty.c Message-ID: <20040215092117.L6303@gamplex.bde.org> In-Reply-To: <20040213192040.I9276@root.org> References: <200402140130.i1E1U6Uw023421@repoman.freebsd.org> <20040213214520.T5430@mail.chesapeake.net> <20040213192040.I9276@root.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 13 Feb 2004, Nate Lawson wrote: > On Fri, 13 Feb 2004, Jeff Roberson wrote: > > On Fri, 13 Feb 2004, Brian Feldman wrote: > > > > > green 2004/02/13 17:30:06 PST > > > > > > FreeBSD src repository > > > > > > Modified files: > > > sys/kern tty.c > > > Log: > > > T -CURRENT DO NOT CRASH UPON ^T K PLZ THX. > > > > Please use more appropriate commit messages. It'd be nice to know why we > > were crashing on ^T. > > I actually saw nothing in this commit that addresses signals, only the > following macro-ization and an include of sched.h, necessary for the > macro. > > > > Also, use sched_pctcpu() instead of assuming td->td_kse is non-NULL. > > > > > > Revision Changes Path > > > 1.208 +2 -1 src/sys/kern/tty.c The problem is actually almost fully described in the log message. Omitting the jargon and/or "Also, " would make it clearer. ^T causes ttyinfo() to be called, and ttyinfo() apparently follows the null pointer td->td_kse. sched_pctcpu() exists partly to centralize not following this null pointer and partly because the contents of td->td_kse might not be up to date (SCHED_4BSD maintains td->td_kse->ke_pctcpu (except, apparently, when td->td_kse is null), but SCHED_ULE needs a more dynamic update). I think it may be a bug that td->td_kse is null, especially in ttyinfo(). ttyinfo() is supposed to choose the most active process, but the choosing is broken for the SCHED_ULE case by comparing kg->kg_estcpu in proc_compare(), but kg->kg_estcpu() is not maintained by SCHED_ULE (it is always 0). This might result in an inactive process being preferred. We display the process's activity in the form of %CPU, so perhaps we should sort on that. However, too much is already done with sched_lock held, and calling sched_pctcpu() in proc_compare() would make this worse. The following related log messages didn't have enough detail for me. % RCS file: /home/ncvs/src/sys/kern/sched_4bsd.c,v % Working file: sched_4bsd.c % head: 1.33 % ... % ---------------------------- % revision 1.27 % date: 2003/11/08 03:03:17; author: davidxu; state: Exp; lines: +2 -0 % Return a reasonable number for top or ps to display for M:N thread, % since there is no direct association between M:N thread and kse, % sometimes, a thread does not have a kse, in that case, return a pctcpu % from its last kse, it is not perfect, but gives a good number to be % displayed. % ---------------------------- Why is this only done for the SCHED_4BSD case? % ... % ---------------------------- % revision 1.25 % date: 2003/10/16 21:13:14; author: jeff; state: Exp; lines: +7 -1 % - The kse may be null in sched_pctcpu(). % % Reported by: kris % ---------------------------- How can it be null? The SCHED_ULE case apparently always checked this. I added a check that the kse isn't null in sched_pctcpu() in the SCHED_4BSD case. It hasn't fired yet, but I don't use threads. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040215092117.L6303>