On Tue, 16 Aug 2005, Ingo Molnar wrote:
> * 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.
In general yes, the patch should be okay. But there are a few things to
check for. Perhaps most of the USB drivers don't care whether interrupts
are enabled or not in their completion routines. However I know of at
least one where it does matter. The current code _is_ SMP-safe, but it
won't remain UP-safe unless you add this patch in addition to your own.
Alan Stern
P.S.: Another routine worth examining is async_completed() in
drivers/usb/core/devio.c.
Index: usb-2.6/drivers/usb/core/message.c
===================================================================
--- usb-2.6.orig/drivers/usb/core/message.c
+++ usb-2.6/drivers/usb/core/message.c
@@ -224,8 +224,9 @@ static void sg_clean (struct usb_sg_requ
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 (&io->lock);
+ spin_lock_irqsave (&io->lock, flags);
/* In 2.5 we require hcds' endpoint queues not to progress after fault
* reports, until the completion callback (this!) returns. That lets
@@ -259,7 +260,7 @@ static void sg_complete (struct urb *urb
* unlink pending urbs so they won't rx/tx bad data.
* careful: unlink can sometimes be synchronous...
*/
- spin_unlock (&io->lock);
+ spin_unlock_irqrestore (&io->lock, flags);
for (i = 0, found = 0; i < io->entries; i++) {
if (!io->urbs [i] || !io->urbs [i]->dev)
continue;
@@ -272,7 +273,7 @@ static void sg_complete (struct urb *urb
} else if (urb == io->urbs [i])
found = 1;
}
- spin_lock (&io->lock);
+ spin_lock_irqsave (&io->lock, flags);
}
urb->dev = NULL;
@@ -282,7 +283,7 @@ static void sg_complete (struct urb *urb
if (!io->count)
complete (&io->complete);
- spin_unlock (&io->lock);
+ spin_unlock_irqrestore (&io->lock, flags);
}
-
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/
