Commit 6d58d234 authored by David Brownell's avatar David Brownell Committed by Greg Kroah-Hartman

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

Alan Stern wrote:
> I suggest you just forget about acquiring the lock in status_dequeue() and
> simply leave it as
>
> 	del_timer_sync (&hcd->rh_timer);
> 	hcd->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.
parent 8453878c
...@@ -324,6 +324,7 @@ static int rh_call_control (struct usb_hcd *hcd, struct urb *urb) ...@@ -324,6 +324,7 @@ static int rh_call_control (struct usb_hcd *hcd, struct urb *urb)
const u8 *bufp = 0; const u8 *bufp = 0;
u8 *ubuf = urb->transfer_buffer; u8 *ubuf = urb->transfer_buffer;
int len = 0; int len = 0;
unsigned long flags;
typeReq = (cmd->bRequestType << 8) | cmd->bRequest; typeReq = (cmd->bRequestType << 8) | cmd->bRequest;
wValue = le16_to_cpu (cmd->wValue); wValue = le16_to_cpu (cmd->wValue);
...@@ -436,7 +437,9 @@ static int rh_call_control (struct usb_hcd *hcd, struct urb *urb) ...@@ -436,7 +437,9 @@ static int rh_call_control (struct usb_hcd *hcd, struct urb *urb)
} }
/* any errors get returned through the urb completion */ /* any errors get returned through the urb completion */
local_irq_save (flags);
usb_hcd_giveback_urb (hcd, urb, NULL); usb_hcd_giveback_urb (hcd, urb, NULL);
local_irq_restore (flags);
return 0; return 0;
} }
...@@ -500,13 +503,13 @@ static void rh_report_status (unsigned long ptr) ...@@ -500,13 +503,13 @@ static void rh_report_status (unsigned long ptr)
/* complete the status urb, or retrigger the timer */ /* complete the status urb, or retrigger the timer */
spin_lock (&hcd_data_lock); spin_lock (&hcd_data_lock);
hcd->rh_timer.data = 0;
if (length > 0) { if (length > 0) {
hcd->rh_timer.data = 0;
urb->actual_length = length; urb->actual_length = length;
urb->status = 0; urb->status = 0;
urb->hcpriv = 0; urb->hcpriv = 0;
} else } else
rh_status_urb (hcd, urb); mod_timer (&hcd->rh_timer, jiffies + HZ/4);
spin_unlock (&hcd_data_lock); spin_unlock (&hcd_data_lock);
spin_unlock (&urb->lock); spin_unlock (&urb->lock);
...@@ -541,15 +544,14 @@ void usb_rh_status_dequeue (struct usb_hcd *hcd, struct urb *urb) ...@@ -541,15 +544,14 @@ void usb_rh_status_dequeue (struct usb_hcd *hcd, struct urb *urb)
{ {
unsigned long flags; unsigned long flags;
spin_lock_irqsave (&hcd_data_lock, flags);
hcd->rh_timer.data = 0;
spin_unlock_irqrestore (&hcd_data_lock, flags);
/* note: always a synchronous unlink */ /* note: always a synchronous unlink */
del_timer_sync (&hcd->rh_timer); del_timer_sync (&hcd->rh_timer);
hcd->rh_timer.data = 0;
local_irq_save (flags);
urb->hcpriv = 0; urb->hcpriv = 0;
usb_hcd_giveback_urb (hcd, urb, NULL); usb_hcd_giveback_urb (hcd, urb, NULL);
local_irq_restore (flags);
} }
/*-------------------------------------------------------------------------*/ /*-------------------------------------------------------------------------*/
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment