On Mon, 11 Jul 2011, Sarah Sharp wrote: > So unless the USB core guarantees that the bandwidth mutex is held > across bandwidth change, I don't think we can avoid this type of race > condition: > > 1. Driver A tries to switch device A to a less-bandwidth intensive > alternate interface setting, and the xHCI driver issues a Configure > Endpoint command. The xHCI driver drops the xhci->lock to wait on a > completion for the command to be processed. > > 2. Driver B tries to switch device B to a more-bandwidth intensive > alt setting. The xhci->lock is acquired and a second Configure > Endpoint command is queued. > > 3. The first Configure Endpoint command succeeds, but device A > stalls the Set Interface request (or perhaps the xHCI host > controller rejects the alt setting because of some other internal > resource constraint, like the number of endpoints the host can > handle). In either case, we need to reinstall the old alt setting. > > 4. The second Configure Endpoint command succeeds, and now device B > is taking all the bus bandwidth. We can't install device A's old > alt setting, and can't install the new alt setting, so the device is > basically useless. > > That's why we need the USB core to hold the bandwidth mutex across both > the call to usb_hcd_alloc_bandwidth() and issuing the Set Interface > command to the device. We need to prevent any changes to the host > controller bandwidth and internal resources until we have fully > installed or finished cleaning up a failed bandwidth change. You're right, of course. Okay, forget about that. > > > Is there any reason a driver would submit an URB and then request a new > > > alt setting? > > > > Yes. Imagine a video device with several different possible > > resolutions, implemented using different altsettings. > > Yes, but why wouldn't the driver cancel the URB that was submitted to > the old endpoint before changing the alt setting and possibly changing > the endpoint characteristics? The xHCI driver is going to change > endpoint rings when an endpoint is changed, so the driver really *needs* > to cancel that URB. It's the same sort of issue we had with drivers not > canceling URBs before issuing a device reset. I don't really care which > way you design the new interface, but I was concerned about this issue > in particular. Sorry, I misunderstood your question. I didn't realize you were asking if a driver would submit an URB and then change altsettings while the URB was still active -- I assumed the driver would wait for the URB to complete before changing altsettings. In the future, usbcore will guarantee that none of the endpoints affected by a config or interface setting change have any active URBs. We can do the same thing with device resets. Basically, this comes down to the fact that we really need two different versions of usb_disable_endpoint. Right now we make do with one function by passing it a flag, which is rather awkward. There really are two distinct actions involved: Prevent any more URBs from being submitted to the endpoint, cancel any outstanding URBs, and wait for the queue to drain. This could be considered a "soft" disable. Tell the HCD to deallocate the endpoint's bandwidth and all the associated hardware and software resources. This is a "hard" disable; in practice it would be preceded by a soft disable Also, this wouldn't be a standalone function; it would be just part of the whole pass-an-array-of-endpoints thing. For a device reset, most HCDs would require only a soft disable. xHCI is unusual in that it knows when a device is reinitialized. It occurred to me that we might want to require xhci-hcd to save the current endpoint settings when doing a reset and restore them automatically afterward. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
