On Tue, 24 Apr 2012, Ming Lei wrote:

> > Instead of changing return codes or adding locks, how about
> > implementing a small state machine for each URB?
> >
> >        Initially the state is ACTIVE.
> >
> >        When the URB times out, acquire the lock.  If the state is not
> >        equal to ACTIVE, drop the lock and return immediately (the URB
> >        is being unlinked concurrently).  Otherwise set the state to
> >        UNLINK_STARTED, drop the lock, call usb_unlink_urb, and
> >        reacquire the lock.  If the state hasn't changed, set it back
> >        to ACTIVE.  But if the state has changed to UNLINK_FINISHED,
> >        set it to ACTIVE and resubmit.
> >
> >        In the completion handler, grab the lock.  If the state
> >        is ACTIVE, resubmit.  But if the state is UNLINK_STARTED,
> >        change it to UNLINK_FINISHED and don't resubmit.
> 
> Sounds a smart solution, but extra care should be put on submit urb
> in other context, maybe the lock is to be held to check URB state before
> submitting it because the lock is released during usb_unlink_urb.

Don't you face a similar problem right now?  What happens if the
timeout expires while the URB completion handler is running?  Then the
timer routine could end up unlinking the resubmitted URB instead of the
original transfer.

You need a sort of "generation number", or something like that, to
prevent this kind of mistake.  The same mechanism should work with the
state machine.

Are you saying that every place where the URB is submitted, the state 
has to be set to ACTIVE first, and this has to done with the lock held?  
Yes, that's right.

> Also basically the URB state is maintained by device driver, so looks
> better to add the state into specific device driver instead of struct URB to
> save each URB storage, but if so, the solution will become more
> complicated than the way of adding lock.

Why?  In one case you add a state variable, in the other case you add a 
spinlock.  One isn't much harder than the other.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html