On Wed, 8 Sep 2004, Andreas Mohr wrote:
> Uhoh, I just found one (linux-2.6.8.1/drivers/usb/core/hcd.c):
>
> irqreturn_t usb_hcd_irq (int irq, void *__hcd, struct pt_regs * r)
> {
> struct usb_hcd *hcd = __hcd;
> int start = hcd->state;
>
> if (unlikely (hcd->state == USB_STATE_HALT)) /* irq sharing? */
> return IRQ_NONE;
>
> hcd->saw_irq = 1;
> if (hcd->driver->irq (hcd, r) == IRQ_NONE)
> return IRQ_NONE;
>
> if (hcd->state != start && hcd->state == USB_STATE_HALT)
> usb_hc_died (hcd);
> return IRQ_HANDLED;
> }
>
> Why the h*ck is that one using unlikely()???
> IIRC, this changes cache probability from 95% to 99% or so, which clearly
> aggravates the (most likely too common) situation of IRQ sharing
> of the USB device with other devices in the system (I've seen IRQ-shared
> USB-UHCD many times already!).
>
> You could remove that unlikely() bracing, maybe that helps a bit.
>
> Also, it unnecessarily calculates hdc->state *twice* before and
> during the IRQ sharing check, so in total it should use
> if (start == USB_STATE_HALT) /* irq sharing? */
> return IRQ_NONE;
> instead, which now should be quite faster.
With all due respect, let me point out that you don't know what you're
talking about.
That unlikely() _really_ does cover an unlikely case -- the case where the
device is turned off. That's what USB_STATE_HALT means. It is indeed
very unlikely for the device to be turned off, since the HC drivers always
leave their devices on unless they encounter an unrecoverable error.
I imagine you were misled by the comment to think that it really was
testing for IRQ sharing. But it isn't; the implication behind the comment
is that _if_ the device is off then the IRQ _must_ be shared, since the
device couldn't have generated it. The _real_ test for IRQ sharing occurs
lower down, where it says:
if (hcd->driver->irq (hcd, r) == IRQ_NONE)
return IRQ_NONE;
As for replacing "hcd->state" by "start" in the body of the unlikely(),
yes, that could be done. Any decent compiler will do it for you,
automatically. The cost of a single extra dereference is minimal in any
case.
Alan Stern
-------------------------------------------------------
This SF.Net email is sponsored by BEA Weblogic Workshop
FREE Java Enterprise J2EE developer tools!
Get your free copy of BEA WebLogic Workshop 8.1 today.
http://ads.osdn.com/?ad_id=5047&alloc_id=10808&op=click
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
