4:08 p.m.
This was prompted by an IHV who shipped a lot of USB storage
attached to SMPs and is mad as hell that it doesn't work.
Ask ootpa about the fireworks :-)
I am guilty somewhat here. When I and Arjan fixed the last hoopla
with USB CD-ROM on Bladecenters & Dells, we were so distraught
by general misdesign of the code that we didn't go far enough.
It is not in upstream, although it seems clear like day to me.
In fact, I ask you to take it regardless of Matt Dharm's approval.
-- Pete
diff -urN -X dontdiff linux-2.4.22-1.2130.nptl/drivers/usb/storage/scsiglue.c
linux-2.4.22-1.2130.nptl-usbstor3/drivers/usb/storage/scsiglue.c
--- linux-2.4.22-1.2130.nptl/drivers/usb/storage/scsiglue.c 2003-06-13 07:51:37.000000000
-0700
+++ linux-2.4.22-1.2130.nptl-usbstor3/drivers/usb/storage/scsiglue.c 2003-12-16
14:04:15.000000000 -0800
@@ -213,9 +213,20 @@
static int device_reset( Scsi_Cmnd *srb )
{
struct us_data *us = (struct us_data *)srb->host->hostdata[0];
+ int rc;
US_DEBUGP("device_reset() called\n" );
- return us->transport_reset(us);
+ spin_unlock_irq(&io_request_lock);
+ down(&(us->dev_semaphore));
+ if (!us->pusb_dev) {
+ up(&(us->dev_semaphore));
+ spin_lock_irq(&io_request_lock);
+ return SUCCESS;
+ }
+ rc = us->transport_reset(us);
+ up(&(us->dev_semaphore));
+ spin_lock_irq(&io_request_lock);
+ return rc;
}
/* This resets the device port, and simulates the device
@@ -230,27 +241,32 @@
/* we use the usb_reset_device() function to handle this for us */
US_DEBUGP("bus_reset() called\n");
+ spin_unlock_irq(&io_request_lock);
+
+ down(&(us->dev_semaphore));
+
/* if the device has been removed, this worked */
if (!us->pusb_dev) {
US_DEBUGP("-- device removed already\n");
+ up(&(us->dev_semaphore));
+ spin_lock_irq(&io_request_lock);
return SUCCESS;
}
- spin_unlock_irq(&io_request_lock);
-
/* release the IRQ, if we have one */
- down(&(us->irq_urb_sem));
if (us->irq_urb) {
US_DEBUGP("-- releasing irq URB\n");
result = usb_unlink_urb(us->irq_urb);
US_DEBUGP("-- usb_unlink_urb() returned %d\n", result);
}
- up(&(us->irq_urb_sem));
/* attempt to reset the port */
if (usb_reset_device(us->pusb_dev) < 0) {
- spin_lock_irq(&io_request_lock);
- return FAILED;
+ /*
+ * Do not return errors, or else the error handler might
+ * invoke host_reset, which is not implemented.
+ */
+ goto bail_out;
}
/* FIXME: This needs to lock out driver probing while it's working
@@ -281,17 +297,18 @@
up(&intf->driver->serialize);
}
+bail_out:
/* re-allocate the IRQ URB and submit it to restore connectivity
* for CBI devices
*/
if (us->protocol == US_PR_CBI) {
- down(&(us->irq_urb_sem));
us->irq_urb->dev = us->pusb_dev;
result = usb_submit_urb(us->irq_urb);
US_DEBUGP("usb_submit_urb() returns %d\n", result);
- up(&(us->irq_urb_sem));
}
-
+
+ up(&(us->dev_semaphore));
+
spin_lock_irq(&io_request_lock);
US_DEBUGP("bus_reset() complete\n");
diff -urN -X dontdiff linux-2.4.22-1.2130.nptl/drivers/usb/storage/usb.c
linux-2.4.22-1.2130.nptl-usbstor3/drivers/usb/storage/usb.c
--- linux-2.4.22-1.2130.nptl/drivers/usb/storage/usb.c 2003-12-08 15:50:01.000000000
-0800
+++ linux-2.4.22-1.2130.nptl-usbstor3/drivers/usb/storage/usb.c 2003-12-16
14:01:41.000000000 -0800
@@ -513,6 +513,9 @@
* strucuture is current. This includes the ep_int field, which gives us
* the endpoint for the interrupt.
* Returns non-zero on failure, zero on success
+ *
+ * ss->dev_semaphore is expected taken, except for a newly minted,
+ * unregistered device.
*/
static int usb_stor_allocate_irq(struct us_data *ss)
{
@@ -522,13 +525,9 @@
US_DEBUGP("Allocating IRQ for CBI transport\n");
- /* lock access to the data structure */
- down(&(ss->irq_urb_sem));
-
/* allocate the URB */
ss->irq_urb = usb_alloc_urb(0);
if (!ss->irq_urb) {
- up(&(ss->irq_urb_sem));
US_DEBUGP("couldn't allocate interrupt URB");
return 1;
}
@@ -549,12 +548,9 @@
US_DEBUGP("usb_submit_urb() returns %d\n", result);
if (result) {
usb_free_urb(ss->irq_urb);
- up(&(ss->irq_urb_sem));
return 2;
}
- /* unlock the data structure and return success */
- up(&(ss->irq_urb_sem));
return 0;
}
@@ -782,7 +778,6 @@
init_completion(&(ss->notify));
init_MUTEX_LOCKED(&(ss->ip_waitq));
spin_lock_init(&(ss->queue_exclusion));
- init_MUTEX(&(ss->irq_urb_sem));
init_MUTEX(&(ss->current_urb_sem));
init_MUTEX(&(ss->dev_semaphore));
@@ -1079,7 +1074,6 @@
down(&(ss->dev_semaphore));
/* release the IRQ, if we have one */
- down(&(ss->irq_urb_sem));
if (ss->irq_urb) {
US_DEBUGP("-- releasing irq URB\n");
result = usb_unlink_urb(ss->irq_urb);
@@ -1087,7 +1081,6 @@
usb_free_urb(ss->irq_urb);
ss->irq_urb = NULL;
}
- up(&(ss->irq_urb_sem));
/* free up the main URB for this device */
US_DEBUGP("-- releasing main URB\n");
diff -urN -X dontdiff linux-2.4.22-1.2130.nptl/drivers/usb/storage/usb.h
linux-2.4.22-1.2130.nptl-usbstor3/drivers/usb/storage/usb.h
--- linux-2.4.22-1.2130.nptl/drivers/usb/storage/usb.h 2003-12-08 15:50:01.000000000
-0800
+++ linux-2.4.22-1.2130.nptl-usbstor3/drivers/usb/storage/usb.h 2003-12-16
14:01:41.000000000 -0800
@@ -118,7 +118,7 @@
struct us_data *next; /* next device */
/* the device we're working with */
- struct semaphore dev_semaphore; /* protect pusb_dev */
+ struct semaphore dev_semaphore; /* protect many things */
struct usb_device *pusb_dev; /* this usb_device */
unsigned int flags; /* from filter initially */
@@ -164,7 +164,6 @@
atomic_t ip_wanted[1]; /* is an IRQ expected? */
/* interrupt communications data */
- struct semaphore irq_urb_sem; /* to protect irq_urb */
struct urb *irq_urb; /* for USB int requests */
unsigned char irqbuf[2]; /* buffer for USB IRQ */
unsigned char irqdata[2]; /* data from USB IRQ */