On Mon, 11 Apr 2005, David Brownell wrote: > > Actually this is a new issue, not a lurking one. The whole point of that > > patch was because I'm going to make the majority of the UHCI driver run in > > a bottom half (a tasklet). This includes most of the work involved in > > submitting and unlinking URBs. > > Erm ... why? That'll mean many of the relevant data structures > will more likely be dcache-cold.
I don't see why it would. Especially if bottom-half handler happens to be running a bit behind, so that it takes care of more than one IRQ invocation in one go. Then the data structures would be dcache-warmer than they are now. > And it'll involve more code to > execute (icache-cold) and maintain too ... More code, yes. Not terribly much more, but some. It's hard to add features without adding code. :-) > For unlinking, the operations should be just patching the schedule > (under spinlock) and requesting an IRQ. That's right, messing around with the schedule. And since unlinking is allowed to happen in_interrupt, it means _nothing_ can touch the schedule without disabling interrupts. That's my main reason for doing this -- so that the heavy-duty work (i.e., the stuff that uses or changes the schedule) can take place without blocking interrupts. > Unlinking is both rare and, > when it happens, on the critical path. Submitting is likewise on > the critical path ... though it happens a lot. Neither of those > would seem to me like good candidates for putting into a tasklet. I agree about submitting. It's a definite weak spot for the projected change. I'm not so sure about unlinking being on the critical path. > I could see in some cases wanting the IRQ processing to be split > into the classic top half (in_irq) and bottom half. I even had > EHCI working that way, originally ... DaveM removed that, mostly > to ensure that pt_regs got delivered to keyboard IRQ handlers > (evidently some keyboard code really wants pt_regs), but also to > make it a bit simpler. It should be possible to deliver pt_regs even with a top-half/bottom-half split. And yes, it's true that splitting things up like this will complicate the driver somewhat. > Is this because of those no-details-provided reports of high IRQ > latencies with USB on one board? It'd have been interesting to > see some accounting for those latencies. For example, oprofile > traces of where the IRQ handlers spend time... That was the main motivation, although I'd had this in the back of my mind for a long time. Yes, it would be nice to see some real numbers for those latencies. I suppose I could add some high-precision timer stuff to the driver, to see just how much time actually is spent with interrupts disabled... > > > To safely run that out of sequence, its queue clearly stopped in some > > > sense. > > > > Not if the URB was never in the hardware queue to begin with. > > From the usbcore perspective, there's only one queue. If you're > proposing that usbcore start needing to think about two separate > queues, that's significantly more invasive than anything that's > happened in usbcore over the past few years (including the almost > total rewrite of the hub and enumeration code)... Now you're reading much more into this than I ever said. All I did was change the API so now it claims that when an URB is unlinked, its queue isn't guaranteed to stop until all the preceding URBs have been completed. Surely you can't object to that? Do you know of any code that relies on the queue being stopped as soon as any URB is unlinked, even if that URB is still 15 deep in the queue? > I want to get picky about API requirements that expect (or > facilitate) complex and error prone logic in HCDs, or which > promote substantially different HCD behavior as visible to > the USB device/interface drivers; yes. It's not at all clear to me that this would be a visible change. Can you think of any way to detect how soon a queue stops from within a driver? > > > OK, so that new text is wrong then. The only problem case has ever > > > been (3b), where the URB completed (at the hardware level) before the > > > queue could be stopped. > > > > Nope, the new text describes exactly how the new driver will work. And > > it's correct. (3b) isn't a problem in the sense that we don't violate the > > guarantees made by the API. The queue _does_ stop, but it might not stop > > until after the hardware has begun working on the URB following the one > > that was unlinked. > > That is, you turn it into (3a) somehow. Which was handled by the original > text ... > > Morphing (3b) into (3a) implies some sort of additional mechanism to couple > completion reports to the (hardware) endpoint queue. I don't know about EHCI or OHCI. With UHCI it would be necessary to change things so that a hardware URB completion isn't reported right away if the URB was unlinked. Is that what you mean? > > We discussed this not long ago, remember? People generally were against > > the idea of removing the "unlink one URB" operation. And there were a few > > cases where the "unlink one" semantics really are needed -- ep0 being > > one example. > > Actually I remember it differently. EP0 being the _only_ example > of a case where it's potentially needed (because it's shared between > interfaces), but otherwise folk liked removing hardware races from > the APIs. Here's another (theoretical) example that came up. Imagine something like a network driver, which queues many independent packets for a bulk endpoint in no particular order and sometimes wants to cancel one of them, say because of a timeout, while leaving the others intact. > I don't remember anyone speaking "against" that, except > in the case of ep0 ... the discussion stopped only because of a lack > of energy to rewrite the HCDs to address such API design bugs. Here's the start of the thread, if you want to refresh your memory: http://marc.theaimsgroup.com/?l=linux-usb-devel&m=110805033904399&w=2 Looking through the messages, I see that Oliver was lukewarm at best, you didn't think it was a particularly high priority because all the HCDs currently handle single-URB unlinking okay and furthermore it would mess up usbtest #10, and Wolfgang Muees suggested that URBs should have unlimited retries (okay, that was a bit OT). _Nobody_ was really in favor of doing it, except possibly me. Alan Stern ------------------------------------------------------- SF email is sponsored by - The IT Product Guide Read honest & candid reviews on hundreds of IT Products from real users. Discover which products truly live up to the hype. Start reading now. http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
