[PATCH 0/5] V2 usb console improvements series

[PATCH 0/5] V2 usb console improvements series

[Jason Wessel]

I aggregated and ported all usb patches I have previously posted which
are not mainlined into a single series aimed at providing a stable usb
console.

Greg Kroah-Hartman already picked up the first 2 patches in the
series, but for completeness they are provided here because one of the
other patches requires the context changes.

Differences in V2 vs V1 patch set
  * The optimize usb sysrq was dropped entirely
  * Removed the unused export symbol per Alan Stern's comments
  * Updated the definition of usb_poll_irq() per Alan Stern's comments


Thanks,
Jason.


-- The recap about synchronous usb serial consoles --

This is picking up where Alan Stern and I left off after a discussion
in September 2009.

The serial uart code implements the console writes synchronously such
that you get the data right away and "pay the toll" in terms of system
performance loss if you do lots of printk's to the console.  The usb
serial console code just eats the data if there is no room left in the
queue, and you do not see the data until the interrupts are restored.

If we assume the main reason you would use a usb console device is to
get printk's then every effort should be made to be able to see the
printk's on the usb console device.  In this patch series, two
conditions must be met before the hcd device would get polled.

1) The data write is a usb serial console write
2) The usb serial driver has set the variable max_in_flight_urbs

Based on Alan's comments about the 1ms minimum response time for the
hcd device I ended up using a maximum 3ms delay loop.  The 3ms was
selected based on testing the usb_debug device and the ftdi usb rs232
port.  I found the usb_debug device needed the 1ms and the ftdi device
required slightly more than 2ms in the worst case else there was some
sporadic data loss.

I also tested incurring the penalty 3ms for each write with the device
in a "failed" capacity.  In that case it introduced a 3ms delay for
each printk() which only showed any noticeable delay with the "SysRq t"
packet.

Part of having the console stable requires that usb sysrq handler get
executed in a tasklet.  The hcd device lock is always held while the
driver call backs are executed.  The means you cannot get the sysrq
printk's out to the usb console device after the in_flight urb queue
is filled.

Thanks,
Jason.

---

The following changes since commit a3d3203e4bb40f253b1541e310dc0f9305be7c84:
  Linus Torvalds (1):
        Merge branch 'release' of git://git.kernel.org/.../lenb/linux-acpi-2.6

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/linux-2.6-kgdb.git for_gregkh

Jason Wessel (5):
      tty_port,usb-console: Fix usb serial console open/close regression
      usb-serial: Use tty_port version of console instead of the usb_serial_port version
      usb-console: pass baud from console to the initial tty open
      usb-hcd,usb-console: poll hcd device to force usb console writes
      usb-serialy,sysrq: Run the sysrq handler in a tasklet

 drivers/char/sysrq.c            |   29 ++++++++++++++++
 drivers/char/tty_port.c         |    2 +-
 drivers/usb/core/hcd.c          |   10 +++++
 drivers/usb/serial/console.c    |   70 ++++++++++++++++++++++----------------
 drivers/usb/serial/ftdi_sio.c   |    9 +++--
 drivers/usb/serial/generic.c    |    6 ++--
 drivers/usb/serial/pl2303.c     |    8 +++--
 drivers/usb/serial/usb-serial.c |    4 +-
 drivers/usb/serial/usb_debug.c  |    2 +-
 include/linux/sysrq.h           |    4 ++
 include/linux/tty.h             |    1 +
 include/linux/usb.h             |    3 ++
 include/linux/usb/serial.h      |    2 -
 13 files changed, 104 insertions(+), 46 deletions(-)

[PATCH 1/5] tty_port,usb-console: Fix usb serial console open/close regression

[Jason Wessel]

Commit e1108a63e10d344284011cccc06328b2cd3e5da3 ("usb_serial: Use the
shutdown() operation") breaks the ability to use a usb console
starting in 2.6.33.  This was observed when using
console=ttyUSB0,115200 as a boot argument with an FTDI device.  The
error is:

ftdi_sio ttyUSB0: ftdi_submit_read_urb - failed submitting read urb, error -22

The handling of the ASYNCB_INITIALIZED changed in 2.6.32 such that in
tty_port_shutdown() it always clears the flag if it is set.  The fix
is to add a variable to the tty_port struct to indicate when the tty
port is a console.

CC: Greg Kroah-Hartman <gregkh@suse.de>
CC: Alan Cox <alan@linux.intel.com>
CC: Alan Stern <stern@rowland.harvard.edu>
CC: Oliver Neukum <oliver@neukum.org>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: linux-usb@vger.kernel.org
Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 drivers/char/tty_port.c      |    2 +-
 drivers/usb/serial/console.c |    1 +
 include/linux/tty.h          |    1 +
 3 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/char/tty_port.c b/drivers/char/tty_port.c
index be492dd..a3bd1d0 100644
--- a/drivers/char/tty_port.c
+++ b/drivers/char/tty_port.c
@@ -119,7 +119,7 @@ EXPORT_SYMBOL(tty_port_tty_set);
 static void tty_port_shutdown(struct tty_port *port)
 {
 	mutex_lock(&port->mutex);
-	if (port->ops->shutdown &&
+	if (port->ops->shutdown && !port->console &&
 		test_and_clear_bit(ASYNCB_INITIALIZED, &port->flags))
 			port->ops->shutdown(port);
 	mutex_unlock(&port->mutex);
diff --git a/drivers/usb/serial/console.c b/drivers/usb/serial/console.c
index b22ac32..f347da2 100644
--- a/drivers/usb/serial/console.c
+++ b/drivers/usb/serial/console.c
@@ -181,6 +181,7 @@ static int usb_console_setup(struct console *co, char *options)
 	/* The console is special in terms of closing the device so
 	 * indicate this port is now acting as a system console. */
 	port->console = 1;
+	port->port.console = 1;
 
 	mutex_unlock(&serial->disc_mutex);
 	return retval;
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 568369a..9c9719e 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -223,6 +223,7 @@ struct tty_port {
 	wait_queue_head_t	close_wait;	/* Close waiters */
 	wait_queue_head_t	delta_msr_wait;	/* Modem status change */
 	unsigned long		flags;		/* TTY flags ASY_*/
+	unsigned char		console:1;	/* port is a console */
 	struct mutex		mutex;		/* Locking */
 	struct mutex		buf_mutex;	/* Buffer alloc lock */
 	unsigned char		*xmit_buf;	/* Optional buffer */
-- 
1.6.4.rc1

[PATCH 2/5] usb-serial: Use tty_port version of console instead of the usb_serial_port version

[Jason Wessel]

Replace all instances of using the console variable in struct
usb_serial_port with the struct tty_port version.

CC: Greg Kroah-Hartman <gregkh@suse.de>
CC: Alan Cox <alan@linux.intel.com>
CC: Alan Stern <stern@rowland.harvard.edu>
CC: Oliver Neukum <oliver@neukum.org>
CC: Andrew Morton <akpm@linux-foundation.org>
CC: linux-usb@vger.kernel.org
Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 drivers/usb/serial/console.c    |    5 ++---
 drivers/usb/serial/ftdi_sio.c   |    2 +-
 drivers/usb/serial/generic.c    |    4 ++--
 drivers/usb/serial/pl2303.c     |    2 +-
 drivers/usb/serial/usb-serial.c |    4 ++--
 include/linux/usb/serial.h      |    2 --
 6 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/serial/console.c b/drivers/usb/serial/console.c
index f347da2..4ea64fe 100644
--- a/drivers/usb/serial/console.c
+++ b/drivers/usb/serial/console.c
@@ -180,7 +180,6 @@ static int usb_console_setup(struct console *co, char *options)
 	--port->port.count;
 	/* The console is special in terms of closing the device so
 	 * indicate this port is now acting as a system console. */
-	port->console = 1;
 	port->port.console = 1;
 
 	mutex_unlock(&serial->disc_mutex);
@@ -217,7 +216,7 @@ static void usb_console_write(struct console *co,
 
 	dbg("%s - port %d, %d byte(s)", __func__, port->number, count);
 
-	if (!port->console) {
+	if (!port->port.console) {
 		dbg("%s - port not opened", __func__);
 		return;
 	}
@@ -313,7 +312,7 @@ void usb_serial_console_exit(void)
 {
 	if (usbcons_info.port) {
 		unregister_console(&usbcons);
-		usbcons_info.port->console = 0;
+		usbcons_info.port->port.console = 0;
 		usbcons_info.port = NULL;
 	}
 }
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 6af0dfa..95ec748 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -2073,7 +2073,7 @@ static int ftdi_process_packet(struct tty_struct *tty,
 		return 0;	/* status only */
 	ch = packet + 2;
 
-	if (!(port->console && port->sysrq) && flag == TTY_NORMAL)
+	if (!(port->port.console && port->sysrq) && flag == TTY_NORMAL)
 		tty_insert_flip_string(tty, ch, len);
 	else {
 		for (i = 0; i < len; i++, ch++) {
diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
index 89fac36..4dea9d7 100644
--- a/drivers/usb/serial/generic.c
+++ b/drivers/usb/serial/generic.c
@@ -437,7 +437,7 @@ static void flush_and_resubmit_read_urb(struct usb_serial_port *port)
 	/* The per character mucking around with sysrq path it too slow for
 	   stuff like 3G modems, so shortcircuit it in the 99.9999999% of cases
 	   where the USB serial is not a console anyway */
-	if (!port->console || !port->sysrq)
+	if (!port->port.console || !port->sysrq)
 		tty_insert_flip_string(tty, ch, urb->actual_length);
 	else {
 		/* Push data to tty */
@@ -556,7 +556,7 @@ void usb_serial_generic_unthrottle(struct tty_struct *tty)
 int usb_serial_handle_sysrq_char(struct tty_struct *tty,
 			struct usb_serial_port *port, unsigned int ch)
 {
-	if (port->sysrq && port->console) {
+	if (port->sysrq && port->port.console) {
 		if (ch && time_before(jiffies, port->sysrq)) {
 			handle_sysrq(ch, tty);
 			port->sysrq = 0;
diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
index 73d5f34..1891cfb 100644
--- a/drivers/usb/serial/pl2303.c
+++ b/drivers/usb/serial/pl2303.c
@@ -1056,7 +1056,7 @@ static void pl2303_push_data(struct tty_struct *tty,
 	if (line_status & UART_OVERRUN_ERROR)
 		tty_insert_flip_char(tty, 0, TTY_OVERRUN);
 
-	if (tty_flag == TTY_NORMAL && !(port->console && port->sysrq))
+	if (tty_flag == TTY_NORMAL && !(port->port.console && port->sysrq))
 		tty_insert_flip_string(tty, data, urb->actual_length);
 	else {
 		int i;
diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index 3873660..f3f6517 100644
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -289,7 +289,7 @@ static void serial_down(struct tty_port *tport)
 	 * The console is magical.  Do not hang up the console hardware
 	 * or there will be tears.
 	 */
-	if (port->console)
+	if (port->port.console)
 		return;
 	if (drv->close)
 		drv->close(port);
@@ -328,7 +328,7 @@ static void serial_cleanup(struct tty_struct *tty)
 	/* The console is magical.  Do not hang up the console hardware
 	 * or there will be tears.
 	 */
-	if (port->console)
+	if (port->port.console)
 		return;
 
 	dbg("%s - port %d", __func__, port->number);
diff --git a/include/linux/usb/serial.h b/include/linux/usb/serial.h
index 0a458b8..b7682fe 100644
--- a/include/linux/usb/serial.h
+++ b/include/linux/usb/serial.h
@@ -66,7 +66,6 @@ enum port_dev_state {
  * @work: work queue entry for the line discipline waking up.
  * @throttled: nonzero if the read urb is inactive to throttle the device
  * @throttle_req: nonzero if the tty wants to throttle us
- * @console: attached usb serial console
  * @dev: pointer to the serial device
  *
  * This structure is used by the usb-serial core and drivers for the specific
@@ -106,7 +105,6 @@ struct usb_serial_port {
 	struct work_struct	work;
 	char			throttled;
 	char			throttle_req;
-	char			console;
 	unsigned long		sysrq; /* sysrq timeout */
 	struct device		dev;
 	enum port_dev_state	dev_state;
-- 
1.6.4.rc1

[PATCH 3/5] usb-console: pass baud from console to the initial tty open

[Jason Wessel]

The usb console code has had a long standing problem of not being able
to pass the baud rate from the kernel argument console=ttyUSB0,BAUD
down to the initial tty open, unless you were willing to settle for
9600 baud.

The solution is to directly use tty_init_termios() in
usb_console_setup() as this will preserve any changes to the initial
termios setting on future opens.

CC: Greg Kroah-Hartman <gregkh@suse.de>
CC: Alan Cox <alan@linux.intel.com>
CC: Alan Stern <stern@rowland.harvard.edu>
CC: linux-usb@vger.kernel.org
Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 drivers/usb/serial/console.c |   22 ++++++++++------------
 1 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/serial/console.c b/drivers/usb/serial/console.c
index 4ea64fe..1ee6b2a 100644
--- a/drivers/usb/serial/console.c
+++ b/drivers/usb/serial/console.c
@@ -66,7 +66,7 @@ static int usb_console_setup(struct console *co, char *options)
 	struct usb_serial_port *port;
 	int retval;
 	struct tty_struct *tty = NULL;
-	struct ktermios *termios = NULL, dummy;
+	struct ktermios dummy;
 
 	dbg("%s", __func__);
 
@@ -141,15 +141,14 @@ static int usb_console_setup(struct console *co, char *options)
 				goto reset_open_count;
 			}
 			kref_init(&tty->kref);
-			termios = kzalloc(sizeof(*termios), GFP_KERNEL);
-			if (!termios) {
+			tty_port_tty_set(&port->port, tty);
+			tty->driver = usb_serial_tty_driver;
+			tty->index = co->index;
+			if (tty_init_termios(tty)) {
 				retval = -ENOMEM;
 				err("no more memory");
 				goto free_tty;
 			}
-			memset(&dummy, 0, sizeof(struct ktermios));
-			tty->termios = termios;
-			tty_port_tty_set(&port->port, tty);
 		}
 
 		/* only call the device specific open if this
@@ -161,16 +160,16 @@ static int usb_console_setup(struct console *co, char *options)
 
 		if (retval) {
 			err("could not open USB console port");
-			goto free_termios;
+			goto fail;
 		}
 
 		if (serial->type->set_termios) {
-			termios->c_cflag = cflag;
-			tty_termios_encode_baud_rate(termios, baud, baud);
+			tty->termios->c_cflag = cflag;
+			tty_termios_encode_baud_rate(tty->termios, baud, baud);
+			memset(&dummy, 0, sizeof(struct ktermios));
 			serial->type->set_termios(tty, port, &dummy);
 
 			tty_port_tty_set(&port->port, NULL);
-			kfree(termios);
 			kfree(tty);
 		}
 		set_bit(ASYNCB_INITIALIZED, &port->port.flags);
@@ -185,8 +184,7 @@ static int usb_console_setup(struct console *co, char *options)
 	mutex_unlock(&serial->disc_mutex);
 	return retval;
 
- free_termios:
-	kfree(termios);
+ fail:
 	tty_port_tty_set(&port->port, NULL);
  free_tty:
 	kfree(tty);
-- 
1.6.4.rc1

[PATCH 4/5] usb-hcd,usb-console: poll hcd device to force usb console writes

[Jason Wessel]

This patch tries to solve the problem that data is lost because there
are too many outstanding transmit urb's while trying to execute
printk's to a console.  The same is true if you try something like
"echo t > /proc/sysrq-trigger".

This patch takes the route of forcibly polling the hcd device to drain
the urb queue in order to initiate the bulk write call backs.  This
only happens if the device is a usb serial console device that sets
the max_in_flight_urbs to a non zero value in the serial device
structure.

A few millisecond penalty will get incurred to allow the hcd controller
to complete a write urb, else the console data is thrown away.

The max_in_flight_urbs was reduced in the usb_debug driver because it
is highly desired to push things out to the console in a timely
fashion and there is no need to have a queue that large for the
interrupt driven mode of operation when used through the tty
interface.

CC: Greg Kroah-Hartman <gregkh@suse.de>
CC: Alan Cox <alan@linux.intel.com>
CC: Alan Stern <stern@rowland.harvard.edu>
CC: Oliver Neukum <oliver@neukum.org>
CC: linux-usb@vger.kernel.org
Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 drivers/usb/core/hcd.c         |   10 +++++++++
 drivers/usb/serial/console.c   |   42 +++++++++++++++++++++++++--------------
 drivers/usb/serial/ftdi_sio.c  |    7 +++--
 drivers/usb/serial/pl2303.c    |    6 +++-
 drivers/usb/serial/usb_debug.c |    2 +-
 include/linux/usb.h            |    3 ++
 6 files changed, 49 insertions(+), 21 deletions(-)

diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 2f8cedd..dd710d7 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2271,6 +2271,16 @@ usb_hcd_platform_shutdown(struct platform_device* dev)
 }
 EXPORT_SYMBOL_GPL(usb_hcd_platform_shutdown);
 
+void
+usb_poll_irq(struct usb_device *udev)
+{
+	struct usb_hcd *hcd;
+
+	hcd = bus_to_hcd(udev->bus);
+	usb_hcd_irq(0, hcd);
+}
+EXPORT_SYMBOL_GPL(usb_poll_irq);
+
 /*-------------------------------------------------------------------------*/
 
 #if defined(CONFIG_USB_MON) || defined(CONFIG_USB_MON_MODULE)
diff --git a/drivers/usb/serial/console.c b/drivers/usb/serial/console.c
index 1ee6b2a..b6b96ff 100644
--- a/drivers/usb/serial/console.c
+++ b/drivers/usb/serial/console.c
@@ -197,13 +197,37 @@ static int usb_console_setup(struct console *co, char *options)
 	return retval;
 }
 
+static void usb_do_console_write(struct usb_serial *serial,
+				 struct usb_serial_port *port,
+				 const char *buf, unsigned count)
+{
+	int retval;
+	int loops = 100;
+try_again:
+	/* pass on to the driver specific version of this function if
+	   it is available */
+	if (serial->type->write)
+		retval = serial->type->write(NULL, port, buf, count);
+	else
+		retval = usb_serial_generic_write(NULL, port, buf, count);
+	if (retval < count && retval >= 0 &&
+	    serial->type->max_in_flight_urbs != 0 && loops--) {
+		/* poll the hcd device because the queue is full */
+		count -= retval;
+		buf += retval;
+		udelay(100);
+		usb_poll_irq(serial->dev);
+		goto try_again;
+	}
+	dbg("%s - return value : %d", __func__, retval);
+}
+
 static void usb_console_write(struct console *co,
 					const char *buf, unsigned count)
 {
 	static struct usbcons_info *info = &usbcons_info;
 	struct usb_serial_port *port = info->port;
 	struct usb_serial *serial;
-	int retval = -ENODEV;
 
 	if (!port || port->serial->dev->state == USB_STATE_NOTATTACHED)
 		return;
@@ -230,23 +254,11 @@ static void usb_console_write(struct console *co,
 				break;
 			}
 		}
-		/* pass on to the driver specific version of this function if
-		   it is available */
-		if (serial->type->write)
-			retval = serial->type->write(NULL, port, buf, i);
-		else
-			retval = usb_serial_generic_write(NULL, port, buf, i);
-		dbg("%s - return value : %d", __func__, retval);
+		usb_do_console_write(serial, port, buf, i);
 		if (lf) {
 			/* append CR after LF */
 			unsigned char cr = 13;
-			if (serial->type->write)
-				retval = serial->type->write(NULL,
-								port, &cr, 1);
-			else
-				retval = usb_serial_generic_write(NULL,
-								port, &cr, 1);
-			dbg("%s - return value : %d", __func__, retval);
+			usb_do_console_write(serial, port, &cr, 1);
 		}
 		buf += i;
 		count -= i;
diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
index 95ec748..c7f559c 100644
--- a/drivers/usb/serial/ftdi_sio.c
+++ b/drivers/usb/serial/ftdi_sio.c
@@ -53,6 +53,9 @@
 #define DRIVER_AUTHOR "Greg Kroah-Hartman <greg@kroah.com>, Bill Ryder <bryder@sgi.com>, Kuba Ober <kuba@mareimbrium.org>, Andreas Mohr"
 #define DRIVER_DESC "USB FTDI Serial Converters Driver"
 
+/* number of outstanding urbs to prevent userspace DoS from happening */
+#define URB_UPPER_LIMIT	42
+
 static int debug;
 static __u16 vendor = FTDI_VID;
 static __u16 product;
@@ -838,6 +841,7 @@ static struct usb_serial_driver ftdi_sio_device = {
 	.ioctl =		ftdi_ioctl,
 	.set_termios =		ftdi_set_termios,
 	.break_ctl =		ftdi_break_ctl,
+	.max_in_flight_urbs =	URB_UPPER_LIMIT,
 };
 
 
@@ -848,9 +852,6 @@ static struct usb_serial_driver ftdi_sio_device = {
 #define HIGH 1
 #define LOW 0
 
-/* number of outstanding urbs to prevent userspace DoS from happening */
-#define URB_UPPER_LIMIT	42
-
 /*
  * ***************************************************************************
  * Utility functions
diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
index 1891cfb..2615fe1 100644
--- a/drivers/usb/serial/pl2303.c
+++ b/drivers/usb/serial/pl2303.c
@@ -453,8 +453,9 @@ static void pl2303_send(struct usb_serial_port *port)
 	port->write_urb->transfer_buffer_length = count;
 	result = usb_submit_urb(port->write_urb, GFP_ATOMIC);
 	if (result) {
-		dev_err(&port->dev, "%s - failed submitting write urb,"
-			" error %d\n", __func__, result);
+		if (!(port->port.console))
+			dev_err(&port->dev, "%s - failed submitting write urb,"
+				" error %d\n", __func__, result);
 		priv->write_urb_in_use = 0;
 		/* TODO: reschedule pl2303_send */
 	}
@@ -1185,6 +1186,7 @@ static struct usb_serial_driver pl2303_device = {
 	.chars_in_buffer =	pl2303_chars_in_buffer,
 	.attach =		pl2303_startup,
 	.release =		pl2303_release,
+	.max_in_flight_urbs =	-1,
 };
 
 static int __init pl2303_init(void)
diff --git a/drivers/usb/serial/usb_debug.c b/drivers/usb/serial/usb_debug.c
index 252cc2d..4a04552 100644
--- a/drivers/usb/serial/usb_debug.c
+++ b/drivers/usb/serial/usb_debug.c
@@ -15,7 +15,7 @@
 #include <linux/usb.h>
 #include <linux/usb/serial.h>
 
-#define URB_DEBUG_MAX_IN_FLIGHT_URBS	4000
+#define URB_DEBUG_MAX_IN_FLIGHT_URBS	42
 #define USB_DEBUG_MAX_PACKET_SIZE	8
 #define USB_DEBUG_BRK_SIZE		8
 static char USB_DEBUG_BRK[USB_DEBUG_BRK_SIZE] = {
diff --git a/include/linux/usb.h b/include/linux/usb.h
index 8c9f053..a7d6cf7 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -569,6 +569,9 @@ static inline void usb_mark_last_busy(struct usb_device *udev)
 
 /*-------------------------------------------------------------------------*/
 
+/* for polling the hcd device */
+extern void usb_poll_irq(struct usb_device *udev);
+
 /* for drivers using iso endpoints */
 extern int usb_get_current_frame_number(struct usb_device *usb_dev);
 
-- 
1.6.4.rc1

[PATCH 5/5] usb-serialy,sysrq: Run the sysrq handler in a tasklet

[Jason Wessel]

If a sysrq is processed on the while holding the usb hcd lock, it is
impossible to drain the queue of urbs via the polling interface and
all the printk output is lost.

Using a tasklet to schedule the sysrq allows the hcd device lock to
free up, and you can issue a sysrq-t to get the task list.

CC: Greg Kroah-Hartman <gregkh@suse.de>
CC: Alan Cox <alan@linux.intel.com>
CC: Alan Stern <stern@rowland.harvard.edu>
CC: linux-usb@vger.kernel.org
Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 drivers/char/sysrq.c         |   29 +++++++++++++++++++++++++++++
 drivers/usb/serial/generic.c |    2 +-
 include/linux/sysrq.h        |    4 ++++
 3 files changed, 34 insertions(+), 1 deletions(-)

diff --git a/drivers/char/sysrq.c b/drivers/char/sysrq.c
index 1ae2de7..ad62e53 100644
--- a/drivers/char/sysrq.c
+++ b/drivers/char/sysrq.c
@@ -584,6 +584,35 @@ int unregister_sysrq_key(int key, struct sysrq_key_op *op_p)
 }
 EXPORT_SYMBOL(unregister_sysrq_key);
 
+/* The sysrq tasklet is used only in the rare case that an
+ * input/output character device processes a sysrq in its input
+ * routine while holding a lock required for the output routine for
+ * the console device.
+ */
+static struct sysrq_tasklet_data {
+	struct tty_struct *tty;
+	unsigned int key;
+	int pending;
+} priv_sysrq_data;
+
+static void sysrq_task(unsigned long args)
+{
+	handle_sysrq(priv_sysrq_data.key, priv_sysrq_data.tty);
+	priv_sysrq_data.pending = 0;
+}
+static DECLARE_TASKLET(sysrq_tasklet, sysrq_task, 0);
+
+void handle_sysrq_tasklet(int key, struct tty_struct *tty)
+{
+	if (priv_sysrq_data.pending)
+		return;
+	priv_sysrq_data.pending = 1;
+	priv_sysrq_data.key = key;
+	priv_sysrq_data.tty = tty;
+	tasklet_schedule(&sysrq_tasklet);
+}
+EXPORT_SYMBOL(handle_sysrq_tasklet);
+
 #ifdef CONFIG_PROC_FS
 /*
  * writing 'C' to /proc/sysrq-trigger is like sysrq-C
diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
index 4dea9d7..206f181 100644
--- a/drivers/usb/serial/generic.c
+++ b/drivers/usb/serial/generic.c
@@ -558,7 +558,7 @@ int usb_serial_handle_sysrq_char(struct tty_struct *tty,
 {
 	if (port->sysrq && port->port.console) {
 		if (ch && time_before(jiffies, port->sysrq)) {
-			handle_sysrq(ch, tty);
+			handle_sysrq_tasklet(ch, tty);
 			port->sysrq = 0;
 			return 1;
 		}
diff --git a/include/linux/sysrq.h b/include/linux/sysrq.h
index 99adcdc..0ff5fb4 100644
--- a/include/linux/sysrq.h
+++ b/include/linux/sysrq.h
@@ -52,6 +52,7 @@ extern int __sysrq_enabled;
  */
 
 void handle_sysrq(int key, struct tty_struct *tty);
+void handle_sysrq_tasklet(int key, struct tty_struct *tty);
 void __handle_sysrq(int key, struct tty_struct *tty, int check_mask);
 int register_sysrq_key(int key, struct sysrq_key_op *op);
 int unregister_sysrq_key(int key, struct sysrq_key_op *op);
@@ -70,6 +71,9 @@ static inline int __reterr(void)
 static inline void handle_sysrq(int key, struct tty_struct *tty)
 {
 }
+static inline void handle_sysrq_tasklet(int key, struct tty_struct *tty)
+{
+}
 
 #define register_sysrq_key(ig,nore) __reterr()
 #define unregister_sysrq_key(ig,nore) __reterr()
-- 
1.6.4.rc1

Re: [PATCH 1/5] tty_port,usb-console: Fix usb serial console open/close regression

[Greg KH]

On Tue, Mar 16, 2010 at 04:05:42PM -0500, Jason Wessel wrote:
> Commit e1108a63e10d344284011cccc06328b2cd3e5da3 ("usb_serial: Use the
> shutdown() operation") breaks the ability to use a usb console
> starting in 2.6.33.  This was observed when using
> console=ttyUSB0,115200 as a boot argument with an FTDI device.  The
> error is:
> 
> ftdi_sio ttyUSB0: ftdi_submit_read_urb - failed submitting read urb, error -22
> 
> The handling of the ASYNCB_INITIALIZED changed in 2.6.32 such that in
> tty_port_shutdown() it always clears the flag if it is set.  The fix
> is to add a variable to the tty_port struct to indicate when the tty
> port is a console.

I've already applied this to my trees, right?


thanks,

greg k-h

Re: [PATCH 2/5] usb-serial: Use tty_port version of console instead of the usb_serial_port version

[Greg KH]

On Tue, Mar 16, 2010 at 04:05:43PM -0500, Jason Wessel wrote:
> Replace all instances of using the console variable in struct
> usb_serial_port with the struct tty_port version.

I've also already applied this one, right?

thanks,

greg k-h

Re: [PATCH 2/5] usb-serial: Use tty_port version of consoleinstead of the usb_serial_port version

[Jason Wessel]

On 03/16/2010 04:23 PM, Greg KH wrote:
> On Tue, Mar 16, 2010 at 04:05:43PM -0500, Jason Wessel wrote:
>   
>> Replace all instances of using the console variable in struct
>> usb_serial_port with the struct tty_port version.
>>     
>
> I've also already applied this one, right?
>   
Correct.  There is no change.

Jason.

Re: [PATCH 1/5] tty_port,usb-console: Fix usb serial consoleopen/close regression

[Jason Wessel]

On 03/16/2010 04:23 PM, Greg KH wrote:
> On Tue, Mar 16, 2010 at 04:05:42PM -0500, Jason Wessel wrote:
>> Commit e1108a63e10d344284011cccc06328b2cd3e5da3 ("usb_serial: Use the
>> shutdown() operation") breaks the ability to use a usb console
>> starting in 2.6.33.  This was observed when using
>> console=ttyUSB0,115200 as a boot argument with an FTDI device.  The
>> error is:
>>
>> ftdi_sio ttyUSB0: ftdi_submit_read_urb - failed submitting read urb, error -22
>>
>> The handling of the ASYNCB_INITIALIZED changed in 2.6.32 such that in
>> tty_port_shutdown() it always clears the flag if it is set.  The fix
>> is to add a variable to the tty_port struct to indicate when the tty
>> port is a console.
> 
> I've already applied this to my trees, right?
> 
> 

Correct.  This is also in your tree.   Patches 3-5 are not.

Jason.

Re: [PATCH 4/5] usb-hcd,usb-console: poll hcd device to force usb console writes

[Greg KH]

On Tue, Mar 16, 2010 at 04:05:45PM -0500, Jason Wessel wrote:
> This patch tries to solve the problem that data is lost because there
> are too many outstanding transmit urb's while trying to execute
> printk's to a console.  The same is true if you try something like
> "echo t > /proc/sysrq-trigger".
> 
> This patch takes the route of forcibly polling the hcd device to drain
> the urb queue in order to initiate the bulk write call backs.  This
> only happens if the device is a usb serial console device that sets
> the max_in_flight_urbs to a non zero value in the serial device
> structure.
> 
> A few millisecond penalty will get incurred to allow the hcd controller
> to complete a write urb, else the console data is thrown away.
> 
> The max_in_flight_urbs was reduced in the usb_debug driver because it
> is highly desired to push things out to the console in a timely
> fashion and there is no need to have a queue that large for the
> interrupt driven mode of operation when used through the tty
> interface.

Will this new interface also be the same one that the usb debugger will
need to tie into the hcd drivers?

Either way, I'd like to get Alan Stern's ack on this before accepting it.

thanks,

greg k-h

Re: [PATCH 4/5] usb-hcd,usb-console: poll hcd device to force usb console writes

[Johan Hovold]

Hi,

On Tue, Mar 16, 2010 at 04:05:45PM -0500, Jason Wessel wrote:
> This patch tries to solve the problem that data is lost because there
> are too many outstanding transmit urb's while trying to execute
> printk's to a console.  The same is true if you try something like
> "echo t > /proc/sysrq-trigger".
> 
> This patch takes the route of forcibly polling the hcd device to drain
> the urb queue in order to initiate the bulk write call backs.  This
> only happens if the device is a usb serial console device that sets
> the max_in_flight_urbs to a non zero value in the serial device
> structure.

Why do you need to use max_in_flight_urbs for this? Shouldn't any usb
serial device be able to use the polling mode? 

Currently the parameter is only used by usb_debug to limit the number of
outstanding urbs for the generic multi-urb write implementation. But now
you're adding a second meaning to the variable and use it as a flag
when you set it to -1 for pl2303 (which uses a fifo-implementation) or
URB_UPPER_LIMIT for ftdi_sio (you could simply have used -1 here as
well). If there are drivers that definitely should not use the polling
mode, it seems to me a new flag such as console_poll (or
no_console_poll) would be more appropriate.

That is, either always poll if a console or if necessary poll only if
serial->type->console_poll is set (or no_console_poll isn't).


There are a few more comments below.


> A few millisecond penalty will get incurred to allow the hcd controller
> to complete a write urb, else the console data is thrown away.
> 
> The max_in_flight_urbs was reduced in the usb_debug driver because it
> is highly desired to push things out to the console in a timely
> fashion and there is no need to have a queue that large for the
> interrupt driven mode of operation when used through the tty
> interface.
> 
> CC: Greg Kroah-Hartman <gregkh@suse.de>
> CC: Alan Cox <alan@linux.intel.com>
> CC: Alan Stern <stern@rowland.harvard.edu>
> CC: Oliver Neukum <oliver@neukum.org>
> CC: linux-usb@vger.kernel.org
> Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
> ---
>  drivers/usb/core/hcd.c         |   10 +++++++++
>  drivers/usb/serial/console.c   |   42 +++++++++++++++++++++++++--------------
>  drivers/usb/serial/ftdi_sio.c  |    7 +++--
>  drivers/usb/serial/pl2303.c    |    6 +++-
>  drivers/usb/serial/usb_debug.c |    2 +-
>  include/linux/usb.h            |    3 ++
>  6 files changed, 49 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 2f8cedd..dd710d7 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -2271,6 +2271,16 @@ usb_hcd_platform_shutdown(struct platform_device* dev)
>  }
>  EXPORT_SYMBOL_GPL(usb_hcd_platform_shutdown);
>  
> +void
> +usb_poll_irq(struct usb_device *udev)
> +{
> +	struct usb_hcd *hcd;
> +
> +	hcd = bus_to_hcd(udev->bus);
> +	usb_hcd_irq(0, hcd);
> +}
> +EXPORT_SYMBOL_GPL(usb_poll_irq);
> +
>  /*-------------------------------------------------------------------------*/
>  
>  #if defined(CONFIG_USB_MON) || defined(CONFIG_USB_MON_MODULE)
> diff --git a/drivers/usb/serial/console.c b/drivers/usb/serial/console.c
> index 1ee6b2a..b6b96ff 100644
> --- a/drivers/usb/serial/console.c
> +++ b/drivers/usb/serial/console.c
> @@ -197,13 +197,37 @@ static int usb_console_setup(struct console *co, char *options)
>  	return retval;
>  }
>  
> +static void usb_do_console_write(struct usb_serial *serial,
> +				 struct usb_serial_port *port,
> +				 const char *buf, unsigned count)
> +{
> +	int retval;
> +	int loops = 100;
> +try_again:
> +	/* pass on to the driver specific version of this function if
> +	   it is available */
> +	if (serial->type->write)
> +		retval = serial->type->write(NULL, port, buf, count);
> +	else
> +		retval = usb_serial_generic_write(NULL, port, buf, count);
> +	if (retval < count && retval >= 0 &&
> +	    serial->type->max_in_flight_urbs != 0 && loops--) {
> +		/* poll the hcd device because the queue is full */
> +		count -= retval;
> +		buf += retval;
> +		udelay(100);
> +		usb_poll_irq(serial->dev);
> +		goto try_again;
> +	}
> +	dbg("%s - return value : %d", __func__, retval);
> +}
> +
>  static void usb_console_write(struct console *co,
>  					const char *buf, unsigned count)
>  {
>  	static struct usbcons_info *info = &usbcons_info;
>  	struct usb_serial_port *port = info->port;
>  	struct usb_serial *serial;
> -	int retval = -ENODEV;
>  
>  	if (!port || port->serial->dev->state == USB_STATE_NOTATTACHED)
>  		return;
> @@ -230,23 +254,11 @@ static void usb_console_write(struct console *co,
>  				break;
>  			}
>  		}
> -		/* pass on to the driver specific version of this function if
> -		   it is available */
> -		if (serial->type->write)
> -			retval = serial->type->write(NULL, port, buf, i);
> -		else
> -			retval = usb_serial_generic_write(NULL, port, buf, i);
> -		dbg("%s - return value : %d", __func__, retval);
> +		usb_do_console_write(serial, port, buf, i);
>  		if (lf) {
>  			/* append CR after LF */
>  			unsigned char cr = 13;
> -			if (serial->type->write)
> -				retval = serial->type->write(NULL,
> -								port, &cr, 1);
> -			else
> -				retval = usb_serial_generic_write(NULL,
> -								port, &cr, 1);
> -			dbg("%s - return value : %d", __func__, retval);
> +			usb_do_console_write(serial, port, &cr, 1);
>  		}
>  		buf += i;
>  		count -= i;
> diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c
> index 95ec748..c7f559c 100644
> --- a/drivers/usb/serial/ftdi_sio.c
> +++ b/drivers/usb/serial/ftdi_sio.c
> @@ -53,6 +53,9 @@
>  #define DRIVER_AUTHOR "Greg Kroah-Hartman <greg@kroah.com>, Bill Ryder <bryder@sgi.com>, Kuba Ober <kuba@mareimbrium.org>, Andreas Mohr"
>  #define DRIVER_DESC "USB FTDI Serial Converters Driver"
>  
> +/* number of outstanding urbs to prevent userspace DoS from happening */
> +#define URB_UPPER_LIMIT	42
> +
>  static int debug;
>  static __u16 vendor = FTDI_VID;
>  static __u16 product;
> @@ -838,6 +841,7 @@ static struct usb_serial_driver ftdi_sio_device = {
>  	.ioctl =		ftdi_ioctl,
>  	.set_termios =		ftdi_set_termios,
>  	.break_ctl =		ftdi_break_ctl,
> +	.max_in_flight_urbs =	URB_UPPER_LIMIT,

Here max_in_flight_urbs is simply used as a flag (could have used -1
here as well).

>  };
>  
>  
> @@ -848,9 +852,6 @@ static struct usb_serial_driver ftdi_sio_device = {
>  #define HIGH 1
>  #define LOW 0
>  
> -/* number of outstanding urbs to prevent userspace DoS from happening */
> -#define URB_UPPER_LIMIT	42
> -
>  /*
>   * ***************************************************************************
>   * Utility functions
> diff --git a/drivers/usb/serial/pl2303.c b/drivers/usb/serial/pl2303.c
> index 1891cfb..2615fe1 100644
> --- a/drivers/usb/serial/pl2303.c
> +++ b/drivers/usb/serial/pl2303.c
> @@ -453,8 +453,9 @@ static void pl2303_send(struct usb_serial_port *port)
>  	port->write_urb->transfer_buffer_length = count;
>  	result = usb_submit_urb(port->write_urb, GFP_ATOMIC);
>  	if (result) {
> -		dev_err(&port->dev, "%s - failed submitting write urb,"
> -			" error %d\n", __func__, result);
> +		if (!(port->port.console))
> +			dev_err(&port->dev, "%s - failed submitting write urb,"
> +				" error %d\n", __func__, result);

Why do you treat pl2303 different from all other drivers (including
ftdi_sio and usb_debug) which all report error when submitting an urb
failed? Is this crucial to not get locked up in some recursion?

>  		priv->write_urb_in_use = 0;
>  		/* TODO: reschedule pl2303_send */
>  	}
> @@ -1185,6 +1186,7 @@ static struct usb_serial_driver pl2303_device = {
>  	.chars_in_buffer =	pl2303_chars_in_buffer,
>  	.attach =		pl2303_startup,
>  	.release =		pl2303_release,
> +	.max_in_flight_urbs =	-1,
>  };

Here max_in_flight_urbs is again used as a flag in a way that is
unrelated to its original meaning as pl2303 does not uses the generic
multi-urb writes but rather a custom fifo and a single urb.

>  static int __init pl2303_init(void)
> diff --git a/drivers/usb/serial/usb_debug.c b/drivers/usb/serial/usb_debug.c
> index 252cc2d..4a04552 100644
> --- a/drivers/usb/serial/usb_debug.c
> +++ b/drivers/usb/serial/usb_debug.c
> @@ -15,7 +15,7 @@
>  #include <linux/usb.h>
>  #include <linux/usb/serial.h>
>  
> -#define URB_DEBUG_MAX_IN_FLIGHT_URBS	4000
> +#define URB_DEBUG_MAX_IN_FLIGHT_URBS	42
>  #define USB_DEBUG_MAX_PACKET_SIZE	8
>  #define USB_DEBUG_BRK_SIZE		8
>  static char USB_DEBUG_BRK[USB_DEBUG_BRK_SIZE] = {
> diff --git a/include/linux/usb.h b/include/linux/usb.h
> index 8c9f053..a7d6cf7 100644
> --- a/include/linux/usb.h
> +++ b/include/linux/usb.h
> @@ -569,6 +569,9 @@ static inline void usb_mark_last_busy(struct usb_device *udev)
>  
>  /*-------------------------------------------------------------------------*/
>  
> +/* for polling the hcd device */
> +extern void usb_poll_irq(struct usb_device *udev);
> +
>  /* for drivers using iso endpoints */
>  extern int usb_get_current_frame_number(struct usb_device *usb_dev);

Regards,
Johan

Re: [PATCH 0/5] V2 usb console improvements series

[Alan Stern]

On Tue, 16 Mar 2010, Jason Wessel wrote:

> Part of having the console stable requires that usb sysrq handler get
> executed in a tasklet.  The hcd device lock is always held while the
> driver call backs are executed.  The means you cannot get the sysrq
> printk's out to the usb console device after the in_flight urb queue
> is filled.

Either I don't understand this comment properly or else it is simply
wrong.  HCDs do not continue to hold their locks when handing completed
URBs back to drivers.  The giveback call occurs with interrupts
disabled, but no locks are held.

Does this mean one of the patches in your series can be simplified?

Alan Stern

Re: [PATCH 4/5] usb-hcd,usb-console: poll hcd device to force usb console writes

[Alan Stern]

On Tue, 16 Mar 2010, Jason Wessel wrote:

> This patch tries to solve the problem that data is lost because there
> are too many outstanding transmit urb's while trying to execute
> printk's to a console.  The same is true if you try something like
> "echo t > /proc/sysrq-trigger".
> 
> This patch takes the route of forcibly polling the hcd device to drain
> the urb queue in order to initiate the bulk write call backs.  This
> only happens if the device is a usb serial console device that sets
> the max_in_flight_urbs to a non zero value in the serial device
> structure.
> 
> A few millisecond penalty will get incurred to allow the hcd controller
> to complete a write urb, else the console data is thrown away.
> 
> The max_in_flight_urbs was reduced in the usb_debug driver because it
> is highly desired to push things out to the console in a timely
> fashion and there is no need to have a queue that large for the
> interrupt driven mode of operation when used through the tty
> interface.

> diff --git a/include/linux/usb.h b/include/linux/usb.h
> index 8c9f053..a7d6cf7 100644
> --- a/include/linux/usb.h
> +++ b/include/linux/usb.h
> @@ -569,6 +569,9 @@ static inline void usb_mark_last_busy(struct usb_device *udev)
>  
>  /*-------------------------------------------------------------------------*/
>  
> +/* for polling the hcd device */
> +extern void usb_poll_irq(struct usb_device *udev);
> +

The points Johan Hovold made about max_in_flight_urbs are well taken
and they should be addressed before this patch is accepted.

The parts about usb_poll_irq() are fine, although the comment above 
should be changed to something more like:

	/* for polling the device's host controller */

since "hcd" is short for "host controller driver", and "polling the 
host controller driver device" doesn't make much sense.  The same goes 
for the comment in usb_do_console_write().

Alan Stern

Re: [PATCH 0/5] V2 usb console improvements series

[Jason Wessel]

On 03/17/2010 09:30 AM, Alan Stern wrote:
> On Tue, 16 Mar 2010, Jason Wessel wrote:
>
>   
>> Part of having the console stable requires that usb sysrq handler get
>> executed in a tasklet.  The hcd device lock is always held while the
>> driver call backs are executed.  The means you cannot get the sysrq
>> printk's out to the usb console device after the in_flight urb queue
>> is filled.
>>     
>
> Either I don't understand this comment properly or else it is simply
> wrong.  HCDs do not continue to hold their locks when handing completed
> URBs back to drivers.  The giveback call occurs with interrupts
> disabled, but no locks are held.
>
>   

The call back from the urb processing holds a lock.  The sysrq is
invoked from that context, which will do a whole lot of printk()
(sysrq-t for example).   The problem is that writes will queue for a bit
and then cannot get drained because the hcd lock is not available. 
Using a tasklet seemed like the obvious choice to move the sysrq
processing to a context where the hcd lock is not likely to be in use.

> Does this mean one of the patches in your series can be simplified?
>   

I will further simplify if you have some recommendations.

Thanks,
Jason.

Re: [PATCH 0/5] V2 usb console improvements series

[Alan Stern]

On Wed, 17 Mar 2010, Jason Wessel wrote:

> On 03/17/2010 09:30 AM, Alan Stern wrote:
> > On Tue, 16 Mar 2010, Jason Wessel wrote:
> >
> >   
> >> Part of having the console stable requires that usb sysrq handler get
> >> executed in a tasklet.  The hcd device lock is always held while the
> >> driver call backs are executed.  The means you cannot get the sysrq
> >> printk's out to the usb console device after the in_flight urb queue
> >> is filled.
> >>     
> >
> > Either I don't understand this comment properly or else it is simply
> > wrong.  HCDs do not continue to hold their locks when handing completed
> > URBs back to drivers.  The giveback call occurs with interrupts
> > disabled, but no locks are held.
> >
> >   
> 
> The call back from the urb processing holds a lock.

Which callback and which lock, exactly?  Surely not the "hcd device
lock" as your description says, and maybe not the "driver call back".

Alan Stern

Re: [PATCH 0/5] V2 usb console improvements series

[Jon Smirl]

On Tue, Mar 16, 2010 at 5:05 PM, Jason Wessel
<jason.wessel@windriver.com> wrote:
> I aggregated and ported all usb patches I have previously posted which
> are not mainlined into a single series aimed at providing a stable usb
> console.

Jason, are you aware that the FTDI chips correctly signals the RTS/CTS
lines when it is busy doing USB processing? They work ok ignoring
RTS/CTS up to about 600Kb/s. For higher baud rates you needs to
implement RTS/CTS. My embedded system has a DMA based serial port that
I am running at 2Mb/s. The RTS/CTS handshake works to automatically
flow control the hardware DMA system. If you don't do this the chip
will simply drop the characters.


>
> Greg Kroah-Hartman already picked up the first 2 patches in the
> series, but for completeness they are provided here because one of the
> other patches requires the context changes.
>
> Differences in V2 vs V1 patch set
>  * The optimize usb sysrq was dropped entirely
>  * Removed the unused export symbol per Alan Stern's comments
>  * Updated the definition of usb_poll_irq() per Alan Stern's comments
>
>
> Thanks,
> Jason.
>
>
> -- The recap about synchronous usb serial consoles --
>
> This is picking up where Alan Stern and I left off after a discussion
> in September 2009.
>
> The serial uart code implements the console writes synchronously such
> that you get the data right away and "pay the toll" in terms of system
> performance loss if you do lots of printk's to the console.  The usb
> serial console code just eats the data if there is no room left in the
> queue, and you do not see the data until the interrupts are restored.
>
> If we assume the main reason you would use a usb console device is to
> get printk's then every effort should be made to be able to see the
> printk's on the usb console device.  In this patch series, two
> conditions must be met before the hcd device would get polled.
>
> 1) The data write is a usb serial console write
> 2) The usb serial driver has set the variable max_in_flight_urbs
>
> Based on Alan's comments about the 1ms minimum response time for the
> hcd device I ended up using a maximum 3ms delay loop.  The 3ms was
> selected based on testing the usb_debug device and the ftdi usb rs232
> port.  I found the usb_debug device needed the 1ms and the ftdi device
> required slightly more than 2ms in the worst case else there was some
> sporadic data loss.
>
> I also tested incurring the penalty 3ms for each write with the device
> in a "failed" capacity.  In that case it introduced a 3ms delay for
> each printk() which only showed any noticeable delay with the "SysRq t"
> packet.
>
> Part of having the console stable requires that usb sysrq handler get
> executed in a tasklet.  The hcd device lock is always held while the
> driver call backs are executed.  The means you cannot get the sysrq
> printk's out to the usb console device after the in_flight urb queue
> is filled.
>
> Thanks,
> Jason.
>
> ---
>
> The following changes since commit a3d3203e4bb40f253b1541e310dc0f9305be7c84:
>  Linus Torvalds (1):
>        Merge branch 'release' of git://git.kernel.org/.../lenb/linux-acpi-2.6
>
> are available in the git repository at:
>
>  git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/linux-2.6-kgdb.git for_gregkh
>
> Jason Wessel (5):
>      tty_port,usb-console: Fix usb serial console open/close regression
>      usb-serial: Use tty_port version of console instead of the usb_serial_port version
>      usb-console: pass baud from console to the initial tty open
>      usb-hcd,usb-console: poll hcd device to force usb console writes
>      usb-serialy,sysrq: Run the sysrq handler in a tasklet
>
>  drivers/char/sysrq.c            |   29 ++++++++++++++++
>  drivers/char/tty_port.c         |    2 +-
>  drivers/usb/core/hcd.c          |   10 +++++
>  drivers/usb/serial/console.c    |   70 ++++++++++++++++++++++----------------
>  drivers/usb/serial/ftdi_sio.c   |    9 +++--
>  drivers/usb/serial/generic.c    |    6 ++--
>  drivers/usb/serial/pl2303.c     |    8 +++--
>  drivers/usb/serial/usb-serial.c |    4 +-
>  drivers/usb/serial/usb_debug.c  |    2 +-
>  include/linux/sysrq.h           |    4 ++
>  include/linux/tty.h             |    1 +
>  include/linux/usb.h             |    3 ++
>  include/linux/usb/serial.h      |    2 -
>  13 files changed, 104 insertions(+), 46 deletions(-)
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>



-- 
Jon Smirl
jonsmirl@gmail.com