USB: serial: fix assumption that throttle/unthrottle cannot sleep
authorOliver Neukum <oliver@neukum.org>
Wed, 7 Oct 2009 08:50:23 +0000 (10:50 +0200)
committerGreg Kroah-Hartman <gregkh@suse.de>
Fri, 9 Oct 2009 20:52:09 +0000 (13:52 -0700)
many serial subdrivers are clearly written as if throttle/unthrottle
cannot sleep. This leads to unneeded atomic submissions. This
patch converts affected drivers in a way to makes very clear that
throttle/unthrottle can sleep. Thus future misdesigns can be avoided
and efficiency and reliability improved.

This removes any such assumption using GFP_KERNEL and spin_lock_irq()

Signed-off-by: Oliver Neukum <oliver@neukum.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
drivers/usb/serial/aircable.c
drivers/usb/serial/cypress_m8.c
drivers/usb/serial/empeg.c
drivers/usb/serial/garmin_gps.c
drivers/usb/serial/keyspan_pda.c
drivers/usb/serial/kl5kusb105.c
drivers/usb/serial/mct_u232.c
drivers/usb/serial/symbolserial.c
drivers/usb/serial/visor.c
drivers/usb/serial/whiteheat.c

index 2cbfab3716e59c3764cf333d5bdb933add1e69a9..b10ac840941137a8516da829a9832b0ace64a665 100644 (file)
@@ -554,13 +554,12 @@ static void aircable_throttle(struct tty_struct *tty)
 {
        struct usb_serial_port *port = tty->driver_data;
        struct aircable_private *priv = usb_get_serial_port_data(port);
-       unsigned long flags;
 
        dbg("%s - port %d", __func__, port->number);
 
-       spin_lock_irqsave(&priv->rx_lock, flags);
+       spin_lock_irq(&priv->rx_lock);
        priv->rx_flags |= THROTTLED;
-       spin_unlock_irqrestore(&priv->rx_lock, flags);
+       spin_unlock_irq(&priv->rx_lock);
 }
 
 /* Based on ftdi_sio.c unthrottle */
@@ -569,14 +568,13 @@ static void aircable_unthrottle(struct tty_struct *tty)
        struct usb_serial_port *port = tty->driver_data;
        struct aircable_private *priv = usb_get_serial_port_data(port);
        int actually_throttled;
-       unsigned long flags;
 
        dbg("%s - port %d", __func__, port->number);
 
-       spin_lock_irqsave(&priv->rx_lock, flags);
+       spin_lock_irq(&priv->rx_lock);
        actually_throttled = priv->rx_flags & ACTUALLY_THROTTLED;
        priv->rx_flags &= ~(THROTTLED | ACTUALLY_THROTTLED);
-       spin_unlock_irqrestore(&priv->rx_lock, flags);
+       spin_unlock_irq(&priv->rx_lock);
 
        if (actually_throttled)
                schedule_work(&priv->rx_work);
index e0a8b715f2f24bcf6046e1cff52dc9cca1a304ff..a591ebec0f89c0af5d0ece8c3e9685fbe3d80d4f 100644 (file)
@@ -1155,13 +1155,12 @@ static void cypress_throttle(struct tty_struct *tty)
 {
        struct usb_serial_port *port = tty->driver_data;
        struct cypress_private *priv = usb_get_serial_port_data(port);
-       unsigned long flags;
 
        dbg("%s - port %d", __func__, port->number);
 
-       spin_lock_irqsave(&priv->lock, flags);
+       spin_lock_irq(&priv->lock);
        priv->rx_flags = THROTTLED;
-       spin_unlock_irqrestore(&priv->lock, flags);
+       spin_unlock_irq(&priv->lock);
 }
 
 
@@ -1170,14 +1169,13 @@ static void cypress_unthrottle(struct tty_struct *tty)
        struct usb_serial_port *port = tty->driver_data;
        struct cypress_private *priv = usb_get_serial_port_data(port);
        int actually_throttled, result;
-       unsigned long flags;
 
        dbg("%s - port %d", __func__, port->number);
 
-       spin_lock_irqsave(&priv->lock, flags);
+       spin_lock_irq(&priv->lock);
        actually_throttled = priv->rx_flags & ACTUALLY_THROTTLED;
        priv->rx_flags = 0;
-       spin_unlock_irqrestore(&priv->lock, flags);
+       spin_unlock_irq(&priv->lock);
 
        if (!priv->comm_is_ok)
                return;
@@ -1185,7 +1183,7 @@ static void cypress_unthrottle(struct tty_struct *tty)
        if (actually_throttled) {
                port->interrupt_in_urb->dev = port->serial->dev;
 
-               result = usb_submit_urb(port->interrupt_in_urb, GFP_ATOMIC);
+               result = usb_submit_urb(port->interrupt_in_urb, GFP_KERNEL);
                if (result) {
                        dev_err(&port->dev, "%s - failed submitting read urb, "
                                        "error %d\n", __func__, result);
index 33c9e9cf9eb2a1f92cb1b299c419d4218cc90747..7dd0e3eadbe64ce3fab049ed2af23e1ba7e424f4 100644 (file)
@@ -391,7 +391,7 @@ static void empeg_unthrottle(struct tty_struct *tty)
        dbg("%s - port %d", __func__, port->number);
 
        port->read_urb->dev = port->serial->dev;
-       result = usb_submit_urb(port->read_urb, GFP_ATOMIC);
+       result = usb_submit_urb(port->read_urb, GFP_KERNEL);
        if (result)
                dev_err(&port->dev,
                        "%s - failed submitting read urb, error %d\n",
index 20432d3455295e5fcac7df0b3958dfd0c6679ad3..5ac900e5a50edbb3a44418271fa900272f882ad8 100644 (file)
@@ -1390,14 +1390,13 @@ static void garmin_throttle(struct tty_struct *tty)
 {
        struct usb_serial_port *port = tty->driver_data;
        struct garmin_data *garmin_data_p = usb_get_serial_port_data(port);
-       unsigned long flags;
 
        dbg("%s - port %d", __func__, port->number);
        /* set flag, data received will be put into a queue
           for later processing */
-       spin_lock_irqsave(&garmin_data_p->lock, flags);
+       spin_lock_irq(&garmin_data_p->lock);
        garmin_data_p->flags |= FLAGS_QUEUING|FLAGS_THROTTLED;
-       spin_unlock_irqrestore(&garmin_data_p->lock, flags);
+       spin_unlock_irq(&garmin_data_p->lock);
 }
 
 
@@ -1405,13 +1404,12 @@ static void garmin_unthrottle(struct tty_struct *tty)
 {
        struct usb_serial_port *port = tty->driver_data;
        struct garmin_data *garmin_data_p = usb_get_serial_port_data(port);
-       unsigned long flags;
        int status;
 
        dbg("%s - port %d", __func__, port->number);
-       spin_lock_irqsave(&garmin_data_p->lock, flags);
+       spin_lock_irq(&garmin_data_p->lock);
        garmin_data_p->flags &= ~FLAGS_THROTTLED;
-       spin_unlock_irqrestore(&garmin_data_p->lock, flags);
+       spin_unlock_irq(&garmin_data_p->lock);
 
        /* in native mode send queued data to tty, in
           serial mode nothing needs to be done here */
@@ -1419,7 +1417,7 @@ static void garmin_unthrottle(struct tty_struct *tty)
                garmin_flush_queue(garmin_data_p);
 
        if (0 != (garmin_data_p->flags & FLAGS_BULK_IN_ACTIVE)) {
-               status = usb_submit_urb(port->read_urb, GFP_ATOMIC);
+               status = usb_submit_urb(port->read_urb, GFP_KERNEL);
                if (status)
                        dev_err(&port->dev,
                                "%s - failed resubmitting read urb, error %d\n",
index 257c16cc6b2abb68793da0ea2c882c3fea664f1f..1296a097f5c354285efdad53d7e10388cbaa6a7b 100644 (file)
@@ -290,7 +290,7 @@ static void keyspan_pda_rx_unthrottle(struct tty_struct *tty)
        /* just restart the receive interrupt URB */
        dbg("keyspan_pda_rx_unthrottle port %d", port->number);
        port->interrupt_in_urb->dev = port->serial->dev;
-       if (usb_submit_urb(port->interrupt_in_urb, GFP_ATOMIC))
+       if (usb_submit_urb(port->interrupt_in_urb, GFP_KERNEL))
                dbg(" usb_submit_urb(read urb) failed");
        return;
 }
index f7373371b137fd996a100ffbdfc74375a847afe4..3a7873806f465aaf02a41213df7c2d41e5f866f1 100644 (file)
@@ -951,7 +951,7 @@ static void klsi_105_unthrottle(struct tty_struct *tty)
        dbg("%s - port %d", __func__, port->number);
 
        port->read_urb->dev = port->serial->dev;
-       result = usb_submit_urb(port->read_urb, GFP_ATOMIC);
+       result = usb_submit_urb(port->read_urb, GFP_KERNEL);
        if (result)
                dev_err(&port->dev,
                        "%s - failed submitting read urb, error %d\n",
index ad4998bbf16f0855609d5ebdef763323413a3615..cd009cb280a50b29d80a50610f8303a4badbd9af 100644 (file)
@@ -777,20 +777,19 @@ static void mct_u232_throttle(struct tty_struct *tty)
 {
        struct usb_serial_port *port = tty->driver_data;
        struct mct_u232_private *priv = usb_get_serial_port_data(port);
-       unsigned long flags;
        unsigned int control_state;
 
        dbg("%s - port %d", __func__, port->number);
 
-       spin_lock_irqsave(&priv->lock, flags);
+       spin_lock_irq(&priv->lock);
        priv->rx_flags |= THROTTLED;
        if (C_CRTSCTS(tty)) {
                priv->control_state &= ~TIOCM_RTS;
                control_state = priv->control_state;
-               spin_unlock_irqrestore(&priv->lock, flags);
+               spin_unlock_irq(&priv->lock);
                (void) mct_u232_set_modem_ctrl(port->serial, control_state);
        } else {
-               spin_unlock_irqrestore(&priv->lock, flags);
+               spin_unlock_irq(&priv->lock);
        }
 }
 
@@ -799,20 +798,19 @@ static void mct_u232_unthrottle(struct tty_struct *tty)
 {
        struct usb_serial_port *port = tty->driver_data;
        struct mct_u232_private *priv = usb_get_serial_port_data(port);
-       unsigned long flags;
        unsigned int control_state;
 
        dbg("%s - port %d", __func__, port->number);
 
-       spin_lock_irqsave(&priv->lock, flags);
+       spin_lock_irq(&priv->lock);
        if ((priv->rx_flags & THROTTLED) && C_CRTSCTS(tty)) {
                priv->rx_flags &= ~THROTTLED;
                priv->control_state |= TIOCM_RTS;
                control_state = priv->control_state;
-               spin_unlock_irqrestore(&priv->lock, flags);
+               spin_unlock_irq(&priv->lock);
                (void) mct_u232_set_modem_ctrl(port->serial, control_state);
        } else {
-               spin_unlock_irqrestore(&priv->lock, flags);
+               spin_unlock_irq(&priv->lock);
        }
 }
 
index f25e5452684340896e4b67747fe47c5e8f057f9b..b282c0f2d8e565101bd17a7f0f37e220b9b17e6d 100644 (file)
@@ -165,33 +165,31 @@ static void symbol_throttle(struct tty_struct *tty)
 {
        struct usb_serial_port *port = tty->driver_data;
        struct symbol_private *priv = usb_get_serial_data(port->serial);
-       unsigned long flags;
 
        dbg("%s - port %d", __func__, port->number);
-       spin_lock_irqsave(&priv->lock, flags);
+       spin_lock_irq(&priv->lock);
        priv->throttled = true;
-       spin_unlock_irqrestore(&priv->lock, flags);
+       spin_unlock_irq(&priv->lock);
 }
 
 static void symbol_unthrottle(struct tty_struct *tty)
 {
        struct usb_serial_port *port = tty->driver_data;
        struct symbol_private *priv = usb_get_serial_data(port->serial);
-       unsigned long flags;
        int result;
        bool was_throttled;
 
        dbg("%s - port %d", __func__, port->number);
 
-       spin_lock_irqsave(&priv->lock, flags);
+       spin_lock_irq(&priv->lock);
        priv->throttled = false;
        was_throttled = priv->actually_throttled;
        priv->actually_throttled = false;
-       spin_unlock_irqrestore(&priv->lock, flags);
+       spin_unlock_irq(&priv->lock);
 
        priv->int_urb->dev = port->serial->dev;
        if (was_throttled) {
-               result = usb_submit_urb(priv->int_urb, GFP_ATOMIC);
+               result = usb_submit_urb(priv->int_urb, GFP_KERNEL);
                if (result)
                        dev_err(&port->dev,
                                "%s - failed submitting read urb, error %d\n",
index 8e2d6dd9add04678149ed605a21de3513a29ab0f..b9341f0e452b32c05ed321e413e0f9c470bdf0c4 100644 (file)
@@ -583,12 +583,11 @@ static void visor_throttle(struct tty_struct *tty)
 {
        struct usb_serial_port *port = tty->driver_data;
        struct visor_private *priv = usb_get_serial_port_data(port);
-       unsigned long flags;
 
        dbg("%s - port %d", __func__, port->number);
-       spin_lock_irqsave(&priv->lock, flags);
+       spin_lock_irq(&priv->lock);
        priv->throttled = 1;
-       spin_unlock_irqrestore(&priv->lock, flags);
+       spin_unlock_irq(&priv->lock);
 }
 
 
@@ -596,17 +595,16 @@ static void visor_unthrottle(struct tty_struct *tty)
 {
        struct usb_serial_port *port = tty->driver_data;
        struct visor_private *priv = usb_get_serial_port_data(port);
-       unsigned long flags;
        int result;
 
        dbg("%s - port %d", __func__, port->number);
-       spin_lock_irqsave(&priv->lock, flags);
+       spin_lock_irq(&priv->lock);
        priv->throttled = 0;
        priv->actually_throttled = 0;
-       spin_unlock_irqrestore(&priv->lock, flags);
+       spin_unlock_irq(&priv->lock);
 
        port->read_urb->dev = port->serial->dev;
-       result = usb_submit_urb(port->read_urb, GFP_ATOMIC);
+       result = usb_submit_urb(port->read_urb, GFP_KERNEL);
        if (result)
                dev_err(&port->dev,
                        "%s - failed submitting read urb, error %d\n",
index 62424eec33ec1ebc7fda3cc909afcbe611a502cc..1093d2eb046ad06a02e4082fe0c1656fc953f332 100644 (file)
@@ -949,13 +949,12 @@ static void whiteheat_throttle(struct tty_struct *tty)
 {
        struct usb_serial_port *port = tty->driver_data;
        struct whiteheat_private *info = usb_get_serial_port_data(port);
-       unsigned long flags;
 
        dbg("%s - port %d", __func__, port->number);
 
-       spin_lock_irqsave(&info->lock, flags);
+       spin_lock_irq(&info->lock);
        info->flags |= THROTTLED;
-       spin_unlock_irqrestore(&info->lock, flags);
+       spin_unlock_irq(&info->lock);
 
        return;
 }
@@ -966,14 +965,13 @@ static void whiteheat_unthrottle(struct tty_struct *tty)
        struct usb_serial_port *port = tty->driver_data;
        struct whiteheat_private *info = usb_get_serial_port_data(port);
        int actually_throttled;
-       unsigned long flags;
 
        dbg("%s - port %d", __func__, port->number);
 
-       spin_lock_irqsave(&info->lock, flags);
+       spin_lock_irq(&info->lock);
        actually_throttled = info->flags & ACTUALLY_THROTTLED;
        info->flags &= ~(THROTTLED | ACTUALLY_THROTTLED);
-       spin_unlock_irqrestore(&info->lock, flags);
+       spin_unlock_irq(&info->lock);
 
        if (actually_throttled)
                rx_data_softint(&info->rx_work);