staging/fwserial: Only reset port status for attached peers

PATCH In review Peter Hurley 2 participants 16 messages Jan 29, 2013 – Jan 30, 2013 Staging

Share & tools

Peter Hurley

author

2013-01-29 03:35 UTC #0 lore raw

PATCH 1/11 staging/fwserial: Only reset port status for attached peers

When a port has been reserved in an attempt to connect to a peer but that attempt does not succeed, releasing the port should not reset the port line status. Although resetting is functionally harmless, it can appear as if a remote peer dropped carrier to a port it was not attached to (which can be confusing).

Signed-off-by Peter Hurley <[email protected]>

---
 drivers/staging/fwserial/fwserial.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

drivers/staging/fwserial/fwserial.c +12 -9

index 48523eb..b1482ef 100644
@@ -1774,10 +1774,11 @@ static struct fwtty_port *fwserial_find_port(struct fwtty_peer *peer)
 	return NULL;
 }
 
-static void fwserial_release_port(struct fwtty_port *port)
+static void fwserial_release_port(struct fwtty_port *port, bool reset)
 {
 	/* drop carrier (and all other line status) */
-	fwtty_update_port_status(port, 0);
+	if (reset)
+		fwtty_update_port_status(port, 0);
 
 	spin_lock_bh(&port->lock);

@@ -1807,7 +1808,7 @@ static void fwserial_plug_timeout(unsigned long data)
 	spin_unlock_bh(&peer->lock);
 
 	if (port)
-		fwserial_release_port(port);
+		fwserial_release_port(port, false);
 }
 
 /**

@@ -1870,7 +1871,7 @@ cancel_timer:
 	peer_revert_state(peer);
 release_port:
 	spin_unlock_bh(&peer->lock);
-	fwserial_release_port(port);
+	fwserial_release_port(port, false);
 free_pkt:
 	kfree(pkt);
 	return err;

@@ -2148,7 +2149,7 @@ static void fwserial_remove_peer(struct fwtty_peer *peer)
 	spin_unlock_bh(&peer->lock);
 
 	if (port)
-		fwserial_release_port(port);
+		fwserial_release_port(port, true);
 
 	synchronize_rcu();
 	kfree(peer);

@@ -2608,7 +2609,7 @@ static void fwserial_handle_plug_req(struct work_struct *work)
 
 	spin_unlock_bh(&peer->lock);
 	if (port)
-		fwserial_release_port(port);
+		fwserial_release_port(port, false);
 
 	rcode = fwserial_send_mgmt_sync(peer, pkt);

@@ -2630,7 +2631,7 @@ static void fwserial_handle_plug_req(struct work_struct *work)
 cleanup:
 	spin_unlock_bh(&peer->lock);
 	if (port)
-		fwserial_release_port(port);
+		fwserial_release_port(port, false);
 	kfree(pkt);
 	return;
 }

@@ -2682,7 +2683,7 @@ static void fwserial_handle_unplug_req(struct work_struct *work)
 cleanup:
 	spin_unlock_bh(&peer->lock);
 	if (port)
-		fwserial_release_port(port);
+		fwserial_release_port(port, true);
 	kfree(pkt);
 	return;
 }

@@ -2693,6 +2694,7 @@ static int fwserial_parse_mgmt_write(struct fwtty_peer *peer,
 				     size_t len)
 {
 	struct fwtty_port *port = NULL;
+	bool reset = false;
 	int rcode;
 
 	if (addr != fwserial_mgmt_addr_handler.offset || len < sizeof(pkt->hdr))

@@ -2768,6 +2770,7 @@ static int fwserial_parse_mgmt_write(struct fwtty_peer *peer,
 			if (be16_to_cpu(pkt->hdr.code) & FWSC_RSP_NACK)
 				fwtty_notice(&peer->unit, "NACK unplug?");
 			port = peer_revert_state(peer);
+			reset = true;
 		}
 		break;

@@ -2779,7 +2782,7 @@ static int fwserial_parse_mgmt_write(struct fwtty_peer *peer,
 	spin_unlock_bh(&peer->lock);
 
 	if (port)
-		fwserial_release_port(port);
+		fwserial_release_port(port, reset);
 
 	return rcode;
 }

Signature · 1 line

1.8.1.1

Peter Hurley

author

2013-01-29 03:37 UTC #1 lore raw

PATCH 2/11 staging/fwserial: Release port regardless of unplug response code

After sending the unplug response, release the port even if an error occurred.

Signed-off-by Peter Hurley <[email protected]>

---
 drivers/staging/fwserial/fwserial.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

drivers/staging/fwserial/fwserial.c +2 -3

index b1482ef..055085d 100644
@@ -2675,10 +2675,9 @@ static void fwserial_handle_unplug_req(struct work_struct *work)
 
 	spin_lock_bh(&peer->lock);
 	if (peer->state == FWPS_UNPLUG_RESPONDING) {
-		if (rcode == RCODE_COMPLETE)
-			port = peer_revert_state(peer);
-		else
+		if (rcode != RCODE_COMPLETE)
 			fwtty_err(&peer->unit, "UNPLUG_RSP error (%d)", rcode);
+		port = peer_revert_state(peer);
 	}
 cleanup:
 	spin_unlock_bh(&peer->lock);

Signature · 1 line

1.8.1.1

Peter Hurley

author

2013-01-29 03:35 UTC #2 lore raw

PATCH 3/11 staging/fwserial: Fix sparse build warnings

In reply to Peter Hurley

Reported-by Stefan Richter <[email protected]>

Signed-off-by Peter Hurley <[email protected]>

---
 drivers/staging/fwserial/fwserial.c | 4 +++-
 drivers/staging/fwserial/fwserial.h | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

drivers/staging/fwserial/fwserial.c +3 -1

index 055085d..473a2db 100644
@@ -78,6 +78,8 @@ static int num_ports;
 /* slab used as pool for struct fwtty_transactions */
 static struct kmem_cache *fwtty_txn_cache;
 
+struct tty_driver *fwtty_driver;
+
 struct fwtty_transaction;
 typedef void (*fwtty_transaction_cb)(struct fw_card *card, int rcode,
 				     void *data, size_t length,

@@ -2543,7 +2545,7 @@ static struct fw_descriptor fwserial_unit_directory = {
  * The management address is in the unit space region but above other known
  * address users (to keep wild writes from causing havoc)
  */
-const struct fw_address_region fwserial_mgmt_addr_region = {
+static const struct fw_address_region fwserial_mgmt_addr_region = {
 	.start = CSR_REGISTER_BASE + 0x1e0000ULL,
 	.end = 0x1000000000000ULL,
 };

drivers/staging/fwserial/fwserial.h +1 -1

index 953ece6..3602809 100644
@@ -352,7 +352,7 @@ struct fw_serial {
 static const char tty_dev_name[] =  TTY_DEV_NAME;
 static const char loop_dev_name[] = "fwloop";
 
-struct tty_driver *fwtty_driver;
+extern struct tty_driver *fwtty_driver;
 
 #define driver_err(s, v...)	pr_err(KBUILD_MODNAME ": " s, ##v)

Signature · 1 line

1.8.1.1

Peter Hurley

author

2013-01-29 03:37 UTC #3 lore raw

PATCH 4/11 staging/fwserial: Create loop device the 'tty' way

In reply to Peter Hurley

Register a second tty driver to create loopback devices for each firewire node. Note that the loopback devices are numbered from 0; the tty->index is transformed when used to index the port table.

Remove the hack that previously enabled this.

Signed-off-by Peter Hurley <[email protected]>

---
 drivers/staging/fwserial/TODO       |  13 ---
 drivers/staging/fwserial/fwserial.c | 176 +++++++++++++++++++-----------------
 2 files changed, 93 insertions(+), 96 deletions(-)

drivers/staging/fwserial/TODO +0 ~~-13~~

index dc61d97..382a795 100644
@@ -12,16 +12,3 @@ TODOs prior to this driver moving out of staging
 1. This driver uses the same unregistered vendor id that the firewire core does
      (0xd00d1e). Perhaps this could be exposed as a define in
      firewire.h?
-
--- Issues with TTY core --
-  1. Hack for alternate device name scheme
-     - because udev no longer allows device renaming, devices should have
-       their proper names on creation. This is an issue for creating the
-       fwloop<n> device with the fwtty<n> devices because although duplicating
-       roughly the same operations as tty_port_register_device() isn't difficult,
-       access to the tty_class & tty_fops is restricted in scope.
-
-       This is currently being worked around in create_loop_device() by
-       extracting the tty_class ptr and tty_fops ptr from the previously created
-       tty devices. Perhaps an add'l api can be added -- eg.,
-       tty_{port_}register_named_device().

drivers/staging/fwserial/fwserial.c +93 ~~-83~~

264 lines not on this page · raw message

Signature · 1 line

1.8.1.1

Peter Hurley

author

2013-01-29 03:35 UTC #4 lore raw

PATCH 5/11 staging/fwserial: Cleanup /proc/tty/driver/ file

In reply to Peter Hurley

Factor out extra stats, data profiles, debugging info and peer info from procfs file in preparation for using debugfs instead.

Signed-off-by Peter Hurley <[email protected]>

---
 drivers/staging/fwserial/fwserial.c | 39 ++++++++++++++-----------------------
 1 file changed, 15 insertions(+), 24 deletions(-)

drivers/staging/fwserial/fwserial.c +15 ~~-24~~

index e143b7a..ba8ffa2 100644
@@ -1500,17 +1500,26 @@ static void fwtty_proc_show_port(struct seq_file *m, struct fwtty_port *port)
 	if (port->port.console)
 		(*port->fwcon_ops->stats)(&stats, port->con_data);
 
-	seq_printf(m, " tx:%d rx:%d", port->icount.tx + stats.xchars,
-		   port->icount.rx);
+	seq_printf(m, " addr:%012llx tx:%d rx:%d", port->rx_handler.offset,
+		   port->icount.tx + stats.xchars, port->icount.rx);
 	seq_printf(m, " cts:%d dsr:%d rng:%d dcd:%d", port->icount.cts,
 		   port->icount.dsr, port->icount.rng, port->icount.dcd);
 	seq_printf(m, " fe:%d oe:%d pe:%d brk:%d", port->icount.frame,
 		   port->icount.overrun, port->icount.parity, port->icount.brk);
+}
+
+static void fwtty_debugfs_show_port(struct seq_file *m, struct fwtty_port *port)
+{
+	struct stats stats;
+
+	memcpy(&stats, &port->stats, sizeof(stats));
+	if (port->port.console)
+		(*port->fwcon_ops->stats)(&stats, port->con_data);
+
 	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, " addr:%012llx", port->rx_handler.offset);
 
 	if (port->port.console) {
 		seq_printf(m, "\n    ");

@@ -1520,7 +1529,7 @@ static void fwtty_proc_show_port(struct seq_file *m, struct fwtty_port *port)
 	dump_profile(m, &port->stats);
 }
 
-static void fwtty_proc_show_peer(struct seq_file *m, struct fwtty_peer *peer)
+static void fwtty_debugfs_show_peer(struct seq_file *m, struct fwtty_peer *peer)
 {
 	int generation = peer->generation;

@@ -1529,21 +1538,14 @@ static void fwtty_proc_show_peer(struct seq_file *m, struct fwtty_peer *peer)
 	seq_printf(m, " node:%04x gen:%d", peer->node_id, generation);
 	seq_printf(m, " sp:%d max:%d guid:%016llx", peer->speed,
 		   peer->max_payload, (unsigned long long) peer->guid);
-
-	if (capable(CAP_SYS_ADMIN)) {
-		seq_printf(m, " mgmt:%012llx",
-			   (unsigned long long) peer->mgmt_addr);
-		seq_printf(m, " addr:%012llx",
-			   (unsigned long long) peer->status_addr);
-	}
+	seq_printf(m, " mgmt:%012llx", (unsigned long long) peer->mgmt_addr);
+	seq_printf(m, " addr:%012llx", (unsigned long long) peer->status_addr);
 	seq_putc(m, '\n');
 }
 
 static int fwtty_proc_show(struct seq_file *m, void *v)
 {
 	struct fwtty_port *port;
-	struct fw_serial *serial;
-	struct fwtty_peer *peer;
 	int i;
 
 	seq_puts(m, "fwserinfo: 1.0 driver: 1.0\n");

@@ -1554,17 +1556,6 @@ static int fwtty_proc_show(struct seq_file *m, void *v)
 		fwtty_port_put(port);
 		seq_printf(m, "\n");
 	}
-	seq_putc(m, '\n');
-
-	rcu_read_lock();
-	list_for_each_entry_rcu(serial, &fwserial_list, list) {
-		seq_printf(m, "card: %s  guid: %016llx\n",
-			   dev_name(serial->card->device),
-			   (unsigned long long) serial->card->guid);
-		list_for_each_entry_rcu(peer, &serial->peer_list, list)
-			fwtty_proc_show_peer(m, peer);
-	}
-	rcu_read_unlock();
 	return 0;
 }

Signature · 1 line

1.8.1.1

Peter Hurley

author

2013-01-29 03:35 UTC #5 lore raw

PATCH 6/11 staging/fwserial: Factor unstable stats/debug/status info to debugfs

In reply to Peter Hurley

Add the following file hierarchy to debugfs:

<debugfs>-+

            +- firewire_serial -+- <unit> -+- peers
                                |          +- stats
                                |
                                +- <unit> -+- peers
                                           +- stats

The 'peers' file (read-only) contains status and configuration info for attached peers for the given fwserial unit.

The 'stats' file (read-only) contains statistics and data profiling information for each tty port for the given fwserial unit.

Signed-off-by Peter Hurley <[email protected]>

---
 drivers/staging/fwserial/fwserial.c | 82 +++++++++++++++++++++++++++++++++++++
 drivers/staging/fwserial/fwserial.h |  2 +
 2 files changed, 84 insertions(+)

drivers/staging/fwserial/fwserial.c +82 -0

index ba8ffa2..66463d1 100644
@@ -84,6 +84,8 @@ static struct kmem_cache *fwtty_txn_cache;
 struct tty_driver *fwtty_driver;
 static struct tty_driver *fwloop_driver;
 
+static struct dentry *fwserial_debugfs;
+
 struct fwtty_transaction;
 typedef void (*fwtty_transaction_cb)(struct fw_card *card, int rcode,
 				     void *data, size_t length,

@@ -1559,11 +1561,71 @@ static int fwtty_proc_show(struct seq_file *m, void *v)
 	return 0;
 }
 
+static int fwtty_debugfs_stats_show(struct seq_file *m, void *v)
+{
+	struct fw_serial *serial = m->private;
+	struct fwtty_port *port;
+	int i;
+
+	for (i = 0; i < num_ports; ++i) {
+		port = fwtty_port_get(serial->ports[i]->index);
+		if (port) {
+			seq_printf(m, "%2d:", port->index);
+			fwtty_proc_show_port(m, port);
+			fwtty_debugfs_show_port(m, port);
+			fwtty_port_put(port);
+			seq_printf(m, "\n");
+		}
+	}
+	return 0;
+}
+
+static int fwtty_debugfs_peers_show(struct seq_file *m, void *v)
+{
+	struct fw_serial *serial = m->private;
+	struct fwtty_peer *peer;
+
+	rcu_read_lock();
+	seq_printf(m, "card: %s  guid: %016llx\n",
+		   dev_name(serial->card->device),
+		   (unsigned long long) serial->card->guid);
+	list_for_each_entry_rcu(peer, &serial->peer_list, list)
+		fwtty_debugfs_show_peer(m, peer);
+	rcu_read_unlock();
+	return 0;
+}
+
 static int fwtty_proc_open(struct inode *inode, struct file *fp)
 {
 	return single_open(fp, fwtty_proc_show, NULL);
 }
 
+static int fwtty_stats_open(struct inode *inode, struct file *fp)
+{
+	return single_open(fp, fwtty_debugfs_stats_show, inode->i_private);
+}
+
+static int fwtty_peers_open(struct inode *inode, struct file *fp)
+{
+	return single_open(fp, fwtty_debugfs_peers_show, inode->i_private);
+}
+
+static const struct file_operations fwtty_stats_fops = {
+	.owner =	THIS_MODULE,
+	.open =		fwtty_stats_open,
+	.read =		seq_read,
+	.llseek =	seq_lseek,
+	.release =	single_release,
+};
+
+static const struct file_operations fwtty_peers_fops = {
+	.owner =	THIS_MODULE,
+	.open =		fwtty_peers_open,
+	.read =		seq_read,
+	.llseek =	seq_lseek,
+	.release =	single_release,
+};
+
 static const struct file_operations fwtty_proc_fops = {
 	.owner =        THIS_MODULE,
 	.open =         fwtty_proc_open,

@@ -2300,6 +2362,17 @@ static int fwserial_create(struct fw_unit *unit)
 		serial->ports[j]->loopback = true;
 	}
 
+	if (!IS_ERR_OR_NULL(fwserial_debugfs)) {
+		serial->debugfs = debugfs_create_dir(dev_name(&unit->device),
+						     fwserial_debugfs);
+		if (!IS_ERR_OR_NULL(serial->debugfs)) {
+			debugfs_create_file("peers", 0444, serial->debugfs,
+					    serial, &fwtty_peers_fops);
+			debugfs_create_file("stats", 0444, serial->debugfs,
+					    serial, &fwtty_stats_fops);
+		}
+	}
+
 	list_add_rcu(&serial->list, &fwserial_list);
 
 	fwtty_notice(&unit, "TTY over FireWire on device %s (guid %016llx)",

@@ -2312,6 +2385,8 @@ static int fwserial_create(struct fw_unit *unit)
 	fwtty_err(&unit, "unable to add peer unit device (%d)", err);
 
 	/* fall-through to error processing */
+	debugfs_remove_recursive(serial->debugfs);
+
 	list_del_rcu(&serial->list);
 	if (create_loop_dev)
 		tty_unregister_device(fwloop_driver, loop_idx(serial->ports[j]));

@@ -2403,6 +2478,8 @@ static int fwserial_remove(struct device *dev)
 		/* unlink from the fwserial_list here */
 		list_del_rcu(&serial->list);
 
+		debugfs_remove_recursive(serial->debugfs);
+
 		for (i = 0; i < num_ttys; ++i)
 			fwserial_close_port(fwtty_driver, serial->ports[i]);
 		if (create_loop_dev)

@@ -2797,6 +2874,9 @@ static int __init fwserial_init(void)
 {
 	int err, num_loops = !!(create_loop_dev);
 
+	/* XXX: placeholder for a "firewire" debugfs node */
+	fwserial_debugfs = debugfs_create_dir(KBUILD_MODNAME, NULL);
+
 	/* num_ttys/num_ports must not be set above the static alloc avail */
 	if (num_ttys + num_loops > MAX_CARD_PORTS)
 		num_ttys = MAX_CARD_PORTS - num_loops;

@@ -2911,6 +2991,7 @@ unregister_driver:
 	tty_unregister_driver(fwtty_driver);
 put_tty:
 	put_tty_driver(fwtty_driver);
+	debugfs_remove_recursive(fwserial_debugfs);
 	return err;
 }

@@ -2926,6 +3007,7 @@ static void __exit fwserial_exit(void)
 	}
 	tty_unregister_driver(fwtty_driver);
 	put_tty_driver(fwtty_driver);
+	debugfs_remove_recursive(fwserial_debugfs);
 }
 
 module_init(fwserial_init);

drivers/staging/fwserial/fwserial.h +2 -0

index 3602809..c768aad 100644
@@ -15,6 +15,7 @@
 #include <linux/serial_reg.h>
 #include <linux/module.h>
 #include <linux/seq_file.h>
+#include <linux/debugfs.h>
 
 #include "dma_fifo.h"

@@ -338,6 +339,7 @@ struct fw_serial {
 	struct fw_card	  *card;
 	struct kref	  kref;
 
+	struct dentry	  *debugfs;
 	struct fwtty_peer *self;
 
 	struct list_head  list;

Signature · 1 line

1.8.1.1

Peter Hurley

author

2013-01-29 03:35 UTC #6 lore raw

PATCH 7/11 staging/fwserial: Don't use deprecated alloc_tty_driver()

In reply to Peter Hurley

Use tty_alloc_driver() instead.

Signed-off-by Peter Hurley <[email protected]>

---
 drivers/staging/fwserial/fwserial.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

drivers/staging/fwserial/fwserial.c +9 ~~-12~~

index 66463d1..912b17a 100644
@@ -2882,9 +2882,10 @@ static int __init fwserial_init(void)
 		num_ttys = MAX_CARD_PORTS - num_loops;
 	num_ports = num_ttys + num_loops;
 
-	fwtty_driver = alloc_tty_driver(MAX_TOTAL_PORTS);
-	if (!fwtty_driver) {
-		err = -ENOMEM;
+	fwtty_driver = tty_alloc_driver(MAX_TOTAL_PORTS, TTY_DRIVER_REAL_RAW
+					| TTY_DRIVER_DYNAMIC_DEV);
+	if (IS_ERR(fwtty_driver)) {
+		err = PTR_ERR(fwtty_driver);
 		return err;
 	}

@@ -2894,9 +2895,6 @@ static int __init fwserial_init(void)
 	fwtty_driver->minor_start	= 0;
 	fwtty_driver->type		= TTY_DRIVER_TYPE_SERIAL;
 	fwtty_driver->subtype		= SERIAL_TYPE_NORMAL;
-	fwtty_driver->flags		= TTY_DRIVER_REAL_RAW |
-						TTY_DRIVER_DYNAMIC_DEV;
-
 	fwtty_driver->init_termios	    = tty_std_termios;
 	fwtty_driver->init_termios.c_cflag  |= CLOCAL;
 	tty_set_operations(fwtty_driver, &fwtty_ops);

@@ -2908,9 +2906,11 @@ static int __init fwserial_init(void)
 	}
 
 	if (create_loop_dev) {
-		fwloop_driver = alloc_tty_driver(MAX_TOTAL_PORTS / num_ports);
-		if (!fwloop_driver) {
-			err = -ENOMEM;
+		fwloop_driver = tty_alloc_driver(MAX_TOTAL_PORTS / num_ports,
+						 TTY_DRIVER_REAL_RAW
+						 | TTY_DRIVER_DYNAMIC_DEV);
+		if (IS_ERR(fwloop_driver)) {
+			err = PTR_ERR(fwloop_driver);
 			goto unregister_driver;
 		}

@@ -2920,9 +2920,6 @@ static int __init fwserial_init(void)
 		fwloop_driver->minor_start	= 0;
 		fwloop_driver->type		= TTY_DRIVER_TYPE_SERIAL;
 		fwloop_driver->subtype		= SERIAL_TYPE_NORMAL;
-		fwloop_driver->flags		= TTY_DRIVER_REAL_RAW |
-							TTY_DRIVER_DYNAMIC_DEV;
-
 		fwloop_driver->init_termios	    = tty_std_termios;
 		fwloop_driver->init_termios.c_cflag  |= CLOCAL;
 		tty_set_operations(fwloop_driver, &fwloop_ops);

Signature · 1 line

1.8.1.1

Peter Hurley

author

2013-01-29 03:35 UTC #7 lore raw

PATCH 8/11 staging/fwserial: Remove reference to removed constant

In reply to Peter Hurley

FWSERIAL_TTY_START_MINOR was removed. The minor_start is allocated by tty_alloc_driver().

Signed-off-by Peter Hurley <[email protected]>

---
 drivers/staging/fwserial/fwserial.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/staging/fwserial/fwserial.h +1 -1

index c768aad..33a3a53 100644
@@ -194,7 +194,7 @@ struct buffered_rx {
  * @port: underlying tty_port
  * @device: tty device
  * @index: index into port_table for this particular port
- *    note: minor = index + FWSERIAL_TTY_START_MINOR
+ *    note: minor = index + minor_start assigned by tty_alloc_driver()
  * @serial: back pointer to the containing fw_serial
  * @rx_handler: bus address handler for unique addr region used by remotes
  *              to communicate with this port. Every port uses

Signature · 1 line

1.8.1.1

Peter Hurley

author

2013-01-29 03:36 UTC #8 lore raw

PATCH 9/11 staging/fwserial: add diagnostic for buffer overflow

In reply to Peter Hurley

Signed-off-by Peter Hurley <[email protected]>

---
 drivers/staging/fwserial/fwserial.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

drivers/staging/fwserial/fwserial.c +4 -1

index 912b17a..912ab77 100644
@@ -569,8 +569,11 @@ 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)
+	if (port->buffered + n > HIGH_WATERMARK) {
+		fwtty_err_ratelimited(port, "overflowed rx buffer: buffered: %d new: %ld wtrmk: %d",
+				      port->buffered, n, HIGH_WATERMARK);
 		return 0;
+	}
 	buf = kmalloc(size, GFP_ATOMIC);
 	if (!buf)
 		return 0;

Signature · 1 line

1.8.1.1

Peter Hurley

author

2013-01-29 03:36 UTC #9 lore raw

PATCH 10/11 staging/fwserial: Fix premature unthrottle

In reply to Peter Hurley

The unthrottle may restart the writer before the rx push has a chance to start emptying the rx buffer, resulting in an overflowed rx buffer and lost data.

Perform the actual device unthrottle with the rx push instead.

Signed-off-by Peter Hurley <[email protected]>

---
 drivers/staging/fwserial/fwserial.c | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

drivers/staging/fwserial/fwserial.c +14 ~~-16~~

index 912ab77..121beff 100644
@@ -463,6 +463,14 @@ static void __fwtty_throttle(struct fwtty_port *port, struct tty_struct *tty)
 		__fwtty_write_port_status(port);
 }
 
+static void __fwtty_unthrottle(struct fwtty_port *port, struct tty_struct *tty)
+{
+	port->mctrl &= ~OOB_RX_THROTTLE;
+	if (C_CRTSCTS(tty))
+		port->mctrl |= TIOCM_RTS;
+	__fwtty_write_port_status(port);
+}
+
 /**
  * fwtty_do_hangup - wait for ldisc to deliver all pending rx; only then hangup
  *

@@ -527,14 +535,15 @@ static void fwtty_emit_breaks(struct work_struct *work)
 	port->icount.brk += brk;
 }
 
-static void fwtty_pushrx(struct work_struct *work)
+static void fwtty_pushrx(struct fwtty_port *port, struct tty_struct *tty)
 {
-	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);
+
+	__fwtty_unthrottle(port, tty);
+
 	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);

@@ -545,11 +554,7 @@ static void fwtty_pushrx(struct work_struct *work)
 				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);
-			}
+			__fwtty_throttle(port, tty);
 			break;
 		} else {
 			list_del(&buf->list);

@@ -1264,14 +1269,7 @@ 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))
-		port->mctrl |= TIOCM_RTS;
-	__fwtty_write_port_status(port);
-	spin_unlock_bh(&port->lock);
+	fwtty_pushrx(port, tty);
 }
 
 static int check_msr_delta(struct fwtty_port *port, unsigned long mask,

Signature · 1 line

1.8.1.1

Greg Kroah-Hartman

maintainer

2013-01-30 08:08 UTC #10 lore raw

Re: [PATCH 10/11] staging/fwserial: Fix premature unthrottle

Quoted 10 lines · Peter Hurley

The unthrottle may restart the writer before the rx push has a chance to start emptying the rx buffer, resulting in an overflowed rx buffer and lost data.
Perform the actual device unthrottle with the rx push instead.
Signed-off-by Peter Hurley <[email protected]>
---
 drivers/staging/fwserial/fwserial.c | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

this patch fails to apply to my tree, but the 9 previous ones were fine, so I'll not apply it, or ght 11/11 patch. Care to fix it up and resend both of them?

thanks,

greg k-h

Peter Hurley

author

2013-01-30 13:11 UTC #11 lore raw

Re: [PATCH 10/11] staging/fwserial: Fix premature unthrottle

Quoted 7 lines · Greg Kroah-Hartman

Quoted 6 lines · Peter Hurley
The unthrottle may restart the writer before the rx push has a chance to start emptying the rx buffer, resulting in an overflowed rx buffer and lost data.
Perform the actual device unthrottle with the rx push instead.

Quoted 8 lines

Quoted 4 lines
Signed-off-by Peter Hurley <[email protected]>
---
 drivers/staging/fwserial/fwserial.c | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)
this patch fails to apply to my tree, but the 9 previous ones were fine, so I'll not apply it, or ght 11/11 patch. Care to fix it up and resend both of them?

Hi Greg,

I can rebase these against your staging-next, if you'd prefer, but it will make the -next merge non-trivial.

Your tty-next tree is carrying patches to this driver from patch series
"TTY: switch flipping functions to tty_port"
which is already in -next.

Let me know what you'd like me to do.

Regards,
Peter Hurley

Greg Kroah-Hartman

maintainer

2013-01-30 13:30 UTC #12 lore raw

Re: [PATCH 10/11] staging/fwserial: Fix premature unthrottle

Quoted 19 lines · Peter Hurley

Quoted 7 lines · Greg Kroah-Hartman
Quoted 6 lines · Peter Hurley
The unthrottle may restart the writer before the rx push has a chance to start emptying the rx buffer, resulting in an overflowed rx buffer and lost data.
Perform the actual device unthrottle with the rx push instead.
Quoted 8 lines
Quoted 4 lines
Signed-off-by Peter Hurley <[email protected]>
---
 drivers/staging/fwserial/fwserial.c | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)
this patch fails to apply to my tree, but the 9 previous ones were fine, so I'll not apply it, or ght 11/11 patch. Care to fix it up and resend both of them?
Hi Greg,

Quoted 8 lines

I can rebase these against your staging-next, if you'd prefer, but it will make the -next merge non-trivial.
Your tty-next tree is carrying patches to this driver from patch series
"TTY: switch flipping functions to tty_port"
which is already in -next.
Let me know what you'd like me to do.

Ugh, that's a mess.

Can these wait until after 3.9-rc1 is out? If so, things should get synced up by then. Otherwise I'll have to figure out how to handle the merge properly...

thanks,

greg k-h

Peter Hurley

author

2013-01-30 13:37 UTC #13 lore raw

Re: [PATCH 10/11] staging/fwserial: Fix premature unthrottle

Quoted 27 lines · Greg Kroah-Hartman

Quoted 26 lines · Peter Hurley
Quoted 7 lines · Greg Kroah-Hartman
Quoted 6 lines · Peter Hurley
The unthrottle may restart the writer before the rx push has a chance to start emptying the rx buffer, resulting in an overflowed rx buffer and lost data.
Perform the actual device unthrottle with the rx push instead.
Quoted 8 lines
Quoted 4 lines
Signed-off-by Peter Hurley <[email protected]>
---
 drivers/staging/fwserial/fwserial.c | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)
this patch fails to apply to my tree, but the 9 previous ones were fine, so I'll not apply it, or ght 11/11 patch. Care to fix it up and resend both of them?
Hi Greg,
I can rebase these against your staging-next, if you'd prefer, but it will make the -next merge non-trivial.
Your tty-next tree is carrying patches to this driver from patch series
"TTY: switch flipping functions to tty_port"
which is already in -next.

Quoted 5 lines

Quoted 1 line
Let me know what you'd like me to do.
Ugh, that's a mess.
Can these wait until after 3.9-rc1 is out?

Ok. Should I plan to resend patches 10 & 11 then (as 1 & 2)?

Regards,
Peter Hurley

Greg Kroah-Hartman

maintainer

2013-01-30 16:23 UTC #14 lore raw

Re: [PATCH 10/11] staging/fwserial: Fix premature unthrottle

Quoted 28 lines · Peter Hurley

Quoted 27 lines · Greg Kroah-Hartman
Quoted 26 lines · Peter Hurley
Quoted 7 lines · Greg Kroah-Hartman
Quoted 6 lines · Peter Hurley
The unthrottle may restart the writer before the rx push has a chance to start emptying the rx buffer, resulting in an overflowed rx buffer and lost data.
Perform the actual device unthrottle with the rx push instead.
Quoted 8 lines
Quoted 4 lines
Signed-off-by Peter Hurley <[email protected]>
---
 drivers/staging/fwserial/fwserial.c | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)
this patch fails to apply to my tree, but the 9 previous ones were fine, so I'll not apply it, or ght 11/11 patch. Care to fix it up and resend both of them?
Hi Greg,
I can rebase these against your staging-next, if you'd prefer, but it will make the -next merge non-trivial.
Your tty-next tree is carrying patches to this driver from patch series
"TTY: switch flipping functions to tty_port"
which is already in -next.

Quoted 7 lines

Quoted 5 lines
Quoted 1 line
Let me know what you'd like me to do.
Ugh, that's a mess.
Can these wait until after 3.9-rc1 is out?
Ok. Should I plan to resend patches 10 & 11 then (as 1 & 2)?

Yes, please do.

thanks,

greg k-h

Peter Hurley

author

2013-01-29 03:46 UTC #15 lore raw

PATCH 11/11 staging/fwserial: Remove unneeded push work

In reply to Peter Hurley

Since rx push can be performed synchronously with unthrottle, remove the push work struct and code references to it.

Signed-off-by Peter Hurley <[email protected]>

---
 drivers/staging/fwserial/fwserial.c | 2 --
 drivers/staging/fwserial/fwserial.h | 4 ----
 2 files changed, 6 deletions(-)

drivers/staging/fwserial/fwserial.c +0 -2

index 121beff..8983911 100644
@@ -1112,7 +1112,6 @@ static void fwtty_port_shutdown(struct tty_port *tty_port)
 
 	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) {

@@ -2300,7 +2299,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);

drivers/staging/fwserial/fwserial.h +0 -4

index 33a3a53..6c179f0 100644
@@ -223,9 +223,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

@@ -267,7 +264,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;

Signature · 1 line

1.8.1.1