<html><head><meta name="color-scheme" content="light dark"></head><body><pre style="word-wrap: break-word; white-space: pre-wrap;">ChangeSet 1.1186, 2003/05/20 11:48:24-07:00, david-b@pacbell.net

[PATCH] USB: Fix machine lockup when unloading HC driver (part 2)

Alan Stern wrote:
&gt; I suggest you just forget about acquiring the lock in status_dequeue() and
&gt; simply leave it as
&gt;
&gt; 	del_timer_sync (&amp;hcd-&gt;rh_timer);
&gt; 	hcd-&gt;rh_timer.data = 0;

Hmm, so if some other URB gets queued in that window,
it'll get trashed?  Unlikely .. the clean fix would be
making the status endpoint have a real URB queue.

I combined your suggested change with two others:
(a) protect the status-unlink and control completion
    handlers against IRQs [ the cases Duncan noted]
(b) use mod_timer to retrigger the timer, instead of the
    heavy weight path.


diff -Nru a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
--- a/drivers/usb/core/hcd.c	Tue May 20 17:25:02 2003
+++ b/drivers/usb/core/hcd.c	Tue May 20 17:25:02 2003
@@ -324,6 +324,7 @@
 	const u8	*bufp = 0;
 	u8		*ubuf = urb-&gt;transfer_buffer;
 	int		len = 0;
+	unsigned long	flags;
 
 	typeReq  = (cmd-&gt;bRequestType &lt;&lt; 8) | cmd-&gt;bRequest;
 	wValue   = le16_to_cpu (cmd-&gt;wValue);
@@ -436,7 +437,9 @@
 	}
 
 	/* any errors get returned through the urb completion */
+	local_irq_save (flags);
 	usb_hcd_giveback_urb (hcd, urb, NULL);
+	local_irq_restore (flags);
 	return 0;
 }
 
@@ -500,13 +503,13 @@
 
 	/* complete the status urb, or retrigger the timer */
 	spin_lock (&amp;hcd_data_lock);
-	hcd-&gt;rh_timer.data = 0;
 	if (length &gt; 0) {
+		hcd-&gt;rh_timer.data = 0;
 		urb-&gt;actual_length = length;
 		urb-&gt;status = 0;
 		urb-&gt;hcpriv = 0;
 	} else
-		rh_status_urb (hcd, urb);
+		mod_timer (&amp;hcd-&gt;rh_timer, jiffies + HZ/4);
 	spin_unlock (&amp;hcd_data_lock);
 	spin_unlock (&amp;urb-&gt;lock);
 
@@ -541,15 +544,14 @@
 {
 	unsigned long	flags;
 
-	spin_lock_irqsave (&amp;hcd_data_lock, flags);
-	hcd-&gt;rh_timer.data = 0;
-	spin_unlock_irqrestore (&amp;hcd_data_lock, flags);
-
 	/* note:  always a synchronous unlink */
 	del_timer_sync (&amp;hcd-&gt;rh_timer);
+	hcd-&gt;rh_timer.data = 0;
 
+	local_irq_save (flags);
 	urb-&gt;hcpriv = 0;
 	usb_hcd_giveback_urb (hcd, urb, NULL);
+	local_irq_restore (flags);
 }
 
 /*-------------------------------------------------------------------------*/
</pre></body></html>