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

[Johannes Stezenbach]

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@xxxxxxxxx>

--- 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;
 }


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/