Re: [PATCH 5/6] n_tty: Fix stuck write wakeup

Johannes Stezenbach
2015-12-13T15:18:27Z

Hi Peter,

On Sat, Dec 12, 2015 at 02:16:38PM -0800, Peter Hurley wrote:
> If signal-driven i/o is disabled while write wakeup is pending (ie.,
> n_tty_write() has set TTY_DO_WRITE_WAKEUP but then signal-driven i/o
> is disabled), the TTY_DO_WRITE_WAKEUP bit will never be cleared and
> will cause tty_wakeup() to always call n_tty_write_wakeup.
>
> Unconditionally clear the write wakeup, and since kill_fasync()
> already checks if the fasync ptr is null, call kill_fasync()
> unconditionally as well.
...
> @@ -230,8 +230,8 @@ static ssize_t chars_in_buffer(struct tty_struct *tty)
>
> static void n_tty_write_wakeup(struct tty_struct *tty)
> {
> - if (tty->fasync && test_and_clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags))
> - kill_fasync(&tty->fasync, SIGIO, POLL_OUT);
> + clear_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
> + kill_fasync(&tty->fasync, SIGIO, POLL_OUT);
> }

There is a related bug that I meant to send a patch, but I
never got around because the issue was found with proprietary
userspace and ancient kernel. Maybe you could take care of it?
The patch might not apply cleanly after your recent changes
or might even be invalid now, please check.

Thanks,
Johannes

---
tty: n_tty: fix SIGIO for output

According to fcntl(2), "a SIGIO signal is sent whenever input
or output becomes possible on that file descriptor", i.e.
after the output buffer was full and now has space for new data.
But in fact SIGIO is sent after every write.

n_tty_write() should set TTY_DO_WRITE_WAKEUP only when
not all data could be written to the buffer.

Signed-off-by: Johannes Stezenbach <js@sig21.net>

--- drivers/char/n_tty.c.orig 2015-11-02 22:26:04.124227148 +0100
+++ drivers/char/n_tty.c 2015-11-02 22:26:10.644212115 +0100
@@ -1925,6 +1925,7 @@ static ssize_t n_tty_write(struct tty_st
DECLARE_WAITQUEUE(wait, current);
int c;
ssize_t retval = 0;
+ size_t count = nr;

/* Job control check -- must be done at start (POSIX.1 7.1.1.4). */
if (L_TOSTOP(tty) && file->f_op->write != redirected_tty_write) {
@@ -1991,7 +1992,7 @@ static ssize_t n_tty_write(struct tty_st
break_out:
__set_current_state(TASK_RUNNING);
remove_wait_queue(&tty->write_wait, &wait);
- if (b - buf != nr && tty->fasync)
+ if (b - buf != count && tty->fasync)
set_bit(TTY_DO_WRITE_WAKEUP, &tty->flags);
return (b - buf) ? b - buf : retval;
}