On Wed, 5 Dec 2007 16:35:26 -0500 (EST), Alan Stern <[EMAIL PROTECTED]> wrote:
> Host controller IRQs are supposed to be serviced with interrupts > disabled. This patch (as1025) adds an IRQF_DISABLED flag to all the > controller drivers that lack it. OK, if you prefer to use a crew-fired weapon in case of a menacing rodent. It's a matter of taste, but the following is wrong IMHO: > It also replaces the > spin_lock_irqsave() and spin_unlock_irqrestore() calls in uhci_irq() > with simple spin_lock() and spin_unlock(). > @@ -415,16 +414,16 @@ static irqreturn_t uhci_irq(struct usb_h > * pending unlinks */ > mod_timer(&hcd->rh_timer, jiffies); > } > - spin_unlock_irqrestore(&uhci->lock, flags); > + spin_unlock(&uhci->lock); > } > } If anything, this bug is a good example when not to do that. Clearly uhci, taken by in reasonable isolation, makes use of timers, and so its atomic code can be re-entered. This is why it uses irqsave, to keep its local invariants safe _in the face of changing externals_ (e.g. one day you want a monstrous IRQF_DISABLED, another day you don't, the code remains correct). -- Pete - To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
