Re: [PATCH tty-next 6/7] n_tty: Only perform wakeups for waiters
On 11/23/2013 07:23 PM, One Thousand Gnomes wrote: On Fri, 22 Nov 2013 10:59:24 -0500 Peter Hurley wrote: Only wakeup the _waiting_ reader, polls and/or writer(s). Signed-off-by: Peter Hurley --- drivers/tty/n_tty.c | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c index 8f2356e..aae28a6 100644 --- a/drivers/tty/n_tty.c +++ b/drivers/tty/n_tty.c @@ -275,7 +275,8 @@ static void n_tty_check_unthrottle(struct tty_struct *tty) return; n_tty_set_room(tty); n_tty_write_wakeup(tty->link); - wake_up_interruptible_poll(>link->write_wait, POLLOUT); + if (waitqueue_active(>link->write_wait)) + wake_up_interruptible_poll(>link->write_wait, POLLOUT); Does this actually microbenchmark faster ? Getting on and off the write_wait queue is actually pretty expensive for the "other" pty (the writer), and the unnecessary wakeup from the reader doesn't help. The other chunks are gratuitous. Regards, Peter Hurley PS - This came up because there is some worst-case behavior that I'm looking into fixing. When the userspace reader is very far behind (say because it's reading char-by-char), it doesn't make sense to keep restarting the input processing worker. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH tty-next 6/7] n_tty: Only perform wakeups for waiters
On Fri, 22 Nov 2013 10:59:24 -0500 Peter Hurley wrote: > Only wakeup the _waiting_ reader, polls and/or writer(s). > > Signed-off-by: Peter Hurley > --- > drivers/tty/n_tty.c | 18 -- > 1 file changed, 12 insertions(+), 6 deletions(-) > > diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c > index 8f2356e..aae28a6 100644 > --- a/drivers/tty/n_tty.c > +++ b/drivers/tty/n_tty.c > @@ -275,7 +275,8 @@ static void n_tty_check_unthrottle(struct tty_struct *tty) > return; > n_tty_set_room(tty); > n_tty_write_wakeup(tty->link); > - wake_up_interruptible_poll(>link->write_wait, POLLOUT); > + if (waitqueue_active(>link->write_wait)) > + wake_up_interruptible_poll(>link->write_wait, > POLLOUT); Does this actually microbenchmark faster ? Alan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH tty-next 6/7] n_tty: Only perform wakeups for waiters
On Fri, 22 Nov 2013 10:59:24 -0500 Peter Hurley pe...@hurleysoftware.com wrote: Only wakeup the _waiting_ reader, polls and/or writer(s). Signed-off-by: Peter Hurley pe...@hurleysoftware.com --- drivers/tty/n_tty.c | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c index 8f2356e..aae28a6 100644 --- a/drivers/tty/n_tty.c +++ b/drivers/tty/n_tty.c @@ -275,7 +275,8 @@ static void n_tty_check_unthrottle(struct tty_struct *tty) return; n_tty_set_room(tty); n_tty_write_wakeup(tty-link); - wake_up_interruptible_poll(tty-link-write_wait, POLLOUT); + if (waitqueue_active(tty-link-write_wait)) + wake_up_interruptible_poll(tty-link-write_wait, POLLOUT); Does this actually microbenchmark faster ? Alan -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH tty-next 6/7] n_tty: Only perform wakeups for waiters
On 11/23/2013 07:23 PM, One Thousand Gnomes wrote: On Fri, 22 Nov 2013 10:59:24 -0500 Peter Hurley pe...@hurleysoftware.com wrote: Only wakeup the _waiting_ reader, polls and/or writer(s). Signed-off-by: Peter Hurley pe...@hurleysoftware.com --- drivers/tty/n_tty.c | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c index 8f2356e..aae28a6 100644 --- a/drivers/tty/n_tty.c +++ b/drivers/tty/n_tty.c @@ -275,7 +275,8 @@ static void n_tty_check_unthrottle(struct tty_struct *tty) return; n_tty_set_room(tty); n_tty_write_wakeup(tty-link); - wake_up_interruptible_poll(tty-link-write_wait, POLLOUT); + if (waitqueue_active(tty-link-write_wait)) + wake_up_interruptible_poll(tty-link-write_wait, POLLOUT); Does this actually microbenchmark faster ? Getting on and off the write_wait queue is actually pretty expensive for the other pty (the writer), and the unnecessary wakeup from the reader doesn't help. The other chunks are gratuitous. Regards, Peter Hurley PS - This came up because there is some worst-case behavior that I'm looking into fixing. When the userspace reader is very far behind (say because it's reading char-by-char), it doesn't make sense to keep restarting the input processing worker. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH tty-next 6/7] n_tty: Only perform wakeups for waiters
Only wakeup the _waiting_ reader, polls and/or writer(s). Signed-off-by: Peter Hurley --- drivers/tty/n_tty.c | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c index 8f2356e..aae28a6 100644 --- a/drivers/tty/n_tty.c +++ b/drivers/tty/n_tty.c @@ -275,7 +275,8 @@ static void n_tty_check_unthrottle(struct tty_struct *tty) return; n_tty_set_room(tty); n_tty_write_wakeup(tty->link); - wake_up_interruptible_poll(>link->write_wait, POLLOUT); + if (waitqueue_active(>link->write_wait)) + wake_up_interruptible_poll(>link->write_wait, POLLOUT); return; } @@ -350,7 +351,8 @@ static void n_tty_packet_mode_flush(struct tty_struct *tty) spin_lock_irqsave(>ctrl_lock, flags); if (tty->link->packet) { tty->ctrl_status |= TIOCPKT_FLUSHREAD; - wake_up_interruptible(>link->read_wait); + if (waitqueue_active(>link->read_wait)) + wake_up_interruptible(>link->read_wait); } spin_unlock_irqrestore(>ctrl_lock, flags); } @@ -1156,7 +1158,8 @@ static void n_tty_receive_break(struct tty_struct *tty) put_tty_queue('\0', ldata); } put_tty_queue('\0', ldata); - wake_up_interruptible(>read_wait); + if (waitqueue_active(>read_wait)) + wake_up_interruptible(>read_wait); } /** @@ -1214,7 +1217,8 @@ static void n_tty_receive_parity_error(struct tty_struct *tty, unsigned char c) put_tty_queue('\0', ldata); else put_tty_queue(c, ldata); - wake_up_interruptible(>read_wait); + if (waitqueue_active(>read_wait)) + wake_up_interruptible(>read_wait); } static void @@ -1808,8 +1812,10 @@ static void n_tty_set_termios(struct tty_struct *tty, struct ktermios *old) start_tty(tty); /* The termios change make the tty ready for I/O */ - wake_up_interruptible(>write_wait); - wake_up_interruptible(>read_wait); + if (waitqueue_active(>write_wait)) + wake_up_interruptible(>write_wait); + if (waitqueue_active(>read_wait)) + wake_up_interruptible(>read_wait); } /** -- 1.8.1.2 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH tty-next 6/7] n_tty: Only perform wakeups for waiters
Only wakeup the _waiting_ reader, polls and/or writer(s). Signed-off-by: Peter Hurley pe...@hurleysoftware.com --- drivers/tty/n_tty.c | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c index 8f2356e..aae28a6 100644 --- a/drivers/tty/n_tty.c +++ b/drivers/tty/n_tty.c @@ -275,7 +275,8 @@ static void n_tty_check_unthrottle(struct tty_struct *tty) return; n_tty_set_room(tty); n_tty_write_wakeup(tty-link); - wake_up_interruptible_poll(tty-link-write_wait, POLLOUT); + if (waitqueue_active(tty-link-write_wait)) + wake_up_interruptible_poll(tty-link-write_wait, POLLOUT); return; } @@ -350,7 +351,8 @@ static void n_tty_packet_mode_flush(struct tty_struct *tty) spin_lock_irqsave(tty-ctrl_lock, flags); if (tty-link-packet) { tty-ctrl_status |= TIOCPKT_FLUSHREAD; - wake_up_interruptible(tty-link-read_wait); + if (waitqueue_active(tty-link-read_wait)) + wake_up_interruptible(tty-link-read_wait); } spin_unlock_irqrestore(tty-ctrl_lock, flags); } @@ -1156,7 +1158,8 @@ static void n_tty_receive_break(struct tty_struct *tty) put_tty_queue('\0', ldata); } put_tty_queue('\0', ldata); - wake_up_interruptible(tty-read_wait); + if (waitqueue_active(tty-read_wait)) + wake_up_interruptible(tty-read_wait); } /** @@ -1214,7 +1217,8 @@ static void n_tty_receive_parity_error(struct tty_struct *tty, unsigned char c) put_tty_queue('\0', ldata); else put_tty_queue(c, ldata); - wake_up_interruptible(tty-read_wait); + if (waitqueue_active(tty-read_wait)) + wake_up_interruptible(tty-read_wait); } static void @@ -1808,8 +1812,10 @@ static void n_tty_set_termios(struct tty_struct *tty, struct ktermios *old) start_tty(tty); /* The termios change make the tty ready for I/O */ - wake_up_interruptible(tty-write_wait); - wake_up_interruptible(tty-read_wait); + if (waitqueue_active(tty-write_wait)) + wake_up_interruptible(tty-write_wait); + if (waitqueue_active(tty-read_wait)) + wake_up_interruptible(tty-read_wait); } /** -- 1.8.1.2 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/