On Thu, 4 Jun 2009, Martin Fuzzey wrote:
> In tests 11 and 12 if the URB completes with an error status (eg babble)
> the asynchrous unlink entered an endless loop trying to unlink
> a non resubmitted URB.
>
> Signed-off-by: Martin Fuzzey <mfuzzey@xxxxxxxxx>
>
> ---
>
> drivers/usb/misc/usbtest.c | 39 +++++++++++++++++++++++++--------------
> 1 files changed, 25 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
> index 5f1a19d..a9f06d7 100644
> --- a/drivers/usb/misc/usbtest.c
> +++ b/drivers/usb/misc/usbtest.c
> @@ -1072,23 +1072,34 @@ static int unlink1 (struct usbtest_dev *dev, int pipe, int size, int async)
> */
> msleep (jiffies % (2 * INTERRUPT_RATE));
> if (async) {
> -retry:
> - retval = usb_unlink_urb (urb);
> - if (retval == -EBUSY || retval == -EIDRM) {
> - /* we can't unlink urbs while they're completing.
> - * or if they've completed, and we haven't resubmitted.
> - * "normal" drivers would prevent resubmission, but
> - * since we're testing unlink paths, we can't.
> - */
> - ERROR(dev, "unlink retry\n");
> - goto retry;
> + while (!completion_done(&completion)) {
> + retval = usb_unlink_urb(urb);
> +
> + switch (retval) {
> + case -EBUSY:
> + case -EIDRM:
> + /* we can't unlink urbs while they're completing
> + * or if they've completed, and we haven't
> + * resubmitted. "normal" drivers would prevent
> + * resubmission, but since we're testing unlink
> + * paths, we can't.
> + */
> + ERROR(dev, "unlink retry\n");
> + continue;
> + case 0:
> + case -EINPROGRESS:
> + break;
> +
> + default:
> + dev_err(&dev->intf->dev,
> + "unlink fail %d\n", retval);
> + return retval;
> + }
> +
> + break;
> }
> } else
> usb_kill_urb (urb);
> - if (!(retval == 0 || retval == -EINPROGRESS)) {
> - dev_err(&dev->intf->dev, "unlink fail %d\n", retval);
> - return retval;
> - }
>
> wait_for_completion (&completion);
> retval = urb->status;
>
For whatever it's worth, I noticed this same bug and wrote the
following patch a couple of years ago. For reasons that escape me now,
the patch was never merged. I suspect it isn't as good as your patch;
what do you think?
Alan Stern
Index: usb-2.6/drivers/usb/misc/usbtest.c
===================================================================
--- usb-2.6.orig/drivers/usb/misc/usbtest.c
+++ usb-2.6/drivers/usb/misc/usbtest.c
@@ -1077,6 +1077,7 @@ static int unlink1 (struct usbtest_dev *
*/
msleep (jiffies % (2 * INTERRUPT_RATE));
if (async) {
+ int count = 0;
retry:
retval = usb_unlink_urb (urb);
if (retval == -EBUSY || retval == -EIDRM) {
@@ -1086,7 +1087,9 @@ retry:
* since we're testing unlink paths, we can't.
*/
dev_dbg (&dev->intf->dev, "unlink retry\n");
- goto retry;
+ if (++count < 5)
+ goto retry;
+ usb_kill_urb(urb);
}
} else
usb_kill_urb (urb);
--
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
