On Sun, Mar 17, 2002, David Brownell <[EMAIL PROTECTED]> wrote: > > There is one small problem with uhci.c (patch appended) where it checks > > urb->status after the completion handler. It should only do this if it's > > an auto resubmitting interrupt URB, but we always checked it. > > > > Other than that, it does not touch the URB after calling the completion > > handler *unless* it's an auto resubmitting interrupt URB ... > > Right, that's my understanding of the right thing to do; and I'm > glad to see that patch. :)
The bug was mostly harmless. Since killed was never checked if resubmit_interrupt was 0, the worst thing that would happen would be reading a random value. The only way I could see it causing a crash was if the URB was freed and then the slab allocator freed the page it was sitting on. But that's a VERY tight race. Anyway, it's the correct thing to do nonetheless :) > > > I think that completion handlers should be able to free URBs, > > > except for periodic URBs that are being resubmitted, but I > > > know that Oliver has problems with that position. > > > > They can't. We need to check to see if it got unlinked and to do that, > > we need to check urb->status. > > Erm ... weren't you just agreeing with what I wrote, though? The > URBs that would be resubmitted are the ones that aren't being > "given back" to the driver. The others can be freed by that handler, > since the HCD knew they were unlinked before that callback and > so wouldn't need to check urb->status. > > The "would be resubmitted" ones could be unlinked in completion, > but not freed at that time. (A later "give it back" callback would then > reflect their new "will not be resubmitted, now fully unlinked" mode.) Yes, I was agreeing with you, but disagreeing with Oliver (I think). What would work is unlinking the URB asynchronously and then freeing the URB in that last callback, atleast for UHCI. But this is also poorly defined. I think the safest thing to do in this situation is never free an auto resubmitting URB in the completion handler for 2.4. Always do it elsewhere. > > All of these exceptions are one of the reasons I dislike the current > > (2.4) URB interface. > > I'm not entirely sure what you mean by that ... although I've begun to > dislike "automagic resubmit" more. The ISO ring stuff is particularly > annoying, there's no clean way to report some errors. And it's very > awkward for INTR-OUT transfers. > > I suspect that what you mean is at least that you'd like to see a simpler > model for periodic URBs ... with no "automagic resubmit", but with each > type of URB being queuable in the HCD. That'd likely make sense to > me as a 2.5 change, once the bandwidth reservation issues get worked. > (Reserve at set_config/set_interface time?? :) Yeah, that's pretty much what I meant. The URB interface pretends that everything is the same (one entry point, usb_submit_urb) but all of the different URB's behave differently. It's very difficult to understand. > > > In practical terms, I believe some of the HC drivers (I thought > > > it was only usb-uhci, actually) continue to access URBs in > > > some cases after they are returned to drivers. I call that sort > > > of behavior buggy, but for now drivers in 2.4 need to cope. > > > > I think the HCD's should be fixed, there's less of them :) Plus, it's > > the rule of least surprise. We shouldn't expect drivers to workaround > > bugs or inconsistencies with HCD's. > > That's why I said "for now" ... :) I agree. But HCDs have a > deservedly notorious reputation for fragility. We could really > use a good regression test framework there. As well as a > simpler and more testable API for the HCDs! I agree. There really isn't much of an excuse for having buggy HCD's. JE _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
