Oliver Neukum <oneukum@xxxxxxx> writes: > Am Dienstag, 26. Juni 2012, 09:23:19 schrieb Ming Lei: >> On Mon, Jun 25, 2012 at 8:10 PM, Oliver Neukum <oliver@xxxxxxxxxx> wrote: > >> > At this point a minidriver must not follow the intfdata pointer, >> > because the interface may again be probed. So if here a minidriver >> >> IMO, probe is serialized strictly with driver unbind since both the parent >> lock and its own device lock have been held, so the probe may only be >> started after driver unbinding is completed. > > Yes, but if you have a driver which claims multiple interfaces and uses > a subdriver, then you will have cases of intfdate being NULL before > disconnect() finishes. This is true regardless of subdriver, isn't it? The subdriver issue is special because it makes this a potentional problem even with a single interface. >> > still uses intfdata, locking will be needed. We want to catch those >> > casees. >> >> Suppose infdata is used here somewhere, it is surely a bug because >> the usbnet instance pointed by intfdata will be freed soon. > > Of course. That is the point. > >> So looks putting the set to NULL after driver_info->unbind is good, >> doesn't it? > > Again, of course. We could drop it (but not the check for NULL in usbnet). > It is a debugging aid. > >> > usb_kill_urb(dev->interrupt); >> > usb_free_urb(dev->interrupt); >> > >> > free_netdev(net); >> > usb_put_dev (xdev); >> > } >> > >> >> > Sure, it is a debugging aid. It has the drawback that minidrivers have >> >> > to be able to deal with intfdata being NULL. That is not hard to do. >> >> >> >> The check isn't needed if the set to NULL is put after driver_info->unbind >> >> in usbnet_disconnect. >> > >> > True, but we don't catch bugs. >> >> If the check is added, the bugs may be hided, and no stack will be >> dumped, :-) > > That is also true. > > Bjørn, > > do you use subdrivers with cdc-ether? No, and I don't think it should ever be added there. It would add complexity to the driver, and IMHO we really don't want that for the generic cdc-ether. Better adding a new minidriver if the need to export some other CDC embedded protocol to userspace shows up. But given that all other embedded protocol examples I've seen are so simple that they've been implemented inside their repective minidrivers (sierra_net, hso, rndis_host), I don't think it's likely that any new need for the cdc-wdm subdriver will show up soon. I could of course be completely wrong, as usual. Bjørn -- 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
