Am 12.10.2012 16:17, schrieb Ming Lei: > On Fri, Oct 12, 2012 at 5:26 PM, Tilman Schmidt <tilman@xxxxxxx> wrote: > >> At first sight, I can simply omit the cancel_work_sync() in >> the pre_reset() case. In the worst case, the uncancelled >> int_in_work() will call usb_clear_halt(), try to resubmit the >> already submitted URB, fail, and trigger another reset >> needlessly. Doesn't sound too bad? >> >> The alternative would be to introduce a new flag in the device >> state to skip the cancel_work_sync() call if int_in_work() is >> in the call to usb_reset_device(). > > The above is not good, running pre_reset() means that the > device is being reset and the device lock has been held, > so int_in_work() can't acquire the device lock and complete > the reset. > > On easy solution is that skipping reset device in int_in_work() > if the device is being reset. I'm not sure I understand your argument. There are two cases I have to worry about: (1) a reset triggered by int_in_work() (2) a reset triggered by some other source external to the driver In case (1) the current design will deadlock because pre_reset() is indirectly called from int_in_work() but calls cancel_work_sync() which will wait for this very work item to complete. This would be fixed by pre_reset() not calling cancel_work_sync() at all. int_in_work() will then run to completion without resubmitting urb_int_in because it's already past that point once it calls usb_reset_device(), so there's no conflict with post_reset() doing that. In case (2) the current design will work fine, but if I remove the call to cancel_work_sync() from pre_reset() there's a slight chance that an uncompleted int_in_work() work item remains uncancelled. So any part of int_in_work() might run asynchronously after pre_reset(). The question is whether this might cause a problem. Which of the two cases does your argument address? Thanks, Tilman -- Tilman Schmidt E-Mail: tilman@xxxxxxx Bonn, Germany Diese Nachricht besteht zu 100% aus wiederverwerteten Bits. Ungeöffnet mindestens haltbar bis: (siehe Rückseite)
Attachment:
signature.asc
Description: OpenPGP digital signature