Lukas Postupa wrote:

Unloading module ehci_hcd after usb storage hung up does not work;
rmmod "hangs".

Hmm, that symptom could be caused by a bug I noticed a while back. I didn't have a chance to test the fix, but it compiles and runs OK in "normal" operations.

See if this patch makes a difference:

 - Simplified irq-safe access to the request spinlock (cheaper)
 - Cleans up properly when faults are reported before all of
   the scatterlist urbs got submitted.

Faults in that window are clearly uncommon, although hardware
locking up the way you described could easily trigger them.

- Dave



--- 1.39/drivers/usb/core/message.c     Tue Oct 28 14:51:28 2003
+++ edited/drivers/usb/core/message.c   Mon Jan 12 13:05:56 2004
@@ -213,9 +213,8 @@
 static void sg_complete (struct urb *urb, struct pt_regs *regs)
 {
        struct usb_sg_request   *io = (struct usb_sg_request *) urb->context;
-       unsigned long           flags;
 
-       spin_lock_irqsave (&io->lock, flags);
+       spin_lock (&io->lock);
 
        /* In 2.5 we require hcds' endpoint queues not to progress after fault
         * reports, until the completion callback (this!) returns.  That lets
@@ -269,7 +268,7 @@
        if (!io->count)
                complete (&io->complete);
 
-       spin_unlock_irqrestore (&io->lock, flags);
+       spin_unlock (&io->lock);
 }
 
 
@@ -441,12 +440,11 @@
  */
 void usb_sg_wait (struct usb_sg_request *io)
 {
-       int             i;
-       unsigned long   flags;
+       int             i, entries = io->entries;
 
        /* queue the urbs.  */
-       spin_lock_irqsave (&io->lock, flags);
-       for (i = 0; i < io->entries && !io->status; i++) {
+       spin_lock_irq (&io->lock);
+       for (i = 0; i < entries && !io->status; i++) {
                int     retval;
 
                io->urbs [i]->dev = io->dev;
@@ -455,7 +453,7 @@
                /* after we submit, let completions or cancelations fire;
                 * we handshake using io->status.
                 */
-               spin_unlock_irqrestore (&io->lock, flags);
+               spin_unlock_irq (&io->lock);
                switch (retval) {
                        /* maybe we retrying will recover */
                case -ENXIO:    // hc didn't queue this one
@@ -479,17 +477,25 @@
 
                        /* fail any uncompleted urbs */
                default:
+                       spin_lock_irq (&io->lock);
+                       io->count -= entries - i;
+                       if (io->status == -EINPROGRESS)
+                               io->status = retval;
+                       if (io->count == 0)
+                               complete (&io->complete);
+                       spin_unlock_irq (&io->lock);
+
                        io->urbs [i]->dev = 0;
                        io->urbs [i]->status = retval;
                        dev_dbg (&io->dev->dev, "%s, submit --> %d\n",
                                __FUNCTION__, retval);
                        usb_sg_cancel (io);
                }
-               spin_lock_irqsave (&io->lock, flags);
+               spin_lock_irq (&io->lock);
                if (retval && io->status == -ECONNRESET)
                        io->status = retval;
        }
-       spin_unlock_irqrestore (&io->lock, flags);
+       spin_unlock_irq (&io->lock);
 
        /* OK, yes, this could be packaged as non-blocking.
         * So could the submit loop above ... but it's easier to

Reply via email to