[PATCH v3 0/5] tty/serial: Process XON/XOFF robustly

[PATCH v3 0/5] tty/serial: Process XON/XOFF robustly

[Ilpo Järvinen]

XON/XOFF are used for software flow-control on serial lines.
XON and XOFF appear as characters within the stream but should
be processed as soon as possible.

The characters received by the UART drivers are in intermediate
buffers until TTY receives them. In the case where the TTY is
not read from, the characters may get stuck into those
intermediate buffers until user-space reads from the TTY. Among
the characters stuck in the buffers, can also be those XON/XOFF
flow control characters. A stuck flow-control character is not
very useful. 

This patch series addresses the issue by checking if TTY is slow
to process characters, that is, eats less than the given amount.
If TTY is slow, a lookahead is invoked for the characters that
remain in the intermediate buffer(s).

Then at a later time, receive_buf needs to ensure the
flow-control actions are not retaken when those same characters
get pushed to TTY. The necessary guards for that are added by
the second patch of the series.

This patch series fixes an issue but I'm not able to pinpoint to
a specific commit id to provide a Fixes tag. The last patch of
the series is not needed for minimal fix (and has a small chance
of regression too), thus that patch shouldn't be sent to stable.

v1 -> v2:
- Add flow ctrl char funcs in separate change & rework logic a bit.
  I believe it's now cleaner than it would have been with the
  suggestions during v1 review, please recheck.
- Renamed n_tty_lookahead_special to n_tty_lookahead_flow_ctrl
- Fixed logic for START_CHAR == STOP_CHAR case
- Use unsigned int for lookahead_count in receive_buf call chain
- Use consistent types in lookahead call chain
- Improved indentation & line splits for function params
- Corrected tty_ldisc.h comments documenting tty_ldisc_ops
- Tweaked comment format

v2 -> v3:
- Split preparatory patch moving/rearranging code to two
- Fix closing path giving change for ... || xx to execute
  instead of skipping the flow-control char
- Use the same flow-control char function on closing path
  (just a cleanup, non-fix as last patch, a small change of
  regression exists)

Ilpo Järvinen (5):
  tty: Add function for handling flow control chars
  tty: Simplify receive flow control char logic
  tty: Add lookahead param to receive_buf
  tty: Implement lookahead to process XON/XOFF timely
  tty: Use flow-control char function on closing path

 drivers/accessibility/speakup/spk_ttyio.c |   2 +-
 drivers/bluetooth/hci_ldisc.c             |   2 +-
 drivers/char/pcmcia/synclink_cs.c         |   2 +-
 drivers/input/serio/serport.c             |   4 +-
 drivers/isdn/capi/capi.c                  |   2 +-
 drivers/misc/ti-st/st_core.c              |   2 +-
 drivers/net/caif/caif_serial.c            |   4 +-
 drivers/net/can/slcan.c                   |   2 +-
 drivers/net/hamradio/6pack.c              |   4 +-
 drivers/net/hamradio/mkiss.c              |   2 +-
 drivers/net/mctp/mctp-serial.c            |   2 +-
 drivers/net/ppp/ppp_async.c               |   2 +-
 drivers/net/ppp/ppp_synctty.c             |   2 +-
 drivers/net/slip/slip.c                   |   2 +-
 drivers/tty/n_gsm.c                       |   2 +-
 drivers/tty/n_hdlc.c                      |   2 +-
 drivers/tty/n_null.c                      |   2 +-
 drivers/tty/n_tty.c                       | 102 +++++++++++++++-------
 drivers/tty/serdev/serdev-ttyport.c       |   3 +-
 drivers/tty/synclink_gt.c                 |   2 +-
 drivers/tty/tty_buffer.c                  |  67 +++++++++++---
 drivers/tty/tty_io.c                      |   2 +-
 drivers/tty/tty_port.c                    |  26 +++++-
 drivers/tty/vt/selection.c                |   2 +-
 include/linux/tty_buffer.h                |   1 +
 include/linux/tty_flip.h                  |   2 +-
 include/linux/tty_ldisc.h                 |  31 +++++--
 include/linux/tty_port.h                  |   5 +-
 net/nfc/nci/uart.c                        |   2 +-
 sound/soc/codecs/cx20442.c                |   2 +-
 sound/soc/ti/ams-delta.c                  |   2 +-
 31 files changed, 205 insertions(+), 84 deletions(-)

-- 
2.30.2

[PATCH v3 1/5] tty: Add function for handling flow control chars

[Ilpo Järvinen]

Move receive path flow control character handling to own function.

No functional changes.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/tty/n_tty.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index efc72104c840..c7edfc001fd0 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1220,21 +1220,28 @@ n_tty_receive_signal_char(struct tty_struct *tty, int signal, unsigned char c)
 		process_echoes(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)
+{
+	if (c == START_CHAR(tty)) {
+		start_tty(tty);
+		process_echoes(tty);
+		return true;
+	}
+	if (c == STOP_CHAR(tty)) {
+		stop_tty(tty);
+		return true;
+	}
+
+	return false;
+}
+
 static void n_tty_receive_char_special(struct tty_struct *tty, unsigned char c)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 
-	if (I_IXON(tty)) {
-		if (c == START_CHAR(tty)) {
-			start_tty(tty);
-			process_echoes(tty);
-			return;
-		}
-		if (c == STOP_CHAR(tty)) {
-			stop_tty(tty);
-			return;
-		}
-	}
+	if (I_IXON(tty) && n_tty_receive_char_flow_ctrl(tty, c))
+		return;
 
 	if (L_ISIG(tty)) {
 		if (c == INTR_CHAR(tty)) {
-- 
2.30.2

[PATCH v3 2/5] tty: Simplify receive flow control char logic

[Ilpo Järvinen]

Add a helper to check if the character is a flow control one.
Reorder return places, add else for the case where START_CHAR
and STOP_CHAR are the same, w/o else both would match.

This seems cleanest approach once skipping due to lookahead
is added by the next patch. Its downside is the duplicated
START_CHAR and STOP_CHAR checks.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/tty/n_tty.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index c7edfc001fd0..90b3e06cbeb1 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1220,20 +1220,25 @@ n_tty_receive_signal_char(struct tty_struct *tty, int signal, unsigned char c)
 		process_echoes(tty);
 }
 
+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)
 {
+	if (!n_tty_is_char_flow_ctrl(tty, c))
+		return false;
+
 	if (c == START_CHAR(tty)) {
 		start_tty(tty);
 		process_echoes(tty);
-		return true;
-	}
-	if (c == STOP_CHAR(tty)) {
+	} else if (c == STOP_CHAR(tty)) {
 		stop_tty(tty);
-		return true;
 	}
 
-	return false;
+	return true;
 }
 
 static void n_tty_receive_char_special(struct tty_struct *tty, unsigned char c)
-- 
2.30.2

[PATCH v3 3/5] tty: Add lookahead param to receive_buf

[Ilpo Järvinen]

After lookahead for XON/XOFF characters is added by the next
patch, the receive side needs to ensure the flow-control
actions are not retaken later on when those same characters
get received by TTY.

Thus, pass lookahead count to receive_buf and skip
flow-control character actions if already taken for the
character in question. Lookahead count will become live after
the next patch.

The logic in n_tty_receive_char_closing is bit tricky.
The lookahead_done checks cannot be combined with the main
conditions because doing so would give chance for the part
after || in the else if condition to execute when lookahead
done is set (rather than just skipping the flow-control
character like it should).

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/accessibility/speakup/spk_ttyio.c |  2 +-
 drivers/bluetooth/hci_ldisc.c             |  2 +-
 drivers/char/pcmcia/synclink_cs.c         |  2 +-
 drivers/input/serio/serport.c             |  4 +-
 drivers/isdn/capi/capi.c                  |  2 +-
 drivers/misc/ti-st/st_core.c              |  2 +-
 drivers/net/caif/caif_serial.c            |  4 +-
 drivers/net/can/slcan.c                   |  2 +-
 drivers/net/hamradio/6pack.c              |  4 +-
 drivers/net/hamradio/mkiss.c              |  2 +-
 drivers/net/mctp/mctp-serial.c            |  2 +-
 drivers/net/ppp/ppp_async.c               |  2 +-
 drivers/net/ppp/ppp_synctty.c             |  2 +-
 drivers/net/slip/slip.c                   |  2 +-
 drivers/tty/n_gsm.c                       |  2 +-
 drivers/tty/n_hdlc.c                      |  2 +-
 drivers/tty/n_null.c                      |  2 +-
 drivers/tty/n_tty.c                       | 53 ++++++++++++++---------
 drivers/tty/serdev/serdev-ttyport.c       |  3 +-
 drivers/tty/synclink_gt.c                 |  2 +-
 drivers/tty/tty_buffer.c                  |  8 ++--
 drivers/tty/tty_io.c                      |  2 +-
 drivers/tty/tty_port.c                    |  5 ++-
 drivers/tty/vt/selection.c                |  2 +-
 include/linux/tty_flip.h                  |  2 +-
 include/linux/tty_ldisc.h                 | 20 ++++++---
 include/linux/tty_port.h                  |  3 +-
 net/nfc/nci/uart.c                        |  2 +-
 sound/soc/codecs/cx20442.c                |  2 +-
 sound/soc/ti/ams-delta.c                  |  2 +-
 30 files changed, 83 insertions(+), 63 deletions(-)

diff --git a/drivers/accessibility/speakup/spk_ttyio.c b/drivers/accessibility/speakup/spk_ttyio.c
index 08cf8a17754b..b33536eea1d3 100644
--- a/drivers/accessibility/speakup/spk_ttyio.c
+++ b/drivers/accessibility/speakup/spk_ttyio.c
@@ -73,7 +73,7 @@ static void spk_ttyio_ldisc_close(struct tty_struct *tty)
 
 static int spk_ttyio_receive_buf2(struct tty_struct *tty,
 				  const unsigned char *cp,
-				  const char *fp, int count)
+				  const char *fp, int count, unsigned int lookahead_count)
 {
 	struct spk_ldisc_data *ldisc_data = tty->disc_data;
 	struct spk_synth *synth = ldisc_data->synth;
diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
index f537673ede17..08c329a4cdcc 100644
--- a/drivers/bluetooth/hci_ldisc.c
+++ b/drivers/bluetooth/hci_ldisc.c
@@ -596,7 +596,7 @@ static void hci_uart_tty_wakeup(struct tty_struct *tty)
  * Return Value:    None
  */
 static void hci_uart_tty_receive(struct tty_struct *tty, const u8 *data,
-				 const char *flags, int count)
+				 const char *flags, int count, unsigned int lookahead_count)
 {
 	struct hci_uart *hu = tty->disc_data;
 
diff --git a/drivers/char/pcmcia/synclink_cs.c b/drivers/char/pcmcia/synclink_cs.c
index 78baba55a8b5..de9c151cfb12 100644
--- a/drivers/char/pcmcia/synclink_cs.c
+++ b/drivers/char/pcmcia/synclink_cs.c
@@ -501,7 +501,7 @@ static void ldisc_receive_buf(struct tty_struct *tty,
 	ld = tty_ldisc_ref(tty);
 	if (ld) {
 		if (ld->ops->receive_buf)
-			ld->ops->receive_buf(tty, data, flags, count);
+			ld->ops->receive_buf(tty, data, flags, count, 0);
 		tty_ldisc_deref(ld);
 	}
 }
diff --git a/drivers/input/serio/serport.c b/drivers/input/serio/serport.c
index 669a728095b8..f0f19b5a2059 100644
--- a/drivers/input/serio/serport.c
+++ b/drivers/input/serio/serport.c
@@ -114,8 +114,8 @@ static void serport_ldisc_close(struct tty_struct *tty)
  * 'interrupt' routine.
  */
 
-static void serport_ldisc_receive(struct tty_struct *tty,
-		const unsigned char *cp, const char *fp, int count)
+static void serport_ldisc_receive(struct tty_struct *tty, const unsigned char *cp,
+				  const char *fp, int count, unsigned int lookahead_count)
 {
 	struct serport *serport = (struct serport*) tty->disc_data;
 	unsigned long flags;
diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
index 0f00be62438d..beb4c78a7219 100644
--- a/drivers/isdn/capi/capi.c
+++ b/drivers/isdn/capi/capi.c
@@ -454,7 +454,7 @@ static int handle_recv_skb(struct capiminor *mp, struct sk_buff *skb)
 		skb_pull(skb, CAPIMSG_LEN(skb->data));
 		pr_debug("capi: DATA_B3_RESP %u len=%d => ldisc\n",
 			 datahandle, skb->len);
-		ld->ops->receive_buf(tty, skb->data, NULL, skb->len);
+		ld->ops->receive_buf(tty, skb->data, NULL, skb->len, 0);
 	} else {
 		printk(KERN_ERR "capi: send DATA_B3_RESP failed=%x\n",
 		       errcode);
diff --git a/drivers/misc/ti-st/st_core.c b/drivers/misc/ti-st/st_core.c
index 7f6976a9f508..b2c96d72c0e3 100644
--- a/drivers/misc/ti-st/st_core.c
+++ b/drivers/misc/ti-st/st_core.c
@@ -797,7 +797,7 @@ static void st_tty_close(struct tty_struct *tty)
 }
 
 static void st_tty_receive(struct tty_struct *tty, const unsigned char *data,
-			   const char *tty_flags, int count)
+			   const char *tty_flags, int count, unsigned int lookahead_count)
 {
 #ifdef VERBOSE
 	print_hex_dump(KERN_DEBUG, ">in>", DUMP_PREFIX_NONE,
diff --git a/drivers/net/caif/caif_serial.c b/drivers/net/caif/caif_serial.c
index 688075859ae4..cc97c436ec88 100644
--- a/drivers/net/caif/caif_serial.c
+++ b/drivers/net/caif/caif_serial.c
@@ -159,7 +159,7 @@ static inline void debugfs_tx(struct ser_device *ser, const u8 *data, int size)
 #endif
 
 static void ldisc_receive(struct tty_struct *tty, const u8 *data,
-			const char *flags, int count)
+			  const char *flags, int count, unsigned int lookahead_count)
 {
 	struct sk_buff *skb = NULL;
 	struct ser_device *ser;
@@ -237,7 +237,7 @@ static int handle_tx(struct ser_device *ser)
 			update_tty_status(ser);
 		} else {
 			tty_wr = len;
-			ldisc_receive(tty, skb->data, NULL, len);
+			ldisc_receive(tty, skb->data, NULL, len, 0);
 		}
 		ser->dev->stats.tx_packets++;
 		ser->dev->stats.tx_bytes += tty_wr;
diff --git a/drivers/net/can/slcan.c b/drivers/net/can/slcan.c
index ec294d0c5722..5e03e14c2d99 100644
--- a/drivers/net/can/slcan.c
+++ b/drivers/net/can/slcan.c
@@ -471,7 +471,7 @@ static void slc_setup(struct net_device *dev)
 
 static void slcan_receive_buf(struct tty_struct *tty,
 			      const unsigned char *cp, const char *fp,
-			      int count)
+			      int count, unsigned int lookahead_count)
 {
 	struct slcan *sl = (struct slcan *) tty->disc_data;
 
diff --git a/drivers/net/hamradio/6pack.c b/drivers/net/hamradio/6pack.c
index 45c3c4a1101b..6ba4b05dff78 100644
--- a/drivers/net/hamradio/6pack.c
+++ b/drivers/net/hamradio/6pack.c
@@ -426,8 +426,8 @@ static void sixpack_write_wakeup(struct tty_struct *tty)
  * a block of 6pack data has been received, which can now be decapsulated
  * and sent on to some IP layer for further processing.
  */
-static void sixpack_receive_buf(struct tty_struct *tty,
-	const unsigned char *cp, const char *fp, int count)
+static void sixpack_receive_buf(struct tty_struct *tty, const unsigned char *cp,
+				const char *fp, int count, unsigned int lookahead_count)
 {
 	struct sixpack *sp;
 	int count1;
diff --git a/drivers/net/hamradio/mkiss.c b/drivers/net/hamradio/mkiss.c
index c251e04ae047..498997ef429a 100644
--- a/drivers/net/hamradio/mkiss.c
+++ b/drivers/net/hamradio/mkiss.c
@@ -875,7 +875,7 @@ static int mkiss_ioctl(struct tty_struct *tty, unsigned int cmd,
  * and sent on to the AX.25 layer for further processing.
  */
 static void mkiss_receive_buf(struct tty_struct *tty, const unsigned char *cp,
-	const char *fp, int count)
+			      const char *fp, int count, unsigned int lookahead_count)
 {
 	struct mkiss *ax = mkiss_get(tty);
 
diff --git a/drivers/net/mctp/mctp-serial.c b/drivers/net/mctp/mctp-serial.c
index 7cd103fd34ef..f1dfab4c54cb 100644
--- a/drivers/net/mctp/mctp-serial.c
+++ b/drivers/net/mctp/mctp-serial.c
@@ -390,7 +390,7 @@ static void mctp_serial_push(struct mctp_serial *dev, unsigned char c)
 
 static void mctp_serial_tty_receive_buf(struct tty_struct *tty,
 					const unsigned char *c,
-					const char *f, int len)
+					const char *f, int len, unsigned int lookahead_count)
 {
 	struct mctp_serial *dev = tty->disc_data;
 	int i;
diff --git a/drivers/net/ppp/ppp_async.c b/drivers/net/ppp/ppp_async.c
index 15a179631903..87c205a02984 100644
--- a/drivers/net/ppp/ppp_async.c
+++ b/drivers/net/ppp/ppp_async.c
@@ -338,7 +338,7 @@ ppp_asynctty_poll(struct tty_struct *tty, struct file *file, poll_table *wait)
 /* May sleep, don't call from interrupt level or with interrupts disabled */
 static void
 ppp_asynctty_receive(struct tty_struct *tty, const unsigned char *buf,
-		  const char *cflags, int count)
+		     const char *cflags, int count, unsigned int lookahead_count)
 {
 	struct asyncppp *ap = ap_get(tty);
 	unsigned long flags;
diff --git a/drivers/net/ppp/ppp_synctty.c b/drivers/net/ppp/ppp_synctty.c
index 18283b7b94bc..b82f4bd82b21 100644
--- a/drivers/net/ppp/ppp_synctty.c
+++ b/drivers/net/ppp/ppp_synctty.c
@@ -331,7 +331,7 @@ ppp_sync_poll(struct tty_struct *tty, struct file *file, poll_table *wait)
 /* May sleep, don't call from interrupt level or with interrupts disabled */
 static void
 ppp_sync_receive(struct tty_struct *tty, const unsigned char *buf,
-		  const char *cflags, int count)
+		 const char *cflags, int count, unsigned int lookahead_count)
 {
 	struct syncppp *ap = sp_get(tty);
 	unsigned long flags;
diff --git a/drivers/net/slip/slip.c b/drivers/net/slip/slip.c
index 88396ff99f03..7fccf28256a8 100644
--- a/drivers/net/slip/slip.c
+++ b/drivers/net/slip/slip.c
@@ -686,7 +686,7 @@ static void sl_setup(struct net_device *dev)
  */
 
 static void slip_receive_buf(struct tty_struct *tty, const unsigned char *cp,
-		const char *fp, int count)
+			     const char *fp, int count, unsigned int lookahead_count)
 {
 	struct slip *sl = tty->disc_data;
 
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index fa92f727fdf8..45f022892b28 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -2500,7 +2500,7 @@ static void gsmld_detach_gsm(struct tty_struct *tty, struct gsm_mux *gsm)
 }
 
 static void gsmld_receive_buf(struct tty_struct *tty, const unsigned char *cp,
-			      const char *fp, int count)
+			      const char *fp, int count, unsigned int lookahead_count)
 {
 	struct gsm_mux *gsm = tty->disc_data;
 	char flags = TTY_NORMAL;
diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index 94c1ec2dd754..2d7c94a38a92 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -379,7 +379,7 @@ static void n_hdlc_tty_wakeup(struct tty_struct *tty)
  * interpreted as one HDLC frame.
  */
 static void n_hdlc_tty_receive(struct tty_struct *tty, const __u8 *data,
-			       const char *flags, int count)
+			       const char *flags, int count, unsigned int lookahead_count)
 {
 	register struct n_hdlc *n_hdlc = tty->disc_data;
 	register struct n_hdlc_buf *buf;
diff --git a/drivers/tty/n_null.c b/drivers/tty/n_null.c
index f913b665af72..6bce76b5bb1c 100644
--- a/drivers/tty/n_null.c
+++ b/drivers/tty/n_null.c
@@ -34,7 +34,7 @@ static ssize_t n_null_write(struct tty_struct *tty, struct file *file,
 
 static void n_null_receivebuf(struct tty_struct *tty,
 				 const unsigned char *cp, const char *fp,
-				 int cnt)
+				 int cnt, unsigned int lookahead_count)
 {
 }
 
diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 90b3e06cbeb1..708cc85ac43d 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1226,11 +1226,15 @@ static bool n_tty_is_char_flow_ctrl(struct tty_struct *tty, unsigned char c)
 }
 
 /* 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)
+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);
@@ -1241,11 +1245,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)) {
@@ -1400,7 +1405,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;
@@ -1408,9 +1414,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))) {
@@ -1495,22 +1504,24 @@ 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, unsigned int lookahead_count)
 {
+	int lookahead_thres = count - lookahead_count;
 	char flag = TTY_NORMAL;
 
 	while (count--) {
 		if (fp)
 			flag = *fp++;
 		if (likely(flag == TTY_NORMAL))
-			n_tty_receive_char_closing(tty, *cp++);
+			n_tty_receive_char_closing(tty, *cp++, count >= lookahead_thres);
 	}
 }
 
-static void n_tty_receive_buf_standard(struct tty_struct *tty,
-		const unsigned char *cp, const char *fp, int count)
+static void n_tty_receive_buf_standard(struct tty_struct *tty, const unsigned char *cp,
+				       const char *fp, int count, unsigned int lookahead_count)
 {
 	struct n_tty_data *ldata = tty->disc_data;
+	int lookahead_thres = count - lookahead_count;
 	char flag = TTY_NORMAL;
 
 	while (count--) {
@@ -1539,14 +1550,14 @@ 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, count >= lookahead_thres);
 		else
 			n_tty_receive_char(tty, c);
 	}
 }
 
 static void __receive_buf(struct tty_struct *tty, const unsigned char *cp,
-			  const char *fp, int count)
+			  const char *fp, int count, unsigned int lookahead_count)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 	bool preops = I_ISTRIP(tty) || (I_IUCLC(tty) && L_IEXTEN(tty));
@@ -1556,9 +1567,9 @@ static void __receive_buf(struct tty_struct *tty, const unsigned char *cp,
 	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);
+		n_tty_receive_buf_closing(tty, cp, fp, count, lookahead_count);
 	else {
-		n_tty_receive_buf_standard(tty, cp, fp, count);
+		n_tty_receive_buf_standard(tty, cp, fp, count, lookahead_count);
 
 		flush_echoes(tty);
 		if (tty->ops->flush_chars)
@@ -1612,7 +1623,7 @@ static void __receive_buf(struct tty_struct *tty, const unsigned char *cp,
  */
 static int
 n_tty_receive_buf_common(struct tty_struct *tty, const unsigned char *cp,
-			 const char *fp, int count, int flow)
+			 const char *fp, int count, unsigned int lookahead_count, int flow)
 {
 	struct n_tty_data *ldata = tty->disc_data;
 	int room, n, rcvd = 0, overflow;
@@ -1654,7 +1665,7 @@ n_tty_receive_buf_common(struct tty_struct *tty, const unsigned char *cp,
 
 		/* ignore parity errors if handling overflow */
 		if (!overflow || !fp || *fp != TTY_PARITY)
-			__receive_buf(tty, cp, fp, n);
+			__receive_buf(tty, cp, fp, n, lookahead_count);
 
 		cp += n;
 		if (fp)
@@ -1681,15 +1692,15 @@ n_tty_receive_buf_common(struct tty_struct *tty, const unsigned char *cp,
 }
 
 static void n_tty_receive_buf(struct tty_struct *tty, const unsigned char *cp,
-			      const char *fp, int count)
+			      const char *fp, int count, unsigned int lookahead_count)
 {
-	n_tty_receive_buf_common(tty, cp, fp, count, 0);
+	n_tty_receive_buf_common(tty, cp, fp, count, lookahead_count, 0);
 }
 
 static int n_tty_receive_buf2(struct tty_struct *tty, const unsigned char *cp,
-			      const char *fp, int count)
+			      const char *fp, int count, unsigned int lookahead_count)
 {
-	return n_tty_receive_buf_common(tty, cp, fp, count, 1);
+	return n_tty_receive_buf_common(tty, cp, fp, count, lookahead_count, 1);
 }
 
 /**
diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c
index d367803e2044..087e081e8e59 100644
--- a/drivers/tty/serdev/serdev-ttyport.c
+++ b/drivers/tty/serdev/serdev-ttyport.c
@@ -23,7 +23,8 @@ struct serport {
  */
 
 static int ttyport_receive_buf(struct tty_port *port, const unsigned char *cp,
-				const unsigned char *fp, size_t count)
+			       const unsigned char *fp, size_t count,
+			       unsigned int lookahead_count)
 {
 	struct serdev_controller *ctrl = port->client_data;
 	struct serport *serport = serdev_controller_get_drvdata(ctrl);
diff --git a/drivers/tty/synclink_gt.c b/drivers/tty/synclink_gt.c
index 25c558e65ece..a669e6e13f90 100644
--- a/drivers/tty/synclink_gt.c
+++ b/drivers/tty/synclink_gt.c
@@ -582,7 +582,7 @@ static void ldisc_receive_buf(struct tty_struct *tty,
 	ld = tty_ldisc_ref(tty);
 	if (ld) {
 		if (ld->ops->receive_buf)
-			ld->ops->receive_buf(tty, data, flags, count);
+			ld->ops->receive_buf(tty, data, flags, count, 0);
 		tty_ldisc_deref(ld);
 	}
 }
diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 646510476c30..c561110c7d4d 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -445,14 +445,14 @@ EXPORT_SYMBOL_GPL(tty_prepare_flip_string);
  * Returns: the number of bytes processed.
  */
 int tty_ldisc_receive_buf(struct tty_ldisc *ld, const unsigned char *p,
-			  const char *f, int count)
+			  const char *f, int count, unsigned int lookahead_count)
 {
 	if (ld->ops->receive_buf2)
-		count = ld->ops->receive_buf2(ld->tty, p, f, count);
+		count = ld->ops->receive_buf2(ld->tty, p, f, count, lookahead_count);
 	else {
 		count = min_t(int, count, ld->tty->receive_room);
 		if (count && ld->ops->receive_buf)
-			ld->ops->receive_buf(ld->tty, p, f, count);
+			ld->ops->receive_buf(ld->tty, p, f, count, lookahead_count);
 	}
 	return count;
 }
@@ -468,7 +468,7 @@ receive_buf(struct tty_port *port, struct tty_buffer *head, int count)
 	if (~head->flags & TTYB_NORMAL)
 		f = flag_buf_ptr(head, head->read);
 
-	n = port->client_ops->receive_buf(port, p, f, count);
+	n = port->client_ops->receive_buf(port, p, f, count, 0);
 	if (n > 0)
 		memset(p, 0, n);
 	return n;
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 8fec1d8648f5..cd0b74a5ceb2 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2290,7 +2290,7 @@ static int tiocsti(struct tty_struct *tty, char __user *p)
 		return -EIO;
 	tty_buffer_lock_exclusive(tty->port);
 	if (ld->ops->receive_buf)
-		ld->ops->receive_buf(tty, &ch, &mbz, 1);
+		ld->ops->receive_buf(tty, &ch, &mbz, 1, 0);
 	tty_buffer_unlock_exclusive(tty->port);
 	tty_ldisc_deref(ld);
 	return 0;
diff --git a/drivers/tty/tty_port.c b/drivers/tty/tty_port.c
index 880608a65773..45cbbf338f24 100644
--- a/drivers/tty/tty_port.c
+++ b/drivers/tty/tty_port.c
@@ -22,7 +22,8 @@
 
 static int tty_port_default_receive_buf(struct tty_port *port,
 					const unsigned char *p,
-					const unsigned char *f, size_t count)
+					const unsigned char *f, size_t count,
+					unsigned int lookahead_count)
 {
 	int ret;
 	struct tty_struct *tty;
@@ -36,7 +37,7 @@ static int tty_port_default_receive_buf(struct tty_port *port,
 	if (!disc)
 		return 0;
 
-	ret = tty_ldisc_receive_buf(disc, p, (char *)f, count);
+	ret = tty_ldisc_receive_buf(disc, p, (char *)f, count, lookahead_count);
 
 	tty_ldisc_deref(disc);
 
diff --git a/drivers/tty/vt/selection.c b/drivers/tty/vt/selection.c
index f7755e73696e..abc3890ecc18 100644
--- a/drivers/tty/vt/selection.c
+++ b/drivers/tty/vt/selection.c
@@ -406,7 +406,7 @@ int paste_selection(struct tty_struct *tty)
 		__set_current_state(TASK_RUNNING);
 		count = vc_sel.buf_len - pasted;
 		count = tty_ldisc_receive_buf(ld, vc_sel.buffer + pasted, NULL,
-					      count);
+					      count, 0);
 		pasted += count;
 	}
 	mutex_unlock(&vc_sel.lock);
diff --git a/include/linux/tty_flip.h b/include/linux/tty_flip.h
index 483d41cbcbb7..53725f6312c0 100644
--- a/include/linux/tty_flip.h
+++ b/include/linux/tty_flip.h
@@ -42,7 +42,7 @@ static inline int tty_insert_flip_string(struct tty_port *port,
 }
 
 int tty_ldisc_receive_buf(struct tty_ldisc *ld, const unsigned char *p,
-		const char *f, int count);
+		const char *f, int count, unsigned int lookahead_count);
 
 void tty_buffer_lock_exclusive(struct tty_port *port);
 void tty_buffer_unlock_exclusive(struct tty_port *port);
diff --git a/include/linux/tty_ldisc.h b/include/linux/tty_ldisc.h
index e85002b56752..d81a39cff9e2 100644
--- a/include/linux/tty_ldisc.h
+++ b/include/linux/tty_ldisc.h
@@ -152,14 +152,17 @@ int ldsem_down_write_nested(struct ld_semaphore *sem, int subclass,
  *	Can sleep.
  *
  * @receive_buf: [DRV] ``void ()(struct tty_struct *tty,
- *		       const unsigned char *cp, const char *fp, int count)``
+ *		       const unsigned char *cp, const char *fp, int count,
+ *		       unsigned int lookahead_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 an array of flag bytes which indicate whether a
  *	character was received with a parity error, etc. @fp may be %NULL to
- *	indicate all data received is %TTY_NORMAL.
+ *	indicate all data received is %TTY_NORMAL. The number of characters is
+ *	indicated with @count and characters for which lookahead was already
+ *	performed with @lookahead_count.
  *
  * @write_wakeup: [DRV] ``void ()(struct tty_struct *tty)``
  *
@@ -176,15 +179,18 @@ int ldsem_down_write_nested(struct ld_semaphore *sem, int subclass,
  *	exclusively by the %N_PPS (Pulse-Per-Second) line discipline.
  *
  * @receive_buf2: [DRV] ``int ()(struct tty_struct *tty,
- *			const unsigned char *cp, const char *fp, int count)``
+ *			const unsigned char *cp, const char *fp, int count,
+ *			unsigned int lookahead_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 an array of flag bytes which indicate whether a
  *	character was received with a parity error, etc. @fp may be %NULL to
- *	indicate all data received is %TTY_NORMAL. If assigned, prefer this
- *	function for automatic flow control.
+ *	indicate all data received is %TTY_NORMAL. The number of characters is
+ *	indicated with @count and characters for which lookahead was already
+ *	performed with @lookahead_count. If assigned, prefer this function for
+ *	automatic flow control.
  *
  * @owner: module containting this ldisc (for reference counting)
  *
@@ -224,11 +230,11 @@ struct tty_ldisc_ops {
 	 * The following routines are called from below.
 	 */
 	void	(*receive_buf)(struct tty_struct *tty, const unsigned char *cp,
-			       const char *fp, int count);
+			       const char *fp, int count, unsigned int lookahead_count);
 	void	(*write_wakeup)(struct tty_struct *tty);
 	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);
+				const char *fp, int count, unsigned int lookahead_count);
 
 	struct  module *owner;
 };
diff --git a/include/linux/tty_port.h b/include/linux/tty_port.h
index 58e9619116b7..402470962d23 100644
--- a/include/linux/tty_port.h
+++ b/include/linux/tty_port.h
@@ -39,7 +39,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);
+	int (*receive_buf)(struct tty_port *port, const unsigned char *,
+			   const unsigned char *, size_t, unsigned int lookahead_count);
 	void (*write_wakeup)(struct tty_port *port);
 };
 
diff --git a/net/nfc/nci/uart.c b/net/nfc/nci/uart.c
index cc8fa9e36159..1daa9cf7eec8 100644
--- a/net/nfc/nci/uart.c
+++ b/net/nfc/nci/uart.c
@@ -296,7 +296,7 @@ static int nci_uart_default_recv_buf(struct nci_uart *nu, const u8 *data,
  * Return Value:    None
  */
 static void nci_uart_tty_receive(struct tty_struct *tty, const u8 *data,
-				 const char *flags, int count)
+				 const char *flags, int count, unsigned int lookahead_count)
 {
 	struct nci_uart *nu = (void *)tty->disc_data;
 
diff --git a/sound/soc/codecs/cx20442.c b/sound/soc/codecs/cx20442.c
index 1af0bf5f1e2f..ad86970ad369 100644
--- a/sound/soc/codecs/cx20442.c
+++ b/sound/soc/codecs/cx20442.c
@@ -259,7 +259,7 @@ static void v253_hangup(struct tty_struct *tty)
 
 /* Line discipline .receive_buf() */
 static void v253_receive(struct tty_struct *tty, const unsigned char *cp,
-		const char *fp, int count)
+			 const char *fp, int count, unsigned int lookahead_count)
 {
 	struct snd_soc_component *component = tty->disc_data;
 	struct cx20442_priv *cx20442;
diff --git a/sound/soc/ti/ams-delta.c b/sound/soc/ti/ams-delta.c
index b1a32545babd..fd9694377e3b 100644
--- a/sound/soc/ti/ams-delta.c
+++ b/sound/soc/ti/ams-delta.c
@@ -337,7 +337,7 @@ static void cx81801_hangup(struct tty_struct *tty)
 
 /* Line discipline .receive_buf() */
 static void cx81801_receive(struct tty_struct *tty, const unsigned char *cp,
-		const char *fp, int count)
+			    const char *fp, int count, unsigned int lookahead_count)
 {
 	struct snd_soc_component *component = tty->disc_data;
 	const unsigned char *c;
-- 
2.30.2

[PATCH v3 4/5] 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.

The guards necessary for ensuring the XON/XOFF character are
processed only once were added by the previous patch. All this patch
needs to do on that front is to pass the lookahead count (that can
now be non-zero) into port->client_ops->receive_buf().

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

diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
index 708cc85ac43d..fede29ed8daf 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1465,6 +1465,23 @@ 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)
+{
+	unsigned char flag = TTY_NORMAL;
+
+	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)
@@ -2420,6 +2437,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 c561110c7d4d..48600bbd40e3 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;
 }
@@ -233,6 +235,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);
@@ -275,13 +278,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)
@@ -458,6 +463,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)
 {
@@ -468,7 +507,7 @@ receive_buf(struct tty_port *port, struct tty_buffer *head, int count)
 	if (~head->flags & TTYB_NORMAL)
 		f = flag_buf_ptr(head, head->read);
 
-	n = port->client_ops->receive_buf(port, p, f, count, 0);
+	n = port->client_ops->receive_buf(port, p, f, count, max(head->lookahead - head->read, 0));
 	if (n > 0)
 		memset(p, 0, n);
 	return n;
@@ -495,7 +534,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))
@@ -518,10 +557,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 45cbbf338f24..47fb8088612a 100644
--- a/drivers/tty/tty_port.c
+++ b/drivers/tty/tty_port.c
@@ -44,6 +44,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);
@@ -56,6 +76,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 d81a39cff9e2..1b181a8cfd95 100644
--- a/include/linux/tty_ldisc.h
+++ b/include/linux/tty_ldisc.h
@@ -192,6 +192,15 @@ int ldsem_down_write_nested(struct ld_semaphore *sem, int subclass,
  *	performed with @lookahead_count. 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.
+ *
  * @owner: module containting this ldisc (for reference counting)
  *
  * This structure defines the interface between the tty line discipline
@@ -235,6 +244,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, unsigned int lookahead_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 402470962d23..7a27cb949c4e 100644
--- a/include/linux/tty_port.h
+++ b/include/linux/tty_port.h
@@ -41,6 +41,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, unsigned int lookahead_count);
+	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

[PATCH v3 5/5] 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.

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 fede29ed8daf..e6f47858f98f 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1414,15 +1414,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

Re: [PATCH v3 5/5] tty: Use flow-control char function on closing path

[Andy Shevchenko]

On Mon, Apr 11, 2022 at 12:48:59PM +0300, Ilpo Järvinen wrote:
> 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.

This is nice, if it flies!
FWIW,
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 fede29ed8daf..e6f47858f98f 100644
> --- a/drivers/tty/n_tty.c
> +++ b/drivers/tty/n_tty.c
> @@ -1414,15 +1414,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
> 

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/5] tty: Simplify receive flow control char logic

[Greg KH]

On Mon, Apr 11, 2022 at 12:48:56PM +0300, Ilpo Järvinen wrote:
> Add a helper to check if the character is a flow control one.
> Reorder return places, add else for the case where START_CHAR
> and STOP_CHAR are the same, w/o else both would match.
> 
> This seems cleanest approach once skipping due to lookahead
> is added by the next patch. Its downside is the duplicated
> START_CHAR and STOP_CHAR checks.
> 
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
>  drivers/tty/n_tty.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
> index c7edfc001fd0..90b3e06cbeb1 100644
> --- a/drivers/tty/n_tty.c
> +++ b/drivers/tty/n_tty.c
> @@ -1220,20 +1220,25 @@ n_tty_receive_signal_char(struct tty_struct *tty, int signal, unsigned char c)
>  		process_echoes(tty);
>  }
>  
> +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)
>  {
> +	if (!n_tty_is_char_flow_ctrl(tty, c))
> +		return false;
> +
>  	if (c == START_CHAR(tty)) {
>  		start_tty(tty);
>  		process_echoes(tty);
> -		return true;
> -	}
> -	if (c == STOP_CHAR(tty)) {
> +	} else if (c == STOP_CHAR(tty)) {

The else is always true here now so why check it again?

Just return in the above if statement and then call stop_tty().

thanks,

greg k-h

Re: [PATCH v3 3/5] tty: Add lookahead param to receive_buf

[Greg KH]

On Mon, Apr 11, 2022 at 12:48:57PM +0300, Ilpo Järvinen wrote:
> After lookahead for XON/XOFF characters is added by the next
> patch,

There is no "next" in a series.  This commit needs to be justified here,
right?

> the receive side needs to ensure the flow-control
> actions are not retaken later on when those same characters
> get received by TTY.
> 
> Thus, pass lookahead count to receive_buf and skip
> flow-control character actions if already taken for the
> character in question. Lookahead count will become live after
> the next patch.

You have all 72 columns, please use them.

> 
> The logic in n_tty_receive_char_closing is bit tricky.
> The lookahead_done checks cannot be combined with the main
> conditions because doing so would give chance for the part
> after || in the else if condition to execute when lookahead
> done is set (rather than just skipping the flow-control
> character like it should).
> 
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
>  drivers/accessibility/speakup/spk_ttyio.c |  2 +-
>  drivers/bluetooth/hci_ldisc.c             |  2 +-
>  drivers/char/pcmcia/synclink_cs.c         |  2 +-
>  drivers/input/serio/serport.c             |  4 +-
>  drivers/isdn/capi/capi.c                  |  2 +-
>  drivers/misc/ti-st/st_core.c              |  2 +-
>  drivers/net/caif/caif_serial.c            |  4 +-
>  drivers/net/can/slcan.c                   |  2 +-
>  drivers/net/hamradio/6pack.c              |  4 +-
>  drivers/net/hamradio/mkiss.c              |  2 +-
>  drivers/net/mctp/mctp-serial.c            |  2 +-
>  drivers/net/ppp/ppp_async.c               |  2 +-
>  drivers/net/ppp/ppp_synctty.c             |  2 +-
>  drivers/net/slip/slip.c                   |  2 +-
>  drivers/tty/n_gsm.c                       |  2 +-
>  drivers/tty/n_hdlc.c                      |  2 +-
>  drivers/tty/n_null.c                      |  2 +-
>  drivers/tty/n_tty.c                       | 53 ++++++++++++++---------
>  drivers/tty/serdev/serdev-ttyport.c       |  3 +-
>  drivers/tty/synclink_gt.c                 |  2 +-
>  drivers/tty/tty_buffer.c                  |  8 ++--
>  drivers/tty/tty_io.c                      |  2 +-
>  drivers/tty/tty_port.c                    |  5 ++-
>  drivers/tty/vt/selection.c                |  2 +-
>  include/linux/tty_flip.h                  |  2 +-
>  include/linux/tty_ldisc.h                 | 20 ++++++---
>  include/linux/tty_port.h                  |  3 +-
>  net/nfc/nci/uart.c                        |  2 +-
>  sound/soc/codecs/cx20442.c                |  2 +-
>  sound/soc/ti/ams-delta.c                  |  2 +-
>  30 files changed, 83 insertions(+), 63 deletions(-)
> 
> diff --git a/drivers/accessibility/speakup/spk_ttyio.c b/drivers/accessibility/speakup/spk_ttyio.c
> index 08cf8a17754b..b33536eea1d3 100644
> --- a/drivers/accessibility/speakup/spk_ttyio.c
> +++ b/drivers/accessibility/speakup/spk_ttyio.c
> @@ -73,7 +73,7 @@ static void spk_ttyio_ldisc_close(struct tty_struct *tty)
>  
>  static int spk_ttyio_receive_buf2(struct tty_struct *tty,
>  				  const unsigned char *cp,
> -				  const char *fp, int count)
> +				  const char *fp, int count, unsigned int lookahead_count)

Ick, adding yet-another-parameter to a function is a mess as it's hard
to know what to do with this and what it means just by looking at when
it is called.


>  {
>  	struct spk_ldisc_data *ldisc_data = tty->disc_data;
>  	struct spk_synth *synth = ldisc_data->synth;
> diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c
> index f537673ede17..08c329a4cdcc 100644
> --- a/drivers/bluetooth/hci_ldisc.c
> +++ b/drivers/bluetooth/hci_ldisc.c
> @@ -596,7 +596,7 @@ static void hci_uart_tty_wakeup(struct tty_struct *tty)
>   * Return Value:    None
>   */
>  static void hci_uart_tty_receive(struct tty_struct *tty, const u8 *data,
> -				 const char *flags, int count)
> +				 const char *flags, int count, unsigned int lookahead_count)
>  {
>  	struct hci_uart *hu = tty->disc_data;
>  
> diff --git a/drivers/char/pcmcia/synclink_cs.c b/drivers/char/pcmcia/synclink_cs.c
> index 78baba55a8b5..de9c151cfb12 100644
> --- a/drivers/char/pcmcia/synclink_cs.c
> +++ b/drivers/char/pcmcia/synclink_cs.c
> @@ -501,7 +501,7 @@ static void ldisc_receive_buf(struct tty_struct *tty,
>  	ld = tty_ldisc_ref(tty);
>  	if (ld) {
>  		if (ld->ops->receive_buf)
> -			ld->ops->receive_buf(tty, data, flags, count);
> +			ld->ops->receive_buf(tty, data, flags, count, 0);
>  		tty_ldisc_deref(ld);
>  	}
>  }
> diff --git a/drivers/input/serio/serport.c b/drivers/input/serio/serport.c
> index 669a728095b8..f0f19b5a2059 100644
> --- a/drivers/input/serio/serport.c
> +++ b/drivers/input/serio/serport.c
> @@ -114,8 +114,8 @@ static void serport_ldisc_close(struct tty_struct *tty)
>   * 'interrupt' routine.
>   */
>  
> -static void serport_ldisc_receive(struct tty_struct *tty,
> -		const unsigned char *cp, const char *fp, int count)
> +static void serport_ldisc_receive(struct tty_struct *tty, const unsigned char *cp,
> +				  const char *fp, int count, unsigned int lookahead_count)
>  {
>  	struct serport *serport = (struct serport*) tty->disc_data;
>  	unsigned long flags;
> diff --git a/drivers/isdn/capi/capi.c b/drivers/isdn/capi/capi.c
> index 0f00be62438d..beb4c78a7219 100644
> --- a/drivers/isdn/capi/capi.c
> +++ b/drivers/isdn/capi/capi.c
> @@ -454,7 +454,7 @@ static int handle_recv_skb(struct capiminor *mp, struct sk_buff *skb)
>  		skb_pull(skb, CAPIMSG_LEN(skb->data));
>  		pr_debug("capi: DATA_B3_RESP %u len=%d => ldisc\n",
>  			 datahandle, skb->len);
> -		ld->ops->receive_buf(tty, skb->data, NULL, skb->len);
> +		ld->ops->receive_buf(tty, skb->data, NULL, skb->len, 0);
>  	} else {
>  		printk(KERN_ERR "capi: send DATA_B3_RESP failed=%x\n",
>  		       errcode);
> diff --git a/drivers/misc/ti-st/st_core.c b/drivers/misc/ti-st/st_core.c
> index 7f6976a9f508..b2c96d72c0e3 100644
> --- a/drivers/misc/ti-st/st_core.c
> +++ b/drivers/misc/ti-st/st_core.c
> @@ -797,7 +797,7 @@ static void st_tty_close(struct tty_struct *tty)
>  }
>  
>  static void st_tty_receive(struct tty_struct *tty, const unsigned char *data,
> -			   const char *tty_flags, int count)
> +			   const char *tty_flags, int count, unsigned int lookahead_count)
>  {
>  #ifdef VERBOSE
>  	print_hex_dump(KERN_DEBUG, ">in>", DUMP_PREFIX_NONE,
> diff --git a/drivers/net/caif/caif_serial.c b/drivers/net/caif/caif_serial.c
> index 688075859ae4..cc97c436ec88 100644
> --- a/drivers/net/caif/caif_serial.c
> +++ b/drivers/net/caif/caif_serial.c
> @@ -159,7 +159,7 @@ static inline void debugfs_tx(struct ser_device *ser, const u8 *data, int size)
>  #endif
>  
>  static void ldisc_receive(struct tty_struct *tty, const u8 *data,
> -			const char *flags, int count)
> +			  const char *flags, int count, unsigned int lookahead_count)
>  {
>  	struct sk_buff *skb = NULL;
>  	struct ser_device *ser;
> @@ -237,7 +237,7 @@ static int handle_tx(struct ser_device *ser)
>  			update_tty_status(ser);
>  		} else {
>  			tty_wr = len;
> -			ldisc_receive(tty, skb->data, NULL, len);
> +			ldisc_receive(tty, skb->data, NULL, len, 0);

See, what does 0 here mean?  Who knows?  Not a good api :(



>  		}
>  		ser->dev->stats.tx_packets++;
>  		ser->dev->stats.tx_bytes += tty_wr;
> diff --git a/drivers/net/can/slcan.c b/drivers/net/can/slcan.c
> index ec294d0c5722..5e03e14c2d99 100644
> --- a/drivers/net/can/slcan.c
> +++ b/drivers/net/can/slcan.c
> @@ -471,7 +471,7 @@ static void slc_setup(struct net_device *dev)
>  
>  static void slcan_receive_buf(struct tty_struct *tty,
>  			      const unsigned char *cp, const char *fp,
> -			      int count)
> +			      int count, unsigned int lookahead_count)
>  {
>  	struct slcan *sl = (struct slcan *) tty->disc_data;
>  
> diff --git a/drivers/net/hamradio/6pack.c b/drivers/net/hamradio/6pack.c
> index 45c3c4a1101b..6ba4b05dff78 100644
> --- a/drivers/net/hamradio/6pack.c
> +++ b/drivers/net/hamradio/6pack.c
> @@ -426,8 +426,8 @@ static void sixpack_write_wakeup(struct tty_struct *tty)
>   * a block of 6pack data has been received, which can now be decapsulated
>   * and sent on to some IP layer for further processing.
>   */
> -static void sixpack_receive_buf(struct tty_struct *tty,
> -	const unsigned char *cp, const char *fp, int count)
> +static void sixpack_receive_buf(struct tty_struct *tty, const unsigned char *cp,
> +				const char *fp, int count, unsigned int lookahead_count)
>  {
>  	struct sixpack *sp;
>  	int count1;
> diff --git a/drivers/net/hamradio/mkiss.c b/drivers/net/hamradio/mkiss.c
> index c251e04ae047..498997ef429a 100644
> --- a/drivers/net/hamradio/mkiss.c
> +++ b/drivers/net/hamradio/mkiss.c
> @@ -875,7 +875,7 @@ static int mkiss_ioctl(struct tty_struct *tty, unsigned int cmd,
>   * and sent on to the AX.25 layer for further processing.
>   */
>  static void mkiss_receive_buf(struct tty_struct *tty, const unsigned char *cp,
> -	const char *fp, int count)
> +			      const char *fp, int count, unsigned int lookahead_count)
>  {
>  	struct mkiss *ax = mkiss_get(tty);
>  
> diff --git a/drivers/net/mctp/mctp-serial.c b/drivers/net/mctp/mctp-serial.c
> index 7cd103fd34ef..f1dfab4c54cb 100644
> --- a/drivers/net/mctp/mctp-serial.c
> +++ b/drivers/net/mctp/mctp-serial.c
> @@ -390,7 +390,7 @@ static void mctp_serial_push(struct mctp_serial *dev, unsigned char c)
>  
>  static void mctp_serial_tty_receive_buf(struct tty_struct *tty,
>  					const unsigned char *c,
> -					const char *f, int len)
> +					const char *f, int len, unsigned int lookahead_count)
>  {
>  	struct mctp_serial *dev = tty->disc_data;
>  	int i;
> diff --git a/drivers/net/ppp/ppp_async.c b/drivers/net/ppp/ppp_async.c
> index 15a179631903..87c205a02984 100644
> --- a/drivers/net/ppp/ppp_async.c
> +++ b/drivers/net/ppp/ppp_async.c
> @@ -338,7 +338,7 @@ ppp_asynctty_poll(struct tty_struct *tty, struct file *file, poll_table *wait)
>  /* May sleep, don't call from interrupt level or with interrupts disabled */
>  static void
>  ppp_asynctty_receive(struct tty_struct *tty, const unsigned char *buf,
> -		  const char *cflags, int count)
> +		     const char *cflags, int count, unsigned int lookahead_count)
>  {
>  	struct asyncppp *ap = ap_get(tty);
>  	unsigned long flags;
> diff --git a/drivers/net/ppp/ppp_synctty.c b/drivers/net/ppp/ppp_synctty.c
> index 18283b7b94bc..b82f4bd82b21 100644
> --- a/drivers/net/ppp/ppp_synctty.c
> +++ b/drivers/net/ppp/ppp_synctty.c
> @@ -331,7 +331,7 @@ ppp_sync_poll(struct tty_struct *tty, struct file *file, poll_table *wait)
>  /* May sleep, don't call from interrupt level or with interrupts disabled */
>  static void
>  ppp_sync_receive(struct tty_struct *tty, const unsigned char *buf,
> -		  const char *cflags, int count)
> +		 const char *cflags, int count, unsigned int lookahead_count)
>  {
>  	struct syncppp *ap = sp_get(tty);
>  	unsigned long flags;
> diff --git a/drivers/net/slip/slip.c b/drivers/net/slip/slip.c
> index 88396ff99f03..7fccf28256a8 100644
> --- a/drivers/net/slip/slip.c
> +++ b/drivers/net/slip/slip.c
> @@ -686,7 +686,7 @@ static void sl_setup(struct net_device *dev)
>   */
>  
>  static void slip_receive_buf(struct tty_struct *tty, const unsigned char *cp,
> -		const char *fp, int count)
> +			     const char *fp, int count, unsigned int lookahead_count)
>  {
>  	struct slip *sl = tty->disc_data;
>  
> diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
> index fa92f727fdf8..45f022892b28 100644
> --- a/drivers/tty/n_gsm.c
> +++ b/drivers/tty/n_gsm.c
> @@ -2500,7 +2500,7 @@ static void gsmld_detach_gsm(struct tty_struct *tty, struct gsm_mux *gsm)
>  }
>  
>  static void gsmld_receive_buf(struct tty_struct *tty, const unsigned char *cp,
> -			      const char *fp, int count)
> +			      const char *fp, int count, unsigned int lookahead_count)
>  {
>  	struct gsm_mux *gsm = tty->disc_data;
>  	char flags = TTY_NORMAL;
> diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
> index 94c1ec2dd754..2d7c94a38a92 100644
> --- a/drivers/tty/n_hdlc.c
> +++ b/drivers/tty/n_hdlc.c
> @@ -379,7 +379,7 @@ static void n_hdlc_tty_wakeup(struct tty_struct *tty)
>   * interpreted as one HDLC frame.
>   */
>  static void n_hdlc_tty_receive(struct tty_struct *tty, const __u8 *data,
> -			       const char *flags, int count)
> +			       const char *flags, int count, unsigned int lookahead_count)
>  {
>  	register struct n_hdlc *n_hdlc = tty->disc_data;
>  	register struct n_hdlc_buf *buf;
> diff --git a/drivers/tty/n_null.c b/drivers/tty/n_null.c
> index f913b665af72..6bce76b5bb1c 100644
> --- a/drivers/tty/n_null.c
> +++ b/drivers/tty/n_null.c
> @@ -34,7 +34,7 @@ static ssize_t n_null_write(struct tty_struct *tty, struct file *file,
>  
>  static void n_null_receivebuf(struct tty_struct *tty,
>  				 const unsigned char *cp, const char *fp,
> -				 int cnt)
> +				 int cnt, unsigned int lookahead_count)
>  {
>  }
>  
> diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
> index 90b3e06cbeb1..708cc85ac43d 100644
> --- a/drivers/tty/n_tty.c
> +++ b/drivers/tty/n_tty.c
> @@ -1226,11 +1226,15 @@ static bool n_tty_is_char_flow_ctrl(struct tty_struct *tty, unsigned char c)
>  }
>  
>  /* 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)
> +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;

Why would this function be called if this option was true?



> +
>  	if (c == START_CHAR(tty)) {
>  		start_tty(tty);
>  		process_echoes(tty);
> @@ -1241,11 +1245,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)) {
> @@ -1400,7 +1405,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)

bool here, yet characters elsewhere?  That's messy :(

the overall idea is good, this implementation isn't quite there yet.

I did take the first patch in the series, it made sense.

thanks,

greg k-h

Re: [PATCH v3 3/5] tty: Add lookahead param to receive_buf

[Ilpo Järvinen]

On Fri, 22 Apr 2022, Greg KH wrote:

> > diff --git a/drivers/accessibility/speakup/spk_ttyio.c b/drivers/accessibility/speakup/spk_ttyio.c
> > index 08cf8a17754b..b33536eea1d3 100644
> > --- a/drivers/accessibility/speakup/spk_ttyio.c
> > +++ b/drivers/accessibility/speakup/spk_ttyio.c
> > @@ -73,7 +73,7 @@ static void spk_ttyio_ldisc_close(struct tty_struct *tty)
> >  
> >  static int spk_ttyio_receive_buf2(struct tty_struct *tty,
> >  				  const unsigned char *cp,
> > -				  const char *fp, int count)
> > +				  const char *fp, int count, unsigned int lookahead_count)
> 
> Ick, adding yet-another-parameter to a function is a mess as it's hard
> to know what to do with this and what it means just by looking at when
> it is called.

To be honest, I didn't like it either but just couldn't find another 
way... That is, not until now that you pushed.

I think I can add lookahead_count into n_tty_data, then both layers 
(n_tty and tty_buffer) that depend on it will indepedently keep track of 
it rather than passing it through the whole callchain.

> >  /* 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)
> > +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;
> 
> Why would this function be called if this option was true?

Agreed, it makes sense to move the check before call (and then I also 
don't need to reorganize this function anymore).

> the overall idea is good, this implementation isn't quite there yet.

Thanks for taking a look.


-- 
 i.

Re: [PATCH v3 3/5] tty: Add lookahead param to receive_buf

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 1707 bytes --]

On Fri, 22 Apr 2022, Ilpo Järvinen wrote:

> On Fri, 22 Apr 2022, Greg KH wrote:
> 
> > >  /* 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)
> > > +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;
> > 
> > Why would this function be called if this option was true?
> 
> Agreed, it makes sense to move the check before call (and then I also 
> don't need to reorganize this function anymore).

I think I want to renege on this. The reason is that on flow control char, 
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

When the check is inside, return value of n_tty_receive_char_flow_ctrl 
decides a), and b) is kept internal to n_tty_receive_char_flow_ctrl().

If I more that lookahead_done check into the caller domain, things get 
IMHO a lot more messy. Effectively, I have three options for the calling 
domain to chose from:

	if (I_IXON(tty)) {
		if (!lookahead_done) {
			if (n_tty_receive_char_flow_ctrl(tty, c))
				return;
		} else if (n_tty_is_char_flow_ctrl(tty, c)) {
			return;
		}
	}

or

	if (I_IXON(tty)) {
		if ((!lookahead_done && n_tty_receive_char_flow_ctrl(tty, c)) ||
		    (lookahead_done && n_tty_is_char_flow_ctrl(tty, c))) {
			return;
	}

vs

        if (I_IXON(tty) && n_tty_receive_char_flow_ctrl(tty, c, lookahead_done))
                return;

I heavily prefer that last option.

-- 
 i.

Re: [PATCH v3 3/5] tty: Add lookahead param to receive_buf

[Greg KH]

On Tue, Apr 26, 2022 at 10:48:40AM +0300, Ilpo Järvinen wrote:
> On Fri, 22 Apr 2022, Ilpo Järvinen wrote:
> 
> > On Fri, 22 Apr 2022, Greg KH wrote:
> > 
> > > >  /* 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)
> > > > +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;
> > > 
> > > Why would this function be called if this option was true?
> > 
> > Agreed, it makes sense to move the check before call (and then I also 
> > don't need to reorganize this function anymore).
> 
> I think I want to renege on this. The reason is that on flow control char, 
> 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
> 
> When the check is inside, return value of n_tty_receive_char_flow_ctrl 
> decides a), and b) is kept internal to n_tty_receive_char_flow_ctrl().
> 
> If I more that lookahead_done check into the caller domain, things get 
> IMHO a lot more messy. Effectively, I have three options for the calling 
> domain to chose from:
> 
> 	if (I_IXON(tty)) {
> 		if (!lookahead_done) {
> 			if (n_tty_receive_char_flow_ctrl(tty, c))
> 				return;
> 		} else if (n_tty_is_char_flow_ctrl(tty, c)) {
> 			return;
> 		}
> 	}
> 
> or
> 
> 	if (I_IXON(tty)) {
> 		if ((!lookahead_done && n_tty_receive_char_flow_ctrl(tty, c)) ||
> 		    (lookahead_done && n_tty_is_char_flow_ctrl(tty, c))) {
> 			return;
> 	}
> 
> vs
> 
>         if (I_IXON(tty) && n_tty_receive_char_flow_ctrl(tty, c, lookahead_done))
>                 return;
> 
> I heavily prefer that last option.

Ok, then document the heck out of it please.

greg k-h