[PATCH v2 3/3] tty: Implement lookahead to process XON/XOFF timely

From: Ilpo Järvinen
Date: Fri Apr 08 2022 - 07:41:06 EST


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@xxxxxxxxxxx>
Tested-by: Gilles Buloz <gilles.buloz@xxxxxxxxxxx>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
---
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 cbea02c662d1..a9d20fcc6595 100644
--- a/drivers/tty/n_tty.c
+++ b/drivers/tty/n_tty.c
@@ -1463,6 +1463,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)
@@ -2418,6 +2435,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