From 126129e174062c2a27423817a459e5113f777789 Mon Sep 17 00:00:00 2001 From: Daniel Drake Date: Thu, 9 Jul 2009 22:09:04 +0100 Subject: [PATCH] Linux: try harder not to lose any data We would previously lose any data that was present on a cancelled URB. Work harder to make sure this doesn't happen. --- libusb/core.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++ libusb/os/linux_usbfs.c | 46 ++++++++++++++++++++++++++++++---------- 2 files changed, 91 insertions(+), 11 deletions(-) diff --git a/libusb/core.c b/libusb/core.c index ce914ce..92eb6bf 100644 --- a/libusb/core.c +++ b/libusb/core.c @@ -238,6 +238,62 @@ if (cfg != desired) * * The above method works because once an interface is claimed, no application * or driver is able to select another configuration. + * + * \section cancel Cancellation, early completion and timeouts + * + * NOTE: This section is currently Linux-centric. I am not sure if any of these + * considerations apply to Darwin or other platforms. + * + * Cancellation of transfers can produce some slightly undesirable results. + * It is important to understand that a transfer timing out can also result in + * cancellation, as can a transfer that completes early (with a short packet). + * + * The timeout case is easy to comprehend; if a transfer times out, libusb + * will cancel it at the point of timeout and inform you of this. Hence timeout + * equals cancellation. + * + * The early completion case needs a little more explanation: When you submit + * a large transfer, libusb may need to divide it into several smaller + * sub-transfers which are then submitted to the host controller in parallel + * for performance reasons. If one sub-transfer completes early, libusb then + * needs to cancel all the subsequent sub-transfers before returning the result + * to you. + * + * All forms of cancellation including the usual libusb_cancel_transfer() can + * add some complications as it is possible that data transfer may be happening + * while libusb is performing cancellation. This can lead to the following + * kinds of situations: + * - A transfer may timeout or be cancelled by the user. While libusb is + * cancelling the transfer (which may consist of several sub-transfers + * which libusb has to cancel one-by-one), data may start to be transferred. + * libusb will report successful cancellation as usual, and will reflect the + * situation with the \ref libusb_transfer::actual_length "actual_length" + * field of the transfer, and for device-to-host transfers, the appropriate + * amount of data will be present in \ref libusb_transfer::buffer "buffer"). + * - Similarly, a few USB-level packets within the transfer may already have + * been transferred when the timeout/cancellation occurred, and more + * packets may complete during the time needed to cancel the transfer. + * - A non-ultimate sub-transfer within a transfer may complete with a short + * packet, prompting libusb to immediately cancel all subsequent + * sub-transfers and report early completion. However, during the time + * needed to cancel the subsequent transfers, more data may be transferred. + * This is a difficult situation because the libusb API does not currently + * have a way of indicating the point at which the transfer ended relative + * to the surplus data that was transferred. + * + * In all cases where the transfer is a device-to-host transfer and surplus + * data is recieved as above, libusb places it in the transfer buffer as if + * it had arrived contiguously and updates + * \ref libusb_transfer::actual_length "actual_length" accordingly. + * + * We hope to eliminate some of these difficulties in the near future, but + * kernel changes may be required. + * + * Ultimately, if you cancel a transfer before it has completed then you are + * obligated to handle the above caveats and to resynchronize with the device + * at the application level. Also, remember that timeouts are simply time-based + * cancellations which libusb makes convenient for you, hence the same + * considerations apply. */ /** diff --git a/libusb/os/linux_usbfs.c b/libusb/os/linux_usbfs.c index f9899cf..1280188 100644 --- a/libusb/os/linux_usbfs.c +++ b/libusb/os/linux_usbfs.c @@ -1710,22 +1710,41 @@ static int handle_bulk_completion(struct usbi_transfer *itransfer, tpriv->num_retired++; - if (urb->status == 0 || - (urb->status == -EOVERFLOW && urb->actual_length > 0)) - itransfer->transferred += urb->actual_length; - if (tpriv->reap_action != NORMAL) { /* cancelled, submit_fail, or completed early */ - if (urb->status == 0 && tpriv->reap_action == COMPLETED_EARLY) { - /* FIXME we could solve this extreme corner case with a memmove - * or something */ - usbi_warn(ITRANSFER_CTX(itransfer), "SOME DATA LOST! " - "(completed early but remaining urb completed)"); + usbi_dbg("abnormal reap: urb status %d", urb->status); + + /* even though we're in the process of cancelling, it's possible that + * we may receive some data in these URBs that we don't want to lose. + * examples: + * 1. while the kernel is cancelling all the packets that make up an + * URB, a few of them might complete. so we get back a successful + * cancellation *and* some data. + * 2. we receive a short URB which marks the early completion condition, + * so we start cancelling the remaining URBs. however, we're too + * slow and another URB completes (or at least completes partially). + * + * When this happens, our objectives are not to lose any "surplus" data, + * and also to stick it at the end of the previously-received data + * (closing any holes), so that libusb reports the total amount of + * transferred data and presents it in a contiguous chunk. + */ + if (urb->actual_length > 0) { + struct libusb_transfer *transfer = + __USBI_TRANSFER_TO_LIBUSB_TRANSFER(itransfer); + unsigned char *target = transfer->buffer + itransfer->transferred; + usbi_dbg("received %d bytes of surplus data", urb->actual_length); + if (urb->buffer != target) { + usbi_dbg("moving surplus data from offset %d to offset %d", + (unsigned char *) urb->buffer - transfer->buffer, + target - transfer->buffer); + memmove(target, urb->buffer, urb->actual_length); + } + itransfer->transferred += urb->actual_length; } - usbi_dbg("CANCEL: urb status %d", urb->status); if (tpriv->num_retired == num_urbs) { - usbi_dbg("CANCEL: last URB handled, reporting"); + usbi_dbg("abnormal reap: last URB handled, reporting"); if (tpriv->reap_action == CANCELLED) { free(tpriv->urbs); tpriv->urbs = NULL; @@ -1739,6 +1758,11 @@ static int handle_bulk_completion(struct usbi_transfer *itransfer, return 0; } + if (urb->status == 0 || + (urb->status == -EOVERFLOW && urb->actual_length > 0)) + itransfer->transferred += urb->actual_length; + + switch (urb->status) { case 0: break; -- 2.7.4