USB: usb_debug, usb_generic_serial: implement multi urb write

The usb_debug driver, when used as the console, will always fail to
insert the carriage return and new line sequence as well as randomly
drop console output.  This is a result of only having the single
write_urb and that the tty layer will have a lock that prevents the
processing of the back to back urb requests.

The solution is to allow more than one urb to be outstanding and have
a slightly deeper transmit queue.  The idea and some code is borrowed
from the ftdi_sio usb driver.

The generic usb serial driver was modified so as to allow the classic
method of 1 write urb, or a multi write urb scheme with N allowed
outstanding urbs where N is controlled by max_in_flight_urbs.  When
max_in_flight_urbs in a "struct usb_serial_driver" is non zero the
multi write urb scheme will be used.

The size of 4000 was selected for the usb_debug driver so that the
driver lowers possibility of losing the queued console messages during
the kernel startup.

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
index be82ea956720184d545a0c43bce6f684c032fb7e..c919686521ff6048578caff283c12229843d1b8d 100644 (file)
--- a/drivers/usb/serial/generic.c
+++ b/drivers/usb/serial/generic.c
@@ -190,6 +190,88 @@ void usb_serial_generic_close(struct usb_serial_port *port)
        generic_cleanup(port);
 }
 
+static int usb_serial_multi_urb_write(struct tty_struct *tty,
+       struct usb_serial_port *port, const unsigned char *buf, int count)
+{
+       unsigned long flags;
+       struct urb *urb;
+       unsigned char *buffer;
+       int status;
+       int towrite;
+       int bwrite = 0;
+
+       dbg("%s - port %d", __func__, port->number);
+
+       if (count == 0)
+               dbg("%s - write request of 0 bytes", __func__);
+
+       while (count > 0) {
+               towrite = (count > port->bulk_out_size) ?
+                       port->bulk_out_size : count;
+               spin_lock_irqsave(&port->lock, flags);
+               if (port->urbs_in_flight >
+                   port->serial->type->max_in_flight_urbs) {
+                       spin_unlock_irqrestore(&port->lock, flags);
+                       dbg("%s - write limit hit\n", __func__);
+                       return bwrite;
+               }
+               port->tx_bytes_flight += towrite;
+               port->urbs_in_flight++;
+               spin_unlock_irqrestore(&port->lock, flags);
+
+               buffer = kmalloc(towrite, GFP_ATOMIC);
+               if (!buffer) {
+                       dev_err(&port->dev,
+                       "%s ran out of kernel memory for urb ...\n", __func__);
+                       goto error_no_buffer;
+               }
+
+               urb = usb_alloc_urb(0, GFP_ATOMIC);
+               if (!urb) {
+                       dev_err(&port->dev, "%s - no more free urbs\n",
+                               __func__);
+                       goto error_no_urb;
+               }
+
+               /* Copy data */
+               memcpy(buffer, buf + bwrite, towrite);
+               usb_serial_debug_data(debug, &port->dev, __func__,
+                                     towrite, buffer);
+               /* fill the buffer and send it */
+               usb_fill_bulk_urb(urb, port->serial->dev,
+                       usb_sndbulkpipe(port->serial->dev,
+                                       port->bulk_out_endpointAddress),
+                       buffer, towrite,
+                       usb_serial_generic_write_bulk_callback, port);
+
+               status = usb_submit_urb(urb, GFP_ATOMIC);
+               if (status) {
+                       dev_err(&port->dev,
+                               "%s - failed submitting write urb, error %d\n",
+                               __func__, status);
+                       goto error;
+               }
+
+               /* This urb is the responsibility of the host driver now */
+               usb_free_urb(urb);
+               dbg("%s write: %d", __func__, towrite);
+               count -= towrite;
+               bwrite += towrite;
+       }
+       return bwrite;
+
+error:
+       usb_free_urb(urb);
+error_no_urb:
+       kfree(buffer);
+error_no_buffer:
+       spin_lock_irqsave(&port->lock, flags);
+       port->urbs_in_flight--;
+       port->tx_bytes_flight -= towrite;
+       spin_unlock_irqrestore(&port->lock, flags);
+       return bwrite;
+}
+
 int usb_serial_generic_write(struct tty_struct *tty,
        struct usb_serial_port *port, const unsigned char *buf, int count)
 {
@@ -207,6 +289,11 @@ int usb_serial_generic_write(struct tty_struct *tty,
        /* only do something if we have a bulk out endpoint */
        if (serial->num_bulk_out) {
                unsigned long flags;
+
+               if (serial->type->max_in_flight_urbs)
+                       return usb_serial_multi_urb_write(tty, port,
+                                                         buf, count);
+
                spin_lock_irqsave(&port->lock, flags);
                if (port->write_urb_busy) {
                        spin_unlock_irqrestore(&port->lock, flags);
@@ -257,15 +344,18 @@ int usb_serial_generic_write_room(struct tty_struct *tty)
 {
        struct usb_serial_port *port = tty->driver_data;
        struct usb_serial *serial = port->serial;
+       unsigned long flags;
        int room = 0;
 
        dbg("%s - port %d", __func__, port->number);
-
-       /* FIXME: Locking */
-       if (serial->num_bulk_out) {
-               if (!(port->write_urb_busy))
+       spin_lock_irqsave(&port->lock, flags);
+       if (serial->type->max_in_flight_urbs) {
+               if (port->urbs_in_flight < serial->type->max_in_flight_urbs)
                        room = port->bulk_out_size;
+       } else if (serial->num_bulk_out && !(port->write_urb_busy)) {
+               room = port->bulk_out_size;
        }
+       spin_unlock_irqrestore(&port->lock, flags);
 
        dbg("%s - returns %d", __func__, room);
        return room;
@@ -276,11 +366,16 @@ int usb_serial_generic_chars_in_buffer(struct tty_struct *tty)
        struct usb_serial_port *port = tty->driver_data;
        struct usb_serial *serial = port->serial;
        int chars = 0;
+       unsigned long flags;
 
        dbg("%s - port %d", __func__, port->number);
 
-       /* FIXME: Locking */
-       if (serial->num_bulk_out) {
+       if (serial->type->max_in_flight_urbs) {
+               spin_lock_irqsave(&port->lock, flags);
+               chars = port->tx_bytes_flight;
+               spin_unlock_irqrestore(&port->lock, flags);
+       } else if (serial->num_bulk_out) {
+               /* FIXME: Locking */
                if (port->write_urb_busy)
                        chars = port->write_urb->transfer_buffer_length;
        }
@@ -363,12 +458,24 @@ EXPORT_SYMBOL_GPL(usb_serial_generic_read_bulk_callback);
 
 void usb_serial_generic_write_bulk_callback(struct urb *urb)
 {
+       unsigned long flags;
        struct usb_serial_port *port = urb->context;
        int status = urb->status;
 
        dbg("%s - port %d", __func__, port->number);
 
-       port->write_urb_busy = 0;
+       if (port->serial->type->max_in_flight_urbs) {
+               spin_lock_irqsave(&port->lock, flags);
+               --port->urbs_in_flight;
+               port->tx_bytes_flight -= urb->transfer_buffer_length;
+               if (port->urbs_in_flight < 0)
+                       port->urbs_in_flight = 0;
+               spin_unlock_irqrestore(&port->lock, flags);
+       } else {
+               /* Handle the case for single urb mode */
+               port->write_urb_busy = 0;
+       }
+
        if (status) {
                dbg("%s - nonzero write bulk status received: %d",
                    __func__, status);

diff --git a/drivers/usb/serial/usb_debug.c b/drivers/usb/serial/usb_debug.c
index 6c9cbb59552a72626181b7ba6b5e9ee3811a1938..a9427a8b86724ae7eec4f19198849293ebabbe44 100644 (file)
--- a/drivers/usb/serial/usb_debug.c
+++ b/drivers/usb/serial/usb_debug.c
@@ -15,6 +15,7 @@
 #include <linux/usb.h>
 #include <linux/usb/serial.h>
 
+#define URB_DEBUG_MAX_IN_FLIGHT_URBS   4000
 #define USB_DEBUG_MAX_PACKET_SIZE      8
 
 static struct usb_device_id id_table [] = {
@@ -46,6 +47,7 @@ static struct usb_serial_driver debug_device = {
        .id_table =             id_table,
        .num_ports =            1,
        .open =                 usb_debug_open,
+       .max_in_flight_urbs =   URB_DEBUG_MAX_IN_FLIGHT_URBS,
 };
 
 static int __init debug_init(void)

diff --git a/include/linux/usb/serial.h b/include/linux/usb/serial.h
index 8cdfed738fe4b8b28d0f7208c8553f0cf2eaee03..e2938fd179e2a8516cd6eb30855a9cd386665b25 100644 (file)
--- a/include/linux/usb/serial.h
+++ b/include/linux/usb/serial.h
@@ -91,6 +91,9 @@ struct usb_serial_port {
        int                     write_urb_busy;
        __u8                    bulk_out_endpointAddress;
 
+       int                     tx_bytes_flight;
+       int                     urbs_in_flight;
+
        wait_queue_head_t       write_wait;
        struct work_struct      work;
        char                    throttled;
@@ -207,6 +210,7 @@ struct usb_serial_driver {
        struct device_driver    driver;
        struct usb_driver       *usb_driver;
        struct usb_dynids       dynids;
+       int                     max_in_flight_urbs;
 
        int (*probe)(struct usb_serial *serial, const struct usb_device_id *id);
        int (*attach)(struct usb_serial *serial);