Hi,
Thanks for the quick review!
On 10/09/2013 07:00 PM, Oliver Neukum wrote:
On Wed, 2013-10-09 at 17:01 +0200, Hans de Goede wrote:
So I have come up with this patch instead, which adds the ability to
suspend wakeups of usb_wait_anchor_empty_timeout() waiters to the usb_anchor
functionality, and uses this in __usb_hcd_giveback_urb() to delay wake-ups
until the completion handler has run.
Hm. This may mean that we lose wakeups. If the driver reanchors (to the
same anchor) no wakeup will be issued, ever.
Correct, and there never was a guarantee one would in the old case too,
if the completion handler re-anchored before the waiting task redid the
list_empty check it would resume waiting anyways.
So re-anchoring and using usb_wait_anchor_empty_timeout() where mutually
exclusive before this patch already.
One could argue the new behavior where the wakeup will never happen is actually
better, because that way people will not accidentally start depending happening
on the race happening in such a way that the wakeup does happen.
I've just checked all in tree users of usb_wait_anchor_empty_timeout(), and
only drivers/staging/ced1401/usb1401.c combines
usb_wait_anchor_empty_timeout() with re-anchoring from the completion handling,
and it sets a flag to stop re-submit + re-anchor from the completion handler,
before calling usb_wait_anchor_empty_timeout().
Regards,
Hans
--
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
