Customers keep complaining that usb-storage is unusable in 2.4,
so I looked at it and WOA! Very simple things, like running
dd if=/dev/sda of=/dev/null bs=8k and then unplugging the
flash cause oopses and lockups; once those are fixed, storage
can get offline forever. Ugh.
2.6 looks much saner, but it's somewhat different and I do not
dare a backport. Here's a patch to add some sanity into the
carnival of semaphores. Oh, and never return failure from
the bus reset - it only serves to fall back to adapter reset,
which we do not implement.
I would like someone to test it, to make sure I am not
breaking things (e.g. if any hangs happen).
Greg & Matt, if you approve, I'll take arguing this with Marcelo,
if you need a help here. This is an issue for us, because
of proliferation of USB storage as factory option. And we
cannot just go to 2.6.
-- Pete
diff -urN -X dontdiff linux-2.4.23/drivers/usb/storage/scsiglue.c
linux-2.4.23-nip/drivers/usb/storage/scsiglue.c
--- linux-2.4.23/drivers/usb/storage/scsiglue.c 2003-06-13 07:51:37.000000000 -0700
+++ linux-2.4.23-nip/drivers/usb/storage/scsiglue.c 2003-12-06 01:10:46.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,28 +297,36 @@
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");
return SUCCESS;
}
-/* FIXME: This doesn't do anything right now */
static int host_reset( Scsi_Cmnd *srb )
{
- printk(KERN_CRIT "usb-storage: host_reset() requested but not implemented\n" );
- return FAILED;
+ struct us_data *us = (struct us_data *)srb->host->hostdata[0];
+
+ spin_unlock_irq(&io_request_lock);
+ down(&(us->dev_semaphore));
+ printk(KERN_CRIT "usb-storage: host_reset() requested but hardly
implemented\n" );
+ up(&(us->dev_semaphore));
+ spin_lock_irq(&io_request_lock);
+ US_DEBUGP("host_reset() complete\n");
+
+ return SUCCESS;
}
/***********************************************************************
@@ -394,6 +418,13 @@
can_queue: 1,
this_id: -1,
+ /*
+ * This is supposed to be an HBA limit. In our case, it is here
+ * because many _devices_ break if transfer is too long, but
+ * we know no reliable way to detect and blacklist them.
+ */
+ max_sectors: 240,
+
sg_tablesize: SG_ALL,
cmd_per_lun: 1,
present: 0,
diff -urN -X dontdiff linux-2.4.23/drivers/usb/storage/usb.c
linux-2.4.23-nip/drivers/usb/storage/usb.c
--- linux-2.4.23/drivers/usb/storage/usb.c 2003-09-03 14:02:38.000000000 -0700
+++ linux-2.4.23-nip/drivers/usb/storage/usb.c 2003-12-05 23:28:16.000000000 -0800
@@ -499,6 +499,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)
{
@@ -508,13 +511,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;
}
@@ -535,12 +534,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;
}
@@ -768,7 +764,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));
@@ -1059,7 +1054,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);
@@ -1067,7 +1061,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.23/drivers/usb/storage/usb.h
linux-2.4.23-nip/drivers/usb/storage/usb.h
--- linux-2.4.23/drivers/usb/storage/usb.h 2003-11-29 19:23:15.000000000 -0800
+++ linux-2.4.23-nip/drivers/usb/storage/usb.h 2003-12-05 23:28:17.000000000 -0800
@@ -116,7 +116,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 */
@@ -162,7 +162,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 */
-------------------------------------------------------
This SF.net email is sponsored by: IBM Linux Tutorials.
Become an expert in LINUX or just sharpen your skills. Sign up for IBM's
Free Linux Tutorials. Learn everything from the bash shell to sys admin.
Click now! http://ads.osdn.com/?ad_id=1278&alloc_id=3371&op=click
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
