* Alan Stern <[EMAIL PROTECTED]> wrote:

> Interrupts are disabled during usb_hcd_giveback_urb because that's how 
> it was done originally and nobody has made an effort to remove this 
> assumption from the USB device drivers.  There's no real reason for it 
> other than historical inertia.  It's not done for serialization -- 
> there's no need for serialization since an URB can't be resubmitted 
> before the previous callback occurs (unless a driver is badly broken).  
> The "detached" method is used simply to avoid an extra pair of 
> enable/disable instructions.

so we can remove it altogether, via the patch below? (If there's any 
unsafe driver, it should already be unsafe on SMP, and with the 
proliferation of HT and dual-core CPUs, SMP will be the norm within a 
year or so - so the sooner we trigger any breakages, the better i 
guess.)

i'll give it a whirl in the -RT tree.

        Ingo

----

remove unnecessarily irqs-off sections from hcd.c.

Signed-off-by: Ingo Molnar <[EMAIL PROTECTED]>

Index: linux/drivers/usb/core/hcd.c
===================================================================
--- linux.orig/drivers/usb/core/hcd.c
+++ linux/drivers/usb/core/hcd.c
@@ -506,13 +506,11 @@ error:
        }
 
        /* any errors get returned through the urb completion */
-       local_irq_save (flags);
-       spin_lock (&urb->lock);
+       spin_lock_irqsave(&urb->lock, flags);
        if (urb->status == -EINPROGRESS)
                urb->status = status;
-       spin_unlock (&urb->lock);
+       spin_unlock_irqrestore(&urb->lock, flags);
        usb_hcd_giveback_urb (hcd, urb, NULL);
-       local_irq_restore (flags);
        return 0;
 }
 
@@ -540,8 +538,7 @@ void usb_hcd_poll_rh_status(struct usb_h
        if (length > 0) {
 
                /* try to complete the status urb */
-               local_irq_save (flags);
-               spin_lock(&hcd_root_hub_lock);
+               spin_lock_irqsave(&hcd_root_hub_lock, flags);
                urb = hcd->status_urb;
                if (urb) {
                        spin_lock(&urb->lock);
@@ -557,14 +554,13 @@ void usb_hcd_poll_rh_status(struct usb_h
                        spin_unlock(&urb->lock);
                } else
                        length = 0;
-               spin_unlock(&hcd_root_hub_lock);
+               spin_unlock_irqrestore(&hcd_root_hub_lock, flags);
 
                /* local irqs are always blocked in completions */
                if (length > 0)
                        usb_hcd_giveback_urb (hcd, urb, NULL);
                else
                        hcd->poll_pending = 1;
-               local_irq_restore (flags);
        }
 
        /* The USB 2.0 spec says 256 ms.  This is close enough and won't
@@ -647,17 +643,15 @@ static int usb_rh_urb_dequeue (struct us
        } else {                                /* Status URB */
                if (!hcd->uses_new_polling)
                        del_timer_sync (&hcd->rh_timer);
-               local_irq_disable ();
-               spin_lock (&hcd_root_hub_lock);
+               spin_lock_irq(&hcd_root_hub_lock);
                if (urb == hcd->status_urb) {
                        hcd->status_urb = NULL;
                        urb->hcpriv = NULL;
                } else
                        urb = NULL;             /* wasn't fully queued */
-               spin_unlock (&hcd_root_hub_lock);
+               spin_unlock_irq(&hcd_root_hub_lock);
                if (urb)
                        usb_hcd_giveback_urb (hcd, urb, NULL);
-               local_irq_enable ();
        }
 
        return 0;
@@ -1367,15 +1361,13 @@ hcd_endpoint_disable (struct usb_device 
        WARN_ON (!HC_IS_RUNNING (hcd->state) && hcd->state != HC_STATE_HALT &&
                        udev->state != USB_STATE_NOTATTACHED);
 
-       local_irq_disable ();
-
        /* FIXME move most of this into message.c as part of its
         * endpoint disable logic
         */
 
        /* ep is already gone from udev->ep_{in,out}[]; no more submits */
 rescan:
-       spin_lock (&hcd_data_lock);
+       spin_lock_irq(&hcd_data_lock);
        list_for_each_entry (urb, &ep->urb_list, urb_list) {
                int     tmp;
 
@@ -1388,13 +1380,13 @@ rescan:
                if (urb->status != -EINPROGRESS)
                        continue;
                usb_get_urb (urb);
-               spin_unlock (&hcd_data_lock);
+               spin_unlock_irq(&hcd_data_lock);
 
-               spin_lock (&urb->lock);
+               spin_lock_irq(&urb->lock);
                tmp = urb->status;
                if (tmp == -EINPROGRESS)
                        urb->status = -ESHUTDOWN;
-               spin_unlock (&urb->lock);
+               spin_unlock_irq(&urb->lock);
 
                /* kick hcd unless it's already returning this */
                if (tmp == -EINPROGRESS) {
@@ -1417,8 +1409,7 @@ rescan:
                /* list contents may have changed */
                goto rescan;
        }
-       spin_unlock (&hcd_data_lock);
-       local_irq_enable ();
+       spin_unlock_irq(&hcd_data_lock);
 
        /* synchronize with the hardware, so old configuration state
         * clears out immediately (and will be freed).
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to