[Libusb-devel] Timeouts & URB handling

[Albert K. <a_k...@ya...>]

Hello libusb:

I've been trying to understand how libusb handles timeouts with respect to URB unlinking & completion.

My understanding is that if all data can not be bulk communicated within the user specified timeout period, then the final URB is unlinked and -ETIMEDOUT is returned.

This mechanism does not allow the caller to know what portion of their data was successfully communicated before the timeout occurred.  For example, on a printer, if the first 500 bytes of a 512 byte print job were sent before timeout, then the caller would recall usb_bulk_write to communicate only the final 12 bytes.  The current implimentation does not allow for this because -ETIMEDOUT is returned masking the fact that some data was infact communicated successfully.

Furthermore, the IOCTL_USB_DISCARDURB request can fail with EINVAL if the URB was completed before the discard request could be processed.  In this case, the URB should be considered to have been successfully executed, and should contribute to bytesdone.

I have worked up the following patch realizing these points.

Please understand that I'm not 100% familiar with all aspects of the libusb project.  So, please consider this submission with all due suspicions.



//////////// begin path




Index: libusb/linux.c
===================================================================
--- libusb/linux.c    (revision 640) 
+++ libusb/linux.c    (working copy) 
@@ -253,25 +253,47 @@
 
   /* If the URB didn't complete in success or error, then let's unlink it */
   if (ret < 0 && !urb.usercontext) {
-    int rc; 
- 
-    if (!waiting) 
-      rc = -ETIMEDOUT; 
-    else 
-      rc = urb.status; 
- 
-    ret = ioctl(dev->fd, IOCTL_USB_DISCARDURB, &urb); 
-    if (ret < 0 && errno != EINVAL && usb_debug >= 1) 
-      fprintf(stderr, "error discarding URB: %s", strerror(errno)); 
- 
-    /* 
-     * When the URB is unlinked, it gets moved to the completed list and 
-     * then we need to reap it or else the next time we call this function, 
-     * we'll get the previous completion and exit early 
+    if (ioctl(dev->fd, IOCTL_USB_DISCARDURB, &urb) != 0) { 
+      /*  
+       * If IOCTL_USB_DISCARDURB failed with EINVAL, then the URB was completed before we could kill it. 
+       * Otherwise something went wrong and we should return that 
+       */ 
+      if (errno != EINVAL) 
+        USB_ERROR_STR(-errno, "error discarding URB: %s", strerror(errno)); 
+    } 
+     
+    /*  
+     * Now we know that the URB has either been completed or discarded 
+     * so we reap it without concern for timeouts 
      */
-    ioctl(dev->fd, IOCTL_USB_REAPURB, &context); 
- 
-    return rc; 
+    while (urb.usercontext == 0) { 
+      context = 0; 
+      if (ioctl(dev->fd, IOCTL_USB_REAPURBNDELAY, &context) != 0) { 
+        /* 
+         * If IOCTL_USB_REAPURBNDELAY failed with EAGAIN, then no URBs are reapable 
+         * Otherwise something went wrong and we should return that 
+         */ 
+        if (errno != EAGAIN) 
+          USB_ERROR_STR(-errno, "error reaping URB: %s", strerror(errno)); 
+        else 
+          continue; 
+      } 
+       
+      if (context && context != &urb) { 
+        /* We reaped somebody else's URB, so we flag it */ 
+        context->usercontext = URB_USERCONTEXT_COOKIE; 
+        continue; 
+      } 
+       
+      /* We reaped our URB and are now done */ 
+      break; 
+    } 
+     
+    bytesdone += urb.actual_length; 
+     
+    /* if we didn't complete any IO, signal timeout */ 
+    if (bytesdone == 0) 
+      return -ETIMEDOUT; 
   }
 
   return bytesdone;



////////////////// end patch


Thank you,

Albert Kennis