[PATCH v5 1/2] tty: Implement lookahead to process XON/XOFF timely

[PATCH v5 1/2] tty: Implement lookahead to process XON/XOFF timely

[Ilpo Järvinen]

When tty is not read from, XON/XOFF may get stuck into an
intermediate buffer. As those characters are there to do software
flow-control, it is not very useful. In the case where neither end
reads from ttys, the receiving ends might not be able receive the
XOFF characters and just keep sending more data to the opposite
direction. This problem is almost guaranteed to occur with DMA
which sends data in large chunks.

If TTY is slow to process characters, that is, eats less than given
amount in receive_buf, invoke lookahead for the rest of the chars
to process potential XON/XOFF characters.

We need to keep track of how many characters have been processed by the
lookahead to avoid processing the flow control char again on the normal
path. Bookkeeping occurs parallel on two layers (tty_buffer and n_tty)
to avoid passing the lookahead_count through the whole callchain.

When a flow-control char is processed, two things must occur:
  a) it must not be treated as normal char
  b) if not yet processed, flow-control actions need to be taken
The return value of n_tty_receive_char_flow_ctrl() tells caller a), and
b) is kept internal to n_tty_receive_char_flow_ctrl().

If characters were previous looked ahead, __receive_buf() makes two
calls to the approriate n_tty_receive_buf_* function. First call is
made with lookahead_done=true for the characters that were subject to
lookahead earlier and then with lookahead=false for the new characters.
Either of the calls might be skipped when it has no characters to
handle.

Reported-by: Gilles Buloz <gilles.buloz@kontron.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/tty/n_tty.c        | 90 +++++++++++++++++++++++++++++++-------
 drivers/tty/tty_buffer.c   | 59 +++++++++++++++++++++----
 drivers/tty/tty_port.c     | 21 +++++++++
 include/linux/tty_buffer.h |  1 +
 include/linux/tty_ldisc.h  | 13 ++++++
 include/linux/tty_port.h   |  2 +
 6 files changed, 161 insertions(+), 25 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 640c9e871044..917b5970b2e0 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -118,6 +118,9 @@ struct n_tty_data {
 	size_t read_tail;
 	size_t line_start;
 
+	/* # of chars looked ahead (to find software flow control chars) */
+	size_t lookahead_count;
+
 	/* protected by output lock */
 	unsigned int column;
 	unsigned int canon_column;
@@ -333,6 +336,8 @@ static void reset_buffer_flags(struct n_tty_data *ldata)
 	ldata->erasing = 0;
 	bitmap_zero(ldata->read_flags, N_TTY_BUF_SIZE);
 	ldata->push = 0;
+
+	ldata->lookahead_count = 0;
 }
 
 static void n_tty_packet_mode_flush(struct tty_struct *tty)
@@ -1225,12 +1230,30 @@ static bool n_tty_is_char_flow_ctrl(struct tty_struct *tty, unsigned char c)
 	return c == START_CHAR(tty) || c == STOP_CHAR(tty);
 }
 
-/* Returns true if c is consumed as flow-control character */
-static bool n_tty_receive_char_flow_ctrl(struct tty_struct *tty, unsigned char c)
+/**
+ * n_tty_receive_char_flow_ctrl - receive flow control chars
+ * @tty: terminal device
+ * @c: character
+ * @lookahead_done: lookahead has processed this character already
+ *
+ * Receive and process flow control character actions.
+ *
+ * In case lookahead for flow control chars already handled the character in
+ * advance to the normal receive, the actions are skipped during normal
+ * receive.
+ *
+ * Returns true if c is consumed as flow-control character, the character
+ * must not be treated as normal character.
+ */
+static bool n_tty_receive_char_flow_ctrl(struct tty_struct *tty, unsigned char c,
+					 bool lookahead_done)
 {
 	if (!n_tty_is_char_flow_ctrl(tty, c))
 		return false;
 
+	if (lookahead_done)
+		return true;
+
 	if (c == START_CHAR(tty)) {
 		start_tty(tty);
 		process_echoes(tty);
@@ -1242,11 +1265,12 @@ static bool n_tty_receive_char_flow_ctrl(struct tty_struct *tty, unsigned char c
 	return true;
 }
 
-static void n_tty_receive_char_special(struct tty_struct *tty, unsigned char c)
+static void n_tty_receive_char_special(struct tty_struct *tty, unsigned char c,
+				       bool lookahead_done)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 
-	if (I_IXON(tty) && n_tty_receive_char_flow_ctrl(tty, c))
+	if (I_IXON(tty) && n_tty_receive_char_flow_ctrl(tty, c, lookahead_done))
 		return;
 
 	if (L_ISIG(tty)) {
@@ -1401,7 +1425,8 @@ static void n_tty_receive_char(struct tty_struct *tty, unsigned char c)
 	put_tty_queue(c, ldata);
 }
 
-static void n_tty_receive_char_closing(struct tty_struct *tty, unsigned char c)
+static void n_tty_receive_char_closing(struct tty_struct *tty, unsigned char c,
+				       bool lookahead_done)
 {
 	if (I_ISTRIP(tty))
 		c &= 0x7f;
@@ -1409,9 +1434,12 @@ static void n_tty_receive_char_closing(struct tty_struct *tty, unsigned char c)
 		c = tolower(c);
 
 	if (I_IXON(tty)) {
-		if (c == STOP_CHAR(tty))
-			stop_tty(tty);
-		else if (c == START_CHAR(tty) ||
+		if (c == STOP_CHAR(tty)) {
+			if (!lookahead_done)
+				stop_tty(tty);
+		} else if (c == START_CHAR(tty) && lookahead_done) {
+			return;
+		} else if (c == START_CHAR(tty) ||
 			 (tty->flow.stopped && !tty->flow.tco_stopped && I_IXANY(tty) &&
 			  c != INTR_CHAR(tty) && c != QUIT_CHAR(tty) &&
 			  c != SUSP_CHAR(tty))) {
@@ -1457,6 +1485,26 @@ n_tty_receive_char_lnext(struct tty_struct *tty, unsigned char c, char flag)
 		n_tty_receive_char_flagged(tty, c, flag);
 }
 
+static void n_tty_lookahead_flow_ctrl(struct tty_struct *tty, const unsigned char *cp,
+				      const unsigned char *fp, unsigned int count)
+{
+	struct n_tty_data *ldata = tty->disc_data;
+	unsigned char flag = TTY_NORMAL;
+
+	ldata->lookahead_count += count;
+
+	if (!I_IXON(tty))
+		return;
+
+	while (count--) {
+		if (fp)
+			flag = *fp++;
+		if (likely(flag == TTY_NORMAL))
+			n_tty_receive_char_flow_ctrl(tty, *cp, false);
+		cp++;
+	}
+}
+
 static void
 n_tty_receive_buf_real_raw(struct tty_struct *tty, const unsigned char *cp,
 			   const char *fp, int count)
@@ -1496,7 +1544,7 @@ n_tty_receive_buf_raw(struct tty_struct *tty, const unsigned char *cp,
 
 static void
 n_tty_receive_buf_closing(struct tty_struct *tty, const unsigned char *cp,
-			  const char *fp, int count)
+			  const char *fp, int count, bool lookahead_done)
 {
 	char flag = TTY_NORMAL;
 
@@ -1504,12 +1552,12 @@ n_tty_receive_buf_closing(struct tty_struct *tty, const unsigned char *cp,
 		if (fp)
 			flag = *fp++;
 		if (likely(flag == TTY_NORMAL))
-			n_tty_receive_char_closing(tty, *cp++);
+			n_tty_receive_char_closing(tty, *cp++, lookahead_done);
 	}
 }
 
 static void n_tty_receive_buf_standard(struct tty_struct *tty,
-		const unsigned char *cp, const char *fp, int count)
+		const unsigned char *cp, const char *fp, int count, bool lookahead_done)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 	char flag = TTY_NORMAL;
@@ -1540,7 +1588,7 @@ static void n_tty_receive_buf_standard(struct tty_struct *tty,
 		}
 
 		if (test_bit(c, ldata->char_map))
-			n_tty_receive_char_special(tty, c);
+			n_tty_receive_char_special(tty, c, lookahead_done);
 		else
 			n_tty_receive_char(tty, c);
 	}
@@ -1551,21 +1599,30 @@ static void __receive_buf(struct tty_struct *tty, const unsigned char *cp,
 {
 	struct n_tty_data *ldata = tty->disc_data;
 	bool preops = I_ISTRIP(tty) || (I_IUCLC(tty) && L_IEXTEN(tty));
+	size_t la_count = min_t(size_t, ldata->lookahead_count, count);
 
 	if (ldata->real_raw)
 		n_tty_receive_buf_real_raw(tty, cp, fp, count);
 	else if (ldata->raw || (L_EXTPROC(tty) && !preops))
 		n_tty_receive_buf_raw(tty, cp, fp, count);
-	else if (tty->closing && !L_EXTPROC(tty))
-		n_tty_receive_buf_closing(tty, cp, fp, count);
-	else {
-		n_tty_receive_buf_standard(tty, cp, fp, count);
+	else if (tty->closing && !L_EXTPROC(tty)) {
+		if (la_count > 0)
+			n_tty_receive_buf_closing(tty, cp, fp, la_count, true);
+		if (count > la_count)
+			n_tty_receive_buf_closing(tty, cp, fp, count - la_count, false);
+	} else {
+		if (la_count > 0)
+			n_tty_receive_buf_standard(tty, cp, fp, la_count, true);
+		if (count > la_count)
+			n_tty_receive_buf_standard(tty, cp, fp, count - la_count, false);
 
 		flush_echoes(tty);
 		if (tty->ops->flush_chars)
 			tty->ops->flush_chars(tty);
 	}
 
+	ldata->lookahead_count -= la_count;
+
 	if (ldata->icanon && !L_EXTPROC(tty))
 		return;
 
@@ -2446,6 +2503,7 @@ static struct tty_ldisc_ops n_tty_ops = {
 	.receive_buf     = n_tty_receive_buf,
 	.write_wakeup    = n_tty_write_wakeup,
 	.receive_buf2	 = n_tty_receive_buf2,
+	.lookahead_buf	 = n_tty_lookahead_flow_ctrl,
 };
 
 /**
diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index bfa431a8e690..754fa43670cc 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -5,6 +5,7 @@
 
 #include <linux/types.h>
 #include <linux/errno.h>
+#include <linux/minmax.h>
 #include <linux/tty.h>
 #include <linux/tty_driver.h>
 #include <linux/tty_flip.h>
@@ -104,6 +105,7 @@ static void tty_buffer_reset(struct tty_buffer *p, size_t size)
 	p->size = size;
 	p->next = NULL;
 	p->commit = 0;
+	p->lookahead = 0;
 	p->read = 0;
 	p->flags = 0;
 }
@@ -234,6 +236,7 @@ void tty_buffer_flush(struct tty_struct *tty, struct tty_ldisc *ld)
 		buf->head = next;
 	}
 	buf->head->read = buf->head->commit;
+	buf->head->lookahead = buf->head->read;
 
 	if (ld && ld->ops->flush_buffer)
 		ld->ops->flush_buffer(tty);
@@ -276,13 +279,15 @@ static int __tty_buffer_request_room(struct tty_port *port, size_t size,
 		if (n != NULL) {
 			n->flags = flags;
 			buf->tail = n;
-			/* paired w/ acquire in flush_to_ldisc(); ensures
-			 * flush_to_ldisc() sees buffer data.
+			/*
+			 * Paired w/ acquire in flush_to_ldisc() and lookahead_bufs()
+			 * ensures they see all buffer data.
 			 */
 			smp_store_release(&b->commit, b->used);
-			/* paired w/ acquire in flush_to_ldisc(); ensures the
-			 * latest commit value can be read before the head is
-			 * advanced to the next buffer
+			/*
+			 * Paired w/ acquire in flush_to_ldisc() and lookahead_bufs()
+			 * ensures the latest commit value can be read before the head
+			 * is advanced to the next buffer.
 			 */
 			smp_store_release(&b->next, n);
 		} else if (change)
@@ -459,6 +464,40 @@ int tty_ldisc_receive_buf(struct tty_ldisc *ld, const unsigned char *p,
 }
 EXPORT_SYMBOL_GPL(tty_ldisc_receive_buf);
 
+static void lookahead_bufs(struct tty_port *port, struct tty_buffer *head)
+{
+	head->lookahead = max(head->lookahead, head->read);
+
+	while (head) {
+		struct tty_buffer *next;
+		unsigned char *p, *f = NULL;
+		unsigned int count;
+
+		/*
+		 * Paired w/ release in __tty_buffer_request_room();
+		 * ensures commit value read is not stale if the head
+		 * is advancing to the next buffer.
+		 */
+		next = smp_load_acquire(&head->next);
+		/*
+		 * Paired w/ release in __tty_buffer_request_room() or in
+		 * tty_buffer_flush(); ensures we see the committed buffer data.
+		 */
+		count = smp_load_acquire(&head->commit) - head->lookahead;
+		if (!count) {
+			head = next;
+			continue;
+		}
+
+		p = char_buf_ptr(head, head->lookahead);
+		if (~head->flags & TTYB_NORMAL)
+			f = flag_buf_ptr(head, head->lookahead);
+
+		port->client_ops->lookahead_buf(port, p, f, count);
+		head->lookahead += count;
+	}
+}
+
 static int
 receive_buf(struct tty_port *port, struct tty_buffer *head, int count)
 {
@@ -496,7 +535,7 @@ static void flush_to_ldisc(struct work_struct *work)
 	while (1) {
 		struct tty_buffer *head = buf->head;
 		struct tty_buffer *next;
-		int count;
+		int count, rcvd;
 
 		/* Ldisc or user is trying to gain exclusive access */
 		if (atomic_read(&buf->priority))
@@ -519,10 +558,12 @@ static void flush_to_ldisc(struct work_struct *work)
 			continue;
 		}
 
-		count = receive_buf(port, head, count);
-		if (!count)
+		rcvd = receive_buf(port, head, count);
+		head->read += rcvd;
+		if (rcvd < count)
+			lookahead_bufs(port, head);
+		if (!rcvd)
 			break;
-		head->read += count;
 
 		if (need_resched())
 			cond_resched();
diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
index 880608a65773..dce08a6d7b5e 100644
--- a/drivers/tty/tty_port.c
+++ b/drivers/tty/tty_port.c
@@ -43,6 +43,26 @@ static int tty_port_default_receive_buf(struct tty_port *port,
 	return ret;
 }
 
+static void tty_port_default_lookahead_buf(struct tty_port *port, const unsigned char *p,
+					   const unsigned char *f, unsigned int count)
+{
+	struct tty_struct *tty;
+	struct tty_ldisc *disc;
+
+	tty = READ_ONCE(port->itty);
+	if (!tty)
+		return;
+
+	disc = tty_ldisc_ref(tty);
+	if (!disc)
+		return;
+
+	if (disc->ops->lookahead_buf)
+		disc->ops->lookahead_buf(disc->tty, p, f, count);
+
+	tty_ldisc_deref(disc);
+}
+
 static void tty_port_default_wakeup(struct tty_port *port)
 {
 	struct tty_struct *tty = tty_port_tty_get(port);
@@ -55,6 +75,7 @@ static void tty_port_default_wakeup(struct tty_port *port)
 
 const struct tty_port_client_operations tty_port_default_client_ops = {
 	.receive_buf = tty_port_default_receive_buf,
+	.lookahead_buf = tty_port_default_lookahead_buf,
 	.write_wakeup = tty_port_default_wakeup,
 };
 EXPORT_SYMBOL_GPL(tty_port_default_client_ops);
diff --git a/include/linux/tty_buffer.h b/include/linux/tty_buffer.h
index 3b9d77604291..1796648c2907 100644
--- a/include/linux/tty_buffer.h
+++ b/include/linux/tty_buffer.h
@@ -15,6 +15,7 @@ struct tty_buffer {
 	int used;
 	int size;
 	int commit;
+	int lookahead;		/* Lazy update on recv, can become less than "read" */
 	int read;
 	int flags;
 	/* Data points here */
diff --git a/include/linux/tty_ldisc.h b/include/linux/tty_ldisc.h
index e85002b56752..bf8143fbcff3 100644
--- a/include/linux/tty_ldisc.h
+++ b/include/linux/tty_ldisc.h
@@ -186,6 +186,17 @@ int ldsem_down_write_nested(struct ld_semaphore *sem, int subclass,
  *	indicate all data received is %TTY_NORMAL. If assigned, prefer this
  *	function for automatic flow control.
  *
+ * @lookahead_buf: [DRV] ``void ()(struct tty_struct *tty,
+ *			const unsigned char *cp, const char *fp, int count)
+ *
+ *	This function is called by the low-level tty driver for characters
+ *	not eaten by receive_buf or receive_buf2. It is useful for processing
+ *	high-priority characters such as software flow-control characters that
+ *	could otherwise get stuck into the intermediate buffer until tty has
+ *	room to receive them. Ldisc must be able to handle later a receive_buf
+ *	or receive_buf2 call for the same characters (e.g. by skipping the
+ *	actions for high-priority characters already handled by lookahead_buf).
+ *
  * @owner: module containting this ldisc (for reference counting)
  *
  * This structure defines the interface between the tty line discipline
@@ -229,6 +240,8 @@ struct tty_ldisc_ops {
 	void	(*dcd_change)(struct tty_struct *tty, unsigned int status);
 	int	(*receive_buf2)(struct tty_struct *tty, const unsigned char *cp,
 				const char *fp, int count);
+	void	(*lookahead_buf)(struct tty_struct *tty, const unsigned char *cp,
+				 const unsigned char *fp, unsigned int count);
 
 	struct  module *owner;
 };
diff --git a/include/linux/tty_port.h b/include/linux/tty_port.h
index 58e9619116b7..fa3c3bdaa234 100644
--- a/include/linux/tty_port.h
+++ b/include/linux/tty_port.h
@@ -40,6 +40,8 @@ struct tty_port_operations {
 
 struct tty_port_client_operations {
 	int (*receive_buf)(struct tty_port *port, const unsigned char *, const unsigned char *, size_t);
+	void (*lookahead_buf)(struct tty_port *port, const unsigned char *cp,
+			      const unsigned char *fp, unsigned int count);
 	void (*write_wakeup)(struct tty_port *port);
 };
 
-- 
2.30.2

Re: [PATCH v5 1/2] tty: Implement lookahead to process XON/XOFF timely

[Andy Shevchenko]

On Mon, Jun 6, 2022 at 3:45 PM Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> When tty is not read from, XON/XOFF may get stuck into an
> intermediate buffer. As those characters are there to do software
> flow-control, it is not very useful. In the case where neither end
> reads from ttys, the receiving ends might not be able receive the
> XOFF characters and just keep sending more data to the opposite
> direction. This problem is almost guaranteed to occur with DMA
> which sends data in large chunks.
>
> If TTY is slow to process characters, that is, eats less than given
> amount in receive_buf, invoke lookahead for the rest of the chars
> to process potential XON/XOFF characters.
>
> We need to keep track of how many characters have been processed by the
> lookahead to avoid processing the flow control char again on the normal
> path. Bookkeeping occurs parallel on two layers (tty_buffer and n_tty)
> to avoid passing the lookahead_count through the whole callchain.

call chain

> When a flow-control char is processed, two things must occur:
>   a) it must not be treated as normal char
>   b) if not yet processed, flow-control actions need to be taken
> The return value of n_tty_receive_char_flow_ctrl() tells caller a), and
> b) is kept internal to n_tty_receive_char_flow_ctrl().
>
> If characters were previous looked ahead, __receive_buf() makes two
> calls to the approriate n_tty_receive_buf_* function. First call is

appropriate

> made with lookahead_done=true for the characters that were subject to
> lookahead earlier and then with lookahead=false for the new characters.
> Either of the calls might be skipped when it has no characters to
> handle.

Seems good to me, but I am not an expert in TTY layer, perhaps Jiri or
somebody else who is more familiar with this can look into. I can
suggest to add Johan Hovold (because he maintains USB serial) and Rob
Herring (because he introduced serdev).

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Also see a few nit-picks below.

> Reported-by: Gilles Buloz <gilles.buloz@kontron.com>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
>  drivers/tty/n_tty.c        | 90 +++++++++++++++++++++++++++++++-------
>  drivers/tty/tty_buffer.c   | 59 +++++++++++++++++++++----
>  drivers/tty/tty_port.c     | 21 +++++++++
>  include/linux/tty_buffer.h |  1 +
>  include/linux/tty_ldisc.h  | 13 ++++++
>  include/linux/tty_port.h   |  2 +
>  6 files changed, 161 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
> index 640c9e871044..917b5970b2e0 100644
> --- a/drivers/tty/n_tty.c
> +++ b/drivers/tty/n_tty.c
> @@ -118,6 +118,9 @@ struct n_tty_data {
>         size_t read_tail;
>         size_t line_start;
>
> +       /* # of chars looked ahead (to find software flow control chars) */
> +       size_t lookahead_count;
> +
>         /* protected by output lock */
>         unsigned int column;
>         unsigned int canon_column;
> @@ -333,6 +336,8 @@ static void reset_buffer_flags(struct n_tty_data *ldata)
>         ldata->erasing = 0;
>         bitmap_zero(ldata->read_flags, N_TTY_BUF_SIZE);
>         ldata->push = 0;
> +
> +       ldata->lookahead_count = 0;
>  }
>
>  static void n_tty_packet_mode_flush(struct tty_struct *tty)
> @@ -1225,12 +1230,30 @@ static bool n_tty_is_char_flow_ctrl(struct tty_struct *tty, unsigned char c)
>         return c == START_CHAR(tty) || c == STOP_CHAR(tty);
>  }
>
> -/* Returns true if c is consumed as flow-control character */
> -static bool n_tty_receive_char_flow_ctrl(struct tty_struct *tty, unsigned char c)
> +/**
> + * n_tty_receive_char_flow_ctrl - receive flow control chars
> + * @tty: terminal device
> + * @c: character
> + * @lookahead_done: lookahead has processed this character already
> + *
> + * Receive and process flow control character actions.
> + *
> + * In case lookahead for flow control chars already handled the character in
> + * advance to the normal receive, the actions are skipped during normal
> + * receive.
> + *
> + * Returns true if c is consumed as flow-control character, the character

@c

> + * must not be treated as normal character.
> + */
> +static bool n_tty_receive_char_flow_ctrl(struct tty_struct *tty, unsigned char c,
> +                                        bool lookahead_done)
>  {
>         if (!n_tty_is_char_flow_ctrl(tty, c))
>                 return false;
>
> +       if (lookahead_done)
> +               return true;
> +
>         if (c == START_CHAR(tty)) {
>                 start_tty(tty);
>                 process_echoes(tty);
> @@ -1242,11 +1265,12 @@ static bool n_tty_receive_char_flow_ctrl(struct tty_struct *tty, unsigned char c
>         return true;
>  }
>
> -static void n_tty_receive_char_special(struct tty_struct *tty, unsigned char c)
> +static void n_tty_receive_char_special(struct tty_struct *tty, unsigned char c,
> +                                      bool lookahead_done)
>  {
>         struct n_tty_data *ldata = tty->disc_data;
>
> -       if (I_IXON(tty) && n_tty_receive_char_flow_ctrl(tty, c))
> +       if (I_IXON(tty) && n_tty_receive_char_flow_ctrl(tty, c, lookahead_done))
>                 return;
>
>         if (L_ISIG(tty)) {
> @@ -1401,7 +1425,8 @@ static void n_tty_receive_char(struct tty_struct *tty, unsigned char c)
>         put_tty_queue(c, ldata);
>  }
>
> -static void n_tty_receive_char_closing(struct tty_struct *tty, unsigned char c)
> +static void n_tty_receive_char_closing(struct tty_struct *tty, unsigned char c,
> +                                      bool lookahead_done)
>  {
>         if (I_ISTRIP(tty))
>                 c &= 0x7f;
> @@ -1409,9 +1434,12 @@ static void n_tty_receive_char_closing(struct tty_struct *tty, unsigned char c)
>                 c = tolower(c);
>
>         if (I_IXON(tty)) {
> -               if (c == STOP_CHAR(tty))
> -                       stop_tty(tty);
> -               else if (c == START_CHAR(tty) ||
> +               if (c == STOP_CHAR(tty)) {
> +                       if (!lookahead_done)
> +                               stop_tty(tty);
> +               } else if (c == START_CHAR(tty) && lookahead_done) {
> +                       return;
> +               } else if (c == START_CHAR(tty) ||
>                          (tty->flow.stopped && !tty->flow.tco_stopped && I_IXANY(tty) &&
>                           c != INTR_CHAR(tty) && c != QUIT_CHAR(tty) &&
>                           c != SUSP_CHAR(tty))) {
> @@ -1457,6 +1485,26 @@ n_tty_receive_char_lnext(struct tty_struct *tty, unsigned char c, char flag)
>                 n_tty_receive_char_flagged(tty, c, flag);
>  }
>
> +static void n_tty_lookahead_flow_ctrl(struct tty_struct *tty, const unsigned char *cp,
> +                                     const unsigned char *fp, unsigned int count)
> +{
> +       struct n_tty_data *ldata = tty->disc_data;
> +       unsigned char flag = TTY_NORMAL;
> +
> +       ldata->lookahead_count += count;
> +
> +       if (!I_IXON(tty))
> +               return;

> +       while (count--) {

If count == 0 at the beginning of this function, this becomes an
infinite loop, perhaps adding a comment that explains that this never
happens?

> +               if (fp)
> +                       flag = *fp++;
> +               if (likely(flag == TTY_NORMAL))
> +                       n_tty_receive_char_flow_ctrl(tty, *cp, false);
> +               cp++;
> +       }
> +}
> +
>  static void
>  n_tty_receive_buf_real_raw(struct tty_struct *tty, const unsigned char *cp,
>                            const char *fp, int count)
> @@ -1496,7 +1544,7 @@ n_tty_receive_buf_raw(struct tty_struct *tty, const unsigned char *cp,
>
>  static void
>  n_tty_receive_buf_closing(struct tty_struct *tty, const unsigned char *cp,
> -                         const char *fp, int count)
> +                         const char *fp, int count, bool lookahead_done)
>  {
>         char flag = TTY_NORMAL;
>
> @@ -1504,12 +1552,12 @@ n_tty_receive_buf_closing(struct tty_struct *tty, const unsigned char *cp,
>                 if (fp)
>                         flag = *fp++;
>                 if (likely(flag == TTY_NORMAL))
> -                       n_tty_receive_char_closing(tty, *cp++);
> +                       n_tty_receive_char_closing(tty, *cp++, lookahead_done);
>         }
>  }
>
>  static void n_tty_receive_buf_standard(struct tty_struct *tty,
> -               const unsigned char *cp, const char *fp, int count)
> +               const unsigned char *cp, const char *fp, int count, bool lookahead_done)
>  {
>         struct n_tty_data *ldata = tty->disc_data;
>         char flag = TTY_NORMAL;
> @@ -1540,7 +1588,7 @@ static void n_tty_receive_buf_standard(struct tty_struct *tty,
>                 }
>
>                 if (test_bit(c, ldata->char_map))
> -                       n_tty_receive_char_special(tty, c);
> +                       n_tty_receive_char_special(tty, c, lookahead_done);
>                 else
>                         n_tty_receive_char(tty, c);
>         }
> @@ -1551,21 +1599,30 @@ static void __receive_buf(struct tty_struct *tty, const unsigned char *cp,
>  {
>         struct n_tty_data *ldata = tty->disc_data;
>         bool preops = I_ISTRIP(tty) || (I_IUCLC(tty) && L_IEXTEN(tty));
> +       size_t la_count = min_t(size_t, ldata->lookahead_count, count);
>
>         if (ldata->real_raw)
>                 n_tty_receive_buf_real_raw(tty, cp, fp, count);
>         else if (ldata->raw || (L_EXTPROC(tty) && !preops))
>                 n_tty_receive_buf_raw(tty, cp, fp, count);
> -       else if (tty->closing && !L_EXTPROC(tty))
> -               n_tty_receive_buf_closing(tty, cp, fp, count);
> -       else {
> -               n_tty_receive_buf_standard(tty, cp, fp, count);
> +       else if (tty->closing && !L_EXTPROC(tty)) {
> +               if (la_count > 0)
> +                       n_tty_receive_buf_closing(tty, cp, fp, la_count, true);
> +               if (count > la_count)
> +                       n_tty_receive_buf_closing(tty, cp, fp, count - la_count, false);
> +       } else {
> +               if (la_count > 0)
> +                       n_tty_receive_buf_standard(tty, cp, fp, la_count, true);
> +               if (count > la_count)
> +                       n_tty_receive_buf_standard(tty, cp, fp, count - la_count, false);
>
>                 flush_echoes(tty);
>                 if (tty->ops->flush_chars)
>                         tty->ops->flush_chars(tty);
>         }
>
> +       ldata->lookahead_count -= la_count;
> +
>         if (ldata->icanon && !L_EXTPROC(tty))
>                 return;
>
> @@ -2446,6 +2503,7 @@ static struct tty_ldisc_ops n_tty_ops = {
>         .receive_buf     = n_tty_receive_buf,
>         .write_wakeup    = n_tty_write_wakeup,
>         .receive_buf2    = n_tty_receive_buf2,
> +       .lookahead_buf   = n_tty_lookahead_flow_ctrl,
>  };
>
>  /**
> diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
> index bfa431a8e690..754fa43670cc 100644
> --- a/drivers/tty/tty_buffer.c
> +++ b/drivers/tty/tty_buffer.c
> @@ -5,6 +5,7 @@
>
>  #include <linux/types.h>
>  #include <linux/errno.h>
> +#include <linux/minmax.h>
>  #include <linux/tty.h>
>  #include <linux/tty_driver.h>
>  #include <linux/tty_flip.h>
> @@ -104,6 +105,7 @@ static void tty_buffer_reset(struct tty_buffer *p, size_t size)
>         p->size = size;
>         p->next = NULL;
>         p->commit = 0;
> +       p->lookahead = 0;
>         p->read = 0;
>         p->flags = 0;
>  }
> @@ -234,6 +236,7 @@ void tty_buffer_flush(struct tty_struct *tty, struct tty_ldisc *ld)
>                 buf->head = next;
>         }
>         buf->head->read = buf->head->commit;
> +       buf->head->lookahead = buf->head->read;
>
>         if (ld && ld->ops->flush_buffer)
>                 ld->ops->flush_buffer(tty);
> @@ -276,13 +279,15 @@ static int __tty_buffer_request_room(struct tty_port *port, size_t size,
>                 if (n != NULL) {
>                         n->flags = flags;
>                         buf->tail = n;
> -                       /* paired w/ acquire in flush_to_ldisc(); ensures
> -                        * flush_to_ldisc() sees buffer data.
> +                       /*
> +                        * Paired w/ acquire in flush_to_ldisc() and lookahead_bufs()
> +                        * ensures they see all buffer data.
>                          */
>                         smp_store_release(&b->commit, b->used);
> -                       /* paired w/ acquire in flush_to_ldisc(); ensures the
> -                        * latest commit value can be read before the head is
> -                        * advanced to the next buffer
> +                       /*
> +                        * Paired w/ acquire in flush_to_ldisc() and lookahead_bufs()
> +                        * ensures the latest commit value can be read before the head
> +                        * is advanced to the next buffer.
>                          */
>                         smp_store_release(&b->next, n);
>                 } else if (change)
> @@ -459,6 +464,40 @@ int tty_ldisc_receive_buf(struct tty_ldisc *ld, const unsigned char *p,
>  }
>  EXPORT_SYMBOL_GPL(tty_ldisc_receive_buf);
>
> +static void lookahead_bufs(struct tty_port *port, struct tty_buffer *head)
> +{
> +       head->lookahead = max(head->lookahead, head->read);
> +
> +       while (head) {
> +               struct tty_buffer *next;
> +               unsigned char *p, *f = NULL;
> +               unsigned int count;
> +
> +               /*
> +                * Paired w/ release in __tty_buffer_request_room();
> +                * ensures commit value read is not stale if the head
> +                * is advancing to the next buffer.
> +                */
> +               next = smp_load_acquire(&head->next);
> +               /*
> +                * Paired w/ release in __tty_buffer_request_room() or in
> +                * tty_buffer_flush(); ensures we see the committed buffer data.
> +                */
> +               count = smp_load_acquire(&head->commit) - head->lookahead;
> +               if (!count) {
> +                       head = next;
> +                       continue;
> +               }
> +
> +               p = char_buf_ptr(head, head->lookahead);
> +               if (~head->flags & TTYB_NORMAL)
> +                       f = flag_buf_ptr(head, head->lookahead);
> +
> +               port->client_ops->lookahead_buf(port, p, f, count);
> +               head->lookahead += count;
> +       }
> +}
> +
>  static int
>  receive_buf(struct tty_port *port, struct tty_buffer *head, int count)
>  {
> @@ -496,7 +535,7 @@ static void flush_to_ldisc(struct work_struct *work)
>         while (1) {
>                 struct tty_buffer *head = buf->head;
>                 struct tty_buffer *next;
> -               int count;
> +               int count, rcvd;
>
>                 /* Ldisc or user is trying to gain exclusive access */
>                 if (atomic_read(&buf->priority))
> @@ -519,10 +558,12 @@ static void flush_to_ldisc(struct work_struct *work)
>                         continue;
>                 }
>
> -               count = receive_buf(port, head, count);
> -               if (!count)
> +               rcvd = receive_buf(port, head, count);
> +               head->read += rcvd;
> +               if (rcvd < count)
> +                       lookahead_bufs(port, head);
> +               if (!rcvd)
>                         break;
> -               head->read += count;
>
>                 if (need_resched())
>                         cond_resched();
> diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
> index 880608a65773..dce08a6d7b5e 100644
> --- a/drivers/tty/tty_port.c
> +++ b/drivers/tty/tty_port.c
> @@ -43,6 +43,26 @@ static int tty_port_default_receive_buf(struct tty_port *port,
>         return ret;
>  }
>
> +static void tty_port_default_lookahead_buf(struct tty_port *port, const unsigned char *p,
> +                                          const unsigned char *f, unsigned int count)
> +{
> +       struct tty_struct *tty;
> +       struct tty_ldisc *disc;
> +
> +       tty = READ_ONCE(port->itty);
> +       if (!tty)
> +               return;
> +
> +       disc = tty_ldisc_ref(tty);
> +       if (!disc)
> +               return;
> +
> +       if (disc->ops->lookahead_buf)
> +               disc->ops->lookahead_buf(disc->tty, p, f, count);
> +
> +       tty_ldisc_deref(disc);
> +}
> +
>  static void tty_port_default_wakeup(struct tty_port *port)
>  {
>         struct tty_struct *tty = tty_port_tty_get(port);
> @@ -55,6 +75,7 @@ static void tty_port_default_wakeup(struct tty_port *port)
>
>  const struct tty_port_client_operations tty_port_default_client_ops = {
>         .receive_buf = tty_port_default_receive_buf,
> +       .lookahead_buf = tty_port_default_lookahead_buf,
>         .write_wakeup = tty_port_default_wakeup,
>  };
>  EXPORT_SYMBOL_GPL(tty_port_default_client_ops);
> diff --git a/include/linux/tty_buffer.h b/include/linux/tty_buffer.h
> index 3b9d77604291..1796648c2907 100644
> --- a/include/linux/tty_buffer.h
> +++ b/include/linux/tty_buffer.h
> @@ -15,6 +15,7 @@ struct tty_buffer {
>         int used;
>         int size;
>         int commit;
> +       int lookahead;          /* Lazy update on recv, can become less than "read" */
>         int read;
>         int flags;
>         /* Data points here */
> diff --git a/include/linux/tty_ldisc.h b/include/linux/tty_ldisc.h
> index e85002b56752..bf8143fbcff3 100644
> --- a/include/linux/tty_ldisc.h
> +++ b/include/linux/tty_ldisc.h
> @@ -186,6 +186,17 @@ int ldsem_down_write_nested(struct ld_semaphore *sem, int subclass,
>   *     indicate all data received is %TTY_NORMAL. If assigned, prefer this
>   *     function for automatic flow control.
>   *
> + * @lookahead_buf: [DRV] ``void ()(struct tty_struct *tty,
> + *                     const unsigned char *cp, const char *fp, int count)
> + *
> + *     This function is called by the low-level tty driver for characters
> + *     not eaten by receive_buf or receive_buf2. It is useful for processing

... by ->receive_buf() or ->receive_buf2().

> + *     high-priority characters such as software flow-control characters that
> + *     could otherwise get stuck into the intermediate buffer until tty has
> + *     room to receive them. Ldisc must be able to handle later a receive_buf
> + *     or receive_buf2 call for the same characters (e.g. by skipping the

Ditto.

> + *     actions for high-priority characters already handled by lookahead_buf).

Similar.

> + *
>   * @owner: module containting this ldisc (for reference counting)
>   *
>   * This structure defines the interface between the tty line discipline
> @@ -229,6 +240,8 @@ struct tty_ldisc_ops {
>         void    (*dcd_change)(struct tty_struct *tty, unsigned int status);
>         int     (*receive_buf2)(struct tty_struct *tty, const unsigned char *cp,
>                                 const char *fp, int count);
> +       void    (*lookahead_buf)(struct tty_struct *tty, const unsigned char *cp,
> +                                const unsigned char *fp, unsigned int count);
>
>         struct  module *owner;
>  };
> diff --git a/include/linux/tty_port.h b/include/linux/tty_port.h
> index 58e9619116b7..fa3c3bdaa234 100644
> --- a/include/linux/tty_port.h
> +++ b/include/linux/tty_port.h
> @@ -40,6 +40,8 @@ struct tty_port_operations {
>
>  struct tty_port_client_operations {
>         int (*receive_buf)(struct tty_port *port, const unsigned char *, const unsigned char *, size_t);
> +       void (*lookahead_buf)(struct tty_port *port, const unsigned char *cp,
> +                             const unsigned char *fp, unsigned int count);
>         void (*write_wakeup)(struct tty_port *port);
>  };
>
> --
> 2.30.2
>


-- 
With Best Regards,
Andy Shevchenko

[PATCH v5 2/2] tty: Use flow-control char function on closing path

[Ilpo Järvinen]

Use n_tty_receive_char_flow_ctrl also on the closing path. This makes
the code cleaner and consistent.

However, there a small change of regression!

The earlier closing path has a small difference compared with the
normal receive path. If START_CHAR and STOP_CHAR are equal, their
precedence is different depending on which path a character is
processed. I don't know whether this difference was intentional or
not, and if equal START_CHAR and STOP_CHAR is actually used anywhere.
But it feels not so useful corner case.

While this change would logically belong to those earlier changes,
having a separate patch for this is useful. If this regresses, bisect
can pinpoint this change rather than the large patch. Also, this
change is not necessary to minimal fix for the issue addressed in
the previous patch.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/tty/n_tty.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 917b5970b2e0..ea4dc316eafb 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1434,15 +1434,10 @@ static void n_tty_receive_char_closing(struct tty_struct *tty, unsigned char c,
 		c = tolower(c);
 
 	if (I_IXON(tty)) {
-		if (c == STOP_CHAR(tty)) {
-			if (!lookahead_done)
-				stop_tty(tty);
-		} else if (c == START_CHAR(tty) && lookahead_done) {
-			return;
-		} else if (c == START_CHAR(tty) ||
-			 (tty->flow.stopped && !tty->flow.tco_stopped && I_IXANY(tty) &&
-			  c != INTR_CHAR(tty) && c != QUIT_CHAR(tty) &&
-			  c != SUSP_CHAR(tty))) {
+		if (!n_tty_receive_char_flow_ctrl(tty, c, lookahead_done) &&
+		    tty->flow.stopped && !tty->flow.tco_stopped && I_IXANY(tty) &&
+		    c != INTR_CHAR(tty) && c != QUIT_CHAR(tty) &&
+		    c != SUSP_CHAR(tty)) {
 			start_tty(tty);
 			process_echoes(tty);
 		}
-- 
2.30.2