From 3fadb8b4facf5106f1127e389e1855013c1701ca Mon Sep 17 00:00:00 2001 From: Pete Batard Date: Tue, 10 Jul 2012 01:59:45 +0100 Subject: [PATCH] Core: Fix unconditional disarming of timerfd * Existing code appears disarms the timerfd always, which cancels pending timeouts as soon as one packet completes. * This fix moves the disarming of the timerfd to arm_timerfd_for_next_timeout(), where it is now done conditionally. It also avoids calling disarm outside of the above call. * This patch also ensures that all handling of the timerfd is done under the flying transfers lock. * Issue reported by Hans de Goede. For more info, see: https://sourceforge.net/mailarchive/message.php?msg_id=29442693 --- libusb/io.c | 173 ++++++++++++++++++++++++-------------------------- libusb/version_nano.h | 2 +- 2 files changed, 83 insertions(+), 92 deletions(-) diff --git a/libusb/io.c b/libusb/io.c index b8d445c..e6d4132 100644 --- a/libusb/io.c +++ b/libusb/io.c @@ -1166,14 +1166,13 @@ static int add_to_flying_list(struct usbi_transfer *transfer) /* if we have no other flying transfers, start the list with this one */ if (list_empty(&ctx->flying_transfers)) { list_add(&transfer->list, &ctx->flying_transfers); - if (timerisset(timeout)) - r = 1; goto out; } /* if we have infinite timeout, append to end of list */ if (!timerisset(timeout)) { list_add_tail(&transfer->list, &ctx->flying_transfers); + /* first is irrelevant in this case */ goto out; } @@ -1186,15 +1185,33 @@ static int add_to_flying_list(struct usbi_transfer *transfer) (cur_tv->tv_sec == timeout->tv_sec && cur_tv->tv_usec > timeout->tv_usec)) { list_add_tail(&transfer->list, &cur->list); - r = first; goto out; } first = 0; } + /* first is 0 at this stage (list not empty) */ /* otherwise we need to be inserted at the end */ list_add_tail(&transfer->list, &ctx->flying_transfers); out: +#ifdef USBI_TIMERFD_AVAILABLE + if (first && usbi_using_timerfd(ctx) && timerisset(timeout)) { + /* if this transfer has the lowest timeout of all active transfers, + * rearm the timerfd with this transfer's timeout */ + const struct itimerspec it = { {0, 0}, + { transfer->timeout.tv_sec, transfer->timeout.tv_usec * 1000 } }; + usbi_dbg("arm timerfd for timeout in %dms (first in line)", + USBI_TRANSFER_TO_LIBUSB_TRANSFER(transfer)->timeout); + r = timerfd_settime(ctx->timerfd, TFD_TIMER_ABSTIME, &it, NULL); + if (r < 0) { + usbi_warn(ctx, "failed to arm first timerfd (error %d)", r); + r = LIBUSB_ERROR_OTHER; + } + } +#else + UNUSED(first); +#endif + usbi_mutex_unlock(&ctx->flying_transfers_lock); return r; } @@ -1272,6 +1289,62 @@ void API_EXPORTED libusb_free_transfer(struct libusb_transfer *transfer) free(itransfer); } +#ifdef USBI_TIMERFD_AVAILABLE +static int disarm_timerfd(struct libusb_context *ctx) +{ + const struct itimerspec disarm_timer = { { 0, 0 }, { 0, 0 } }; + int r; + + usbi_dbg(""); + r = timerfd_settime(ctx->timerfd, 0, &disarm_timer, NULL); + if (r < 0) + return LIBUSB_ERROR_OTHER; + else + return 0; +} + +/* iterates through the flying transfers, and rearms the timerfd based on the + * next upcoming timeout. + * must be called with flying_list locked. + * returns 0 if there was no timeout to arm, 1 if the next timeout was armed, + * or a LIBUSB_ERROR code on failure. + */ +static int arm_timerfd_for_next_timeout(struct libusb_context *ctx) +{ + struct usbi_transfer *transfer; + + list_for_each_entry(transfer, &ctx->flying_transfers, list, struct usbi_transfer) { + struct timeval *cur_tv = &transfer->timeout; + + /* if we've reached transfers of infinite timeout, then we have no + * arming to do */ + if (!timerisset(cur_tv)) + goto disarm; + + /* act on first transfer that is not already cancelled */ + if (!(transfer->flags & USBI_TRANSFER_TIMED_OUT)) { + int r; + const struct itimerspec it = { {0, 0}, + { cur_tv->tv_sec, cur_tv->tv_usec * 1000 } }; + usbi_dbg("next timeout originally %dms", USBI_TRANSFER_TO_LIBUSB_TRANSFER(transfer)->timeout); + r = timerfd_settime(ctx->timerfd, TFD_TIMER_ABSTIME, &it, NULL); + if (r < 0) + return LIBUSB_ERROR_OTHER; + return 1; + } + } + +disarm: + return disarm_timerfd(ctx); +} +#else +static int arm_timerfd_for_next_timeout(struct libusb_context *ctx) +{ + (void)ctx; + return 0; +} +#endif + /** \ingroup asyncio * Submit a transfer. This function will fire off the USB transfer and then * return immediately. @@ -1290,7 +1363,6 @@ int API_EXPORTED libusb_submit_transfer(struct libusb_transfer *transfer) struct usbi_transfer *itransfer = LIBUSB_TRANSFER_TO_USBI_TRANSFER(transfer); int r; - int first; int updated_fds; usbi_mutex_lock(&itransfer->lock); @@ -1302,27 +1374,16 @@ int API_EXPORTED libusb_submit_transfer(struct libusb_transfer *transfer) goto out; } - first = add_to_flying_list(itransfer); + r = add_to_flying_list(itransfer); + if (r) + goto out; r = usbi_backend->submit_transfer(itransfer); if (r) { usbi_mutex_lock(&ctx->flying_transfers_lock); list_del(&itransfer->list); + arm_timerfd_for_next_timeout(ctx); usbi_mutex_unlock(&ctx->flying_transfers_lock); } -#ifdef USBI_TIMERFD_AVAILABLE - else if (first && usbi_using_timerfd(ctx)) { - /* if this transfer has the lowest timeout of all active transfers, - * rearm the timerfd with this transfer's timeout */ - const struct itimerspec it = { {0, 0}, - { itransfer->timeout.tv_sec, itransfer->timeout.tv_usec * 1000 } }; - usbi_dbg("arm timerfd for timeout in %dms (first in line)", transfer->timeout); - r = timerfd_settime(ctx->timerfd, TFD_TIMER_ABSTIME, &it, NULL); - if (r < 0) - r = LIBUSB_ERROR_OTHER; - } -#else - (void)first; -#endif out: updated_fds = (itransfer->flags & USBI_TRANSFER_UPDATED_FDS); @@ -1372,66 +1433,6 @@ int API_EXPORTED libusb_cancel_transfer(struct libusb_transfer *transfer) return r; } -#ifdef USBI_TIMERFD_AVAILABLE -static int disarm_timerfd(struct libusb_context *ctx) -{ - const struct itimerspec disarm_timer = { { 0, 0 }, { 0, 0 } }; - int r; - - usbi_dbg(""); - r = timerfd_settime(ctx->timerfd, 0, &disarm_timer, NULL); - if (r < 0) - return LIBUSB_ERROR_OTHER; - else - return 0; -} - -/* iterates through the flying transfers, and rearms the timerfd based on the - * next upcoming timeout. - * must be called with flying_list locked. - * returns 0 if there was no timeout to arm, 1 if the next timeout was armed, - * or a LIBUSB_ERROR code on failure. - */ -static int arm_timerfd_for_next_timeout(struct libusb_context *ctx) -{ - struct usbi_transfer *transfer; - - list_for_each_entry(transfer, &ctx->flying_transfers, list, struct usbi_transfer) { - struct timeval *cur_tv = &transfer->timeout; - - /* if we've reached transfers of infinite timeout, then we have no - * arming to do */ - if (!timerisset(cur_tv)) - return 0; - - /* act on first transfer that is not already cancelled */ - if (!(transfer->flags & USBI_TRANSFER_TIMED_OUT)) { - int r; - const struct itimerspec it = { {0, 0}, - { cur_tv->tv_sec, cur_tv->tv_usec * 1000 } }; - usbi_dbg("next timeout originally %dms", USBI_TRANSFER_TO_LIBUSB_TRANSFER(transfer)->timeout); - r = timerfd_settime(ctx->timerfd, TFD_TIMER_ABSTIME, &it, NULL); - if (r < 0) - return LIBUSB_ERROR_OTHER; - return 1; - } - } - - return 0; -} -#else -static int disarm_timerfd(struct libusb_context *ctx) -{ - (void)ctx; - return 0; -} -static int arm_timerfd_for_next_timeout(struct libusb_context *ctx) -{ - (void)ctx; - return 0; -} -#endif - /* Handle completion of a transfer (completion might be an error condition). * This will invoke the user-supplied callback function, which may end up * freeing the transfer. Therefore you cannot use the transfer structure @@ -1459,14 +1460,8 @@ int usbi_handle_transfer_completion(struct usbi_transfer *itransfer, if (usbi_using_timerfd(ctx)) r = arm_timerfd_for_next_timeout(ctx); usbi_mutex_unlock(&ctx->flying_transfers_lock); - - if (usbi_using_timerfd(ctx)) { - if (r < 0) - return r; - r = disarm_timerfd(ctx); - if (r < 0) - return r; - } + if (usbi_using_timerfd(ctx) && (r < 0)) + return r; if (status == LIBUSB_TRANSFER_COMPLETED && transfer->flags & LIBUSB_TRANSFER_SHORT_NOT_OK) { @@ -1829,10 +1824,6 @@ static int handle_timerfd_trigger(struct libusb_context *ctx) { int r; - r = disarm_timerfd(ctx); - if (r < 0) - return r; - usbi_mutex_lock(&ctx->flying_transfers_lock); /* process the timeout that just happened */ diff --git a/libusb/version_nano.h b/libusb/version_nano.h index fbf5397..a43c750 100644 --- a/libusb/version_nano.h +++ b/libusb/version_nano.h @@ -1 +1 @@ -#define LIBUSB_NANO 10543 +#define LIBUSB_NANO 10544 -- 2.7.4