On Wed, 21 Mar 2012, Ming Lei wrote: > On Wed, Mar 21, 2012 at 10:35 PM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > > On Wed, 21 Mar 2012, Ming Lei wrote: > > > >> Looks it is a general issue about usb_hcd_unlink_urb. > >> > >> Alan, how about increasing URB reference count before calling unlink1 > >> inside usb_hcd_unlink_urb to fix this kind of problem? > > > > No, that won't fix the problem. �The URB could complete and be > > deallocated even before usb_hcd_unlink_urb() is called, so nothing that > > function can do will prevent the error. > > IMO, driver should not call usb_hcd_unlink_urb after urb is freed from > the driver, > but this problem is that URB may be freed during usb_hcd_unlink_urb. Drivers don't call usb_hcd_unlink_urb; they call usb_unlink_urb. This sort of thing can happen: Driver Interrupt handler ------ ----------------- call usb_unlink_urb URB completion interrupt occurs call usb_hcd_giveback_urb completion routine calls usb_free_urb URB is deallocated call usb_hcd_unlink_urb try to increment URB's refcount oops because URB is gone > In fact, it is allowed that usb_free_urb is called inside .complete handler, > at least as said in Documentation/URB.txt: > > "You may free an urb that you've submitted,..." > > So looks reasonable to increase the URB reference count before calling > unlink1(), just like that done inside usb_hcd_flush_endpoint(). And I > think it is a general solution for avoiding this kind of problem. It will not solve the problem illustrated above. The driver must avoid freeing the URB before usb_unlink_urb returns. In this case, increasing the refcount around the unlink call would work. > > It is the caller's responsibility to make sure that the URB does not > > get freed before usb_unlink_urb() or usb_kill_urb() returns. �We > > probably should mention this in the kerneldoc... > > If so, looks it is a bit contrary with Documentation/URB.txt, also > this may add extra constraint(maybe unnecessary) to the driver. It's not contradictory. You may indeed free an URB that you have submitted, so long as you don't free it while usb_unlink_urb (or related routines like usb_kill_urb) is running. The extra constraint on the driver is indeed necessary. However the driver can avoid complications by using anchors. Alan Stern -- 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
