Re: [PATCH] net/usbnet: avoid recursive locking in usbnet_stop()

On 02/20/2012 09:31 PM, Alan Stern wrote:

On Mon, 20 Feb 2012, Sebastian Andrzej Siewior wrote:

quoted

On 02/20/2012 05:21 PM, Oliver Neukum wrote:

quoted

quoted

defer_bh() takes the lok which is hold during unlink_urbs(). The safe
walk suggest that the skb will be removed from the list and this is done
by defer_bh() so it seems to be okay to drop the lock here.

I am afraid there's something wrong in the hcd driver. Async unlink must
be possible with a lock held. I cannot approve this patch.

Hmmm. The comment above unlink() says that. Looking through other hcds
it seems that musb is not the only one doing it wrong. Oh well...

What's the issue here?

If a driver calls usb_unlink_urb() while holding a lock, and the
completion routine tries to acquire the same lock, then deadlock is
possible.  The fact that usb_unlink_urb() is asynchronous is not a
guarantee of anything; the HCD is allowed to call the completion
handler from within usb_unlink_urb().

It's true that the kerneldoc for usb_unlink_urb() says "This request is
always asynchronous".  It might be a good idea to remove the word
"always", because it seems to give people the wrong idea.

I see. So you approve that patch and suggest to remove the "always"
wording plus adding something like "the hcd might call complete routine
during unlink" ?

Alan Stern

Sebastian
--
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