linux-kernel - [PATCH v3 1/6] n_tty: Eliminate receive_room() from consumer/exclusive paths

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Fri, 16 Jan 2015 15:05:34 -0500
From:	Peter Hurley <peter@...leysoftware.com>
To:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:	Jiri Slaby <jslaby@...e.cz>,
	Christian Riesch <christian.riesch@...cron.at>,
	linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org,
	Denis Du <dudenis2000@...oo.ca>,
	Måns Rullgård <mans@...sr.com>,
	Peter Hurley <peter@...leysoftware.com>
Subject: [PATCH v3 1/6] n_tty: Eliminate receive_room() from consumer/exclusive paths

The input worker never reschedules itself; it only processes input until
either there is no more input or the read buffer is full. So the reader
is responsible for restarting the input worker only if the read buffer
was previously full (no_room == 1) _and_ space is now available to process
more input because the reader has consumed data from the read buffer.

However, computing the actual space available is not required to determine
if the reader has consumed data from the read buffer. This condition is
evaluated in 5 situations, each of which the space avail is already known:
1. n_tty_flush_buffer() - the read buffer is empty; kick the worker
2. n_tty_set_termios() - no data has been consumed; do not kick the worker
       (although it may have kicked the reader so data _will be_ consumed)
3. n_tty_check_unthrottle - avail space > 3968; kick the worker
4. n_tty_read, before leaving - only kick the worker if the reader has
       moved the tail. This prevents unnecessarily kicking the worker
       when timeout-style reading is used.
5. n_tty_read, before sleeping - although it is possible for the read
       buffer to be full and input_available_p() to be false, this can
       only happen when the input worker is racing the reader, in which
       case the reader will have been woken and won't sleep.

Rename n_tty_set_room() to n_tty_kick_worker() to reflect what the
function actually does.

Signed-off-by: Peter Hurley <peter@...leysoftware.com>
---
 drivers/tty/n_tty.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 4ddfa60..b60b043 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -187,10 +187,10 @@ static int receive_room(struct tty_struct *tty)
 }
 
 /**
- *	n_tty_set_room	-	receive space
+ *	n_tty_kick_worker - start input worker (if required)
  *	@tty: terminal
  *
- *	Re-schedules the flip buffer work if space just became available.
+ *	Re-schedules the flip buffer work if it may have stopped
  *
  *	Caller holds exclusive termios_rwsem
  *	   or
@@ -198,12 +198,12 @@ static int receive_room(struct tty_struct *tty)
  *		holds non-exclusive termios_rwsem
  */
 
-static void n_tty_set_room(struct tty_struct *tty)
+static void n_tty_kick_worker(struct tty_struct *tty)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 
-	/* Did this open up the receive buffer? We may need to flip */
-	if (unlikely(ldata->no_room) && receive_room(tty)) {
+	/* Did the input worker stop? Restart it */
+	if (unlikely(ldata->no_room)) {
 		ldata->no_room = 0;
 
 		WARN_RATELIMIT(tty->port->itty == NULL,
@@ -274,7 +274,7 @@ static void n_tty_check_unthrottle(struct tty_struct *tty)
 			return;
 		if (!tty->count)
 			return;
-		n_tty_set_room(tty);
+		n_tty_kick_worker(tty);
 		n_tty_write_wakeup(tty->link);
 		if (waitqueue_active(&tty->link->write_wait))
 			wake_up_interruptible_poll(&tty->link->write_wait, POLLOUT);
@@ -296,7 +296,7 @@ static void n_tty_check_unthrottle(struct tty_struct *tty)
 			break;
 		if (!tty->count)
 			break;
-		n_tty_set_room(tty);
+		n_tty_kick_worker(tty);
 		unthrottled = tty_unthrottle_safe(tty);
 		if (!unthrottled)
 			break;
@@ -379,7 +379,7 @@ static void n_tty_flush_buffer(struct tty_struct *tty)
 {
 	down_write(&tty->termios_rwsem);
 	reset_buffer_flags(tty->disc_data);
-	n_tty_set_room(tty);
+	n_tty_kick_worker(tty);
 
 	if (tty->link)
 		n_tty_packet_mode_flush(tty);
@@ -1817,7 +1817,6 @@ static void n_tty_set_termios(struct tty_struct *tty, struct ktermios *old)
 		else
 			ldata->real_raw = 0;
 	}
-	n_tty_set_room(tty);
 	/*
 	 * Fix tty hang when I_IXON(tty) is cleared, but the tty
 	 * been stopped by STOP_CHAR(tty) before it.
@@ -2130,6 +2129,7 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
 	ssize_t retval = 0;
 	long timeout;
 	int packet;
+	size_t tail;
 
 	c = job_control(tty, file);
 	if (c < 0)
@@ -2166,6 +2166,7 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
 	}
 
 	packet = tty->packet;
+	tail = ldata->read_tail;
 
 	add_wait_queue(&tty->read_wait, &wait);
 	while (nr) {
@@ -2208,7 +2209,6 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
 				retval = -ERESTARTSYS;
 				break;
 			}
-			n_tty_set_room(tty);
 			up_read(&tty->termios_rwsem);
 
 			timeout = wait_woken(&wait, TASK_INTERRUPTIBLE,
@@ -2253,7 +2253,8 @@ static ssize_t n_tty_read(struct tty_struct *tty, struct file *file,
 		if (time)
 			timeout = time;
 	}
-	n_tty_set_room(tty);
+	if (tail != ldata->read_tail)
+		n_tty_kick_worker(tty);
 	up_read(&tty->termios_rwsem);
 
 	remove_wait_queue(&tty->read_wait, &wait);
-- 
2.2.2

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