[PATCH] USB: add ability for usb-serial drivers to determine if their write urb is...

This removes a lot of racy and buggy code by trying to check the status of the urb.

Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

diff --git a/drivers/usb/serial/cyberjack.c b/drivers/usb/serial/cyberjack.c
index 46a204cd40e19c614a795407e63b4c91debb6a64..b5b431067b086bedabcf3f83851b5d55a2c2e214 100644 (file)
--- a/drivers/usb/serial/cyberjack.c
+++ b/drivers/usb/serial/cyberjack.c
@@ -213,10 +213,14 @@ static int cyberjack_write (struct usb_serial_port *port, const unsigned char *b
                return (0);
        }
 
-       if (port->write_urb->status == -EINPROGRESS) {
+       spin_lock(&port->lock);
+       if (port->write_urb_busy) {
+               spin_unlock(&port->lock);
                dbg("%s - already writing", __FUNCTION__);
-               return (0);
+               return 0;
        }
+       port->write_urb_busy = 1;
+       spin_unlock(&port->lock);
 
        spin_lock_irqsave(&priv->lock, flags);
 
@@ -224,6 +228,7 @@ static int cyberjack_write (struct usb_serial_port *port, const unsigned char *b
                /* To much data for buffer. Reset buffer. */
                priv->wrfilled=0;
                spin_unlock_irqrestore(&priv->lock, flags);
+               port->write_urb_busy = 0;
                return (0);
        }
 
@@ -268,6 +273,7 @@ static int cyberjack_write (struct usb_serial_port *port, const unsigned char *b
                        priv->wrfilled=0;
                        priv->wrsent=0;
                        spin_unlock_irqrestore(&priv->lock, flags);
+                       port->write_urb_busy = 0;
                        return 0;
                }
 
@@ -412,7 +418,8 @@ static void cyberjack_write_bulk_callback (struct urb *urb, struct pt_regs *regs
        struct cyberjack_private *priv = usb_get_serial_port_data(port);
 
        dbg("%s - port %d", __FUNCTION__, port->number);
-       
+
+       port->write_urb_busy = 0;
        if (urb->status) {
                dbg("%s - nonzero write bulk status received: %d", __FUNCTION__, urb->status);
                return;
@@ -424,12 +431,6 @@ static void cyberjack_write_bulk_callback (struct urb *urb, struct pt_regs *regs
        if( priv->wrfilled ) {
                int length, blksize, result;
 
-               if (port->write_urb->status == -EINPROGRESS) {
-                       dbg("%s - already writing", __FUNCTION__);
-                       spin_unlock(&priv->lock);
-                       return;
-               }
-
                dbg("%s - transmitting data (frame n)", __FUNCTION__);
 
                length = ((priv->wrfilled - priv->wrsent) > port->bulk_out_size) ?

diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c
index 99214aa3cd19ba03f05c1db91eef572e72c59083..ddde5fb13f6b33a9003cebcd94a118ee1f37d821 100644 (file)
--- a/drivers/usb/serial/generic.c
+++ b/drivers/usb/serial/generic.c
@@ -174,10 +174,14 @@ int usb_serial_generic_write(struct usb_serial_port *port, const unsigned char *
 
        /* only do something if we have a bulk out endpoint */
        if (serial->num_bulk_out) {
-               if (port->write_urb->status == -EINPROGRESS) {
+               spin_lock(&port->lock);
+               if (port->write_urb_busy) {
+                       spin_unlock(&port->lock);
                        dbg("%s - already writing", __FUNCTION__);
-                       return (0);
+                       return 0;
                }
+               port->write_urb_busy = 1;
+               spin_unlock(&port->lock);
 
                count = (count > port->bulk_out_size) ? port->bulk_out_size : count;
 
@@ -195,17 +199,20 @@ int usb_serial_generic_write(struct usb_serial_port *port, const unsigned char *
                                     usb_serial_generic_write_bulk_callback), port);
 
                /* send the data out the bulk port */
+               port->write_urb_busy = 1;
                result = usb_submit_urb(port->write_urb, GFP_ATOMIC);
-               if (result)
+               if (result) {
                        dev_err(&port->dev, "%s - failed submitting write urb, error %d\n", __FUNCTION__, result);
-               else
+                       /* don't have to grab the lock here, as we will retry if != 0 */
+                       port->write_urb_busy = 0;
+               } else
                        result = count;
 
                return result;
        }
 
        /* no bulk out, so return 0 bytes written */
-       return (0);
+       return 0;
 }
 
 int usb_serial_generic_write_room (struct usb_serial_port *port)
@@ -214,9 +221,9 @@ int usb_serial_generic_write_room (struct usb_serial_port *port)
        int room = 0;
 
        dbg("%s - port %d", __FUNCTION__, port->number);
-       
+
        if (serial->num_bulk_out) {
-               if (port->write_urb->status != -EINPROGRESS)
+               if (port->write_urb_busy)
                        room = port->bulk_out_size;
        }
 
@@ -232,7 +239,7 @@ int usb_serial_generic_chars_in_buffer (struct usb_serial_port *port)
        dbg("%s - port %d", __FUNCTION__, port->number);
 
        if (serial->num_bulk_out) {
-               if (port->write_urb->status == -EINPROGRESS)
+               if (port->write_urb_busy)
                        chars = port->write_urb->transfer_buffer_length;
        }
 
@@ -291,6 +298,7 @@ void usb_serial_generic_write_bulk_callback (struct urb *urb, struct pt_regs *re
 
        dbg("%s - port %d", __FUNCTION__, port->number);
 
+       port->write_urb_busy = 0;
        if (urb->status) {
                dbg("%s - nonzero write bulk status received: %d", __FUNCTION__, urb->status);
                return;

diff --git a/drivers/usb/serial/ipaq.c b/drivers/usb/serial/ipaq.c
index 3bd69c4ef24b6bdaf6e9f3b837c48f64b2d0e6a7..c05c2a2a0f31ddf31f0bde26cd68c32a59560824 100644 (file)
--- a/drivers/usb/serial/ipaq.c
+++ b/drivers/usb/serial/ipaq.c
@@ -818,11 +818,6 @@ static void ipaq_write_gather(struct usb_serial_port *port)
        struct ipaq_packet      *pkt, *tmp;
        struct urb              *urb = port->write_urb;
 
-       if (urb->status == -EINPROGRESS) {
-               /* Should never happen */
-               err("%s - flushing while urb is active !", __FUNCTION__);
-               return;
-       }
        room = URBDATA_SIZE;
        list_for_each_entry_safe(pkt, tmp, &priv->queue, list) {
                count = min(room, (int)(pkt->len - pkt->written));

diff --git a/drivers/usb/serial/ipw.c b/drivers/usb/serial/ipw.c
index 11105d74f461dec2961193ffab0f89091b4781be..85e242459c2765449b7bc2ee3e0ec3dc189c4308 100644 (file)
--- a/drivers/usb/serial/ipw.c
+++ b/drivers/usb/serial/ipw.c
@@ -399,16 +399,21 @@ static int ipw_write(struct usb_serial_port *port, const unsigned char *buf, int
                dbg("%s - write request of 0 bytes", __FUNCTION__);
                return 0;
        }
-       
-       /* Racy and broken, FIXME properly! */
-       if (port->write_urb->status == -EINPROGRESS)
+
+       spin_lock(&port->lock);
+       if (port->write_urb_busy) {
+               spin_unlock(&port->lock);
+               dbg("%s - already writing", __FUNCTION__);
                return 0;
+       }
+       port->write_urb_busy = 1;
+       spin_unlock(&port->lock);
 
        count = min(count, port->bulk_out_size);
        memcpy(port->bulk_out_buffer, buf, count);
 
        dbg("%s count now:%d", __FUNCTION__, count);
-       
+
        usb_fill_bulk_urb(port->write_urb, dev,
                          usb_sndbulkpipe(dev, port->bulk_out_endpointAddress),
                          port->write_urb->transfer_buffer,
@@ -418,6 +423,7 @@ static int ipw_write(struct usb_serial_port *port, const unsigned char *buf, int
 
        ret = usb_submit_urb(port->write_urb, GFP_ATOMIC);
        if (ret != 0) {
+               port->write_urb_busy = 0;
                dbg("%s - usb_submit_urb(write bulk) failed with error = %d", __FUNCTION__, ret);
                return ret;
        }

diff --git a/drivers/usb/serial/ir-usb.c b/drivers/usb/serial/ir-usb.c
index 59f234df5f89555e96beb87fd54c9cb6dc1d7465..937b2fdd7171ab04a045caac5edffb2775f8e210 100644 (file)
--- a/drivers/usb/serial/ir-usb.c
+++ b/drivers/usb/serial/ir-usb.c
@@ -341,10 +341,14 @@ static int ir_write (struct usb_serial_port *port, const unsigned char *buf, int
        if (count == 0)
                return 0;
 
-       if (port->write_urb->status == -EINPROGRESS) {
-               dbg ("%s - already writing", __FUNCTION__);
+       spin_lock(&port->lock);
+       if (port->write_urb_busy) {
+               spin_unlock(&port->lock);
+               dbg("%s - already writing", __FUNCTION__);
                return 0;
        }
+       port->write_urb_busy = 1;
+       spin_unlock(&port->lock);
 
        transfer_buffer = port->write_urb->transfer_buffer;
        transfer_size = min(count, port->bulk_out_size - 1);
@@ -374,9 +378,10 @@ static int ir_write (struct usb_serial_port *port, const unsigned char *buf, int
        port->write_urb->transfer_flags = URB_ZERO_PACKET;
 
        result = usb_submit_urb (port->write_urb, GFP_ATOMIC);
-       if (result)
+       if (result) {
+               port->write_urb_busy = 0;
                dev_err(&port->dev, "%s - failed submitting write urb, error %d\n", __FUNCTION__, result);
-       else
+       } else
                result = transfer_size;
 
        return result;
@@ -387,7 +392,8 @@ static void ir_write_bulk_callback (struct urb *urb, struct pt_regs *regs)
        struct usb_serial_port *port = (struct usb_serial_port *)urb->context;
 
        dbg("%s - port %d", __FUNCTION__, port->number);
-       
+
+       port->write_urb_busy = 0;
        if (urb->status) {
                dbg("%s - nonzero write bulk status received: %d", __FUNCTION__, urb->status);
                return;

diff --git a/drivers/usb/serial/keyspan_pda.c b/drivers/usb/serial/keyspan_pda.c
index 7fd0aa9eccf6e104db982f96ff30acbf7822c704..635c384cb15a600219d111d5b5eccd3be5695cb0 100644 (file)
--- a/drivers/usb/serial/keyspan_pda.c
+++ b/drivers/usb/serial/keyspan_pda.c
@@ -520,9 +520,13 @@ static int keyspan_pda_write(struct usb_serial_port *port,
           the TX urb is in-flight (wait until it completes)
           the device is full (wait until it says there is room)
        */
-       if (port->write_urb->status == -EINPROGRESS || priv->tx_throttled ) {
-               return( 0 );
+       spin_lock(&port->lock);
+       if (port->write_urb_busy || priv->tx_throttled) {
+               spin_unlock(&port->lock);
+               return 0;
        }
+       port->write_urb_busy = 1;
+       spin_unlock(&port->lock);
 
        /* At this point the URB is in our control, nobody else can submit it
           again (the only sudden transition was the one from EINPROGRESS to
@@ -570,7 +574,7 @@ static int keyspan_pda_write(struct usb_serial_port *port,
                memcpy (port->write_urb->transfer_buffer, buf, count);
                /* send the data out the bulk port */
                port->write_urb->transfer_buffer_length = count;
-               
+
                priv->tx_room -= count;
 
                port->write_urb->dev = port->serial->dev;
@@ -593,6 +597,8 @@ static int keyspan_pda_write(struct usb_serial_port *port,
 
        rc = count;
 exit:
+       if (rc < 0)
+               port->write_urb_busy = 0;
        return rc;
 }
 
@@ -602,6 +608,7 @@ static void keyspan_pda_write_bulk_callback (struct urb *urb, struct pt_regs *re
        struct usb_serial_port *port = (struct usb_serial_port *)urb->context;
        struct keyspan_pda_private *priv;
 
+       port->write_urb_busy = 0;
        priv = usb_get_serial_port_data(port);
 
        /* queue up a wakeup at scheduler time */
@@ -626,12 +633,12 @@ static int keyspan_pda_write_room (struct usb_serial_port *port)
 static int keyspan_pda_chars_in_buffer (struct usb_serial_port *port)
 {
        struct keyspan_pda_private *priv;
-       
+
        priv = usb_get_serial_port_data(port);
-       
+
        /* when throttled, return at least WAKEUP_CHARS to tell select() (via
           n_tty.c:normal_poll() ) that we're not writeable. */
-       if( port->write_urb->status == -EINPROGRESS || priv->tx_throttled )
+       if (port->write_urb_busy || priv->tx_throttled)
                return 256;
        return 0;
 }

diff --git a/drivers/usb/serial/omninet.c b/drivers/usb/serial/omninet.c
index b5f2c06d4f3e6a69e713dc4250cd6519ac00ec75..6a99ae192df110d8022d6d8c157d4741808ceaa6 100644 (file)
--- a/drivers/usb/serial/omninet.c
+++ b/drivers/usb/serial/omninet.c
@@ -254,10 +254,15 @@ static int omninet_write (struct usb_serial_port *port, const unsigned char *buf
                dbg("%s - write request of 0 bytes", __FUNCTION__);
                return (0);
        }
-       if (wport->write_urb->status == -EINPROGRESS) {
+
+       spin_lock(&port->lock);
+       if (port->write_urb_busy) {
+               spin_unlock(&port->lock);
                dbg("%s - already writing", __FUNCTION__);
-               return (0);
+               return 0;
        }
+       port->write_urb_busy = 1;
+       spin_unlock(&port->lock);
 
        count = (count > OMNINET_BULKOUTSIZE) ? OMNINET_BULKOUTSIZE : count;
 
@@ -275,9 +280,10 @@ static int omninet_write (struct usb_serial_port *port, const unsigned char *buf
 
        wport->write_urb->dev = serial->dev;
        result = usb_submit_urb(wport->write_urb, GFP_ATOMIC);
-       if (result)
+       if (result) {
+               port->write_urb_busy = 0;
                err("%s - failed submitting write urb, error %d", __FUNCTION__, result);
-       else
+       } else
                result = count;
 
        return result;
@@ -291,7 +297,7 @@ static int omninet_write_room (struct usb_serial_port *port)
 
        int room = 0; // Default: no room
 
-       if (wport->write_urb->status != -EINPROGRESS)
+       if (wport->write_urb_busy)
                room = wport->bulk_out_size - OMNINET_HEADERLEN;
 
 //     dbg("omninet_write_room returns %d", room);
@@ -306,6 +312,7 @@ static void omninet_write_bulk_callback (struct urb *urb, struct pt_regs *regs)
 
 //     dbg("omninet_write_bulk_callback, port %0x\n", port);
 
+       port->write_urb_busy = 0;
        if (urb->status) {
                dbg("%s - nonzero write bulk status received: %d", __FUNCTION__, urb->status);
                return;

diff --git a/drivers/usb/serial/safe_serial.c b/drivers/usb/serial/safe_serial.c
index 0e85ed6c6c195ced68b8e9b0c99769cb4517981f..96a17568cbf1f4cdc8a684984c9b78c5d7e3f858 100644 (file)
--- a/drivers/usb/serial/safe_serial.c
+++ b/drivers/usb/serial/safe_serial.c
@@ -299,10 +299,14 @@ static int safe_write (struct usb_serial_port *port, const unsigned char *buf, i
                dbg ("%s - write request of 0 bytes", __FUNCTION__);
                return (0);
        }
-       if (port->write_urb->status == -EINPROGRESS) {
-               dbg ("%s - already writing", __FUNCTION__);
-               return (0);
+       spin_lock(&port->lock);
+       if (port->write_urb_busy) {
+               spin_unlock(&port->lock);
+               dbg("%s - already writing", __FUNCTION__);
+               return 0;
        }
+       port->write_urb_busy = 1;
+       spin_unlock(&port->lock);
 
        packet_length = port->bulk_out_size;    // get max packetsize
 
@@ -354,6 +358,7 @@ static int safe_write (struct usb_serial_port *port, const unsigned char *buf, i
 #endif
        port->write_urb->dev = port->serial->dev;
        if ((result = usb_submit_urb (port->write_urb, GFP_KERNEL))) {
+               port->write_urb_busy = 0;
                err ("%s - failed submitting write urb, error %d", __FUNCTION__, result);
                return 0;
        }
@@ -368,7 +373,7 @@ static int safe_write_room (struct usb_serial_port *port)
 
        dbg ("%s", __FUNCTION__);
 
-       if (port->write_urb->status != -EINPROGRESS)
+       if (port->write_urb_busy)
                room = port->bulk_out_size - (safe ? 2 : 0);
 
        if (room) {

diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c
index 5da76dd8fb285681d4f5774b2468dc14794ec910..0267b26dde18ae5b72b38c0d02722b0ac982e07b 100644 (file)
--- a/drivers/usb/serial/usb-serial.c
+++ b/drivers/usb/serial/usb-serial.c
@@ -1047,6 +1047,7 @@ int usb_serial_probe(struct usb_interface *interface,
                memset(port, 0x00, sizeof(struct usb_serial_port));
                port->number = i + serial->minor;
                port->serial = serial;
+               spin_lock_init(&port->lock);
                INIT_WORK(&port->work, usb_serial_port_softint, port);
                serial->port[i] = port;
        }

diff --git a/drivers/usb/serial/usb-serial.h b/drivers/usb/serial/usb-serial.h
index d1f0c4057fa61e88be238a5cd0ac8ff99e388487..57f92f054c75ded3572f5ee9ee0845f69ab317fc 100644 (file)
--- a/drivers/usb/serial/usb-serial.h
+++ b/drivers/usb/serial/usb-serial.h
@@ -69,6 +69,7 @@
  * usb_serial_port: structure for the specific ports of a device.
  * @serial: pointer back to the struct usb_serial owner of this port.
  * @tty: pointer to the corresponding tty for this port.
+ * @lock: spinlock to grab when updating portions of this structure.
  * @number: the number of the port (the minor number).
  * @interrupt_in_buffer: pointer to the interrupt in buffer for this port.
  * @interrupt_in_urb: pointer to the interrupt in struct urb for this port.
@@ -98,6 +99,7 @@
 struct usb_serial_port {
        struct usb_serial *     serial;
        struct tty_struct *     tty;
+       spinlock_t              lock;
        unsigned char           number;
 
        unsigned char *         interrupt_in_buffer;
@@ -117,6 +119,7 @@ struct usb_serial_port {
        unsigned char *         bulk_out_buffer;
        int                     bulk_out_size;
        struct urb *            write_urb;
+       int                     write_urb_busy;
        __u8                    bulk_out_endpointAddress;
 
        wait_queue_head_t       write_wait;