Date: Mon, 20 Sep 2010 04:21:31 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Ed Schouten <ed@80386.nl> Cc: Kostik Belousov <kostikbel@gmail.com>, svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r212860 - head/sys/kern Message-ID: <20100920021713.Y950@delplex.bde.org> In-Reply-To: <20100919150628.GN56986@hoeg.nl> References: <201009191421.o8JELdNY004586@svn.freebsd.org> <20100919143047.GB2389@deviant.kiev.zoral.com.ua> <20100919150628.GN56986@hoeg.nl>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, 19 Sep 2010, Ed Schouten wrote:
> * Kostik Belousov <kostikbel@gmail.com> wrote:
>> Shouldn't you always report CLOCAL for console then ?
>
> Hmmm... That would be a lot more elegant, also for callout devices. The
> change I just committed, doesn't take a loss of SER_DCD into account
> after opening the device.
This should be handled like it was by non-broken serial device drivers:
- CLOCAL defaults to on for the console only
- CLOCAL is also locked on for the console only
- otherwise, CLOCAL is not specially handled. If the user wants to
default and lock it differently, then he changes these defaults in
the initial and lock state devices
- locking the speed for the console only is even more important
- locking of other termios state (parity and stop bits...) is also
important, but has never been done by default AFAIK. This can be
fixed by programming the lock state device at boot time using
rc.d/serial or directly, but this is unobvious and painful.
> Any comments on the following patch?
>
> %%%
> Index: sys/kern/tty.c
> ===================================================================
> --- sys/kern/tty.c (revision 212860)
> +++ sys/kern/tty.c (working copy)
> @@ -263,12 +263,14 @@
>
> if (!tty_opened(tp)) {
> /* Set proper termios flags. */
> - if (TTY_CALLOUT(tp, dev)) {
> + if (TTY_CALLOUT(tp, dev))
> tp->t_termios = tp->t_termios_init_out;
> - } else {
> + else
> tp->t_termios = tp->t_termios_init_in;
> - }
> ttydevsw_param(tp, &tp->t_termios);
> + /* Prevent modem control on callout devices and /dev/console. */
> + if (TTY_CALLOUT(tp, dev) || dev == dev_console)
> + tp->t_termios.c_cflag |= CLOCAL;
This seems to just break the user's defaults set in the initial state
devices.
The uart driver even refuses to honor the users changes of CLOCAL,
HUPCL and the speed for consoles at tcsetattr() time. Normally it is
a mistake for the user to change these, but if he has reprogrammed the
lock state device so that changes are possible, then the changes should
be honored. Not honoring the changes in the driver is equivalent to
having a lock on parts of the lock device, with bugs in reads of the
locked device (parts of it that are effectively locked can be changed,
but such changes don't work).
CLOCAL doesn't seem to be set in the lock device anywhere, but it is
set for the initial state device for consoles. This is enough for
open() (since changing it in the initial state device should require
the same privilege as changing it in the lock device. But users should
be especially careful in changing it for the console, since for the
console its setting should start as locked on, and this locking becomes
negatively useful if the initial state is changed to off so that the
full setting becomes locked off).
Defaulting to CLOCAL for the callout device is wrong. Callout devices
should only ignore carrier in open(). Then open() normally succeeds
without carrier, and any off-to-on transition of carrier has little
effect. But on-to-off transitions of carrier should be handled,
according to the user's setting of CLOCAL, and this setting should not
normally be locked (unlike in the callin case where locking it may be
good). If the device is actually local, then hanging up is not very
useful, but callout devices aren't really needed for that case either
(I use them out of habit, but almost the same behaviour can be obtained
using callin devices with CLOCAL set in their initial state and unset
after open()).
>
> ttydevsw_modem(tp, SER_DTR|SER_RTS, 0);
>
> @@ -281,9 +283,8 @@
> }
>
> /* Wait for Carrier Detect. */
> - if (!TTY_CALLOUT(tp, dev) && (oflags & O_NONBLOCK) == 0 &&
> - (tp->t_termios.c_cflag & CLOCAL) == 0 &&
> - dev != dev_console) {
> + if ((oflags & O_NONBLOCK) == 0 &&
> + (tp->t_termios.c_cflag & CLOCAL) == 0) {
> while ((ttydevsw_modem(tp, 0, 0) & SER_DCD) == 0) {
> error = tty_wait(tp, &tp->t_dcdwait);
> if (error != 0)
> %%%
It has always intentionally not been done like this, so that CLOCAL works
normally for callout devices except for open. CLOCAL is in effect forced
to be temporarily ignored during open. This is called "SOFT_CARRIER" in
the sio driver: from a lost comment:
% /*
% * Handle initial DCD. Callout devices get a fake initial
% * DCD (trapdoor DCD). If we are callout, then any sleeping
% * callin opens get woken up and resume sleeping on "siobi"
% * instead of "siodcd".
% */
% /*
% * XXX `mynor & CALLOUT_MASK' should be
% * `tp->t_cflag & (SOFT_CARRIER | TRAPDOOR_CARRIER) where
% * TRAPDOOR_CARRIER is the default initial state for callout
% * devices and SOFT_CARRIER is like CLOCAL except it hides
% * the true carrier.
% */
I never got around to de-magicing this by putting SOFT_CARRIER in
t_cflag and somehow making the callout'ness more programmable using
TRAPDOOR_CARRIER. Most systems never needed both the callin and callout
devices, and it would have been nice to able to create a callout device
on demand for occasional use (no devfs please, but without creating a
new device it is hard to find a context for the existing callin device
users (blocked in open) to sleep in). Now, even fewer systems need
both. Linux axed callout devices about 15 years ago. Serial modem
connections were still useful then, so I think this mainly moved the
complications to userland where they are larger.
Other termios flags defaults lossage:
>From the detached sio driver:
% /*
% * We don't use all the flags from <sys/ttydefaults.h> since they
% * are only relevant for logins. It's important to have echo off
% * initially so that the line doesn't start blathering before the
% * echo flag can be turned off.
% */
% com->it_in.c_iflag = 0;
% com->it_in.c_oflag = 0;
% com->it_in.c_cflag = TTYDEF_CFLAG;
% com->it_in.c_lflag = 0;
(We should actually use the "raw" flags, which aren't quite all 0's -- see
cfmakeraw().)
>From -current:
% static void
% tty_init_termios(struct tty *tp)
% {
% struct termios *t = &tp->t_termios_init_in;
%
% t->c_cflag = TTYDEF_CFLAG;
% t->c_iflag = TTYDEF_IFLAG;
% t->c_lflag = TTYDEF_LFLAG;
% t->c_oflag = TTYDEF_OFLAG;
Even the comment saying why these defaults are unusable in the kernel
has been detached. An intermediate stage lost the comment but avoided
the problem with echos by using TTYDEF_LFLAG for consoles only and
TTYDEF_LFLAG_NOECHO otherwise. Now, TTYDEF_LFLAG_NOECHO is still
defined, but is no longer used.
Bad defaults for all the flags can be fixed by reprogramming at boot
time, except possibly if an echo war makes the system unbootable, but
this is just as painful as for the missing console lock device
initialization. I have many sloppy test scripts that depend on the
default state being raw.
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100920021713.Y950>