linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH tty-next 0/5] Halve tty buffer memory consumption
@ 2013-11-22 17:09 Peter Hurley
  2013-11-22 17:09 ` [PATCH tty-next 1/5] tty: Always handle NULL flag ptr Peter Hurley
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Peter Hurley @ 2013-11-22 17:09 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Jiri Slaby, linux-kernel, linux-serial, Peter Hurley, Dmitry Torokhov

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Greg,

This patchset implements a method of encoding tty 'flip' buffers
such that storage is not allocated for per-byte-received 'flag' bytes
if the data received contains no error.

The N_TTY line discipline already contains logic which assumes that
if the flags buffer ptr is NULL, all data is assumed to be TTY_NORMAL.

Patch 1 documents this behavior and fixes line disciplines not already
handling this condition. The INPUT maintainer is Cc'd on the N_MOUSE
changes.
Patch 2 enables the port driver to alter the default 64k limit
imposed by the tty_buffer core. Hi-throughput drivers (>10MB/sec) can
use this to reduce/eliminate dropped data. A forthcoming fwserial patchset
does just this to remove local rx buffering (and 100 lines of code).
Patch 3 s/memory_used/mem_used
Patch 4 removes the unused tty_prepare_flip_string_flags() function
Patch 5 implements the NULL flag buffer encoding scheme.

For each allocated flip buffer, a bitfield is added which is
used to differentiate a flip buffer without a flags buffer (in which
all data is TTY_NORMAL) from a flip buffer containing 'flagged' data.

If the current flip buffer has no flags field and new 'flagged' data
is inserted, the current flip buffer is committed and a new current
flip buffer is allocated with a flags buffer.

Users of tty_insert_flip_string_flag() always allocates a flags
buffer on the assumption that data is not entirely TTY_NORMAL.

Regards,

Peter Hurley (5):
  tty: Always handle NULL flag ptr
  tty: Enable configurable tty flip buffer limit
  tty: Rename tty buffer memory_used field
  tty: Remove tty_prepare_flip_string_flags()
  tty: Halve flip buffer GFP_ATOMIC memory consumption

 drivers/input/serio/serport.c |  28 +++++------
 drivers/tty/n_gsm.c           |   5 +-
 drivers/tty/n_r3964.c         |   2 +-
 drivers/tty/tty_buffer.c      | 105 ++++++++++++++++++++++++------------------
 include/linux/tty.h           |   7 ++-
 include/linux/tty_flip.h      |  11 +++--
 include/linux/tty_ldisc.h     |   6 ++-
 7 files changed, 96 insertions(+), 68 deletions(-)

-- 
1.8.1.2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH tty-next 1/5] tty: Always handle NULL flag ptr
  2013-11-22 17:09 [PATCH tty-next 0/5] Halve tty buffer memory consumption Peter Hurley
@ 2013-11-22 17:09 ` Peter Hurley
  2013-11-22 22:20   ` Dmitry Torokhov
  2013-11-22 17:09 ` [PATCH tty-next 2/5] tty: Enable configurable tty flip buffer limit Peter Hurley
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 13+ messages in thread
From: Peter Hurley @ 2013-11-22 17:09 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Jiri Slaby, linux-kernel, linux-serial, Peter Hurley, Dmitry Torokhov

Most line disciplines already handle the undocumented NULL flag
ptr in their .receive_buf method; however, several don't.

Document the NULL flag ptr, and correct handling in the
N_MOUSE, N_GSM0710 and N_R394 line disciplines.

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/input/serio/serport.c | 28 +++++++++++++++-------------
 drivers/tty/n_gsm.c           |  5 +++--
 drivers/tty/n_r3964.c         |  2 +-
 include/linux/tty_ldisc.h     |  6 ++++--
 4 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/drivers/input/serio/serport.c b/drivers/input/serio/serport.c
index 8755f5f..72b4633 100644
--- a/drivers/input/serio/serport.c
+++ b/drivers/input/serio/serport.c
@@ -124,7 +124,7 @@ static void serport_ldisc_receive(struct tty_struct *tty, const unsigned char *c
 {
 	struct serport *serport = (struct serport*) tty->disc_data;
 	unsigned long flags;
-	unsigned int ch_flags;
+	unsigned int ch_flags = TTY_NORMAL;
 	int i;
 
 	spin_lock_irqsave(&serport->lock, flags);
@@ -133,18 +133,20 @@ static void serport_ldisc_receive(struct tty_struct *tty, const unsigned char *c
 		goto out;
 
 	for (i = 0; i < count; i++) {
-		switch (fp[i]) {
-		case TTY_FRAME:
-			ch_flags = SERIO_FRAME;
-			break;
-
-		case TTY_PARITY:
-			ch_flags = SERIO_PARITY;
-			break;
-
-		default:
-			ch_flags = 0;
-			break;
+		if (fp) {
+			switch (fp[i]) {
+			case TTY_FRAME:
+				ch_flags = SERIO_FRAME;
+				break;
+
+			case TTY_PARITY:
+				ch_flags = SERIO_PARITY;
+				break;
+
+			default:
+				ch_flags = TTY_NORMAL;
+				break;
+			}
 		}
 
 		serio_interrupt(serport->serio, cp[i], ch_flags);
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index c0f76da..c09db11 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -2269,14 +2269,15 @@ static void gsmld_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 	char *f;
 	int i;
 	char buf[64];
-	char flags;
+	char flags = TTY_NORMAL;
 
 	if (debug & 4)
 		print_hex_dump_bytes("gsmld_receive: ", DUMP_PREFIX_OFFSET,
 				     cp, count);
 
 	for (i = count, dp = cp, f = fp; i; i--, dp++) {
-		flags = *f++;
+		if (f)
+			flags = *f++;
 		switch (flags) {
 		case TTY_NORMAL:
 			gsm->receive(gsm, *dp);
diff --git a/drivers/tty/n_r3964.c b/drivers/tty/n_r3964.c
index 1e64050..8b157d6 100644
--- a/drivers/tty/n_r3964.c
+++ b/drivers/tty/n_r3964.c
@@ -1244,7 +1244,7 @@ static void r3964_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 {
 	struct r3964_info *pInfo = tty->disc_data;
 	const unsigned char *p;
-	char *f, flags = 0;
+	char *f, flags = TTY_NORMAL;
 	int i;
 
 	for (i = count, p = cp, f = fp; i; i--, p++) {
diff --git a/include/linux/tty_ldisc.h b/include/linux/tty_ldisc.h
index f15c898..b8347c2 100644
--- a/include/linux/tty_ldisc.h
+++ b/include/linux/tty_ldisc.h
@@ -84,7 +84,8 @@
  *	processing.  <cp> is a pointer to the buffer of input
  *	character received by the device.  <fp> is a pointer to a
  *	pointer of flag bytes which indicate whether a character was
- *	received with a parity error, etc.
+ *	received with a parity error, etc. <fp> may be NULL to indicate
+ *	all data received is TTY_NORMAL.
  *
  * void	(*write_wakeup)(struct tty_struct *);
  *
@@ -118,7 +119,8 @@
  *	processing.  <cp> is a pointer to the buffer of input
  *	character received by the device.  <fp> is a pointer to a
  *	pointer of flag bytes which indicate whether a character was
- *	received with a parity error, etc.
+ *	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.
  */
 
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH tty-next 2/5] tty: Enable configurable tty flip buffer limit
  2013-11-22 17:09 [PATCH tty-next 0/5] Halve tty buffer memory consumption Peter Hurley
  2013-11-22 17:09 ` [PATCH tty-next 1/5] tty: Always handle NULL flag ptr Peter Hurley
@ 2013-11-22 17:09 ` Peter Hurley
  2013-11-22 17:09 ` [PATCH tty-next 3/5] tty: Rename tty buffer memory_used field Peter Hurley
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Peter Hurley @ 2013-11-22 17:09 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, linux-serial, Peter Hurley

Allow driver to configure its maximum flip buffer memory
consumption/limit. This is necessary for very-high speed line
rates (in excess of 10MB/sec) because the flip buffers can
be saturated before the line discipline has a chance to
throttle the input.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_buffer.c | 24 +++++++++++++++++++++---
 include/linux/tty.h      |  1 +
 include/linux/tty_flip.h |  1 +
 3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index c043136f..57eb34b 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -26,7 +26,7 @@
  * Byte threshold to limit memory consumption for flip buffers.
  * The actual memory limit is > 2x this amount.
  */
-#define TTYB_MEM_LIMIT	65536
+#define TTYB_DEFAULT_MEM_LIMIT	65536
 
 /*
  * We default to dicing tty buffer allocations to this many characters
@@ -89,7 +89,7 @@ void tty_buffer_unlock_exclusive(struct tty_port *port)
 
 int tty_buffer_space_avail(struct tty_port *port)
 {
-	int space = TTYB_MEM_LIMIT - atomic_read(&port->buf.memory_used);
+	int space = port->buf.mem_limit - atomic_read(&port->buf.memory_used);
 	return max(space, 0);
 }
 
@@ -162,7 +162,7 @@ static struct tty_buffer *tty_buffer_alloc(struct tty_port *port, size_t size)
 
 	/* Should possibly check if this fails for the largest buffer we
 	   have queued and recycle that ? */
-	if (atomic_read(&port->buf.memory_used) > TTYB_MEM_LIMIT)
+	if (atomic_read(&port->buf.memory_used) > port->buf.mem_limit)
 		return NULL;
 	p = kmalloc(sizeof(struct tty_buffer) + 2 * size, GFP_ATOMIC);
 	if (p == NULL)
@@ -536,4 +536,22 @@ void tty_buffer_init(struct tty_port *port)
 	atomic_set(&buf->memory_used, 0);
 	atomic_set(&buf->priority, 0);
 	INIT_WORK(&buf->work, flush_to_ldisc);
+	buf->mem_limit = TTYB_DEFAULT_MEM_LIMIT;
 }
+
+/**
+ *	tty_buffer_set_limit	-	change the tty buffer memory limit
+ *	@port: tty port to change
+ *
+ *	Change the tty buffer memory limit.
+ *	Must be called before the other tty buffer functions are used.
+ */
+
+int tty_buffer_set_limit(struct tty_port *port, int limit)
+{
+	if (limit < MIN_TTYB_SIZE)
+		return -EINVAL;
+	port->buf.mem_limit = limit;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(tty_buffer_set_limit);
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 2f47989..58a34e7 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -61,6 +61,7 @@ struct tty_bufhead {
 	struct tty_buffer sentinel;
 	struct llist_head free;		/* Free queue head */
 	atomic_t	   memory_used; /* In-use buffers excluding free list */
+	int		   mem_limit;
 	struct tty_buffer *tail;	/* Active buffer */
 };
 /*
diff --git a/include/linux/tty_flip.h b/include/linux/tty_flip.h
index 21ddd7d..2da8bc2 100644
--- a/include/linux/tty_flip.h
+++ b/include/linux/tty_flip.h
@@ -1,6 +1,7 @@
 #ifndef _LINUX_TTY_FLIP_H
 #define _LINUX_TTY_FLIP_H
 
+extern int tty_buffer_set_limit(struct tty_port *port, int limit);
 extern int tty_buffer_space_avail(struct tty_port *port);
 extern int tty_buffer_request_room(struct tty_port *port, size_t size);
 extern int tty_insert_flip_string_flags(struct tty_port *port,
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH tty-next 3/5] tty: Rename tty buffer memory_used field
  2013-11-22 17:09 [PATCH tty-next 0/5] Halve tty buffer memory consumption Peter Hurley
  2013-11-22 17:09 ` [PATCH tty-next 1/5] tty: Always handle NULL flag ptr Peter Hurley
  2013-11-22 17:09 ` [PATCH tty-next 2/5] tty: Enable configurable tty flip buffer limit Peter Hurley
@ 2013-11-22 17:09 ` Peter Hurley
  2013-11-22 17:09 ` [PATCH tty-next 4/5] tty: Remove tty_prepare_flip_string_flags() Peter Hurley
  2013-11-22 17:09 ` [PATCH tty-next 5/5] tty: Halve flip buffer GFP_ATOMIC memory consumption Peter Hurley
  4 siblings, 0 replies; 13+ messages in thread
From: Peter Hurley @ 2013-11-22 17:09 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, linux-serial, Peter Hurley

Trim up the memory_used field name to mem_used.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_buffer.c | 12 ++++++------
 include/linux/tty.h      |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 57eb34b..e8f22f8 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -89,7 +89,7 @@ void tty_buffer_unlock_exclusive(struct tty_port *port)
 
 int tty_buffer_space_avail(struct tty_port *port)
 {
-	int space = port->buf.mem_limit - atomic_read(&port->buf.memory_used);
+	int space = port->buf.mem_limit - atomic_read(&port->buf.mem_used);
 	return max(space, 0);
 }
 
@@ -129,7 +129,7 @@ void tty_buffer_free_all(struct tty_port *port)
 	buf->head = &buf->sentinel;
 	buf->tail = &buf->sentinel;
 
-	atomic_set(&buf->memory_used, 0);
+	atomic_set(&buf->mem_used, 0);
 }
 
 /**
@@ -162,7 +162,7 @@ static struct tty_buffer *tty_buffer_alloc(struct tty_port *port, size_t size)
 
 	/* Should possibly check if this fails for the largest buffer we
 	   have queued and recycle that ? */
-	if (atomic_read(&port->buf.memory_used) > port->buf.mem_limit)
+	if (atomic_read(&port->buf.mem_used) > port->buf.mem_limit)
 		return NULL;
 	p = kmalloc(sizeof(struct tty_buffer) + 2 * size, GFP_ATOMIC);
 	if (p == NULL)
@@ -170,7 +170,7 @@ static struct tty_buffer *tty_buffer_alloc(struct tty_port *port, size_t size)
 
 found:
 	tty_buffer_reset(p, size);
-	atomic_add(size, &port->buf.memory_used);
+	atomic_add(size, &port->buf.mem_used);
 	return p;
 }
 
@@ -188,7 +188,7 @@ static void tty_buffer_free(struct tty_port *port, struct tty_buffer *b)
 	struct tty_bufhead *buf = &port->buf;
 
 	/* Dumb strategy for now - should keep some stats */
-	WARN_ON(atomic_sub_return(b->size, &buf->memory_used) < 0);
+	WARN_ON(atomic_sub_return(b->size, &buf->mem_used) < 0);
 
 	if (b->size > MIN_TTYB_SIZE)
 		kfree(b);
@@ -533,7 +533,7 @@ void tty_buffer_init(struct tty_port *port)
 	buf->head = &buf->sentinel;
 	buf->tail = &buf->sentinel;
 	init_llist_head(&buf->free);
-	atomic_set(&buf->memory_used, 0);
+	atomic_set(&buf->mem_used, 0);
 	atomic_set(&buf->priority, 0);
 	INIT_WORK(&buf->work, flush_to_ldisc);
 	buf->mem_limit = TTYB_DEFAULT_MEM_LIMIT;
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 58a34e7..ed9aedd 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -60,7 +60,7 @@ struct tty_bufhead {
 	atomic_t	   priority;
 	struct tty_buffer sentinel;
 	struct llist_head free;		/* Free queue head */
-	atomic_t	   memory_used; /* In-use buffers excluding free list */
+	atomic_t	   mem_used;    /* In-use buffers excluding free list */
 	int		   mem_limit;
 	struct tty_buffer *tail;	/* Active buffer */
 };
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH tty-next 4/5] tty: Remove tty_prepare_flip_string_flags()
  2013-11-22 17:09 [PATCH tty-next 0/5] Halve tty buffer memory consumption Peter Hurley
                   ` (2 preceding siblings ...)
  2013-11-22 17:09 ` [PATCH tty-next 3/5] tty: Rename tty buffer memory_used field Peter Hurley
@ 2013-11-22 17:09 ` Peter Hurley
  2013-11-22 17:09 ` [PATCH tty-next 5/5] tty: Halve flip buffer GFP_ATOMIC memory consumption Peter Hurley
  4 siblings, 0 replies; 13+ messages in thread
From: Peter Hurley @ 2013-11-22 17:09 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, linux-serial, Peter Hurley

There is no in-tree user of tty_prepare_flip_string_flags(); remove.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_buffer.c | 28 ----------------------------
 include/linux/tty_flip.h |  2 --
 2 files changed, 30 deletions(-)

diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index e8f22f8..6a3620e 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -373,34 +373,6 @@ int tty_prepare_flip_string(struct tty_port *port, unsigned char **chars,
 }
 EXPORT_SYMBOL_GPL(tty_prepare_flip_string);
 
-/**
- *	tty_prepare_flip_string_flags	-	make room for characters
- *	@port: tty port
- *	@chars: return pointer for character write area
- *	@flags: return pointer for status flag write area
- *	@size: desired size
- *
- *	Prepare a block of space in the buffer for data. Returns the length
- *	available and buffer pointer to the space which is now allocated and
- *	accounted for as ready for characters. This is used for drivers
- *	that need their own block copy routines into the buffer. There is no
- *	guarantee the buffer is a DMA target!
- */
-
-int tty_prepare_flip_string_flags(struct tty_port *port,
-			unsigned char **chars, char **flags, size_t size)
-{
-	int space = tty_buffer_request_room(port, size);
-	if (likely(space)) {
-		struct tty_buffer *tb = port->buf.tail;
-		*chars = char_buf_ptr(tb, tb->used);
-		*flags = flag_buf_ptr(tb, tb->used);
-		tb->used += space;
-	}
-	return space;
-}
-EXPORT_SYMBOL_GPL(tty_prepare_flip_string_flags);
-
 
 static int
 receive_buf(struct tty_struct *tty, struct tty_buffer *head, int count)
diff --git a/include/linux/tty_flip.h b/include/linux/tty_flip.h
index 2da8bc2..3f821e9 100644
--- a/include/linux/tty_flip.h
+++ b/include/linux/tty_flip.h
@@ -10,8 +10,6 @@ extern int tty_insert_flip_string_fixed_flag(struct tty_port *port,
 		const unsigned char *chars, char flag, size_t size);
 extern int tty_prepare_flip_string(struct tty_port *port,
 		unsigned char **chars, size_t size);
-extern int tty_prepare_flip_string_flags(struct tty_port *port,
-		unsigned char **chars, char **flags, size_t size);
 extern void tty_flip_buffer_push(struct tty_port *port);
 void tty_schedule_flip(struct tty_port *port);
 
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH tty-next 5/5] tty: Halve flip buffer GFP_ATOMIC memory consumption
  2013-11-22 17:09 [PATCH tty-next 0/5] Halve tty buffer memory consumption Peter Hurley
                   ` (3 preceding siblings ...)
  2013-11-22 17:09 ` [PATCH tty-next 4/5] tty: Remove tty_prepare_flip_string_flags() Peter Hurley
@ 2013-11-22 17:09 ` Peter Hurley
  2013-12-09  1:01   ` Greg Kroah-Hartman
  4 siblings, 1 reply; 13+ messages in thread
From: Peter Hurley @ 2013-11-22 17:09 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, linux-serial, Peter Hurley

tty flip buffers use GFP_ATOMIC allocations for received data
which is to be processed by the line discipline. For each byte
received, an extra byte is used to indicate the error status of
that byte.

Instead, if the received data is error-free, encode the entire
buffer without status bytes.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/tty_buffer.c | 45 +++++++++++++++++++++++++++++++++++----------
 include/linux/tty.h      |  4 ++++
 include/linux/tty_flip.h |  8 ++++++--
 3 files changed, 45 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 6a3620e..c4fe20e 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -100,6 +100,7 @@ static void tty_buffer_reset(struct tty_buffer *p, size_t size)
 	p->next = NULL;
 	p->commit = 0;
 	p->read = 0;
+	p->flags = 0;
 }
 
 /**
@@ -230,31 +231,49 @@ void tty_buffer_flush(struct tty_struct *tty)
  *	tty_buffer_request_room		-	grow tty buffer if needed
  *	@tty: tty structure
  *	@size: size desired
+ *	@flags: buffer flags if new buffer allocated (default = 0)
  *
  *	Make at least size bytes of linear space available for the tty
  *	buffer. If we fail return the size we managed to find.
+ *
+ *	Will change over to a new buffer if the current buffer is encoded as
+ *	TTY_NORMAL (so has no flags buffer) and the new buffer requires
+ *	a flags buffer.
  */
-int tty_buffer_request_room(struct tty_port *port, size_t size)
+int __tty_buffer_request_room(struct tty_port *port, size_t size, int flags)
 {
 	struct tty_bufhead *buf = &port->buf;
 	struct tty_buffer *b, *n;
-	int left;
+	int left, change;
 
 	b = buf->tail;
-	left = b->size - b->used;
+	if (b->flags & TTYB_NORMAL)
+		left = 2 * b->size - b->used;
+	else
+		left = b->size - b->used;
 
-	if (left < size) {
+	change = (b->flags & TTYB_NORMAL) && (~flags & TTYB_NORMAL);
+	if (change || left < size) {
 		/* This is the slow path - looking for new buffers to use */
 		if ((n = tty_buffer_alloc(port, size)) != NULL) {
+			n->flags = flags;
 			buf->tail = n;
 			b->commit = b->used;
 			smp_mb();
 			b->next = n;
-		} else
+		} else if (change)
+			size = 0;
+		else
 			size = left;
 	}
 	return size;
 }
+EXPORT_SYMBOL_GPL(__tty_buffer_request_room);
+
+int tty_buffer_request_room(struct tty_port *port, size_t size)
+{
+	return __tty_buffer_request_room(port, size, 0);
+}
 EXPORT_SYMBOL_GPL(tty_buffer_request_room);
 
 /**
@@ -274,12 +293,14 @@ int tty_insert_flip_string_fixed_flag(struct tty_port *port,
 	int copied = 0;
 	do {
 		int goal = min_t(size_t, size - copied, TTY_BUFFER_PAGE);
-		int space = tty_buffer_request_room(port, goal);
+		int flags = (flag == TTY_NORMAL) ? TTYB_NORMAL : 0;
+		int space = __tty_buffer_request_room(port, goal, flags);
 		struct tty_buffer *tb = port->buf.tail;
 		if (unlikely(space == 0))
 			break;
 		memcpy(char_buf_ptr(tb, tb->used), chars, space);
-		memset(flag_buf_ptr(tb, tb->used), flag, space);
+		if (~tb->flags & TTYB_NORMAL)
+			memset(flag_buf_ptr(tb, tb->used), flag, space);
 		tb->used += space;
 		copied += space;
 		chars += space;
@@ -362,11 +383,12 @@ EXPORT_SYMBOL(tty_schedule_flip);
 int tty_prepare_flip_string(struct tty_port *port, unsigned char **chars,
 		size_t size)
 {
-	int space = tty_buffer_request_room(port, size);
+	int space = __tty_buffer_request_room(port, size, TTYB_NORMAL);
 	if (likely(space)) {
 		struct tty_buffer *tb = port->buf.tail;
 		*chars = char_buf_ptr(tb, tb->used);
-		memset(flag_buf_ptr(tb, tb->used), TTY_NORMAL, space);
+		if (~tb->flags & TTYB_NORMAL)
+			memset(flag_buf_ptr(tb, tb->used), TTY_NORMAL, space);
 		tb->used += space;
 	}
 	return space;
@@ -379,7 +401,10 @@ receive_buf(struct tty_struct *tty, struct tty_buffer *head, int count)
 {
 	struct tty_ldisc *disc = tty->ldisc;
 	unsigned char *p = char_buf_ptr(head, head->read);
-	char	      *f = flag_buf_ptr(head, head->read);
+	char	      *f = NULL;
+
+	if (~head->flags & TTYB_NORMAL)
+		f = flag_buf_ptr(head, head->read);
 
 	if (disc->ops->receive_buf2)
 		count = disc->ops->receive_buf2(tty, p, f, count);
diff --git a/include/linux/tty.h b/include/linux/tty.h
index ed9aedd..9d234dc 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -39,10 +39,14 @@ struct tty_buffer {
 	int size;
 	int commit;
 	int read;
+	int flags;
 	/* Data points here */
 	unsigned long data[0];
 };
 
+/* Values for .flags field of tty_buffer */
+#define TTYB_NORMAL	1	/* buffer has no flags buffer */
+
 static inline unsigned char *char_buf_ptr(struct tty_buffer *b, int ofs)
 {
 	return ((unsigned char *)b->data) + ofs;
diff --git a/include/linux/tty_flip.h b/include/linux/tty_flip.h
index 3f821e9..c28dd52 100644
--- a/include/linux/tty_flip.h
+++ b/include/linux/tty_flip.h
@@ -17,8 +17,12 @@ static inline int tty_insert_flip_char(struct tty_port *port,
 					unsigned char ch, char flag)
 {
 	struct tty_buffer *tb = port->buf.tail;
-	if (tb && tb->used < tb->size) {
-		*flag_buf_ptr(tb, tb->used) = flag;
+	int change;
+
+	change = (tb->flags & TTYB_NORMAL) && (flag != TTY_NORMAL);
+	if (!change && tb->used < tb->size) {
+		if (~tb->flags & TTYB_NORMAL)
+			*flag_buf_ptr(tb, tb->used) = flag;
 		*char_buf_ptr(tb, tb->used++) = ch;
 		return 1;
 	}
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH tty-next 1/5] tty: Always handle NULL flag ptr
  2013-11-22 17:09 ` [PATCH tty-next 1/5] tty: Always handle NULL flag ptr Peter Hurley
@ 2013-11-22 22:20   ` Dmitry Torokhov
  2013-11-26  2:00     ` Peter Hurley
  2013-12-02 18:56     ` [PATCH v2 " Peter Hurley
  0 siblings, 2 replies; 13+ messages in thread
From: Dmitry Torokhov @ 2013-11-22 22:20 UTC (permalink / raw)
  To: Peter Hurley; +Cc: Greg Kroah-Hartman, Jiri Slaby, linux-kernel, linux-serial

Hi Peter,

On Fri, Nov 22, 2013 at 12:09:54PM -0500, Peter Hurley wrote:
> Most line disciplines already handle the undocumented NULL flag
> ptr in their .receive_buf method; however, several don't.
> 
> Document the NULL flag ptr, and correct handling in the
> N_MOUSE, N_GSM0710 and N_R394 line disciplines.
> 
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
> ---
>  drivers/input/serio/serport.c | 28 +++++++++++++++-------------
>  drivers/tty/n_gsm.c           |  5 +++--
>  drivers/tty/n_r3964.c         |  2 +-
>  include/linux/tty_ldisc.h     |  6 ++++--
>  4 files changed, 23 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/input/serio/serport.c b/drivers/input/serio/serport.c
> index 8755f5f..72b4633 100644
> --- a/drivers/input/serio/serport.c
> +++ b/drivers/input/serio/serport.c
> @@ -124,7 +124,7 @@ static void serport_ldisc_receive(struct tty_struct *tty, const unsigned char *c
>  {
>  	struct serport *serport = (struct serport*) tty->disc_data;
>  	unsigned long flags;
> -	unsigned int ch_flags;
> +	unsigned int ch_flags = TTY_NORMAL;

We should not be passing tty constants into serio layer as they are
different subsystems (even though TTY_NORMAL happens to be encoded as 0
and thus happens to work). Please use 0 for 'no flags' as the original
code did.

Other than that the serio portion looks good to me.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH tty-next 1/5] tty: Always handle NULL flag ptr
  2013-11-22 22:20   ` Dmitry Torokhov
@ 2013-11-26  2:00     ` Peter Hurley
  2013-12-02 18:56     ` [PATCH v2 " Peter Hurley
  1 sibling, 0 replies; 13+ messages in thread
From: Peter Hurley @ 2013-11-26  2:00 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Greg Kroah-Hartman, Jiri Slaby, linux-kernel, linux-serial

On 11/22/2013 05:20 PM, Dmitry Torokhov wrote:
> Hi Peter,
>
> On Fri, Nov 22, 2013 at 12:09:54PM -0500, Peter Hurley wrote:
>> Most line disciplines already handle the undocumented NULL flag
>> ptr in their .receive_buf method; however, several don't.
>>
>> Document the NULL flag ptr, and correct handling in the
>> N_MOUSE, N_GSM0710 and N_R394 line disciplines.
>>
>> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
>> ---
>>   drivers/input/serio/serport.c | 28 +++++++++++++++-------------
>>   drivers/tty/n_gsm.c           |  5 +++--
>>   drivers/tty/n_r3964.c         |  2 +-
>>   include/linux/tty_ldisc.h     |  6 ++++--
>>   4 files changed, 23 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/input/serio/serport.c b/drivers/input/serio/serport.c
>> index 8755f5f..72b4633 100644
>> --- a/drivers/input/serio/serport.c
>> +++ b/drivers/input/serio/serport.c
>> @@ -124,7 +124,7 @@ static void serport_ldisc_receive(struct tty_struct *tty, const unsigned char *c
>>   {
>>   	struct serport *serport = (struct serport*) tty->disc_data;
>>   	unsigned long flags;
>> -	unsigned int ch_flags;
>> +	unsigned int ch_flags = TTY_NORMAL;
>
> We should not be passing tty constants into serio layer as they are
> different subsystems (even though TTY_NORMAL happens to be encoded as 0
> and thus happens to work). Please use 0 for 'no flags' as the original
> code did.

Ah, I see your point. I'll respin this for your ack.

Regards,
Peter Hurley


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 tty-next 1/5] tty: Always handle NULL flag ptr
  2013-11-22 22:20   ` Dmitry Torokhov
  2013-11-26  2:00     ` Peter Hurley
@ 2013-12-02 18:56     ` Peter Hurley
  2013-12-02 19:09       ` Dmitry Torokhov
  1 sibling, 1 reply; 13+ messages in thread
From: Peter Hurley @ 2013-12-02 18:56 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Dmitry Torokhov
  Cc: linux-serial, linux-kernel, Jiri Slaby, Peter Hurley

Most line disciplines already handle the undocumented NULL flag
ptr in their .receive_buf method; however, several don't.

Document the NULL flag ptr, and correct handling in the
N_MOUSE, N_GSM0710 and N_R394 line disciplines.

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---

v2 - Uses 0 for no flags to serio layer per Dmitry.

 drivers/input/serio/serport.c | 28 +++++++++++++++-------------
 drivers/tty/n_gsm.c           |  5 +++--
 drivers/tty/n_r3964.c         |  2 +-
 include/linux/tty_ldisc.h     |  6 ++++--
 4 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/drivers/input/serio/serport.c b/drivers/input/serio/serport.c
index 8755f5f..0cb7ef5 100644
--- a/drivers/input/serio/serport.c
+++ b/drivers/input/serio/serport.c
@@ -124,7 +124,7 @@ static void serport_ldisc_receive(struct tty_struct *tty, const unsigned char *c
 {
 	struct serport *serport = (struct serport*) tty->disc_data;
 	unsigned long flags;
-	unsigned int ch_flags;
+	unsigned int ch_flags = 0;
 	int i;
 
 	spin_lock_irqsave(&serport->lock, flags);
@@ -133,18 +133,20 @@ static void serport_ldisc_receive(struct tty_struct *tty, const unsigned char *c
 		goto out;
 
 	for (i = 0; i < count; i++) {
-		switch (fp[i]) {
-		case TTY_FRAME:
-			ch_flags = SERIO_FRAME;
-			break;
-
-		case TTY_PARITY:
-			ch_flags = SERIO_PARITY;
-			break;
-
-		default:
-			ch_flags = 0;
-			break;
+		if (fp) {
+			switch (fp[i]) {
+			case TTY_FRAME:
+				ch_flags = SERIO_FRAME;
+				break;
+
+			case TTY_PARITY:
+				ch_flags = SERIO_PARITY;
+				break;
+
+			default:
+				ch_flags = 0;
+				break;
+			}
 		}
 
 		serio_interrupt(serport->serio, cp[i], ch_flags);
diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
index c0f76da..c09db11 100644
--- a/drivers/tty/n_gsm.c
+++ b/drivers/tty/n_gsm.c
@@ -2269,14 +2269,15 @@ static void gsmld_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 	char *f;
 	int i;
 	char buf[64];
-	char flags;
+	char flags = TTY_NORMAL;
 
 	if (debug & 4)
 		print_hex_dump_bytes("gsmld_receive: ", DUMP_PREFIX_OFFSET,
 				     cp, count);
 
 	for (i = count, dp = cp, f = fp; i; i--, dp++) {
-		flags = *f++;
+		if (f)
+			flags = *f++;
 		switch (flags) {
 		case TTY_NORMAL:
 			gsm->receive(gsm, *dp);
diff --git a/drivers/tty/n_r3964.c b/drivers/tty/n_r3964.c
index 1e64050..8b157d6 100644
--- a/drivers/tty/n_r3964.c
+++ b/drivers/tty/n_r3964.c
@@ -1244,7 +1244,7 @@ static void r3964_receive_buf(struct tty_struct *tty, const unsigned char *cp,
 {
 	struct r3964_info *pInfo = tty->disc_data;
 	const unsigned char *p;
-	char *f, flags = 0;
+	char *f, flags = TTY_NORMAL;
 	int i;
 
 	for (i = count, p = cp, f = fp; i; i--, p++) {
diff --git a/include/linux/tty_ldisc.h b/include/linux/tty_ldisc.h
index f15c898..b8347c2 100644
--- a/include/linux/tty_ldisc.h
+++ b/include/linux/tty_ldisc.h
@@ -84,7 +84,8 @@
  *	processing.  <cp> is a pointer to the buffer of input
  *	character received by the device.  <fp> is a pointer to a
  *	pointer of flag bytes which indicate whether a character was
- *	received with a parity error, etc.
+ *	received with a parity error, etc. <fp> may be NULL to indicate
+ *	all data received is TTY_NORMAL.
  *
  * void	(*write_wakeup)(struct tty_struct *);
  *
@@ -118,7 +119,8 @@
  *	processing.  <cp> is a pointer to the buffer of input
  *	character received by the device.  <fp> is a pointer to a
  *	pointer of flag bytes which indicate whether a character was
- *	received with a parity error, etc.
+ *	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.
  */
 
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 tty-next 1/5] tty: Always handle NULL flag ptr
  2013-12-02 18:56     ` [PATCH v2 " Peter Hurley
@ 2013-12-02 19:09       ` Dmitry Torokhov
  0 siblings, 0 replies; 13+ messages in thread
From: Dmitry Torokhov @ 2013-12-02 19:09 UTC (permalink / raw)
  To: Peter Hurley; +Cc: Greg Kroah-Hartman, linux-serial, linux-kernel, Jiri Slaby

On Mon, Dec 02, 2013 at 01:56:03PM -0500, Peter Hurley wrote:
> Most line disciplines already handle the undocumented NULL flag
> ptr in their .receive_buf method; however, several don't.
> 
> Document the NULL flag ptr, and correct handling in the
> N_MOUSE, N_GSM0710 and N_R394 line disciplines.
> 
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>

For serport:

	Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Thanks.

> ---
> 
> v2 - Uses 0 for no flags to serio layer per Dmitry.
> 
>  drivers/input/serio/serport.c | 28 +++++++++++++++-------------
>  drivers/tty/n_gsm.c           |  5 +++--
>  drivers/tty/n_r3964.c         |  2 +-
>  include/linux/tty_ldisc.h     |  6 ++++--
>  4 files changed, 23 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/input/serio/serport.c b/drivers/input/serio/serport.c
> index 8755f5f..0cb7ef5 100644
> --- a/drivers/input/serio/serport.c
> +++ b/drivers/input/serio/serport.c
> @@ -124,7 +124,7 @@ static void serport_ldisc_receive(struct tty_struct *tty, const unsigned char *c
>  {
>  	struct serport *serport = (struct serport*) tty->disc_data;
>  	unsigned long flags;
> -	unsigned int ch_flags;
> +	unsigned int ch_flags = 0;
>  	int i;
>  
>  	spin_lock_irqsave(&serport->lock, flags);
> @@ -133,18 +133,20 @@ static void serport_ldisc_receive(struct tty_struct *tty, const unsigned char *c
>  		goto out;
>  
>  	for (i = 0; i < count; i++) {
> -		switch (fp[i]) {
> -		case TTY_FRAME:
> -			ch_flags = SERIO_FRAME;
> -			break;
> -
> -		case TTY_PARITY:
> -			ch_flags = SERIO_PARITY;
> -			break;
> -
> -		default:
> -			ch_flags = 0;
> -			break;
> +		if (fp) {
> +			switch (fp[i]) {
> +			case TTY_FRAME:
> +				ch_flags = SERIO_FRAME;
> +				break;
> +
> +			case TTY_PARITY:
> +				ch_flags = SERIO_PARITY;
> +				break;
> +
> +			default:
> +				ch_flags = 0;
> +				break;
> +			}
>  		}
>  
>  		serio_interrupt(serport->serio, cp[i], ch_flags);
> diff --git a/drivers/tty/n_gsm.c b/drivers/tty/n_gsm.c
> index c0f76da..c09db11 100644
> --- a/drivers/tty/n_gsm.c
> +++ b/drivers/tty/n_gsm.c
> @@ -2269,14 +2269,15 @@ static void gsmld_receive_buf(struct tty_struct *tty, const unsigned char *cp,
>  	char *f;
>  	int i;
>  	char buf[64];
> -	char flags;
> +	char flags = TTY_NORMAL;
>  
>  	if (debug & 4)
>  		print_hex_dump_bytes("gsmld_receive: ", DUMP_PREFIX_OFFSET,
>  				     cp, count);
>  
>  	for (i = count, dp = cp, f = fp; i; i--, dp++) {
> -		flags = *f++;
> +		if (f)
> +			flags = *f++;
>  		switch (flags) {
>  		case TTY_NORMAL:
>  			gsm->receive(gsm, *dp);
> diff --git a/drivers/tty/n_r3964.c b/drivers/tty/n_r3964.c
> index 1e64050..8b157d6 100644
> --- a/drivers/tty/n_r3964.c
> +++ b/drivers/tty/n_r3964.c
> @@ -1244,7 +1244,7 @@ static void r3964_receive_buf(struct tty_struct *tty, const unsigned char *cp,
>  {
>  	struct r3964_info *pInfo = tty->disc_data;
>  	const unsigned char *p;
> -	char *f, flags = 0;
> +	char *f, flags = TTY_NORMAL;
>  	int i;
>  
>  	for (i = count, p = cp, f = fp; i; i--, p++) {
> diff --git a/include/linux/tty_ldisc.h b/include/linux/tty_ldisc.h
> index f15c898..b8347c2 100644
> --- a/include/linux/tty_ldisc.h
> +++ b/include/linux/tty_ldisc.h
> @@ -84,7 +84,8 @@
>   *	processing.  <cp> is a pointer to the buffer of input
>   *	character received by the device.  <fp> is a pointer to a
>   *	pointer of flag bytes which indicate whether a character was
> - *	received with a parity error, etc.
> + *	received with a parity error, etc. <fp> may be NULL to indicate
> + *	all data received is TTY_NORMAL.
>   *
>   * void	(*write_wakeup)(struct tty_struct *);
>   *
> @@ -118,7 +119,8 @@
>   *	processing.  <cp> is a pointer to the buffer of input
>   *	character received by the device.  <fp> is a pointer to a
>   *	pointer of flag bytes which indicate whether a character was
> - *	received with a parity error, etc.
> + *	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.
>   */
>  
> -- 
> 1.8.1.2
> 

-- 
Dmitry

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH tty-next 5/5] tty: Halve flip buffer GFP_ATOMIC memory consumption
  2013-11-22 17:09 ` [PATCH tty-next 5/5] tty: Halve flip buffer GFP_ATOMIC memory consumption Peter Hurley
@ 2013-12-09  1:01   ` Greg Kroah-Hartman
  2013-12-09 13:27     ` Peter Hurley
  2013-12-09 14:23     ` [PATCH tty-next v3 " Peter Hurley
  0 siblings, 2 replies; 13+ messages in thread
From: Greg Kroah-Hartman @ 2013-12-09  1:01 UTC (permalink / raw)
  To: Peter Hurley; +Cc: Jiri Slaby, linux-kernel, linux-serial

On Fri, Nov 22, 2013 at 12:09:58PM -0500, Peter Hurley wrote:
> tty flip buffers use GFP_ATOMIC allocations for received data
> which is to be processed by the line discipline. For each byte
> received, an extra byte is used to indicate the error status of
> that byte.
> 
> Instead, if the received data is error-free, encode the entire
> buffer without status bytes.
> 
> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
> ---
>  drivers/tty/tty_buffer.c | 45 +++++++++++++++++++++++++++++++++++----------
>  include/linux/tty.h      |  4 ++++
>  include/linux/tty_flip.h |  8 ++++++--
>  3 files changed, 45 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
> index 6a3620e..c4fe20e 100644
> --- a/drivers/tty/tty_buffer.c
> +++ b/drivers/tty/tty_buffer.c
> @@ -100,6 +100,7 @@ static void tty_buffer_reset(struct tty_buffer *p, size_t size)
>  	p->next = NULL;
>  	p->commit = 0;
>  	p->read = 0;
> +	p->flags = 0;
>  }
>  
>  /**
> @@ -230,31 +231,49 @@ void tty_buffer_flush(struct tty_struct *tty)
>   *	tty_buffer_request_room		-	grow tty buffer if needed
>   *	@tty: tty structure
>   *	@size: size desired
> + *	@flags: buffer flags if new buffer allocated (default = 0)
>   *
>   *	Make at least size bytes of linear space available for the tty
>   *	buffer. If we fail return the size we managed to find.
> + *
> + *	Will change over to a new buffer if the current buffer is encoded as
> + *	TTY_NORMAL (so has no flags buffer) and the new buffer requires
> + *	a flags buffer.
>   */
> -int tty_buffer_request_room(struct tty_port *port, size_t size)
> +int __tty_buffer_request_room(struct tty_port *port, size_t size, int flags)
>  {
>  	struct tty_bufhead *buf = &port->buf;
>  	struct tty_buffer *b, *n;
> -	int left;
> +	int left, change;
>  
>  	b = buf->tail;
> -	left = b->size - b->used;
> +	if (b->flags & TTYB_NORMAL)
> +		left = 2 * b->size - b->used;
> +	else
> +		left = b->size - b->used;
>  
> -	if (left < size) {
> +	change = (b->flags & TTYB_NORMAL) && (~flags & TTYB_NORMAL);
> +	if (change || left < size) {
>  		/* This is the slow path - looking for new buffers to use */
>  		if ((n = tty_buffer_alloc(port, size)) != NULL) {
> +			n->flags = flags;
>  			buf->tail = n;
>  			b->commit = b->used;
>  			smp_mb();
>  			b->next = n;
> -		} else
> +		} else if (change)
> +			size = 0;
> +		else
>  			size = left;
>  	}
>  	return size;
>  }
> +EXPORT_SYMBOL_GPL(__tty_buffer_request_room);

Why are you exporting this?  There is no function prototype anywhere, so
no one can even try to call it, it should just be static, right?

I've applied the first 4 patches in this series, care to fix this one
up?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH tty-next 5/5] tty: Halve flip buffer GFP_ATOMIC memory consumption
  2013-12-09  1:01   ` Greg Kroah-Hartman
@ 2013-12-09 13:27     ` Peter Hurley
  2013-12-09 14:23     ` [PATCH tty-next v3 " Peter Hurley
  1 sibling, 0 replies; 13+ messages in thread
From: Peter Hurley @ 2013-12-09 13:27 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, linux-serial

On 12/08/2013 08:01 PM, Greg Kroah-Hartman wrote:
> On Fri, Nov 22, 2013 at 12:09:58PM -0500, Peter Hurley wrote:
>> tty flip buffers use GFP_ATOMIC allocations for received data
>> which is to be processed by the line discipline. For each byte
>> received, an extra byte is used to indicate the error status of
>> that byte.
>>
>> Instead, if the received data is error-free, encode the entire
>> buffer without status bytes.
>>
>> Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
>> ---
>>   drivers/tty/tty_buffer.c | 45 +++++++++++++++++++++++++++++++++++----------
>>   include/linux/tty.h      |  4 ++++
>>   include/linux/tty_flip.h |  8 ++++++--
>>   3 files changed, 45 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
>> index 6a3620e..c4fe20e 100644
>> --- a/drivers/tty/tty_buffer.c
>> +++ b/drivers/tty/tty_buffer.c
>> @@ -100,6 +100,7 @@ static void tty_buffer_reset(struct tty_buffer *p, size_t size)
>>   	p->next = NULL;
>>   	p->commit = 0;
>>   	p->read = 0;
>> +	p->flags = 0;
>>   }
>>
>>   /**
>> @@ -230,31 +231,49 @@ void tty_buffer_flush(struct tty_struct *tty)
>>    *	tty_buffer_request_room		-	grow tty buffer if needed
>>    *	@tty: tty structure
>>    *	@size: size desired
>> + *	@flags: buffer flags if new buffer allocated (default = 0)
>>    *
>>    *	Make at least size bytes of linear space available for the tty
>>    *	buffer. If we fail return the size we managed to find.
>> + *
>> + *	Will change over to a new buffer if the current buffer is encoded as
>> + *	TTY_NORMAL (so has no flags buffer) and the new buffer requires
>> + *	a flags buffer.
>>    */
>> -int tty_buffer_request_room(struct tty_port *port, size_t size)
>> +int __tty_buffer_request_room(struct tty_port *port, size_t size, int flags)
>>   {
>>   	struct tty_bufhead *buf = &port->buf;
>>   	struct tty_buffer *b, *n;
>> -	int left;
>> +	int left, change;
>>
>>   	b = buf->tail;
>> -	left = b->size - b->used;
>> +	if (b->flags & TTYB_NORMAL)
>> +		left = 2 * b->size - b->used;
>> +	else
>> +		left = b->size - b->used;
>>
>> -	if (left < size) {
>> +	change = (b->flags & TTYB_NORMAL) && (~flags & TTYB_NORMAL);
>> +	if (change || left < size) {
>>   		/* This is the slow path - looking for new buffers to use */
>>   		if ((n = tty_buffer_alloc(port, size)) != NULL) {
>> +			n->flags = flags;
>>   			buf->tail = n;
>>   			b->commit = b->used;
>>   			smp_mb();
>>   			b->next = n;
>> -		} else
>> +		} else if (change)
>> +			size = 0;
>> +		else
>>   			size = left;
>>   	}
>>   	return size;
>>   }
>> +EXPORT_SYMBOL_GPL(__tty_buffer_request_room);
>
> Why are you exporting this?  There is no function prototype anywhere, so
> no one can even try to call it, it should just be static, right?

Yes, sorry. Thanks for catching this.

> I've applied the first 4 patches in this series, care to fix this one
> up?

Patch coming.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH tty-next v3 5/5] tty: Halve flip buffer GFP_ATOMIC memory consumption
  2013-12-09  1:01   ` Greg Kroah-Hartman
  2013-12-09 13:27     ` Peter Hurley
@ 2013-12-09 14:23     ` Peter Hurley
  1 sibling, 0 replies; 13+ messages in thread
From: Peter Hurley @ 2013-12-09 14:23 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-kernel, linux-serial, Peter Hurley

tty flip buffers use GFP_ATOMIC allocations for received data
which is to be processed by the line discipline. For each byte
received, an extra byte is used to indicate the error status of
that byte.

Instead, if the received data is error-free, encode the entire
buffer without status bytes.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---

v3 - Make __tty_buffer_request_room static; don't export symbol. duh.

 drivers/tty/tty_buffer.c | 45 +++++++++++++++++++++++++++++++++++----------
 include/linux/tty.h      |  4 ++++
 include/linux/tty_flip.h |  8 ++++++--
 3 files changed, 45 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index 6a3620e..7ba69b6 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -100,6 +100,7 @@ static void tty_buffer_reset(struct tty_buffer *p, size_t size)
 	p->next = NULL;
 	p->commit = 0;
 	p->read = 0;
+	p->flags = 0;
 }
 
 /**
@@ -230,31 +231,49 @@ void tty_buffer_flush(struct tty_struct *tty)
  *	tty_buffer_request_room		-	grow tty buffer if needed
  *	@tty: tty structure
  *	@size: size desired
+ *	@flags: buffer flags if new buffer allocated (default = 0)
  *
  *	Make at least size bytes of linear space available for the tty
  *	buffer. If we fail return the size we managed to find.
+ *
+ *	Will change over to a new buffer if the current buffer is encoded as
+ *	TTY_NORMAL (so has no flags buffer) and the new buffer requires
+ *	a flags buffer.
  */
-int tty_buffer_request_room(struct tty_port *port, size_t size)
+static int __tty_buffer_request_room(struct tty_port *port, size_t size,
+				     int flags)
 {
 	struct tty_bufhead *buf = &port->buf;
 	struct tty_buffer *b, *n;
-	int left;
+	int left, change;
 
 	b = buf->tail;
-	left = b->size - b->used;
+	if (b->flags & TTYB_NORMAL)
+		left = 2 * b->size - b->used;
+	else
+		left = b->size - b->used;
 
-	if (left < size) {
+	change = (b->flags & TTYB_NORMAL) && (~flags & TTYB_NORMAL);
+	if (change || left < size) {
 		/* This is the slow path - looking for new buffers to use */
 		if ((n = tty_buffer_alloc(port, size)) != NULL) {
+			n->flags = flags;
 			buf->tail = n;
 			b->commit = b->used;
 			smp_mb();
 			b->next = n;
-		} else
+		} else if (change)
+			size = 0;
+		else
 			size = left;
 	}
 	return size;
 }
+
+int tty_buffer_request_room(struct tty_port *port, size_t size)
+{
+	return __tty_buffer_request_room(port, size, 0);
+}
 EXPORT_SYMBOL_GPL(tty_buffer_request_room);
 
 /**
@@ -274,12 +293,14 @@ int tty_insert_flip_string_fixed_flag(struct tty_port *port,
 	int copied = 0;
 	do {
 		int goal = min_t(size_t, size - copied, TTY_BUFFER_PAGE);
-		int space = tty_buffer_request_room(port, goal);
+		int flags = (flag == TTY_NORMAL) ? TTYB_NORMAL : 0;
+		int space = __tty_buffer_request_room(port, goal, flags);
 		struct tty_buffer *tb = port->buf.tail;
 		if (unlikely(space == 0))
 			break;
 		memcpy(char_buf_ptr(tb, tb->used), chars, space);
-		memset(flag_buf_ptr(tb, tb->used), flag, space);
+		if (~tb->flags & TTYB_NORMAL)
+			memset(flag_buf_ptr(tb, tb->used), flag, space);
 		tb->used += space;
 		copied += space;
 		chars += space;
@@ -362,11 +383,12 @@ EXPORT_SYMBOL(tty_schedule_flip);
 int tty_prepare_flip_string(struct tty_port *port, unsigned char **chars,
 		size_t size)
 {
-	int space = tty_buffer_request_room(port, size);
+	int space = __tty_buffer_request_room(port, size, TTYB_NORMAL);
 	if (likely(space)) {
 		struct tty_buffer *tb = port->buf.tail;
 		*chars = char_buf_ptr(tb, tb->used);
-		memset(flag_buf_ptr(tb, tb->used), TTY_NORMAL, space);
+		if (~tb->flags & TTYB_NORMAL)
+			memset(flag_buf_ptr(tb, tb->used), TTY_NORMAL, space);
 		tb->used += space;
 	}
 	return space;
@@ -379,7 +401,10 @@ receive_buf(struct tty_struct *tty, struct tty_buffer *head, int count)
 {
 	struct tty_ldisc *disc = tty->ldisc;
 	unsigned char *p = char_buf_ptr(head, head->read);
-	char	      *f = flag_buf_ptr(head, head->read);
+	char	      *f = NULL;
+
+	if (~head->flags & TTYB_NORMAL)
+		f = flag_buf_ptr(head, head->read);
 
 	if (disc->ops->receive_buf2)
 		count = disc->ops->receive_buf2(tty, p, f, count);
diff --git a/include/linux/tty.h b/include/linux/tty.h
index ed9aedd..9d234dc 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -39,10 +39,14 @@ struct tty_buffer {
 	int size;
 	int commit;
 	int read;
+	int flags;
 	/* Data points here */
 	unsigned long data[0];
 };
 
+/* Values for .flags field of tty_buffer */
+#define TTYB_NORMAL	1	/* buffer has no flags buffer */
+
 static inline unsigned char *char_buf_ptr(struct tty_buffer *b, int ofs)
 {
 	return ((unsigned char *)b->data) + ofs;
diff --git a/include/linux/tty_flip.h b/include/linux/tty_flip.h
index 3f821e9..c28dd52 100644
--- a/include/linux/tty_flip.h
+++ b/include/linux/tty_flip.h
@@ -17,8 +17,12 @@ static inline int tty_insert_flip_char(struct tty_port *port,
 					unsigned char ch, char flag)
 {
 	struct tty_buffer *tb = port->buf.tail;
-	if (tb && tb->used < tb->size) {
-		*flag_buf_ptr(tb, tb->used) = flag;
+	int change;
+
+	change = (tb->flags & TTYB_NORMAL) && (flag != TTY_NORMAL);
+	if (!change && tb->used < tb->size) {
+		if (~tb->flags & TTYB_NORMAL)
+			*flag_buf_ptr(tb, tb->used) = flag;
 		*char_buf_ptr(tb, tb->used++) = ch;
 		return 1;
 	}
-- 
1.8.1.2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2013-12-09 14:24 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-22 17:09 [PATCH tty-next 0/5] Halve tty buffer memory consumption Peter Hurley
2013-11-22 17:09 ` [PATCH tty-next 1/5] tty: Always handle NULL flag ptr Peter Hurley
2013-11-22 22:20   ` Dmitry Torokhov
2013-11-26  2:00     ` Peter Hurley
2013-12-02 18:56     ` [PATCH v2 " Peter Hurley
2013-12-02 19:09       ` Dmitry Torokhov
2013-11-22 17:09 ` [PATCH tty-next 2/5] tty: Enable configurable tty flip buffer limit Peter Hurley
2013-11-22 17:09 ` [PATCH tty-next 3/5] tty: Rename tty buffer memory_used field Peter Hurley
2013-11-22 17:09 ` [PATCH tty-next 4/5] tty: Remove tty_prepare_flip_string_flags() Peter Hurley
2013-11-22 17:09 ` [PATCH tty-next 5/5] tty: Halve flip buffer GFP_ATOMIC memory consumption Peter Hurley
2013-12-09  1:01   ` Greg Kroah-Hartman
2013-12-09 13:27     ` Peter Hurley
2013-12-09 14:23     ` [PATCH tty-next v3 " Peter Hurley

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).