> > > should device drivers call usb_get_dev() like some > > > currently do (storage, some network, and the usbvideo core)? Does this > > > protect something from happening that I don't see? > > > > I think earlier on, when the whole stack was less well developed, > > device drivers doing that refcounting were getting extra protection > > against various core/hcd bugs that often showed up in disconnect > > paths ... which bugs are now gone. > > We hope :)
True, but if there still were such bugs they'd be showing up a lot more generally. From the email, I can tell I've spent more time finding/fixing such bugs than most folk. It's been a long time since a core/hcd bug in this area has turned up, and those were the ones I referred to. On the other hand, bugs in the device drivers still turn up pretty regularly. The "printer" and "usbfs" drivers had disconnect() bugs until quite recently, which caused oopsing. (Recently meaning they both oops in 2.4.18, as I recall.) Of course, many of those bugs were finally isolated (after what, three years?) by the BUG() calls that offend Johannes so much. > > On the other hand, arguably an even better solution to this flamage > > would be to hide refcounting from device drivers entirely. There > > are quite a few Linux driver frameworks that don't expose it. And > > since the device drivers and khubd agree on when the devices are > > really going away (and need to!), it's easy to argue that such device > > refcounting doesn't need to be in the device driver API at all. > > No, we need refcounting. Inside usbcore, sure. Visible to device drivers, no. Because the interface claiming and disconnect() make driver-visible refcounting superfluous. Think about it ... they act as implicit refcounts, so there's no need to make it both explicit AND driver-visible. Even if those probe/claim refcounts were explicit (incrementing the count), there'd still be no need for the counting to be driver-visible. > See my previous response to Johannes about > what Documentation/CodingStyle says. You quoted "if another thread can find your data structure, and you don't have a reference count on it you almost certainly have a bug". But see above -- the count does not need to be driver-visible. As always, usable style guidelines are not ironclad rules, since there's no way for them to capture every possible situation and still be usable by non-lawyers. > What I'm more worried about is, do you think these two patches contain > anything that would cause a bug? They absolutely do ... they allow the *hci_free_dev() calls to be made in_interrupt() contexts, which changes the API. Arguably, one problem is that this refcounting is being used in a way it shouldn't be. It's co-mingling memory management with device management. If I can restate Johannes' claims without needing to put on asbestos underwear, he wants hci_free_dev() to just be a memory management call ... when it's really a call for device management, and those calls aren't done in_interrupt(). I don't actually buy that argument, because I can't see any situation where the device management and the memory management can justifiably be out of sync. Once the device is gone, it's gone. - Dave _______________________________________________________________ Have big pipes? SourceForge.net is looking for download mirrors. We supply the hardware. You get the recognition. Email Us: [EMAIL PROTECTED] _______________________________________________ [EMAIL PROTECTED] To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel