From owner-cvs-all@FreeBSD.ORG Sat Feb 14 14:44:33 2004 Return-Path: Delivered-To: cvs-all@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id D3A6616A4CE; Sat, 14 Feb 2004 14:44:33 -0800 (PST) Received: from mailout2.pacific.net.au (mailout2.pacific.net.au [61.8.0.85]) by mx1.FreeBSD.org (Postfix) with ESMTP id 590AA43D1D; Sat, 14 Feb 2004 14:44:33 -0800 (PST) (envelope-from bde@zeta.org.au) Received: from mailproxy1.pacific.net.au (mailproxy1.pacific.net.au [61.8.0.86])i1EMiW5O031782; Sun, 15 Feb 2004 09:44:32 +1100 Received: from gamplex.bde.org (katana.zip.com.au [61.8.7.246]) i1EMiS0I028359; Sun, 15 Feb 2004 09:44:29 +1100 Date: Sun, 15 Feb 2004 09:44:28 +1100 (EST) From: Bruce Evans X-X-Sender: bde@gamplex.bde.org To: Nate Lawson In-Reply-To: <20040213192040.I9276@root.org> Message-ID: <20040215092117.L6303@gamplex.bde.org> References: <200402140130.i1E1U6Uw023421@repoman.freebsd.org> <20040213214520.T5430@mail.chesapeake.net> <20040213192040.I9276@root.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII cc: Jeff Roberson cc: src-committers@FreeBSD.org cc: cvs-src@FreeBSD.org cc: cvs-all@FreeBSD.org cc: Brian Feldman Subject: Re: cvs commit: src/sys/kern tty.c X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 14 Feb 2004 22:44:34 -0000 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