[PATCH staging-next 0/4] fwserial changes for 3.14

[PATCH staging-next 0/4] fwserial changes for 3.14

[Peter Hurley]

**This patchset requires 'Halve tty buffer memory consumption' patchset **

Greg,

I have not forgotten fwserial :)

Patches 1 & 2 remove the hacky rx buffering, and instead rely on
configurable tty buffers. [Interestingly, tty buffers were originally
introduced by Alan Cox to replace flip buffers specifically to support
higher throughput serial devices which had mutated special-purpose
rx buffering.]

Patch 3 remedies an ugly (private interface) name.
Patch 4 adds build configuration options which control how many
fwserial devices can exist (per adapter and in total). This is
so some build configurations can minimize resources while still
providing firewire serial console autoconnect (say for a debugging
console or a headless server) while another build configuration
could support many headless server consoles running on a single
machine.

I hope to get the stage 1 console driver into -next in this
cycle as well.

Regards,

Peter Hurley (4):
  staging/fwserial: Rip out rx buffering
  staging/fwserial: Up the tty buffer limit to 128K
  staging/fwserial: Rename data profiling functions
  staging/fwserial: Add Kconfig options for max ports

 drivers/staging/fwserial/Kconfig    |  20 +++++
 drivers/staging/fwserial/fwserial.c | 151 ++++++++++--------------------------
 drivers/staging/fwserial/fwserial.h |  24 +-----
 drivers/tty/tty_buffer.c            |   1 +
 4 files changed, 64 insertions(+), 132 deletions(-)

-- 
1.8.1.2

[PATCH staging-next 1/4] staging/fwserial: Rip out rx buffering

[Peter Hurley]

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/staging/fwserial/fwserial.c | 128 ++++++++----------------------------
 drivers/staging/fwserial/fwserial.h |  16 -----
 drivers/tty/tty_buffer.c            |   1 +
 3 files changed, 28 insertions(+), 117 deletions(-)

diff --git a/drivers/staging/fwserial/fwserial.c b/drivers/staging/fwserial/fwserial.c
index ff92f34..c96f62e 100644
--- a/drivers/staging/fwserial/fwserial.c
+++ b/drivers/staging/fwserial/fwserial.c
@@ -456,16 +456,27 @@ static int fwtty_write_port_status(struct fwtty_port *port)
 	return err;
 }
 
-static void __fwtty_throttle(struct fwtty_port *port, struct tty_struct *tty)
+static void fwtty_throttle_port(struct fwtty_port *port)
 {
+	struct tty_struct *tty;
 	unsigned old;
 
+	tty = tty_port_tty_get(&port->port);
+	if (!tty)
+		return;
+
+	spin_lock_bh(&port->lock);
+
 	old = port->mctrl;
 	port->mctrl |= OOB_RX_THROTTLE;
 	if (C_CRTSCTS(tty))
 		port->mctrl &= ~TIOCM_RTS;
 	if (~old & OOB_RX_THROTTLE)
 		__fwtty_write_port_status(port);
+
+	spin_unlock_bh(&port->lock);
+
+	tty_kref_put(tty);
 }
 
 /**
@@ -532,74 +543,8 @@ static void fwtty_emit_breaks(struct work_struct *work)
 	port->icount.brk += brk;
 }
 
-static void fwtty_pushrx(struct work_struct *work)
-{
-	struct fwtty_port *port = to_port(work, push);
-	struct tty_struct *tty;
-	struct buffered_rx *buf, *next;
-	int n, c = 0;
-
-	spin_lock_bh(&port->lock);
-	list_for_each_entry_safe(buf, next, &port->buf_list, list) {
-		n = tty_insert_flip_string_fixed_flag(&port->port, buf->data,
-						      TTY_NORMAL, buf->n);
-		c += n;
-		port->buffered -= n;
-		if (n < buf->n) {
-			if (n > 0) {
-				memmove(buf->data, buf->data + n, buf->n - n);
-				buf->n -= n;
-			}
-			tty = tty_port_tty_get(&port->port);
-			if (tty) {
-				__fwtty_throttle(port, tty);
-				tty_kref_put(tty);
-			}
-			break;
-		} else {
-			list_del(&buf->list);
-			kfree(buf);
-		}
-	}
-	if (c > 0)
-		tty_flip_buffer_push(&port->port);
-
-	if (list_empty(&port->buf_list))
-		clear_bit(BUFFERING_RX, &port->flags);
-	spin_unlock_bh(&port->lock);
-}
-
-static int fwtty_buffer_rx(struct fwtty_port *port, unsigned char *d, size_t n)
-{
-	struct buffered_rx *buf;
-	size_t size = (n + sizeof(struct buffered_rx) + 0xFF) & ~0xFF;
-
-	if (port->buffered + n > HIGH_WATERMARK) {
-		fwtty_err_ratelimited(port, "overflowed rx buffer: buffered: %d new: %zu wtrmk: %d\n",
-				      port->buffered, n, HIGH_WATERMARK);
-		return 0;
-	}
-	buf = kmalloc(size, GFP_ATOMIC);
-	if (!buf)
-		return 0;
-	INIT_LIST_HEAD(&buf->list);
-	buf->n = n;
-	memcpy(buf->data, d, n);
-
-	spin_lock_bh(&port->lock);
-	list_add_tail(&buf->list, &port->buf_list);
-	port->buffered += n;
-	if (port->buffered > port->stats.watermark)
-		port->stats.watermark = port->buffered;
-	set_bit(BUFFERING_RX, &port->flags);
-	spin_unlock_bh(&port->lock);
-
-	return n;
-}
-
 static int fwtty_rx(struct fwtty_port *port, unsigned char *data, size_t len)
 {
-	struct tty_struct *tty;
 	int c, n = len;
 	unsigned lsr;
 	int err = 0;
@@ -636,31 +581,24 @@ static int fwtty_rx(struct fwtty_port *port, unsigned char *data, size_t len)
 		goto out;
 	}
 
-	if (!test_bit(BUFFERING_RX, &port->flags)) {
-		c = tty_insert_flip_string_fixed_flag(&port->port, data,
-				TTY_NORMAL, n);
-		if (c > 0)
-			tty_flip_buffer_push(&port->port);
-		n -= c;
-
-		if (n) {
-			/* start buffering and throttling */
-			n -= fwtty_buffer_rx(port, &data[c], n);
-
-			tty = tty_port_tty_get(&port->port);
-			if (tty) {
-				spin_lock_bh(&port->lock);
-				__fwtty_throttle(port, tty);
-				spin_unlock_bh(&port->lock);
-				tty_kref_put(tty);
-			}
-		}
-	} else
-		n -= fwtty_buffer_rx(port, data, n);
+	c = tty_insert_flip_string_fixed_flag(&port->port, data, TTY_NORMAL, n);
+	if (c > 0)
+		tty_flip_buffer_push(&port->port);
+	n -= c;
 
 	if (n) {
 		port->overrun = true;
 		err = -EIO;
+		fwtty_err_ratelimited(port, "flip buffer overrun\n");
+
+	} else {
+		/* throttle the sender if remaining flip buffer space has
+		 * reached high watermark to avoid losing data which may be
+		 * in-flight. Since the AR request context is 32k, that much
+		 * data may have _already_ been acked.
+		 */
+		if (tty_buffer_space_avail(&port->port) < HIGH_WATERMARK)
+			fwtty_throttle_port(port);
 	}
 
 out:
@@ -1101,20 +1039,13 @@ static int fwtty_port_activate(struct tty_port *tty_port,
 static void fwtty_port_shutdown(struct tty_port *tty_port)
 {
 	struct fwtty_port *port = to_port(tty_port, port);
-	struct buffered_rx *buf, *next;
 
 	/* TODO: cancel outstanding transactions */
 
 	cancel_delayed_work_sync(&port->emit_breaks);
 	cancel_delayed_work_sync(&port->drain);
-	cancel_work_sync(&port->push);
 
 	spin_lock_bh(&port->lock);
-	list_for_each_entry_safe(buf, next, &port->buf_list, list) {
-		list_del(&buf->list);
-		kfree(buf);
-	}
-	port->buffered = 0;
 	port->flags = 0;
 	port->break_ctl = 0;
 	port->overrun = 0;
@@ -1264,8 +1195,6 @@ static void fwtty_unthrottle(struct tty_struct *tty)
 
 	profile_fifo_avail(port, port->stats.unthrottle);
 
-	schedule_work(&port->push);
-
 	spin_lock_bh(&port->lock);
 	port->mctrl &= ~OOB_RX_THROTTLE;
 	if (C_CRTSCTS(tty))
@@ -1523,8 +1452,7 @@ static void fwtty_debugfs_show_port(struct seq_file *m, struct fwtty_port *port)
 
 	seq_printf(m, " dr:%d st:%d err:%d lost:%d", stats.dropped,
 		   stats.tx_stall, stats.fifo_errs, stats.lost);
-	seq_printf(m, " pkts:%d thr:%d wtrmk:%d", stats.sent, stats.throttled,
-		   stats.watermark);
+	seq_printf(m, " pkts:%d thr:%d", stats.sent, stats.throttled);
 
 	if (port->port.console) {
 		seq_puts(m, "\n    ");
@@ -2302,8 +2230,6 @@ static int fwserial_create(struct fw_unit *unit)
 		INIT_DELAYED_WORK(&port->drain, fwtty_drain_tx);
 		INIT_DELAYED_WORK(&port->emit_breaks, fwtty_emit_breaks);
 		INIT_WORK(&port->hangup, fwtty_do_hangup);
-		INIT_WORK(&port->push, fwtty_pushrx);
-		INIT_LIST_HEAD(&port->buf_list);
 		init_waitqueue_head(&port->wait_tx);
 		port->max_payload = link_speed_to_max_payload(SCODE_100);
 		dma_fifo_init(&port->tx_fifo);
diff --git a/drivers/staging/fwserial/fwserial.h b/drivers/staging/fwserial/fwserial.h
index 2463501..149b24d 100644
--- a/drivers/staging/fwserial/fwserial.h
+++ b/drivers/staging/fwserial/fwserial.h
@@ -166,7 +166,6 @@ struct stats {
 	unsigned	sent;
 	unsigned	lost;
 	unsigned	throttled;
-	unsigned	watermark;
 	unsigned	reads[DISTRIBUTION_MAX_INDEX + 1];
 	unsigned	writes[DISTRIBUTION_MAX_INDEX + 1];
 	unsigned	txns[DISTRIBUTION_MAX_INDEX + 1];
@@ -183,12 +182,6 @@ struct fwconsole_ops {
 #define FWCON_NOTIFY_ATTACH		1
 #define FWCON_NOTIFY_DETACH		2
 
-struct buffered_rx {
-	struct list_head	list;
-	size_t			n;
-	unsigned char		data[0];
-};
-
 /**
  * fwtty_port: structure used to track/represent underlying tty_port
  * @port: underlying tty_port
@@ -223,11 +216,6 @@ struct buffered_rx {
  *         The work can race with the writer but concurrent sending is
  *         prevented with the IN_TX flag. Scheduled under lock to
  *         limit scheduling when fifo has just been drained.
- * @push: work responsible for pushing buffered rx to the ldisc.
- *	  rx can become buffered if the tty buffer is filled before the
- *	  ldisc throttles the sender.
- * @buf_list: list of buffered rx yet to be sent to ldisc
- * @buffered: byte count of buffered rx
  * @tx_fifo: fifo used to store & block-up writes for dma to remote
  * @max_payload: max bytes transmissable per dma (based on peer's max_payload)
  * @status_mask: UART_LSR_* bitmask significant to rx (based on termios)
@@ -267,9 +255,6 @@ struct fwtty_port {
 	spinlock_t		   lock;
 	unsigned		   mctrl;
 	struct delayed_work	   drain;
-	struct work_struct	   push;
-	struct list_head	   buf_list;
-	int			   buffered;
 	struct dma_fifo		   tx_fifo;
 	int			   max_payload;
 	unsigned		   status_mask;
@@ -291,7 +276,6 @@ struct fwtty_port {
 /* bit #s for flags field */
 #define IN_TX                      0
 #define STOP_TX                    1
-#define BUFFERING_RX		   2
 
 /* bitmasks for special mctrl/mstatus bits */
 #define OOB_RX_THROTTLE   0x00010000
diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index c4fe20e..0f2211f 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -92,6 +92,7 @@ int tty_buffer_space_avail(struct tty_port *port)
 	int space = port->buf.mem_limit - atomic_read(&port->buf.mem_used);
 	return max(space, 0);
 }
+EXPORT_SYMBOL_GPL(tty_buffer_space_avail);
 
 static void tty_buffer_reset(struct tty_buffer *p, size_t size)
 {
-- 
1.8.1.2

[PATCH staging-next 2/4] staging/fwserial: Up the tty buffer limit to 128K

[Peter Hurley]

The firewire driver can overrun the tty buffers before the
line discipline can throttle the input; up the buffer limit.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/staging/fwserial/fwserial.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/fwserial/fwserial.c b/drivers/staging/fwserial/fwserial.c
index c96f62e..cad9e3f 100644
--- a/drivers/staging/fwserial/fwserial.c
+++ b/drivers/staging/fwserial/fwserial.c
@@ -2225,6 +2225,7 @@ static int fwserial_create(struct fw_unit *unit)
 		port->index = FWTTY_INVALID_INDEX;
 		port->port.ops = &fwtty_port_ops;
 		port->serial = serial;
+		tty_buffer_set_limit(&port->port, 128 * 1024);
 
 		spin_lock_init(&port->lock);
 		INIT_DELAYED_WORK(&port->drain, fwtty_drain_tx);
-- 
1.8.1.2

[PATCH staging-next 3/4] staging/fwserial: Rename data profiling functions

[Peter Hurley]

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/staging/fwserial/fwserial.c | 22 +++++++++++-----------
 drivers/staging/fwserial/fwserial.h |  4 ++--
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/fwserial/fwserial.c b/drivers/staging/fwserial/fwserial.c
index cad9e3f..12bd002 100644
--- a/drivers/staging/fwserial/fwserial.c
+++ b/drivers/staging/fwserial/fwserial.c
@@ -136,14 +136,14 @@ static struct fwtty_peer *__fwserial_peer_by_node_id(struct fw_card *card,
 
 #ifdef FWTTY_PROFILING
 
-static void profile_fifo_avail(struct fwtty_port *port, unsigned *stat)
+static void fwtty_profile_fifo(struct fwtty_port *port, unsigned *stat)
 {
 	spin_lock_bh(&port->lock);
-	profile_size_distrib(stat, dma_fifo_avail(&port->tx_fifo));
+	fwtty_profile_data(stat, dma_fifo_avail(&port->tx_fifo));
 	spin_unlock_bh(&port->lock);
 }
 
-static void dump_profile(struct seq_file *m, struct stats *stats)
+static void fwtty_dump_profile(struct seq_file *m, struct stats *stats)
 {
 	/* for each stat, print sum of 0 to 2^k, then individually */
 	int k = 4;
@@ -183,8 +183,8 @@ static void dump_profile(struct seq_file *m, struct stats *stats)
 }
 
 #else
-#define profile_fifo_avail(port, stat)
-#define dump_profile(m, stats)
+#define fwtty_profile_fifo(port, stat)
+#define fwtty_dump_profile(m, stats)
 #endif
 
 /*
@@ -550,7 +550,7 @@ static int fwtty_rx(struct fwtty_port *port, unsigned char *data, size_t len)
 	int err = 0;
 
 	fwtty_dbg(port, "%d\n", n);
-	profile_size_distrib(port->stats.reads, n);
+	fwtty_profile_data(port->stats.reads, n);
 
 	if (port->write_only) {
 		n = 0;
@@ -759,7 +759,7 @@ static int fwtty_tx(struct fwtty_port *port, bool drain)
 			if (n == -EAGAIN)
 				++port->stats.tx_stall;
 			else if (n == -ENODATA)
-				profile_size_distrib(port->stats.txns, 0);
+				fwtty_profile_data(port->stats.txns, 0);
 			else {
 				++port->stats.fifo_errs;
 				fwtty_err_ratelimited(port, "fifo err: %d\n",
@@ -768,7 +768,7 @@ static int fwtty_tx(struct fwtty_port *port, bool drain)
 			break;
 		}
 
-		profile_size_distrib(port->stats.txns, txn->dma_pended.len);
+		fwtty_profile_data(port->stats.txns, txn->dma_pended.len);
 
 		fwtty_send_txn_async(peer, txn, TCODE_WRITE_BLOCK_REQUEST,
 				     peer->fifo_addr, txn->dma_pended.data,
@@ -1115,7 +1115,7 @@ static int fwtty_write(struct tty_struct *tty, const unsigned char *buf, int c)
 	int n, len;
 
 	fwtty_dbg(port, "%d\n", c);
-	profile_size_distrib(port->stats.writes, c);
+	fwtty_profile_data(port->stats.writes, c);
 
 	spin_lock_bh(&port->lock);
 	n = dma_fifo_in(&port->tx_fifo, buf, c);
@@ -1193,7 +1193,7 @@ static void fwtty_unthrottle(struct tty_struct *tty)
 
 	fwtty_dbg(port, "CRTSCTS: %d\n", (C_CRTSCTS(tty) != 0));
 
-	profile_fifo_avail(port, port->stats.unthrottle);
+	fwtty_profile_fifo(port, port->stats.unthrottle);
 
 	spin_lock_bh(&port->lock);
 	port->mctrl &= ~OOB_RX_THROTTLE;
@@ -1459,7 +1459,7 @@ static void fwtty_debugfs_show_port(struct seq_file *m, struct fwtty_port *port)
 		(*port->fwcon_ops->proc_show)(m, port->con_data);
 	}
 
-	dump_profile(m, &port->stats);
+	fwtty_dump_profile(m, &port->stats);
 }
 
 static void fwtty_debugfs_show_peer(struct seq_file *m, struct fwtty_peer *peer)
diff --git a/drivers/staging/fwserial/fwserial.h b/drivers/staging/fwserial/fwserial.h
index 149b24d..eab85b4 100644
--- a/drivers/staging/fwserial/fwserial.h
+++ b/drivers/staging/fwserial/fwserial.h
@@ -22,14 +22,14 @@
 #ifdef FWTTY_PROFILING
 #define DISTRIBUTION_MAX_SIZE     8192
 #define DISTRIBUTION_MAX_INDEX    (ilog2(DISTRIBUTION_MAX_SIZE) + 1)
-static inline void profile_size_distrib(unsigned stat[], unsigned val)
+static inline void fwtty_profile_data(unsigned stat[], unsigned val)
 {
 	int n = (val) ? min(ilog2(val) + 1, DISTRIBUTION_MAX_INDEX) : 0;
 	++stat[n];
 }
 #else
 #define DISTRIBUTION_MAX_INDEX    0
-#define profile_size_distrib(st, n)
+#define fwtty_profile_data(st, n)
 #endif
 
 /* Parameters for both VIRT_CABLE_PLUG & VIRT_CABLE_PLUG_RSP mgmt codes */
-- 
1.8.1.2

[PATCH staging-next 4/4] staging/fwserial: Add Kconfig options for max ports

[Peter Hurley]

Allow kernel configuration of max supported ports for
TTY-over-Firewire driver.

Signed-off-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/staging/fwserial/Kconfig    | 20 ++++++++++++++++++++
 drivers/staging/fwserial/fwserial.h |  4 ++--
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/fwserial/Kconfig b/drivers/staging/fwserial/Kconfig
index a0812d9..9c7c926 100644
--- a/drivers/staging/fwserial/Kconfig
+++ b/drivers/staging/fwserial/Kconfig
@@ -9,3 +9,23 @@ config FIREWIRE_SERIAL
 
 	  To compile this driver as a module, say M here:  the module will
 	  be called firewire-serial.
+
+if FIREWIRE_SERIAL
+
+config FWTTY_MAX_TOTAL_PORTS
+       int "Maximum number of serial ports supported"
+       default "64"
+       help
+          Set this to the maximum number of serial ports you want the
+	  firewire-serial driver to support.
+
+config FWTTY_MAX_CARD_PORTS
+       int "Maximum number of serial ports supported per adapter"
+       range 0 FWTTY_MAX_TOTAL_PORTS
+       default "32"
+       help
+          Set this to the maximum number of serial ports each firewire
+	  adapter supports. The actual number of serial ports registered
+	  is set with the module parameter "ttys".
+
+endif
diff --git a/drivers/staging/fwserial/fwserial.h b/drivers/staging/fwserial/fwserial.h
index eab85b4..54f7f9b 100644
--- a/drivers/staging/fwserial/fwserial.h
+++ b/drivers/staging/fwserial/fwserial.h
@@ -291,8 +291,8 @@ struct fwtty_port {
 #define FREQ_BREAKS        (HZ / 50)
 
 /* Ports are allocated in blocks of num_ports for each fw_card */
-#define MAX_CARD_PORTS           32	/* max # of ports per card */
-#define MAX_TOTAL_PORTS          64	/* max # of ports total    */
+#define MAX_CARD_PORTS           CONFIG_FWTTY_MAX_CARD_PORTS
+#define MAX_TOTAL_PORTS          CONFIG_FWTTY_MAX_TOTAL_PORTS
 
 /* tuning parameters */
 #define FWTTY_PORT_TXFIFO_LEN	4096
-- 
1.8.1.2

Re: [PATCH staging-next 0/4] fwserial changes for 3.14

[Greg Kroah-Hartman]

On Fri, Nov 22, 2013 at 01:06:07PM -0500, Peter Hurley wrote:
> **This patchset requires 'Halve tty buffer memory consumption' patchset **

Requires it to build or just work properly?

I'm trying to figure out what tree to take this in, (tty-next or
staging-next).

thanks,

greg k-h

Re: [PATCH staging-next 0/4] fwserial changes for 3.14

[Peter Hurley]

On 11/25/2013 03:43 PM, Greg Kroah-Hartman wrote:
> On Fri, Nov 22, 2013 at 01:06:07PM -0500, Peter Hurley wrote:
>> **This patchset requires 'Halve tty buffer memory consumption' patchset **
>
> Requires it to build or just work properly?
>
> I'm trying to figure out what tree to take this in, (tty-next or
> staging-next).

To build.

The 'Halve tty buffer memory consumption' patchset exports new
methods that allow the driver to configure the maximum memory
consumption by the tty buffers.

This fwserial patchset uses that new interface to rely on tty
buffers to provide adequate rx buffering, rather than add'l rx
buffering being done by the driver.

By halving the buffer memory consumption, I don't feel so
guilty about doubling the limit :)

Regards,
Peter Hurley

Re: [PATCH staging-next 0/4] fwserial changes for 3.14

[Greg Kroah-Hartman]

On Mon, Nov 25, 2013 at 04:00:23PM -0500, Peter Hurley wrote:
> On 11/25/2013 03:43 PM, Greg Kroah-Hartman wrote:
> >On Fri, Nov 22, 2013 at 01:06:07PM -0500, Peter Hurley wrote:
> >>**This patchset requires 'Halve tty buffer memory consumption' patchset **
> >
> >Requires it to build or just work properly?
> >
> >I'm trying to figure out what tree to take this in, (tty-next or
> >staging-next).
> 
> To build.
> 
> The 'Halve tty buffer memory consumption' patchset exports new
> methods that allow the driver to configure the maximum memory
> consumption by the tty buffers.
> 
> This fwserial patchset uses that new interface to rely on tty
> buffers to provide adequate rx buffering, rather than add'l rx
> buffering being done by the driver.
> 
> By halving the buffer memory consumption, I don't feel so
> guilty about doubling the limit :)

Ok, thanks, I'll take it through my tty-next tree then.

greg k-h