found an error in sierra autosuspend #2

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

Please disregard the last version as it was still buggy. To get rid
of te races we simply need to be merciless to the VM and use GFP_ATOMIC.
This version should fix it. Could you test?

	Regards
		Oliver

--

commit 5221b37f1c1b7a79fe4a6e59fa8758251c097a2f
Author: Oliver Neukum <oliver@xxxxxxxxxx>
Date:   Wed Aug 26 19:00:58 2009 +0200

    usb:autosuspend for the sierra driver supporting always online connections

diff --git a/drivers/usb/serial/sierra.c b/drivers/usb/serial/sierra.c
index f48d05e..9ca86df 100644
--- a/drivers/usb/serial/sierra.c
+++ b/drivers/usb/serial/sierra.c
@@ -51,6 +51,12 @@ struct sierra_iface_info {
 	const u8  *ifaceinfo;	/* pointer to the array holding the numbers */
 };
 
+struct sierra_intf_private {
+	spinlock_t susp_lock;
+	unsigned int suspended:1;
+	int in_flight;
+};
+
 static int sierra_set_power_state(struct usb_device *udev, __u16 swiState)
 {
 	int result;
@@ -144,6 +150,7 @@ static int sierra_probe(struct usb_serial *serial,
 {
 	int result = 0;
 	struct usb_device *udev;
+	struct sierra_intf_private *data;
 	u8 ifnum;
 
 	udev = serial->dev;
@@ -171,6 +178,11 @@ static int sierra_probe(struct usb_serial *serial,
 		return -ENODEV;
 	}
 
+	data = serial->private = kzalloc(sizeof(struct sierra_intf_private), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+	spin_lock_init(&data->susp_lock);
+
 	return result;
 }
 
@@ -261,6 +273,8 @@ static struct usb_driver sierra_driver = {
 	.name       = "sierra",
 	.probe      = usb_serial_probe,
 	.disconnect = usb_serial_disconnect,
+	.suspend    = usb_serial_suspend,
+	.resume     = usb_serial_resume,
 	.id_table   = id_table,
 	.no_dynamic_id = 	1,
 };
@@ -268,6 +282,8 @@ static struct usb_driver sierra_driver = {
 struct sierra_port_private {
 	spinlock_t lock;	/* lock the structure */
 	int outstanding_urbs;	/* number of out urbs in flight */
+	struct usb_anchor active;
+	struct usb_anchor delayed;
 
 	/* Input endpoints and buffers for this port */
 	struct urb *in_urbs[N_IN_URB];
@@ -279,6 +295,8 @@ struct sierra_port_private {
 	int dsr_state;
 	int dcd_state;
 	int ri_state;
+
+	unsigned int opened:1;
 };
 
 static int sierra_send_setup(struct usb_serial_port *port)
@@ -390,21 +408,25 @@ static void sierra_outdat_callback(struct urb *urb)
 {
 	struct usb_serial_port *port = urb->context;
 	struct sierra_port_private *portdata = usb_get_serial_port_data(port);
+	struct sierra_intf_private *intfdata;
 	int status = urb->status;
-	unsigned long flags;
 
 	dev_dbg(&port->dev, "%s - port %d\n", __func__, port->number);
+	intfdata = port->serial->private;
 
 	/* free up the transfer buffer, as usb_free_urb() does not do this */
 	kfree(urb->transfer_buffer);
-
+	usb_autopm_put_interface_async(port->serial->interface);
 	if (status)
 		dev_dbg(&port->dev, "%s - nonzero write bulk status "
 		    "received: %d\n", __func__, status);
 
-	spin_lock_irqsave(&portdata->lock, flags);
+	spin_lock(&portdata->lock);
 	--portdata->outstanding_urbs;
-	spin_unlock_irqrestore(&portdata->lock, flags);
+	spin_unlock(&portdata->lock);
+	spin_lock(&intfdata->susp_lock);
+	--intfdata->in_flight;
+	spin_unlock(&intfdata->susp_lock);
 
 	usb_serial_port_softint(port);
 }
@@ -414,6 +436,7 @@ static int sierra_write(struct tty_struct *tty, struct usb_serial_port *port,
 					const unsigned char *buf, int count)
 {
 	struct sierra_port_private *portdata = usb_get_serial_port_data(port);
+	struct sierra_intf_private *intfdata;
 	struct usb_serial *serial = port->serial;
 	unsigned long flags;
 	unsigned char *buffer;
@@ -426,9 +449,9 @@ static int sierra_write(struct tty_struct *tty, struct usb_serial_port *port,
 		return 0;
 
 	portdata = usb_get_serial_port_data(port);
+	intfdata = serial->private;
 
 	dev_dbg(&port->dev, "%s: write (%zd bytes)\n", __func__, writesize);
-
 	spin_lock_irqsave(&portdata->lock, flags);
 	dev_dbg(&port->dev, "%s - outstanding_urbs: %d\n", __func__,
 		portdata->outstanding_urbs);
@@ -442,6 +465,10 @@ static int sierra_write(struct tty_struct *tty, struct usb_serial_port *port,
 		portdata->outstanding_urbs);
 	spin_unlock_irqrestore(&portdata->lock, flags);
 
+	retval = usb_autopm_get_interface_async(serial->interface);
+	if (retval < 0)
+		goto error_simple;
+
 	buffer = kmalloc(writesize, GFP_ATOMIC);
 	if (!buffer) {
 		dev_err(&port->dev, "out of memory\n");
@@ -468,14 +495,29 @@ static int sierra_write(struct tty_struct *tty, struct usb_serial_port *port,
 	/* Handle the need to send a zero length packet */
 	urb->transfer_flags |= URB_ZERO_PACKET;
 
+	spin_lock_irqsave(&intfdata->susp_lock, flags);
+
+	if (intfdata->suspended) {
+		usb_anchor_urb(urb, &portdata->delayed);
+		spin_unlock_irqrestore(&intfdata->susp_lock, flags);
+		goto skip_power;
+	} else {
+		usb_anchor_urb(urb, &portdata->active);
+	}
 	/* send it down the pipe */
 	retval = usb_submit_urb(urb, GFP_ATOMIC);
 	if (retval) {
+		usb_unanchor_urb(urb);
+		spin_unlock_irqrestore(&intfdata->susp_lock, flags);
 		dev_err(&port->dev, "%s - usb_submit_urb(write bulk) failed "
 			"with status = %d\n", __func__, retval);
 		goto error;
+	} else {
+		intfdata->in_flight++;
+		spin_unlock_irqrestore(&intfdata->susp_lock, flags);
 	}
 
+skip_power:
 	/* we are done with this urb, so let the host driver
 	 * really free it when it is finished with it */
 	usb_free_urb(urb);
@@ -491,6 +533,8 @@ error_no_buffer:
 	dev_dbg(&port->dev, "%s - 2. outstanding_urbs: %d\n", __func__,
 		portdata->outstanding_urbs);
 	spin_unlock_irqrestore(&portdata->lock, flags);
+	usb_autopm_put_interface_async(serial->interface);
+error_simple:
 	return retval;
 }
 
@@ -530,6 +574,7 @@ static void sierra_indat_callback(struct urb *urb)
 
 	/* Resubmit urb so we continue receiving */
 	if (port->port.count && status != -ESHUTDOWN && status != -EPERM) {
+		usb_mark_last_busy(port->serial->dev);
 		err = usb_submit_urb(urb, GFP_ATOMIC);
 		if (err)
 			dev_err(&port->dev, "resubmit read urb failed."
@@ -591,6 +636,7 @@ static void sierra_instat_callback(struct urb *urb)
 
 	/* Resubmit urb so we continue receiving IRQ data */
 	if (port->port.count && status != -ESHUTDOWN && status != -ENOENT) {
+		usb_mark_last_busy(serial->dev);
 		urb->dev = serial->dev;
 		err = usb_submit_urb(urb, GFP_ATOMIC);
 		if (err)
@@ -711,6 +757,8 @@ static void sierra_close(struct usb_serial_port *port)
 	int i;
 	struct usb_serial *serial = port->serial;
 	struct sierra_port_private *portdata;
+	struct sierra_intf_private *intfdata = port->serial->private;
+
 
 	dev_dbg(&port->dev, "%s\n", __func__);
 	portdata = usb_get_serial_port_data(port);
@@ -723,6 +771,10 @@ static void sierra_close(struct usb_serial_port *port)
 		if (!serial->disconnected)
 			sierra_send_setup(port);
 		mutex_unlock(&serial->disc_mutex);
+		spin_lock_irq(&intfdata->susp_lock);
+		portdata->opened = 0;
+		spin_unlock_irq(&intfdata->susp_lock);
+
 
 		/* Stop reading urbs */
 		sierra_stop_rx_urbs(port);
@@ -731,6 +783,8 @@ static void sierra_close(struct usb_serial_port *port)
 			sierra_release_urb(portdata->in_urbs[i]);
 			portdata->in_urbs[i] = NULL;
 		}
+		usb_autopm_get_interface(serial->interface);
+		serial->interface->needs_remote_wakeup = 0;	
 	}
 }
 
@@ -739,6 +793,7 @@ static int sierra_open(struct tty_struct *tty,
 {
 	struct sierra_port_private *portdata;
 	struct usb_serial *serial = port->serial;
+	struct sierra_intf_private *intfdata = serial->private;
 	int i;
 	int err;
 	int endpoint;
@@ -772,6 +827,12 @@ static int sierra_open(struct tty_struct *tty,
 	}
 	sierra_send_setup(port);
 
+	serial->interface->needs_remote_wakeup = 1;
+	spin_lock_irq(&intfdata->susp_lock);
+	portdata->opened = 1;
+	spin_unlock_irq(&intfdata->susp_lock);
+	usb_autopm_put_interface(serial->interface);
+
 	return 0;
 }
 
@@ -819,6 +880,8 @@ static int sierra_startup(struct usb_serial *serial)
 			return -ENOMEM;
 		}
 		spin_lock_init(&portdata->lock);
+		init_usb_anchor(&portdata->active);
+		init_usb_anchor(&portdata->delayed);
 		/* Set the port private data pointer */
 		usb_set_serial_port_data(port, portdata);
 	}
@@ -845,6 +908,83 @@ static void sierra_release(struct usb_serial *serial)
 	}
 }
 
+static void stop_read_write_urbs(struct usb_serial *serial)
+{
+	int i, j;
+	struct usb_serial_port *port;
+	struct sierra_port_private *portdata;
+
+	/* Stop reading/writing urbs */
+	for (i = 0; i < serial->num_ports; ++i) {
+		port = serial->port[i];
+		portdata = usb_get_serial_port_data(port);
+		for (j = 0; j < N_IN_URB; j++)
+			usb_kill_urb(portdata->in_urbs[j]);
+		usb_kill_anchored_urbs(&portdata->active);
+	}
+}
+
+static int sierra_suspend(struct usb_serial *serial, pm_message_t message)
+{
+	struct sierra_intf_private *intfdata;
+	int b;
+
+	if (serial->dev->auto_pm) {
+		intfdata = serial->private;
+		spin_lock_irq(&intfdata->susp_lock);
+		b = intfdata->in_flight;
+				
+		if (b) {
+			spin_unlock_irq(&intfdata->susp_lock);
+			return -EBUSY;
+		} else {
+			intfdata->suspended = 1;
+			spin_unlock_irq(&intfdata->susp_lock);
+		}
+	}
+	stop_read_write_urbs(serial);
+
+	return 0;
+}
+
+static int sierra_resume(struct usb_serial *serial)
+{
+	struct usb_serial_port *port;
+	struct sierra_intf_private *intfdata = serial->private;
+	struct sierra_port_private *portdata;
+	struct urb *urb;
+	int ec = 0;
+	int i, err;
+
+	spin_lock_irq(&intfdata->susp_lock);
+	for (i = 0; i < serial->num_ports; i++) {
+		port = serial->port[i];
+		portdata = usb_get_serial_port_data(port);
+
+		while ((urb = usb_get_from_anchor(&portdata->delayed))) {
+			usb_anchor_urb(urb, &portdata->active); 
+			intfdata->in_flight++;
+			err = usb_submit_urb(urb, GFP_ATOMIC);
+			if (err < 0) {
+				intfdata->in_flight--;
+				usb_unanchor_urb(urb);
+				usb_scuttle_anchored_urbs(&portdata->delayed);
+				break;
+			}
+		}
+
+		if (portdata->opened) {
+			err = sierra_submit_rx_urbs(port, GFP_ATOMIC);
+			if (err)
+				ec++;
+		}
+	}
+	intfdata->suspended = 0;
+	spin_unlock_irq(&intfdata->susp_lock);
+
+	return ec ? -EIO : 0;
+}
+
 static struct usb_serial_driver sierra_device = {
 	.driver = {
 		.owner =	THIS_MODULE,
@@ -865,6 +1005,8 @@ static struct usb_serial_driver sierra_device = {
 	.tiocmset          = sierra_tiocmset,
 	.attach            = sierra_startup,
 	.release           = sierra_release,
+	.suspend	   = sierra_suspend,
+	.resume		   = sierra_resume,
 	.read_int_callback = sierra_instat_callback,
 };
 

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux