Messages in this thread Patch in this message | | | Date | Tue, 16 Aug 2005 12:56:12 -0400 (EDT) | From | Alan Stern <> | Subject | Re: [patch] Real-Time Preemption, -RT-2.6.13-rc6-V0.7.53-11 |
| |
On Tue, 16 Aug 2005, Ingo Molnar wrote:
> * Alan Stern <stern@rowland.harvard.edu> 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |