cvs commit: src/sys/kern tty.c

[Bruce Evans]

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