On Mon, 11 Jul 2011, Sarah Sharp wrote: > > However, you should never need to walk the device tree in the first > > place. What you should do (and I say this without having yet looked at > > the code you posted) is save a private data structure for each enabled > > endpoint, containing whatever scheduling parameters you need, such as > > possibly a pointer to the usb_host_endpoint structure. > > Yes, there's already a structure in the xhci_virt_ep that stores that > information, so I'm not walking the tree. I can change it to include a > pointer to the endpoint descriptor, rather than storing duplicate copies > of the max packet size and number of additional opportunities per > microframe. I might also need to store a pointer to the SuperSpeed > endpoint companion descriptor eventually. Whether to copy the information or not is up to you. > > In addition, for each scheduling domain you should store an array > > holding the number of microseconds allocated for each frame or > > microframe. The length of the array doesn't have to be terribly long. > > Perhaps 256 uframes for HS/SS and 32 frames for FS/LS (which is what > > uhci-hcd and ohci-hcd use). Any endpoint with a period longer than > > that would get charged with more bandwidth than it actually uses, but I > > don't think this will matter much. > > I don't actually have control over which endpoints are scheduled in > which microframe with the Intel hardware. That's handled dynamically in > the hardware, so I can't assume a particular periodic endpoint is going > to be scheduled in a particular microframe. Instead, the bandwidth > algorithm computes the worst possible microframe (or at least no worse > than the worst microframe). Ah, yes. The weird part of your scheduling algorithm -- you have to tell whether a schedule is valie without actually knowing how things will be scheduled! :-) Okay, so xhci-hcd won't need this information. But other HCDs will. > For each bandwidth domain, I have a table (xhci_interval_bw_table) that > stores information about endpoints that have the same periodic interval. > That's used to calculate the most packed microframe, see if it's > possible to shuffle transfers out of over-packed microframes and still > meet the interval contract, and finally calculate the overall worst case > bandwidth used. > > > Some extra information might also be needed. For example, ehci-hcd > > will need to keep a sorted list of all active periodic endpoints below > > each TT; xhci-hcd might need something similar. > > Yes, I also need to keep track of which endpoints are below each TT. > It's further exasperated by the fact that the USB core only assigns one > usb_tt structure per HS hub, not per port on a multi-TT hub. I really > only need to keep periodic endpoints in the list, and I have the list > sorted with the greatest max packet size first, which makes it easy for > the bandwidth checking algorithm. ehci-hcd will be very much like that, except that the sorting order will be different. Endpoints will be sorted by decreasing interval, then by starting uframe, then by max payload size. > > I wonder if we might not put some of this stuff in the core. It should > > be generally useful. > > Yes, some of it might be useful in the USB core. However, I'm under a > bit of pressure from the distros. They really didn't like backporting > the big USB core changes for the USB 3.0 hub patches, because they then > needed to revalidate all the other host controller drivers. (You could > argue they should just take the latest upstream kernel, but that's not > an argument I'm going to win.) So to try and make it more palatable for > them to take the Intel xHCI support patches, I'd like to do a gradual > patchset that adds the bandwidth checking to the xHCI driver, and then > converts the code to use any shared values in the USB core. It will > probably look pretty silly in the long run, but that's the sort of > rock-and-hard place I'm stuck in. I appreciate the constraints you're under. > I'm not opposed to making big USB core changes and getting EHCI to do > static bandwidth checking in the long run, but I'd like to get the xHCI > driver working for the Intel xHCI hosts that need software bandwidth > before we focus on improving other drivers. Sure. > > If you're more concerned about the short term, we ought to be able to > > avoid the most recent set of problems by making usb_disable_device > > acquire the bandwidth_mutex rather than having the caller do it. The > > idea is that we first unbind all the drivers and unregister all the > > interfaces, then hold the mutex while releasing the old bandwidth and > > allocating the new. > > Yes, that's probably a better solution. Do you want to make that patch > or should I? You're closer to this issue than I am, so you should write the patch. When you do, you can take the word "FIXME" out of the comment added in my second regression-fix (but leave everything else). :-) 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
