On Mon, Dec 16, 2013 at 12:50:15AM +0000, Marcel Moolenaar wrote:
> Author: marcel
> Date: Mon Dec 16 00:50:14 2013
> New Revision: 259441
> URL: http://svnweb.freebsd.org/changeset/base/259441
> Log:
> Properly drain the TTY when both revoke(2) and close(2) end up closing
> the TTY. In such a case, ttydev_close() is called multiple times and
> each time, t_revokecnt is incremented and cv_broadcast() is called for
> both the t_outwait and t_inwait condition variables.
> Let's say revoke(2) comes in first and gets to call tty_drain() from
> ttydev_leave(). Let's say that the revoke comes from init(8) as the
> result of running "shutdown -r now". Since shutdown prints various
> messages to the console before announing that the machine will reboot
> immediately, let's also say that the output queue is not empty and
> that tty_drain() has something to do. Let's assume this all happens
> on a 9600 baud serial console, so it takes a time to drain.
> The shutdown command will exit(2) and as such will end up closing
> stdout. Let's say this close will come in second, bump t_revokecnt
> and call tty_wakeup(). This has tty_wait() return prematurely and
> the next thing that will happen is that the thread doing revoke(2)
> will flush the TTY. Since the drain wasn't complete, the flush will
> effectively drop whatever is left in t_outq.
> This change takes into account that tty_drain() will return ERESTART
> due to the fact that t_revokecnt was bumped and in that case simply
> call tty_drain() again. The thread in question is already performing
> the close so it can safely finish draining the TTY before destroying
> the TTY structure.
> Now all messages from shutdown will be printed on the serial console.
> Obtained from: Juniper Networks, Inc.
> Modified:
> head/sys/kern/tty.c
> Modified: head/sys/kern/tty.c
> ==============================================================================
> --- head/sys/kern/tty.c Sun Dec 15 23:49:42 2013 (r259440)
> +++ head/sys/kern/tty.c Mon Dec 16 00:50:14 2013 (r259441)
> @@ -191,8 +191,10 @@ ttydev_leave(struct tty *tp)
>
> /* Drain any output. */
> MPASS((tp->t_flags & TF_STOPPED) == 0);
> - if (!tty_gone(tp))
> - tty_drain(tp);
> + if (!tty_gone(tp)) {
> + while (tty_drain(tp) == ERESTART)
> + ;
> + }
>
> ttydisc_close(tp);
>
Unfortunately, this is only correct if the [ERESTART] is because of the
(tp->t_revokecnt != revokecnt) check. If cv_wait_sig() returned
ERESTART, it will return ERESTART immediately the next time it is
called, until the signal has been handled. Therefore, if a process
performing the last close of a TTY via close(2) receives a signal with
SA_RESTART set, it will spin until the data has drained. Formerly, the
data would be discarded when the signal was received (without reflecting
this in close(2)'s return value, since ttydev_leave() returns nothing
and ttydev_close() always returns 0).
One way to fix this would be to replicate the revokecnt check into
ttydev_leave() so that tty_drain() is only retried if t_revokecnt
changed.
Discarding data because of a signal is not ideal, but I think leaving a
process unkillable because of a drain that will never complete is worse.
A process that cares about data in TTY buffers can use tcdrain(3) and
handle signals and errors normally.
--
Jilles Tjoelker
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
