On Wed, Jun 12, 2013 at 6:19 PM, Oliver Neukum <oli...@neukum.org> wrote: > On Wednesday 12 June 2013 18:11:34 Ming Lei wrote: >> On Wed, Jun 12, 2013 at 5:11 PM, Oliver Neukum <oli...@neukum.org> wrote: >> > On Tuesday 11 June 2013 15:10:03 Alan Stern wrote: >> >> In order to prevent this from happening, you would have to change the >> >> spin_lock() call in the completion handler to spin_lock_irqsave(). >> >> Furthermore, you will have to audit every USB driver to make sure that >> >> all the completion handlers get fixed. >> > >> > Yes. However, it can be done mechanically. And we know only >> > the handlers for complete need to be fixed. >> >> I am wondering if the change is needed since timer function is still >> run in softirq context instead of hard irq. >> >> Looks Alan concerned that one USB interface driver may have another >> hard interrupt handler involved. Is there such kind of USB driver/device >> in tree? > > No. Suppose a USB network driver. > The complete() handler is written on the assumption that interrupts are off. > So it takes a spinlock from the network subsystem. It does so with spin_lock()
Looks I misses the case, but IMO, it might not be very common, generally subsystem provides API for drivers to do something(handle rx/report tx) inside complete(), and seldom exports subsystem-wide lock directly to drivers. API has no context info, and should have called spin_lock_irqrestore(). > > Other network drivers also take the lock. And they may take it from an IRQ > handler. > If such an IRQ interrupts the tasklet complete() is running in, the CPU will > deadlock. Looks no usbnet drivers hold subsystem(network) locks in its complete(). Both the locks held are per device/per skb list. In usbnet_bh(), there is one, eg. netif_rx(), which is a API and disables local IRQ. > > The danger is not interrupt handlers in the same driver but IRQ handlers of > _other_ > drivers (PCI, ...) a lock is shared with. Right, so generally drivers which spin_lock(subsystem lock) in complete() might be affected. > > You need to go through all USB drivers and change every spin_lock() that goes > for a lock that is exported to a spin_lock_irqsave() Yes, that is the safest way. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
