[PATCH 0/7] n_tty fixes

[PATCH 0/7] n_tty fixes

[Peter Hurley]

This patch series combines a number of fixes for the N_TTY
line discipline.

These address unsafe use of the foreground process group id:
  n_tty: Fix unsafe driver-side signals
  n_tty: Lock access to tty->pgrp for POSIX job control

These continue Jiri's work to separate ldisc from tty:
  tty: Fix checkpatch errors in tty_ldisc.h
  n_tty: Encapsulate minimum_to_wake within N_TTY

The rest are self-explanatory:
  n_tty: Untangle read completion variables
  n_tty: Fix unsafe update of available buffer space
  n_tty: Buffer work should not reschedule itself


Peter Hurley (7):
  n_tty: Fix unsafe driver-side signals
  n_tty: Lock access to tty->pgrp for POSIX job control
  tty: Fix checkpatch errors in tty_ldisc.h
  n_tty: Encapsulate minimum_to_wake within N_TTY
  n_tty: Untangle read completion variables
  n_tty: Fix unsafe update of available buffer space
  n_tty: Buffer work should not reschedule itself

 drivers/tty/n_tty.c       | 144 +++++++++++++++++++++++++---------------------
 drivers/tty/tty_io.c      |  17 +++---
 include/linux/tty.h       |   1 -
 include/linux/tty_ldisc.h | 138 +++++++++++++++++++++++---------------------
 4 files changed, 161 insertions(+), 139 deletions(-)

-- 
1.8.1.2

[PATCH 1/7] n_tty: Fix unsafe driver-side signals

[Peter Hurley]

An ldisc reference is insufficient guarantee the foreground process
group is not in the process of being signalled from a hangup.

1) Reads of tty->pgrp must be locked with ctrl_lock
2) The group pid must be referenced for the duration of signalling.
   Because the driver-side is not process-context, a pid reference
   must be acquired.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/n_tty.c | 34 ++++++++++++++--------------------
 1 file changed, 14 insertions(+), 20 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 66ce178..cd9ba3d 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1038,23 +1038,19 @@ static void eraser(unsigned char c, struct tty_struct *tty)
  *	isig		-	handle the ISIG optio
  *	@sig: signal
  *	@tty: terminal
- *	@flush: force flush
  *
- *	Called when a signal is being sent due to terminal input. This
- *	may caus terminal flushing to take place according to the termios
- *	settings and character used. Called from the driver receive_buf
- *	path so serialized.
+ *	Called when a signal is being sent due to terminal input.
+ *	Called from the driver receive_buf path so serialized.
  *
- *	Locking: ctrl_lock, read_lock (both via flush buffer)
+ *	Locking: ctrl_lock
  */
 
-static inline void isig(int sig, struct tty_struct *tty, int flush)
+static inline void isig(int sig, struct tty_struct *tty)
 {
-	if (tty->pgrp)
-		kill_pgrp(tty->pgrp, sig, 1);
-	if (flush || !L_NOFLSH(tty)) {
-		n_tty_flush_buffer(tty);
-		tty_driver_flush_buffer(tty);
+	struct pid *tty_pgrp = tty_get_pgrp(tty);
+	if (tty_pgrp) {
+		kill_pgrp(tty_pgrp, sig, 1);
+		put_pid(tty_pgrp);
 	}
 }
 
@@ -1075,7 +1071,11 @@ static inline void n_tty_receive_break(struct tty_struct *tty)
 	if (I_IGNBRK(tty))
 		return;
 	if (I_BRKINT(tty)) {
-		isig(SIGINT, tty, 1);
+		isig(SIGINT, tty);
+		if (!L_NOFLSH(tty)) {
+			n_tty_flush_buffer(tty);
+			tty_driver_flush_buffer(tty);
+		}
 		return;
 	}
 	if (I_PARMRK(tty)) {
@@ -1242,11 +1242,6 @@ static inline void n_tty_receive_char(struct tty_struct *tty, unsigned char c)
 		signal = SIGTSTP;
 		if (c == SUSP_CHAR(tty)) {
 send_signal:
-			/*
-			 * Note that we do not use isig() here because we want
-			 * the order to be:
-			 * 1) flush, 2) echo, 3) signal
-			 */
 			if (!L_NOFLSH(tty)) {
 				n_tty_flush_buffer(tty);
 				tty_driver_flush_buffer(tty);
@@ -1257,8 +1252,7 @@ send_signal:
 				echo_char(c, tty);
 				process_echoes(tty);
 			}
-			if (tty->pgrp)
-				kill_pgrp(tty->pgrp, signal, 1);
+			isig(signal, tty);
 			return;
 		}
 	}
-- 
1.8.1.2

[PATCH 2/7] n_tty: Lock access to tty->pgrp for POSIX job control

[Peter Hurley]

Concurrent access to tty->pgrp must be protected with tty->ctrl_lock.
Also, as noted in the comments, reading current->signal->tty is
safe because either,
  1) current->signal->tty is assigned by current, or
  2) current->signal->tty is set to NULL.

NB: for reference, tty_check_change() implements a similar POSIX
check for the ioctls corresponding to tcflush(), tcdrain(),
tcsetattr(), tcsetpgrp(), tcflow() and tcsendbreak().

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/n_tty.c | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index cd9ba3d..61797c4 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1744,10 +1744,9 @@ extern ssize_t redirected_tty_write(struct file *, const char __user *,
  *	and if appropriate send any needed signals and return a negative
  *	error code if action should be taken.
  *
- *	FIXME:
- *	Locking: None - redirected write test is safe, testing
- *	current->signal should possibly lock current->sighand
- *	pgrp locking ?
+ *	Locking: redirected write test is safe
+ *		 current->signal->tty check is safe
+ *		 ctrl_lock to safely reference tty->pgrp
  */
 
 static int job_control(struct tty_struct *tty, struct file *file)
@@ -1757,19 +1756,22 @@ static int job_control(struct tty_struct *tty, struct file *file)
 	/* NOTE: not yet done after every sleep pending a thorough
 	   check of the logic of this change. -- jlc */
 	/* don't stop on /dev/console */
-	if (file->f_op->write != redirected_tty_write &&
-	    current->signal->tty == tty) {
-		if (!tty->pgrp)
-			printk(KERN_ERR "n_tty_read: no tty->pgrp!\n");
-		else if (task_pgrp(current) != tty->pgrp) {
-			if (is_ignored(SIGTTIN) ||
-			    is_current_pgrp_orphaned())
-				return -EIO;
-			kill_pgrp(task_pgrp(current), SIGTTIN, 1);
-			set_thread_flag(TIF_SIGPENDING);
-			return -ERESTARTSYS;
-		}
+	if (file->f_op->write == redirected_tty_write ||
+	    current->signal->tty != tty)
+		return 0;
+
+	spin_lock_irq(&tty->ctrl_lock);
+	if (!tty->pgrp)
+		printk(KERN_ERR "n_tty_read: no tty->pgrp!\n");
+	else if (task_pgrp(current) != tty->pgrp) {
+		spin_unlock_irq(&tty->ctrl_lock);
+		if (is_ignored(SIGTTIN) || is_current_pgrp_orphaned())
+			return -EIO;
+		kill_pgrp(task_pgrp(current), SIGTTIN, 1);
+		set_thread_flag(TIF_SIGPENDING);
+		return -ERESTARTSYS;
 	}
+	spin_unlock_irq(&tty->ctrl_lock);
 	return 0;
 }
 
-- 
1.8.1.2

[PATCH 3/7] tty: Fix checkpatch errors in tty_ldisc.h

[Peter Hurley]

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 include/linux/tty_ldisc.h | 132 +++++++++++++++++++++++-----------------------
 1 file changed, 66 insertions(+), 66 deletions(-)

diff --git a/include/linux/tty_ldisc.h b/include/linux/tty_ldisc.h
index a8c58c2..1ef449e 100644
--- a/include/linux/tty_ldisc.h
+++ b/include/linux/tty_ldisc.h
@@ -9,89 +9,89 @@
  *
  * int	(*open)(struct tty_struct *);
  *
- * 	This function is called when the line discipline is associated
- * 	with the tty.  The line discipline can use this as an
- * 	opportunity to initialize any state needed by the ldisc routines.
- * 
+ *	This function is called when the line discipline is associated
+ *	with the tty.  The line discipline can use this as an
+ *	opportunity to initialize any state needed by the ldisc routines.
+ *
  * void	(*close)(struct tty_struct *);
  *
  *	This function is called when the line discipline is being
- * 	shutdown, either because the tty is being closed or because
- * 	the tty is being changed to use a new line discipline
- * 
+ *	shutdown, either because the tty is being closed or because
+ *	the tty is being changed to use a new line discipline
+ *
  * void	(*flush_buffer)(struct tty_struct *tty);
  *
- * 	This function instructs the line discipline to clear its
- * 	buffers of any input characters it may have queued to be
- * 	delivered to the user mode process.
- * 
+ *	This function instructs the line discipline to clear its
+ *	buffers of any input characters it may have queued to be
+ *	delivered to the user mode process.
+ *
  * ssize_t (*chars_in_buffer)(struct tty_struct *tty);
  *
- * 	This function returns the number of input characters the line
+ *	This function returns the number of input characters the line
  *	discipline may have queued up to be delivered to the user mode
  *	process.
- * 
+ *
  * ssize_t (*read)(struct tty_struct * tty, struct file * file,
  *		   unsigned char * buf, size_t nr);
  *
- * 	This function is called when the user requests to read from
- * 	the tty.  The line discipline will return whatever characters
- * 	it has buffered up for the user.  If this function is not
- * 	defined, the user will receive an EIO error.
- * 
+ *	This function is called when the user requests to read from
+ *	the tty.  The line discipline will return whatever characters
+ *	it has buffered up for the user.  If this function is not
+ *	defined, the user will receive an EIO error.
+ *
  * ssize_t (*write)(struct tty_struct * tty, struct file * file,
- * 		    const unsigned char * buf, size_t nr);
- *
- * 	This function is called when the user requests to write to the
- * 	tty.  The line discipline will deliver the characters to the
- * 	low-level tty device for transmission, optionally performing
- * 	some processing on the characters first.  If this function is
- * 	not defined, the user will receive an EIO error.
- * 
+ *		    const unsigned char * buf, size_t nr);
+ *
+ *	This function is called when the user requests to write to the
+ *	tty.  The line discipline will deliver the characters to the
+ *	low-level tty device for transmission, optionally performing
+ *	some processing on the characters first.  If this function is
+ *	not defined, the user will receive an EIO error.
+ *
  * int	(*ioctl)(struct tty_struct * tty, struct file * file,
- * 		 unsigned int cmd, unsigned long arg);
+ *		 unsigned int cmd, unsigned long arg);
  *
  *	This function is called when the user requests an ioctl which
- * 	is not handled by the tty layer or the low-level tty driver.
- * 	It is intended for ioctls which affect line discpline
- * 	operation.  Note that the search order for ioctls is (1) tty
- * 	layer, (2) tty low-level driver, (3) line discpline.  So a
- * 	low-level driver can "grab" an ioctl request before the line
- * 	discpline has a chance to see it.
- * 
+ *	is not handled by the tty layer or the low-level tty driver.
+ *	It is intended for ioctls which affect line discpline
+ *	operation.  Note that the search order for ioctls is (1) tty
+ *	layer, (2) tty low-level driver, (3) line discpline.  So a
+ *	low-level driver can "grab" an ioctl request before the line
+ *	discpline has a chance to see it.
+ *
  * long	(*compat_ioctl)(struct tty_struct * tty, struct file * file,
- * 		        unsigned int cmd, unsigned long arg);
+ *		        unsigned int cmd, unsigned long arg);
  *
- *      Process ioctl calls from 32-bit process on 64-bit system
+ *	Process ioctl calls from 32-bit process on 64-bit system
  *
  * void	(*set_termios)(struct tty_struct *tty, struct ktermios * old);
  *
- * 	This function notifies the line discpline that a change has
- * 	been made to the termios structure.
- * 
+ *	This function notifies the line discpline that a change has
+ *	been made to the termios structure.
+ *
  * int	(*poll)(struct tty_struct * tty, struct file * file,
- * 		  poll_table *wait);
+ *		  poll_table *wait);
  *
- * 	This function is called when a user attempts to select/poll on a
- * 	tty device.  It is solely the responsibility of the line
- * 	discipline to handle poll requests.
+ *	This function is called when a user attempts to select/poll on a
+ *	tty device.  It is solely the responsibility of the line
+ *	discipline to handle poll requests.
  *
  * void	(*receive_buf)(struct tty_struct *, const unsigned char *cp,
- * 		       char *fp, int count);
- *
- * 	This function is called by the low-level tty driver to send
- * 	characters received by the hardware to the line discpline for
- * 	processing.  <cp> is a pointer to the buffer of input
- * 	character received by the device.  <fp> is a pointer to a
- * 	pointer of flag bytes which indicate whether a character was
- * 	received with a parity error, etc.
- * 
+ *		       char *fp, int count);
+ *
+ *	This function is called by the low-level tty driver to send
+ *	characters received by the hardware to the line discpline for
+ *	processing.  <cp> is a pointer to the buffer of input
+ *	character received by the device.  <fp> is a pointer to a
+ *	pointer of flag bytes which indicate whether a character was
+ *	received with a parity error, etc.
+ *
  * void	(*write_wakeup)(struct tty_struct *);
  *
- * 	This function is called by the low-level tty driver to signal
- * 	that line discpline should try to send more characters to the
- * 	low-level driver for transmission.  If the line discpline does
- * 	not have any more data to send, it can just return.
+ *	This function is called by the low-level tty driver to signal
+ *	that line discpline should try to send more characters to the
+ *	low-level driver for transmission.  If the line discpline does
+ *	not have any more data to send, it can just return.
  *
  * int (*hangup)(struct tty_struct *)
  *
@@ -162,7 +162,7 @@ struct tty_ldisc_ops {
 	char	*name;
 	int	num;
 	int	flags;
-	
+
 	/*
 	 * The following routines are called from above.
 	 */
@@ -170,19 +170,19 @@ struct tty_ldisc_ops {
 	void	(*close)(struct tty_struct *);
 	void	(*flush_buffer)(struct tty_struct *tty);
 	ssize_t	(*chars_in_buffer)(struct tty_struct *tty);
-	ssize_t	(*read)(struct tty_struct * tty, struct file * file,
-			unsigned char __user * buf, size_t nr);
-	ssize_t	(*write)(struct tty_struct * tty, struct file * file,
-			 const unsigned char * buf, size_t nr);	
-	int	(*ioctl)(struct tty_struct * tty, struct file * file,
+	ssize_t	(*read)(struct tty_struct *tty, struct file *file,
+			unsigned char __user *buf, size_t nr);
+	ssize_t	(*write)(struct tty_struct *tty, struct file *file,
+			 const unsigned char *buf, size_t nr);
+	int	(*ioctl)(struct tty_struct *tty, struct file *file,
 			 unsigned int cmd, unsigned long arg);
-	long	(*compat_ioctl)(struct tty_struct * tty, struct file * file,
+	long	(*compat_ioctl)(struct tty_struct *tty, struct file *file,
 				unsigned int cmd, unsigned long arg);
-	void	(*set_termios)(struct tty_struct *tty, struct ktermios * old);
+	void	(*set_termios)(struct tty_struct *tty, struct ktermios *old);
 	unsigned int (*poll)(struct tty_struct *, struct file *,
 			     struct poll_table_struct *);
 	int	(*hangup)(struct tty_struct *tty);
-	
+
 	/*
 	 * The following routines are called from below.
 	 */
@@ -192,7 +192,7 @@ struct tty_ldisc_ops {
 	void	(*dcd_change)(struct tty_struct *, unsigned int);
 
 	struct  module *owner;
-	
+
 	int refcount;
 };
 
-- 
1.8.1.2

[PATCH 4/7] n_tty: Encapsulate minimum_to_wake within N_TTY

[Peter Hurley]

minimum_to_wake is unique to N_TTY processing, and belongs in
per-ldisc data.

Add the ldisc method, ldisc_ops::fasync(), to notify line disciplines
when signal-driven I/O is enabled or disabled. When enabled for N_TTY
(by fcntl(F_SETFL, O_ASYNC)), blocking reader/polls will be woken
for any readable input. When disabled, blocking reader/polls are not
woken until the read buffer is full.

Canonical mode (L_ICANON(tty), n_tty_data::icanon) is not affected by
the minimum_to_wake setting.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/n_tty.c       | 39 +++++++++++++++++++++++++++------------
 drivers/tty/tty_io.c      | 17 +++++++++--------
 include/linux/tty.h       |  1 -
 include/linux/tty_ldisc.h |  6 ++++++
 4 files changed, 42 insertions(+), 21 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 61797c4..3fd657b 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -89,6 +89,7 @@ struct n_tty_data {
 	int read_head;
 	int read_tail;
 	int read_cnt;
+	int minimum_to_wake;
 
 	unsigned char *echo_buf;
 	unsigned int echo_pos;
@@ -1471,7 +1472,7 @@ static void n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 
 	n_tty_set_room(tty);
 
-	if ((!ldata->icanon && (ldata->read_cnt >= tty->minimum_to_wake)) ||
+	if ((!ldata->icanon && (ldata->read_cnt >= ldata->minimum_to_wake)) ||
 		L_EXTPROC(tty)) {
 		kill_fasync(&tty->fasync, SIGIO, POLL_IN);
 		if (waitqueue_active(&tty->read_wait))
@@ -1649,7 +1650,7 @@ static int n_tty_open(struct tty_struct *tty)
 	tty->disc_data = ldata;
 	reset_buffer_flags(tty->disc_data);
 	ldata->column = 0;
-	tty->minimum_to_wake = 1;
+	ldata->minimum_to_wake = 1;
 	tty->closing = 0;
 	/* indicate buffer work may resume */
 	clear_bit(TTY_LDISC_HALTED, &tty->flags);
@@ -1817,17 +1818,17 @@ do_it_again:
 		minimum = MIN_CHAR(tty);
 		if (minimum) {
 			if (time)
-				tty->minimum_to_wake = 1;
+				ldata->minimum_to_wake = 1;
 			else if (!waitqueue_active(&tty->read_wait) ||
-				 (tty->minimum_to_wake > minimum))
-				tty->minimum_to_wake = minimum;
+				 (ldata->minimum_to_wake > minimum))
+				ldata->minimum_to_wake = minimum;
 		} else {
 			timeout = 0;
 			if (time) {
 				timeout = time;
 				time = 0;
 			}
-			tty->minimum_to_wake = minimum = 1;
+			ldata->minimum_to_wake = minimum = 1;
 		}
 	}
 
@@ -1867,9 +1868,9 @@ do_it_again:
 		   TASK_RUNNING. */
 		set_current_state(TASK_INTERRUPTIBLE);
 
-		if (((minimum - (b - buf)) < tty->minimum_to_wake) &&
+		if (((minimum - (b - buf)) < ldata->minimum_to_wake) &&
 		    ((minimum - (b - buf)) >= 1))
-			tty->minimum_to_wake = (minimum - (b - buf));
+			ldata->minimum_to_wake = (minimum - (b - buf));
 
 		if (!input_available_p(tty, 0)) {
 			if (test_bit(TTY_OTHER_CLOSED, &tty->flags)) {
@@ -1979,7 +1980,7 @@ do_it_again:
 	remove_wait_queue(&tty->read_wait, &wait);
 
 	if (!waitqueue_active(&tty->read_wait))
-		tty->minimum_to_wake = minimum;
+		ldata->minimum_to_wake = minimum;
 
 	__set_current_state(TASK_RUNNING);
 	size = b - buf;
@@ -2111,6 +2112,7 @@ break_out:
 static unsigned int n_tty_poll(struct tty_struct *tty, struct file *file,
 							poll_table *wait)
 {
+	struct n_tty_data *ldata = tty->disc_data;
 	unsigned int mask = 0;
 
 	poll_wait(file, &tty->read_wait, wait);
@@ -2125,9 +2127,9 @@ static unsigned int n_tty_poll(struct tty_struct *tty, struct file *file,
 		mask |= POLLHUP;
 	if (!(mask & (POLLHUP | POLLIN | POLLRDNORM))) {
 		if (MIN_CHAR(tty) && !TIME_CHAR(tty))
-			tty->minimum_to_wake = MIN_CHAR(tty);
+			ldata->minimum_to_wake = MIN_CHAR(tty);
 		else
-			tty->minimum_to_wake = 1;
+			ldata->minimum_to_wake = 1;
 	}
 	if (tty->ops->write && !tty_is_writelocked(tty) &&
 			tty_chars_in_buffer(tty) < WAKEUP_CHARS &&
@@ -2175,6 +2177,18 @@ static int n_tty_ioctl(struct tty_struct *tty, struct file *file,
 	}
 }
 
+static void n_tty_fasync(struct tty_struct *tty, int on)
+{
+	struct n_tty_data *ldata = tty->disc_data;
+
+	if (!waitqueue_active(&tty->read_wait)) {
+		if (on)
+			ldata->minimum_to_wake = 1;
+		else if (!tty->fasync)
+			ldata->minimum_to_wake = N_TTY_BUF_SIZE;
+	}
+}
+
 struct tty_ldisc_ops tty_ldisc_N_TTY = {
 	.magic           = TTY_LDISC_MAGIC,
 	.name            = "n_tty",
@@ -2188,7 +2202,8 @@ struct tty_ldisc_ops tty_ldisc_N_TTY = {
 	.set_termios     = n_tty_set_termios,
 	.poll            = n_tty_poll,
 	.receive_buf     = n_tty_receive_buf,
-	.write_wakeup    = n_tty_write_wakeup
+	.write_wakeup    = n_tty_write_wakeup,
+	.fasync		 = n_tty_fasync,
 };
 
 /**
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index adc1d01..525c915 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2077,6 +2077,7 @@ static unsigned int tty_poll(struct file *filp, poll_table *wait)
 static int __tty_fasync(int fd, struct file *filp, int on)
 {
 	struct tty_struct *tty = file_tty(filp);
+	struct tty_ldisc *ldisc;
 	unsigned long flags;
 	int retval = 0;
 
@@ -2087,11 +2088,17 @@ static int __tty_fasync(int fd, struct file *filp, int on)
 	if (retval <= 0)
 		goto out;
 
+	ldisc = tty_ldisc_ref(tty);
+	if (ldisc) {
+		if (ldisc->ops->fasync)
+			ldisc->ops->fasync(tty, on);
+		tty_ldisc_deref(ldisc);
+	}
+
 	if (on) {
 		enum pid_type type;
 		struct pid *pid;
-		if (!waitqueue_active(&tty->read_wait))
-			tty->minimum_to_wake = 1;
+
 		spin_lock_irqsave(&tty->ctrl_lock, flags);
 		if (tty->pgrp) {
 			pid = tty->pgrp;
@@ -2104,13 +2111,7 @@ static int __tty_fasync(int fd, struct file *filp, int on)
 		spin_unlock_irqrestore(&tty->ctrl_lock, flags);
 		retval = __f_setown(filp, pid, type, 0);
 		put_pid(pid);
-		if (retval)
-			goto out;
-	} else {
-		if (!tty->fasync && !waitqueue_active(&tty->read_wait))
-			tty->minimum_to_wake = N_TTY_BUF_SIZE;
 	}
-	retval = 0;
 out:
 	return retval;
 }
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 2c109a3..39f0317 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -272,7 +272,6 @@ struct tty_struct {
 #define N_TTY_BUF_SIZE 4096
 
 	unsigned char closing:1;
-	unsigned short minimum_to_wake;
 	unsigned char *write_buf;
 	int write_cnt;
 	/* If the tty has a pending do_SAK, queue it here - akpm */
diff --git a/include/linux/tty_ldisc.h b/include/linux/tty_ldisc.h
index 1ef449e..44584d9 100644
--- a/include/linux/tty_ldisc.h
+++ b/include/linux/tty_ldisc.h
@@ -100,6 +100,11 @@
  *	seek to perform this action quickly but should wait until
  *	any pending driver I/O is completed.
  *
+ * void (*fasync)(struct tty_struct *, int on)
+ *
+ *	Notify line discipline when signal-driven I/O is enabled or
+ *	disabled.
+ *
  * void (*dcd_change)(struct tty_struct *tty, unsigned int status)
  *
  *	Tells the discipline that the DCD pin has changed its status.
@@ -190,6 +195,7 @@ struct tty_ldisc_ops {
 			       char *fp, int count);
 	void	(*write_wakeup)(struct tty_struct *);
 	void	(*dcd_change)(struct tty_struct *, unsigned int);
+	void	(*fasync)(struct tty_struct *tty, int on);
 
 	struct  module *owner;
 
-- 
1.8.1.2

[PATCH 5/7] n_tty: Untangle read completion variables

[Peter Hurley]

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/n_tty.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 3fd657b..b2f621f 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1814,20 +1814,16 @@ do_it_again:
 	minimum = time = 0;
 	timeout = MAX_SCHEDULE_TIMEOUT;
 	if (!ldata->icanon) {
-		time = (HZ / 10) * TIME_CHAR(tty);
 		minimum = MIN_CHAR(tty);
 		if (minimum) {
+			time = (HZ / 10) * TIME_CHAR(tty);
 			if (time)
 				ldata->minimum_to_wake = 1;
 			else if (!waitqueue_active(&tty->read_wait) ||
 				 (ldata->minimum_to_wake > minimum))
 				ldata->minimum_to_wake = minimum;
 		} else {
-			timeout = 0;
-			if (time) {
-				timeout = time;
-				time = 0;
-			}
+			timeout = (HZ / 10) * TIME_CHAR(tty);
 			ldata->minimum_to_wake = minimum = 1;
 		}
 	}
-- 
1.8.1.2

[PATCH 6/7] n_tty: Fix unsafe update of available buffer space

[Peter Hurley]

receive_room is used to control the amount of data the flip
buffer work can push to the read buffer. This update is unsafe:

  CPU 0                        |  CPU 1
                               |
                               | n_tty_read()
                               |   n_tty_set_room()
                               |     left = <calc of space>
n_tty_receive_buf()            |
  <push data to buffer>        |
  n_tty_set_room()             |
    left = <calc of space>     |
    tty->receive_room = left   |
                               |     tty->receive_room = left

receive_room is now updated with a stale calculation of the
available buffer space, and the subsequent work loop will likely
overwrite unread data in the input buffer.

Update receive_room atomically with the calculation of the
available buffer space.

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

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index b2f621f..61a55f4 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -115,13 +115,14 @@ static inline int tty_put_user(struct tty_struct *tty, unsigned char x,
 }
 
 /**
- *	n_tty_set__room	-	receive space
+ *	n_tty_set_room	-	receive space
  *	@tty: terminal
  *
- *	Called by the driver to find out how much data it is
- *	permitted to feed to the line discipline without any being lost
- *	and thus to manage flow control. Not serialized. Answers for the
- *	"instant".
+ *	Sets tty->receive_room to reflect the currently available space
+ *	in the input buffer, and re-schedules the flip buffer work if space
+ *	just became available.
+ *
+ *	Locks: Concurrent update is protected with read_lock
  */
 
 static void n_tty_set_room(struct tty_struct *tty)
@@ -129,8 +130,10 @@ static void n_tty_set_room(struct tty_struct *tty)
 	struct n_tty_data *ldata = tty->disc_data;
 	int left;
 	int old_left;
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&ldata->read_lock, flags);
 
-	/* ldata->read_cnt is not read locked ? */
 	if (I_PARMRK(tty)) {
 		/* Multiply read_cnt by 3, since each byte might take up to
 		 * three times as many spaces when PARMRK is set (depending on
@@ -150,6 +153,8 @@ static void n_tty_set_room(struct tty_struct *tty)
 	old_left = tty->receive_room;
 	tty->receive_room = left;
 
+	raw_spin_unlock_irqrestore(&ldata->read_lock, flags);
+
 	/* Did this open up the receive buffer? We may need to flip */
 	if (left && !old_left) {
 		WARN_RATELIMIT(tty->port->itty == NULL,
@@ -1885,7 +1890,6 @@ do_it_again:
 				retval = -ERESTARTSYS;
 				break;
 			}
-			/* FIXME: does n_tty_set_room need locking ? */
 			n_tty_set_room(tty);
 			timeout = schedule_timeout(timeout);
 			continue;
-- 
1.8.1.2

[PATCH 7/7] n_tty: Buffer work should not reschedule itself

[Peter Hurley]

Although the driver-side input path must update the available
buffer space, it should not reschedule itself. If space is still
available and the flip buffers are not empty, flush_to_ldisc()
will loop again.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/n_tty.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 61a55f4..6f9694d 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -118,14 +118,14 @@ static inline int tty_put_user(struct tty_struct *tty, unsigned char x,
  *	n_tty_set_room	-	receive space
  *	@tty: terminal
  *
- *	Sets tty->receive_room to reflect the currently available space
+ *	Updates tty->receive_room to reflect the currently available space
  *	in the input buffer, and re-schedules the flip buffer work if space
  *	just became available.
  *
  *	Locks: Concurrent update is protected with read_lock
  */
 
-static void n_tty_set_room(struct tty_struct *tty)
+static int set_room(struct tty_struct *tty)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 	int left;
@@ -155,8 +155,13 @@ static void n_tty_set_room(struct tty_struct *tty)
 
 	raw_spin_unlock_irqrestore(&ldata->read_lock, flags);
 
+	return left && !old_left;
+}
+
+static void n_tty_set_room(struct tty_struct *tty)
+{
 	/* Did this open up the receive buffer? We may need to flip */
-	if (left && !old_left) {
+	if (set_room(tty)) {
 		WARN_RATELIMIT(tty->port->itty == NULL,
 				"scheduling with invalid itty\n");
 		/* see if ldisc has been killed - if so, this means that
@@ -1475,7 +1480,7 @@ static void n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 			tty->ops->flush_chars(tty);
 	}
 
-	n_tty_set_room(tty);
+	set_room(tty);
 
 	if ((!ldata->icanon && (ldata->read_cnt >= ldata->minimum_to_wake)) ||
 		L_EXTPROC(tty)) {
-- 
1.8.1.2

Re: [PATCH 4/7] n_tty: Encapsulate minimum_to_wake within N_TTY

[Greg Kroah-Hartman]

On Wed, Mar 06, 2013 at 08:38:22AM -0500, Peter Hurley wrote:
> minimum_to_wake is unique to N_TTY processing, and belongs in
> per-ldisc data.
> 
> Add the ldisc method, ldisc_ops::fasync(), to notify line disciplines
> when signal-driven I/O is enabled or disabled. When enabled for N_TTY
> (by fcntl(F_SETFL, O_ASYNC)), blocking reader/polls will be woken
> for any readable input. When disabled, blocking reader/polls are not
> woken until the read buffer is full.
> 
> Canonical mode (L_ICANON(tty), n_tty_data::icanon) is not affected by
> the minimum_to_wake setting.
> 
> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>

For some reason, this patch doesn't apply.  Care to refresh this one,
and the rest in this series, and resend?

thanks,

greg k-h

Re: [PATCH 4/7] n_tty: Encapsulate minimum_to_wake within N_TTY

[Peter Hurley]

On Mon, 2013-03-18 at 16:15 -0700, Greg Kroah-Hartman wrote:
> On Wed, Mar 06, 2013 at 08:38:22AM -0500, Peter Hurley wrote:
> > minimum_to_wake is unique to N_TTY processing, and belongs in
> > per-ldisc data.
> > 
> > Add the ldisc method, ldisc_ops::fasync(), to notify line disciplines
> > when signal-driven I/O is enabled or disabled. When enabled for N_TTY
> > (by fcntl(F_SETFL, O_ASYNC)), blocking reader/polls will be woken
> > for any readable input. When disabled, blocking reader/polls are not
> > woken until the read buffer is full.
> > 
> > Canonical mode (L_ICANON(tty), n_tty_data::icanon) is not affected by
> > the minimum_to_wake setting.
> > 
> > Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
> 
> For some reason, this patch doesn't apply.  Care to refresh this one,
> and the rest in this series, and resend?

Sorry. There was probably some accidental dependency on one of the other
patchsets of mine you did apply.

This and patch 5 now apply without error to tty-next.

Re: [PATCH 4/7] n_tty: Encapsulate minimum_to_wake within N_TTY

[Greg Kroah-Hartman]

On Tue, Mar 19, 2013 at 09:57:09AM -0400, Peter Hurley wrote:
> On Mon, 2013-03-18 at 16:15 -0700, Greg Kroah-Hartman wrote:
> > On Wed, Mar 06, 2013 at 08:38:22AM -0500, Peter Hurley wrote:
> > > minimum_to_wake is unique to N_TTY processing, and belongs in
> > > per-ldisc data.
> > > 
> > > Add the ldisc method, ldisc_ops::fasync(), to notify line disciplines
> > > when signal-driven I/O is enabled or disabled. When enabled for N_TTY
> > > (by fcntl(F_SETFL, O_ASYNC)), blocking reader/polls will be woken
> > > for any readable input. When disabled, blocking reader/polls are not
> > > woken until the read buffer is full.
> > > 
> > > Canonical mode (L_ICANON(tty), n_tty_data::icanon) is not affected by
> > > the minimum_to_wake setting.
> > > 
> > > Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
> > 
> > For some reason, this patch doesn't apply.  Care to refresh this one,
> > and the rest in this series, and resend?
> 
> Sorry. There was probably some accidental dependency on one of the other
> patchsets of mine you did apply.
> 
> This and patch 5 now apply without error to tty-next.

Ok, but they are now long gone from my queue.  Can you please resend
what I haven't applied?

thanks,

greg k-h

[PATCH 4/7] n_tty: Encapsulate minimum_to_wake within N_TTY

[Peter Hurley]

minimum_to_wake is unique to N_TTY processing, and belongs in
per-ldisc data.

Add the ldisc method, ldisc_ops::fasync(), to notify line disciplines
when signal-driven I/O is enabled or disabled. When enabled for N_TTY
(by fcntl(F_SETFL, O_ASYNC)), blocking reader/polls will be woken
for any readable input. When disabled, blocking reader/polls are not
woken until the read buffer is full.

Canonical mode (L_ICANON(tty), n_tty_data::icanon) is not affected by
the minimum_to_wake setting.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/n_tty.c       | 39 +++++++++++++++++++++++++++------------
 drivers/tty/tty_io.c      | 17 +++++++++--------
 include/linux/tty.h       |  1 -
 include/linux/tty_ldisc.h |  6 ++++++
 4 files changed, 42 insertions(+), 21 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 61797c4..3fd657b 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -89,6 +89,7 @@ struct n_tty_data {
 	int read_head;
 	int read_tail;
 	int read_cnt;
+	int minimum_to_wake;
 
 	unsigned char *echo_buf;
 	unsigned int echo_pos;
@@ -1471,7 +1472,7 @@ static void n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 
 	n_tty_set_room(tty);
 
-	if ((!ldata->icanon && (ldata->read_cnt >= tty->minimum_to_wake)) ||
+	if ((!ldata->icanon && (ldata->read_cnt >= ldata->minimum_to_wake)) ||
 		L_EXTPROC(tty)) {
 		kill_fasync(&tty->fasync, SIGIO, POLL_IN);
 		if (waitqueue_active(&tty->read_wait))
@@ -1649,7 +1650,7 @@ static int n_tty_open(struct tty_struct *tty)
 	tty->disc_data = ldata;
 	reset_buffer_flags(tty->disc_data);
 	ldata->column = 0;
-	tty->minimum_to_wake = 1;
+	ldata->minimum_to_wake = 1;
 	tty->closing = 0;
 	/* indicate buffer work may resume */
 	clear_bit(TTY_LDISC_HALTED, &tty->flags);
@@ -1817,17 +1818,17 @@ do_it_again:
 		minimum = MIN_CHAR(tty);
 		if (minimum) {
 			if (time)
-				tty->minimum_to_wake = 1;
+				ldata->minimum_to_wake = 1;
 			else if (!waitqueue_active(&tty->read_wait) ||
-				 (tty->minimum_to_wake > minimum))
-				tty->minimum_to_wake = minimum;
+				 (ldata->minimum_to_wake > minimum))
+				ldata->minimum_to_wake = minimum;
 		} else {
 			timeout = 0;
 			if (time) {
 				timeout = time;
 				time = 0;
 			}
-			tty->minimum_to_wake = minimum = 1;
+			ldata->minimum_to_wake = minimum = 1;
 		}
 	}
 
@@ -1867,9 +1868,9 @@ do_it_again:
 		   TASK_RUNNING. */
 		set_current_state(TASK_INTERRUPTIBLE);
 
-		if (((minimum - (b - buf)) < tty->minimum_to_wake) &&
+		if (((minimum - (b - buf)) < ldata->minimum_to_wake) &&
 		    ((minimum - (b - buf)) >= 1))
-			tty->minimum_to_wake = (minimum - (b - buf));
+			ldata->minimum_to_wake = (minimum - (b - buf));
 
 		if (!input_available_p(tty, 0)) {
 			if (test_bit(TTY_OTHER_CLOSED, &tty->flags)) {
@@ -1979,7 +1980,7 @@ do_it_again:
 	remove_wait_queue(&tty->read_wait, &wait);
 
 	if (!waitqueue_active(&tty->read_wait))
-		tty->minimum_to_wake = minimum;
+		ldata->minimum_to_wake = minimum;
 
 	__set_current_state(TASK_RUNNING);
 	size = b - buf;
@@ -2111,6 +2112,7 @@ break_out:
 static unsigned int n_tty_poll(struct tty_struct *tty, struct file *file,
 							poll_table *wait)
 {
+	struct n_tty_data *ldata = tty->disc_data;
 	unsigned int mask = 0;
 
 	poll_wait(file, &tty->read_wait, wait);
@@ -2125,9 +2127,9 @@ static unsigned int n_tty_poll(struct tty_struct *tty, struct file *file,
 		mask |= POLLHUP;
 	if (!(mask & (POLLHUP | POLLIN | POLLRDNORM))) {
 		if (MIN_CHAR(tty) && !TIME_CHAR(tty))
-			tty->minimum_to_wake = MIN_CHAR(tty);
+			ldata->minimum_to_wake = MIN_CHAR(tty);
 		else
-			tty->minimum_to_wake = 1;
+			ldata->minimum_to_wake = 1;
 	}
 	if (tty->ops->write && !tty_is_writelocked(tty) &&
 			tty_chars_in_buffer(tty) < WAKEUP_CHARS &&
@@ -2175,6 +2177,18 @@ static int n_tty_ioctl(struct tty_struct *tty, struct file *file,
 	}
 }
 
+static void n_tty_fasync(struct tty_struct *tty, int on)
+{
+	struct n_tty_data *ldata = tty->disc_data;
+
+	if (!waitqueue_active(&tty->read_wait)) {
+		if (on)
+			ldata->minimum_to_wake = 1;
+		else if (!tty->fasync)
+			ldata->minimum_to_wake = N_TTY_BUF_SIZE;
+	}
+}
+
 struct tty_ldisc_ops tty_ldisc_N_TTY = {
 	.magic           = TTY_LDISC_MAGIC,
 	.name            = "n_tty",
@@ -2188,7 +2202,8 @@ struct tty_ldisc_ops tty_ldisc_N_TTY = {
 	.set_termios     = n_tty_set_termios,
 	.poll            = n_tty_poll,
 	.receive_buf     = n_tty_receive_buf,
-	.write_wakeup    = n_tty_write_wakeup
+	.write_wakeup    = n_tty_write_wakeup,
+	.fasync		 = n_tty_fasync,
 };
 
 /**
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index adc1d01..525c915 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2077,6 +2077,7 @@ static unsigned int tty_poll(struct file *filp, poll_table *wait)
 static int __tty_fasync(int fd, struct file *filp, int on)
 {
 	struct tty_struct *tty = file_tty(filp);
+	struct tty_ldisc *ldisc;
 	unsigned long flags;
 	int retval = 0;
 
@@ -2087,11 +2088,17 @@ static int __tty_fasync(int fd, struct file *filp, int on)
 	if (retval <= 0)
 		goto out;
 
+	ldisc = tty_ldisc_ref(tty);
+	if (ldisc) {
+		if (ldisc->ops->fasync)
+			ldisc->ops->fasync(tty, on);
+		tty_ldisc_deref(ldisc);
+	}
+
 	if (on) {
 		enum pid_type type;
 		struct pid *pid;
-		if (!waitqueue_active(&tty->read_wait))
-			tty->minimum_to_wake = 1;
+
 		spin_lock_irqsave(&tty->ctrl_lock, flags);
 		if (tty->pgrp) {
 			pid = tty->pgrp;
@@ -2104,13 +2111,7 @@ static int __tty_fasync(int fd, struct file *filp, int on)
 		spin_unlock_irqrestore(&tty->ctrl_lock, flags);
 		retval = __f_setown(filp, pid, type, 0);
 		put_pid(pid);
-		if (retval)
-			goto out;
-	} else {
-		if (!tty->fasync && !waitqueue_active(&tty->read_wait))
-			tty->minimum_to_wake = N_TTY_BUF_SIZE;
 	}
-	retval = 0;
 out:
 	return retval;
 }
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 2c109a3..39f0317 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -272,7 +272,6 @@ struct tty_struct {
 #define N_TTY_BUF_SIZE 4096
 
 	unsigned char closing:1;
-	unsigned short minimum_to_wake;
 	unsigned char *write_buf;
 	int write_cnt;
 	/* If the tty has a pending do_SAK, queue it here - akpm */
diff --git a/include/linux/tty_ldisc.h b/include/linux/tty_ldisc.h
index 1ef449e..44584d9 100644
--- a/include/linux/tty_ldisc.h
+++ b/include/linux/tty_ldisc.h
@@ -100,6 +100,11 @@
  *	seek to perform this action quickly but should wait until
  *	any pending driver I/O is completed.
  *
+ * void (*fasync)(struct tty_struct *, int on)
+ *
+ *	Notify line discipline when signal-driven I/O is enabled or
+ *	disabled.
+ *
  * void (*dcd_change)(struct tty_struct *tty, unsigned int status)
  *
  *	Tells the discipline that the DCD pin has changed its status.
@@ -190,6 +195,7 @@ struct tty_ldisc_ops {
 			       char *fp, int count);
 	void	(*write_wakeup)(struct tty_struct *);
 	void	(*dcd_change)(struct tty_struct *, unsigned int);
+	void	(*fasync)(struct tty_struct *tty, int on);
 
 	struct  module *owner;
 
-- 
1.8.1.2

[PATCH 5/7] n_tty: Untangle read completion variables

[Peter Hurley]

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/n_tty.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 3fd657b..b2f621f 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1814,20 +1814,16 @@ do_it_again:
 	minimum = time = 0;
 	timeout = MAX_SCHEDULE_TIMEOUT;
 	if (!ldata->icanon) {
-		time = (HZ / 10) * TIME_CHAR(tty);
 		minimum = MIN_CHAR(tty);
 		if (minimum) {
+			time = (HZ / 10) * TIME_CHAR(tty);
 			if (time)
 				ldata->minimum_to_wake = 1;
 			else if (!waitqueue_active(&tty->read_wait) ||
 				 (ldata->minimum_to_wake > minimum))
 				ldata->minimum_to_wake = minimum;
 		} else {
-			timeout = 0;
-			if (time) {
-				timeout = time;
-				time = 0;
-			}
+			timeout = (HZ / 10) * TIME_CHAR(tty);
 			ldata->minimum_to_wake = minimum = 1;
 		}
 	}
-- 
1.8.1.2

[PATCH 6/7] n_tty: Fix unsafe update of available buffer space

[Peter Hurley]

receive_room is used to control the amount of data the flip
buffer work can push to the read buffer. This update is unsafe:

  CPU 0                        |  CPU 1
                               |
                               | n_tty_read()
                               |   n_tty_set_room()
                               |     left = <calc of space>
n_tty_receive_buf()            |
  <push data to buffer>        |
  n_tty_set_room()             |
    left = <calc of space>     |
    tty->receive_room = left   |
                               |     tty->receive_room = left

receive_room is now updated with a stale calculation of the
available buffer space, and the subsequent work loop will likely
overwrite unread data in the input buffer.

Update receive_room atomically with the calculation of the
available buffer space.

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

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index b2f621f..61a55f4 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -115,13 +115,14 @@ static inline int tty_put_user(struct tty_struct *tty, unsigned char x,
 }
 
 /**
- *	n_tty_set__room	-	receive space
+ *	n_tty_set_room	-	receive space
  *	@tty: terminal
  *
- *	Called by the driver to find out how much data it is
- *	permitted to feed to the line discipline without any being lost
- *	and thus to manage flow control. Not serialized. Answers for the
- *	"instant".
+ *	Sets tty->receive_room to reflect the currently available space
+ *	in the input buffer, and re-schedules the flip buffer work if space
+ *	just became available.
+ *
+ *	Locks: Concurrent update is protected with read_lock
  */
 
 static void n_tty_set_room(struct tty_struct *tty)
@@ -129,8 +130,10 @@ static void n_tty_set_room(struct tty_struct *tty)
 	struct n_tty_data *ldata = tty->disc_data;
 	int left;
 	int old_left;
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&ldata->read_lock, flags);
 
-	/* ldata->read_cnt is not read locked ? */
 	if (I_PARMRK(tty)) {
 		/* Multiply read_cnt by 3, since each byte might take up to
 		 * three times as many spaces when PARMRK is set (depending on
@@ -150,6 +153,8 @@ static void n_tty_set_room(struct tty_struct *tty)
 	old_left = tty->receive_room;
 	tty->receive_room = left;
 
+	raw_spin_unlock_irqrestore(&ldata->read_lock, flags);
+
 	/* Did this open up the receive buffer? We may need to flip */
 	if (left && !old_left) {
 		WARN_RATELIMIT(tty->port->itty == NULL,
@@ -1885,7 +1890,6 @@ do_it_again:
 				retval = -ERESTARTSYS;
 				break;
 			}
-			/* FIXME: does n_tty_set_room need locking ? */
 			n_tty_set_room(tty);
 			timeout = schedule_timeout(timeout);
 			continue;
-- 
1.8.1.2

[PATCH 7/7] n_tty: Buffer work should not reschedule itself

[Peter Hurley]

Although the driver-side input path must update the available
buffer space, it should not reschedule itself. If space is still
available and the flip buffers are not empty, flush_to_ldisc()
will loop again.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/n_tty.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 61a55f4..6f9694d 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -118,14 +118,14 @@ static inline int tty_put_user(struct tty_struct *tty, unsigned char x,
  *	n_tty_set_room	-	receive space
  *	@tty: terminal
  *
- *	Sets tty->receive_room to reflect the currently available space
+ *	Updates tty->receive_room to reflect the currently available space
  *	in the input buffer, and re-schedules the flip buffer work if space
  *	just became available.
  *
  *	Locks: Concurrent update is protected with read_lock
  */
 
-static void n_tty_set_room(struct tty_struct *tty)
+static int set_room(struct tty_struct *tty)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 	int left;
@@ -155,8 +155,13 @@ static void n_tty_set_room(struct tty_struct *tty)
 
 	raw_spin_unlock_irqrestore(&ldata->read_lock, flags);
 
+	return left && !old_left;
+}
+
+static void n_tty_set_room(struct tty_struct *tty)
+{
 	/* Did this open up the receive buffer? We may need to flip */
-	if (left && !old_left) {
+	if (set_room(tty)) {
 		WARN_RATELIMIT(tty->port->itty == NULL,
 				"scheduling with invalid itty\n");
 		/* see if ldisc has been killed - if so, this means that
@@ -1475,7 +1480,7 @@ static void n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 			tty->ops->flush_chars(tty);
 	}
 
-	n_tty_set_room(tty);
+	set_room(tty);
 
 	if ((!ldata->icanon && (ldata->read_cnt >= ldata->minimum_to_wake)) ||
 		L_EXTPROC(tty)) {
-- 
1.8.1.2

Re: [PATCH 6/7] n_tty: Fix unsafe update of available buffer space

[Ilya Zykov]

On 19.03.2013 18:26, Peter Hurley wrote:
> receive_room is used to control the amount of data the flip
> buffer work can push to the read buffer. This update is unsafe:
> 
>   CPU 0                        |  CPU 1
>                                |
>                                | n_tty_read()
>                                |   n_tty_set_room()
>                                |     left = <calc of space>
> n_tty_receive_buf()            |
>   <push data to buffer>        |
>   n_tty_set_room()             |
>     left = <calc of space>     |
>     tty->receive_room = left   |
>                                |     tty->receive_room = left
> 
> receive_room is now updated with a stale calculation of the
> available buffer space, and the subsequent work loop will likely
> overwrite unread data in the input buffer.
> 

Sounds reasonable to me.
Thank you.
Ilya Zykov <linux@izyk.ru>

Re: [PATCH 7/7] n_tty: Buffer work should not reschedule itself

[Ilya Zykov]

On 19.03.2013 18:26, Peter Hurley wrote:
> Although the driver-side input path must update the available
> buffer space, it should not reschedule itself. If space is still
> available and the flip buffers are not empty, flush_to_ldisc()
> will loop again.
> 

Sounds reasonable to me.
Thank you.
Ilya Zykov <linux@izyk.ru>

[PATCH 0/4] [resend] n_tty fixes

[Peter Hurley]

On 03/19/2013 10:10 AM, Greg Kroah-Hartman wrote:> On Tue, Mar 19, 2013 at 09:57:09AM -0400, Peter Hurley wrote:
>> On Mon, 2013-03-18 at 16:15 -0700, Greg Kroah-Hartman wrote:
>>> On Wed, Mar 06, 2013 at 08:38:22AM -0500, Peter Hurley wrote:
>>>> minimum_to_wake is unique to N_TTY processing, and belongs in
>>>> per-ldisc data.
>>>>
>>>> Add the ldisc method, ldisc_ops::fasync(), to notify line disciplines
>>>> when signal-driven I/O is enabled or disabled. When enabled for N_TTY
>>>> (by fcntl(F_SETFL, O_ASYNC)), blocking reader/polls will be woken
>>>> for any readable input. When disabled, blocking reader/polls are not
>>>> woken until the read buffer is full.
>>>>
>>>> Canonical mode (L_ICANON(tty), n_tty_data::icanon) is not affected by
>>>> the minimum_to_wake setting.
>>>>
>>>> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
>>>
>>> For some reason, this patch doesn't apply.  Care to refresh this one,
>>> and the rest in this series, and resend?
>>
>> Sorry. There was probably some accidental dependency on one of the other
>> patchsets of mine you did apply.
>>
>> This and patch 5 now apply without error to tty-next.
> 
> Ok, but they are now long gone from my queue.  Can you please resend
> what I haven't applied?

Greg,

I resent these back on 19 Mar but they never got applied. (maybe because
I resent them as 4,5,6 & 7/7 ??)

Anyway, these apply cleanly to tty-next.

Regards,
Peter Hurley


Peter Hurley (4):
  n_tty: Encapsulate minimum_to_wake within N_TTY
  n_tty: Untangle read completion variables
  n_tty: Fix unsafe update of available buffer space
  n_tty: Buffer work should not reschedule itself

 drivers/tty/n_tty.c       | 76 ++++++++++++++++++++++++++++++-----------------
 drivers/tty/tty_io.c      | 17 ++++++-----
 include/linux/tty.h       |  1 -
 include/linux/tty_ldisc.h |  6 ++++
 4 files changed, 63 insertions(+), 37 deletions(-)

-- 
1.8.1.2

[PATCH 1/4] n_tty: Encapsulate minimum_to_wake within N_TTY

[Peter Hurley]

minimum_to_wake is unique to N_TTY processing, and belongs in
per-ldisc data.

Add the ldisc method, ldisc_ops::fasync(), to notify line disciplines
when signal-driven I/O is enabled or disabled. When enabled for N_TTY
(by fcntl(F_SETFL, O_ASYNC)), blocking reader/polls will be woken
for any readable input. When disabled, blocking reader/polls are not
woken until the read buffer is full.

Canonical mode (L_ICANON(tty), n_tty_data::icanon) is not affected by
the minimum_to_wake setting.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/n_tty.c       | 39 +++++++++++++++++++++++++++------------
 drivers/tty/tty_io.c      | 17 +++++++++--------
 include/linux/tty.h       |  1 -
 include/linux/tty_ldisc.h |  6 ++++++
 4 files changed, 42 insertions(+), 21 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index cdcdb0e..f1806de 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -89,6 +89,7 @@ struct n_tty_data {
 	int read_head;
 	int read_tail;
 	int read_cnt;
+	int minimum_to_wake;
 
 	unsigned char *echo_buf;
 	unsigned int echo_pos;
@@ -1455,7 +1456,7 @@ static void n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 
 	n_tty_set_room(tty);
 
-	if ((!ldata->icanon && (ldata->read_cnt >= tty->minimum_to_wake)) ||
+	if ((!ldata->icanon && (ldata->read_cnt >= ldata->minimum_to_wake)) ||
 		L_EXTPROC(tty)) {
 		kill_fasync(&tty->fasync, SIGIO, POLL_IN);
 		if (waitqueue_active(&tty->read_wait))
@@ -1636,7 +1637,7 @@ static int n_tty_open(struct tty_struct *tty)
 	tty->disc_data = ldata;
 	reset_buffer_flags(tty->disc_data);
 	ldata->column = 0;
-	tty->minimum_to_wake = 1;
+	ldata->minimum_to_wake = 1;
 	tty->closing = 0;
 	/* indicate buffer work may resume */
 	clear_bit(TTY_LDISC_HALTED, &tty->flags);
@@ -1804,17 +1805,17 @@ do_it_again:
 		minimum = MIN_CHAR(tty);
 		if (minimum) {
 			if (time)
-				tty->minimum_to_wake = 1;
+				ldata->minimum_to_wake = 1;
 			else if (!waitqueue_active(&tty->read_wait) ||
-				 (tty->minimum_to_wake > minimum))
-				tty->minimum_to_wake = minimum;
+				 (ldata->minimum_to_wake > minimum))
+				ldata->minimum_to_wake = minimum;
 		} else {
 			timeout = 0;
 			if (time) {
 				timeout = time;
 				time = 0;
 			}
-			tty->minimum_to_wake = minimum = 1;
+			ldata->minimum_to_wake = minimum = 1;
 		}
 	}
 
@@ -1854,9 +1855,9 @@ do_it_again:
 		   TASK_RUNNING. */
 		set_current_state(TASK_INTERRUPTIBLE);
 
-		if (((minimum - (b - buf)) < tty->minimum_to_wake) &&
+		if (((minimum - (b - buf)) < ldata->minimum_to_wake) &&
 		    ((minimum - (b - buf)) >= 1))
-			tty->minimum_to_wake = (minimum - (b - buf));
+			ldata->minimum_to_wake = (minimum - (b - buf));
 
 		if (!input_available_p(tty, 0)) {
 			if (test_bit(TTY_OTHER_CLOSED, &tty->flags)) {
@@ -1973,7 +1974,7 @@ do_it_again:
 	remove_wait_queue(&tty->read_wait, &wait);
 
 	if (!waitqueue_active(&tty->read_wait))
-		tty->minimum_to_wake = minimum;
+		ldata->minimum_to_wake = minimum;
 
 	__set_current_state(TASK_RUNNING);
 	size = b - buf;
@@ -2105,6 +2106,7 @@ break_out:
 static unsigned int n_tty_poll(struct tty_struct *tty, struct file *file,
 							poll_table *wait)
 {
+	struct n_tty_data *ldata = tty->disc_data;
 	unsigned int mask = 0;
 
 	poll_wait(file, &tty->read_wait, wait);
@@ -2119,9 +2121,9 @@ static unsigned int n_tty_poll(struct tty_struct *tty, struct file *file,
 		mask |= POLLHUP;
 	if (!(mask & (POLLHUP | POLLIN | POLLRDNORM))) {
 		if (MIN_CHAR(tty) && !TIME_CHAR(tty))
-			tty->minimum_to_wake = MIN_CHAR(tty);
+			ldata->minimum_to_wake = MIN_CHAR(tty);
 		else
-			tty->minimum_to_wake = 1;
+			ldata->minimum_to_wake = 1;
 	}
 	if (tty->ops->write && !tty_is_writelocked(tty) &&
 			tty_chars_in_buffer(tty) < WAKEUP_CHARS &&
@@ -2169,6 +2171,18 @@ static int n_tty_ioctl(struct tty_struct *tty, struct file *file,
 	}
 }
 
+static void n_tty_fasync(struct tty_struct *tty, int on)
+{
+	struct n_tty_data *ldata = tty->disc_data;
+
+	if (!waitqueue_active(&tty->read_wait)) {
+		if (on)
+			ldata->minimum_to_wake = 1;
+		else if (!tty->fasync)
+			ldata->minimum_to_wake = N_TTY_BUF_SIZE;
+	}
+}
+
 struct tty_ldisc_ops tty_ldisc_N_TTY = {
 	.magic           = TTY_LDISC_MAGIC,
 	.name            = "n_tty",
@@ -2182,7 +2196,8 @@ struct tty_ldisc_ops tty_ldisc_N_TTY = {
 	.set_termios     = n_tty_set_termios,
 	.poll            = n_tty_poll,
 	.receive_buf     = n_tty_receive_buf,
-	.write_wakeup    = n_tty_write_wakeup
+	.write_wakeup    = n_tty_write_wakeup,
+	.fasync		 = n_tty_fasync,
 };
 
 /**
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 5ee51ba..6cd08d9 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2137,6 +2137,7 @@ static unsigned int tty_poll(struct file *filp, poll_table *wait)
 static int __tty_fasync(int fd, struct file *filp, int on)
 {
 	struct tty_struct *tty = file_tty(filp);
+	struct tty_ldisc *ldisc;
 	unsigned long flags;
 	int retval = 0;
 
@@ -2147,11 +2148,17 @@ static int __tty_fasync(int fd, struct file *filp, int on)
 	if (retval <= 0)
 		goto out;
 
+	ldisc = tty_ldisc_ref(tty);
+	if (ldisc) {
+		if (ldisc->ops->fasync)
+			ldisc->ops->fasync(tty, on);
+		tty_ldisc_deref(ldisc);
+	}
+
 	if (on) {
 		enum pid_type type;
 		struct pid *pid;
-		if (!waitqueue_active(&tty->read_wait))
-			tty->minimum_to_wake = 1;
+
 		spin_lock_irqsave(&tty->ctrl_lock, flags);
 		if (tty->pgrp) {
 			pid = tty->pgrp;
@@ -2164,13 +2171,7 @@ static int __tty_fasync(int fd, struct file *filp, int on)
 		spin_unlock_irqrestore(&tty->ctrl_lock, flags);
 		retval = __f_setown(filp, pid, type, 0);
 		put_pid(pid);
-		if (retval)
-			goto out;
-	} else {
-		if (!tty->fasync && !waitqueue_active(&tty->read_wait))
-			tty->minimum_to_wake = N_TTY_BUF_SIZE;
 	}
-	retval = 0;
 out:
 	return retval;
 }
diff --git a/include/linux/tty.h b/include/linux/tty.h
index a665c7e..7269daf 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -272,7 +272,6 @@ struct tty_struct {
 #define N_TTY_BUF_SIZE 4096
 
 	unsigned char closing:1;
-	unsigned short minimum_to_wake;
 	unsigned char *write_buf;
 	int write_cnt;
 	/* If the tty has a pending do_SAK, queue it here - akpm */
diff --git a/include/linux/tty_ldisc.h b/include/linux/tty_ldisc.h
index 22ee107..23bdd9d 100644
--- a/include/linux/tty_ldisc.h
+++ b/include/linux/tty_ldisc.h
@@ -100,6 +100,11 @@
  *	seek to perform this action quickly but should wait until
  *	any pending driver I/O is completed.
  *
+ * void (*fasync)(struct tty_struct *, int on)
+ *
+ *	Notify line discipline when signal-driven I/O is enabled or
+ *	disabled.
+ *
  * void (*dcd_change)(struct tty_struct *tty, unsigned int status)
  *
  *	Tells the discipline that the DCD pin has changed its status.
@@ -189,6 +194,7 @@ struct tty_ldisc_ops {
 			       char *fp, int count);
 	void	(*write_wakeup)(struct tty_struct *);
 	void	(*dcd_change)(struct tty_struct *, unsigned int);
+	void	(*fasync)(struct tty_struct *tty, int on);
 
 	struct  module *owner;
 
-- 
1.8.1.2

[PATCH 2/4] n_tty: Untangle read completion variables

[Peter Hurley]

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/n_tty.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index f1806de..fa5cb46 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1801,20 +1801,16 @@ do_it_again:
 	minimum = time = 0;
 	timeout = MAX_SCHEDULE_TIMEOUT;
 	if (!ldata->icanon) {
-		time = (HZ / 10) * TIME_CHAR(tty);
 		minimum = MIN_CHAR(tty);
 		if (minimum) {
+			time = (HZ / 10) * TIME_CHAR(tty);
 			if (time)
 				ldata->minimum_to_wake = 1;
 			else if (!waitqueue_active(&tty->read_wait) ||
 				 (ldata->minimum_to_wake > minimum))
 				ldata->minimum_to_wake = minimum;
 		} else {
-			timeout = 0;
-			if (time) {
-				timeout = time;
-				time = 0;
-			}
+			timeout = (HZ / 10) * TIME_CHAR(tty);
 			ldata->minimum_to_wake = minimum = 1;
 		}
 	}
-- 
1.8.1.2

[PATCH 3/4] n_tty: Fix unsafe update of available buffer space

[Peter Hurley]

receive_room is used to control the amount of data the flip
buffer work can push to the read buffer. This update is unsafe:

  CPU 0                        |  CPU 1
                               |
                               | n_tty_read()
                               |   n_tty_set_room()
                               |     left = <calc of space>
n_tty_receive_buf()            |
  <push data to buffer>        |
  n_tty_set_room()             |
    left = <calc of space>     |
    tty->receive_room = left   |
                               |     tty->receive_room = left

receive_room is now updated with a stale calculation of the
available buffer space, and the subsequent work loop will likely
overwrite unread data in the input buffer.

Update receive_room atomically with the calculation of the
available buffer space.

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

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index fa5cb46..0646165 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -115,13 +115,14 @@ static inline int tty_put_user(struct tty_struct *tty, unsigned char x,
 }
 
 /**
- *	n_tty_set__room	-	receive space
+ *	n_tty_set_room	-	receive space
  *	@tty: terminal
  *
- *	Called by the driver to find out how much data it is
- *	permitted to feed to the line discipline without any being lost
- *	and thus to manage flow control. Not serialized. Answers for the
- *	"instant".
+ *	Sets tty->receive_room to reflect the currently available space
+ *	in the input buffer, and re-schedules the flip buffer work if space
+ *	just became available.
+ *
+ *	Locks: Concurrent update is protected with read_lock
  */
 
 static void n_tty_set_room(struct tty_struct *tty)
@@ -129,8 +130,10 @@ static void n_tty_set_room(struct tty_struct *tty)
 	struct n_tty_data *ldata = tty->disc_data;
 	int left;
 	int old_left;
+	unsigned long flags;
+
+	raw_spin_lock_irqsave(&ldata->read_lock, flags);
 
-	/* ldata->read_cnt is not read locked ? */
 	if (I_PARMRK(tty)) {
 		/* Multiply read_cnt by 3, since each byte might take up to
 		 * three times as many spaces when PARMRK is set (depending on
@@ -150,6 +153,8 @@ static void n_tty_set_room(struct tty_struct *tty)
 	old_left = tty->receive_room;
 	tty->receive_room = left;
 
+	raw_spin_unlock_irqrestore(&ldata->read_lock, flags);
+
 	/* Did this open up the receive buffer? We may need to flip */
 	if (left && !old_left) {
 		WARN_RATELIMIT(tty->port->itty == NULL,
@@ -1872,7 +1877,6 @@ do_it_again:
 				retval = -ERESTARTSYS;
 				break;
 			}
-			/* FIXME: does n_tty_set_room need locking ? */
 			n_tty_set_room(tty);
 			timeout = schedule_timeout(timeout);
 			continue;
-- 
1.8.1.2

[PATCH 4/4] n_tty: Buffer work should not reschedule itself

[Peter Hurley]

Although the driver-side input path must update the available
buffer space, it should not reschedule itself. If space is still
available and the flip buffers are not empty, flush_to_ldisc()
will loop again.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/n_tty.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 0646165..4bf0fc0 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -118,14 +118,14 @@ static inline int tty_put_user(struct tty_struct *tty, unsigned char x,
  *	n_tty_set_room	-	receive space
  *	@tty: terminal
  *
- *	Sets tty->receive_room to reflect the currently available space
+ *	Updates tty->receive_room to reflect the currently available space
  *	in the input buffer, and re-schedules the flip buffer work if space
  *	just became available.
  *
  *	Locks: Concurrent update is protected with read_lock
  */
 
-static void n_tty_set_room(struct tty_struct *tty)
+static int set_room(struct tty_struct *tty)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 	int left;
@@ -155,8 +155,13 @@ static void n_tty_set_room(struct tty_struct *tty)
 
 	raw_spin_unlock_irqrestore(&ldata->read_lock, flags);
 
+	return left && !old_left;
+}
+
+static void n_tty_set_room(struct tty_struct *tty)
+{
 	/* Did this open up the receive buffer? We may need to flip */
-	if (left && !old_left) {
+	if (set_room(tty)) {
 		WARN_RATELIMIT(tty->port->itty == NULL,
 				"scheduling with invalid itty\n");
 		/* see if ldisc has been killed - if so, this means that
@@ -1459,7 +1464,7 @@ static void n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 			tty->ops->flush_chars(tty);
 	}
 
-	n_tty_set_room(tty);
+	set_room(tty);
 
 	if ((!ldata->icanon && (ldata->read_cnt >= ldata->minimum_to_wake)) ||
 		L_EXTPROC(tty)) {
-- 
1.8.1.2

Re: [PATCH 0/4] [resend] n_tty fixes

[Greg Kroah-Hartman]

On Sat, Jun 15, 2013 at 07:28:27AM -0400, Peter Hurley wrote:
> On 03/19/2013 10:10 AM, Greg Kroah-Hartman wrote:> On Tue, Mar 19, 2013 at 09:57:09AM -0400, Peter Hurley wrote:
> >> On Mon, 2013-03-18 at 16:15 -0700, Greg Kroah-Hartman wrote:
> >>> On Wed, Mar 06, 2013 at 08:38:22AM -0500, Peter Hurley wrote:
> >>>> minimum_to_wake is unique to N_TTY processing, and belongs in
> >>>> per-ldisc data.
> >>>>
> >>>> Add the ldisc method, ldisc_ops::fasync(), to notify line disciplines
> >>>> when signal-driven I/O is enabled or disabled. When enabled for N_TTY
> >>>> (by fcntl(F_SETFL, O_ASYNC)), blocking reader/polls will be woken
> >>>> for any readable input. When disabled, blocking reader/polls are not
> >>>> woken until the read buffer is full.
> >>>>
> >>>> Canonical mode (L_ICANON(tty), n_tty_data::icanon) is not affected by
> >>>> the minimum_to_wake setting.
> >>>>
> >>>> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
> >>>
> >>> For some reason, this patch doesn't apply.  Care to refresh this one,
> >>> and the rest in this series, and resend?
> >>
> >> Sorry. There was probably some accidental dependency on one of the other
> >> patchsets of mine you did apply.
> >>
> >> This and patch 5 now apply without error to tty-next.
> > 
> > Ok, but they are now long gone from my queue.  Can you please resend
> > what I haven't applied?
> 
> Greg,
> 
> I resent these back on 19 Mar but they never got applied. (maybe because
> I resent them as 4,5,6 & 7/7 ??)

Yes, they were burried, sorry about that :)

> Anyway, these apply cleanly to tty-next.

Now applied, thanks.

greg k-h