On Wed, Jul 03, 2002, David Brownell <[EMAIL PROTECTED]> wrote: > >>Of course, if you took a different strategy that wasn't > >>focussed on an URB list (like both UHCIs do), that issue > >>needn't come up. For example, scanning active QHs for TDs > >>that have completed (successfully or with errors), you know > >>the urb associated with each TD. > > > > 1) Not all transfers have QH's (Interrupt, Isochronous) > > Hmm, I thought that only ISO TDs were allowed to live outside > a QH.
Sure it can. I think ISO TD's can even live inside of QH's too. The QH is just for management of queue's of TD's. I could write a driver which doesn't use QH's at all, but it would be very inefficient. > Regardless, the point is actually that scanning the > schedule for TDs (normally done through QHs) lets you get to > those URBs directly; you don't need another list of URBs. > > This is an area where the EHCI and UHCI hardware data structures > are _very_ similar. Both have periodic schedule tables that have > ISO TDs followed by interrupt transfers (QHs), and an async > schedule (control/bulk) arranged in a ring. And the hardware > does similar updates for both: TDs get marked, QHs get updated. > > Differences include the fact that EHCI lets each part of the > schedule be turned off independently, the UHCI "ring" is a > driver policy issue (for FSBR) which can be disabled, and > in UHCI those async transfers hang out at the end of the > periodic schedule. > > None of those differences is enough to make UHCI drivers need > a list of URBs (and corresponding added locks) ... EHCI needs > very similar handling, but does fine with just one lock per HC. Actually, you *MUST* maintain a list of outstanding URB's. Since the HC doesn't tell the HCD which URB's have completed, we need to check all of them to determine which one's completed. The only other solution is to walk the schedule, but that requires complicated code to convert bus addresses into kernel addresses. Not to mention it's effectively a list anyway and it still needs a lock. The only difference is what form the list takes and the since using urb_list is so much easier to use than walking the schedule, that's what uhci.c uses. > > 2) When we submit a new URB, we have to add that new QH to the list > > > > The problem here is no matter which piece of identifying information we > > use, we need to put it into a list and *that's* the problem. > > Right, but how many lists? It's already living in one list that's > locked, and which moreover is sorted in a useful order (the schedule), > so there's no need to have those requests also live in parallel data > structures with a different locking policy. The reason we don't do it that way is because it's dumb. I'm sure if you try to implement what you're proposing you'll see just how complicated it is. > >>During completion callbacks there are some invariants you > >>can rely on, like that URB not changing (not that it should > >>matter, all its TDs finished!) and that no URB could queue > >>in front of the TDs you already scanned. > > > > But that requires dropping the lock and that means *anything* can happen > > to the list. If you save the position you're currently on and acquire > > the lock and continue after the callback, you may find the URB you're > > working on disappeared already, either because of the callback or > > another CPU. > > No, not "anything". The normal case is URB getting unlinked _at > the same time as_ the callback. Only interrupt transfers don't work > that way (and I hope to see that change later in 2.5, it'll simplify > quite a bit of stuff at several layers in the USB stack). Consider the case where a callback unlinks a different URB, or submits a different URB. If we hold the list lock during this call chain, we'll deadlock on ourselves. That's why we need to drop to lock. However, when we drop the lock, the other CPU's are free to do pretty much anything to the list. > If you work by scanning the schedule rather than a list of URBs, > there's even less variability: the HC updates every QH so TDs > that have finished don't show up there, and marks non-QH TDs. > Scan the list of TDs that _were_ linked to the schedule, issuing > completions as needed, then stop when you get to a TD the HC is > still working on. We already do that, but we don't have any the complicated logic that's involved with converting bus addresses to kernel addresses. Plus, you still need a lock to prevent multiple CPU's from modifying the schedule. > The only lock needed is a "schedule lock" to keep other threads > from tweaking the schedule while you scan it; drop that during the > completion callback. In the rare cases that the part of the > schedule you're looking at got updated when you get the lock back, > recovery is simple. How so? Have you looked at how usb-uhci.c does this? I refuse to put anything like that in uhci.c. > > Take a look at uhci_interrupt in usb-uhci.c for their processing loop > > and tell me that's something you would prefer over having 2 locks :) > > I was saying that _both_ UHCI HCDs should be simpler. Both > have too many locks, their queueing code (the hairiest part > of any HCD, I expect you'll agree!) is too complex ... those > factors are related. Umm, I didn't say usb-uhci.c shouldn't be simpler. In fact, I think every single time I've ever said anything about usb-uhci.c, I've said it should be simpler. What I did say is that they have implemented the necessary the logic that you're advocating and it's hacky, kludgy, contains arbitrary processing limits and nothing like it is ever going to see uhci.c. > Take a look at ehci_irq() in ehci-hcd.c (and the schedule > scanners it arranges to be called later) and then explain > why it should convert to use more locks ... :) Hmm, I don't ever recall advocating using more locks. Maybe you misinterpreted something? I'm looking at the ehci-hcd driver right now and I'm not sure how you can be sure it's safe. Perhaps it's just you haven't seen any problems because of the lack of USB 2.0 devices? But you have the same locking problems with lists that usb-uhci does, except you don't even try to jump through hoops to workaround it. JE ------------------------------------------------------- This sf.net email is sponsored by:ThinkGeek No, I will not fix your computer. http://thinkgeek.com/sf _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
