From 3b371f1a6ab30bdebdadfcde1b96a9b98fb806ca Mon Sep 17 00:00:00 2001 From: Chris Dickens Date: Mon, 17 Nov 2014 23:53:13 -0800 Subject: core: Eliminate hotplug pipe, using list and event pipe instead To further consolidate libusb internal events, this change removes the hotplug pipe. Hotplug messages are now kept in a list within the context, and the event pipe is signalled when a new hotplug message is added. When handling events, the hotplug messages will be processed from the list instead of from a separate pipe. This change is greatly beneficial for the Windows/WinCE backends which do not allow pipes to be used in the WaitFor* functions. Signed-off-by: Chris Dickens --- libusb/core.c | 37 +++++++-------------- libusb/hotplug.c | 30 ++++++++++++----- libusb/hotplug.h | 8 +++++ libusb/io.c | 91 +++++++++++++++++++-------------------------------- libusb/libusbi.h | 7 +++- libusb/version_nano.h | 2 +- 6 files changed, 81 insertions(+), 94 deletions(-) diff --git a/libusb/core.c b/libusb/core.c index 13100da..626fabc 100644 --- a/libusb/core.c +++ b/libusb/core.c @@ -684,12 +684,8 @@ struct libusb_device *usbi_alloc_device(struct libusb_context *ctx, void usbi_connect_device(struct libusb_device *dev) { - libusb_hotplug_message message; - ssize_t ret; + struct libusb_context *ctx = DEVICE_CTX(dev); - memset(&message, 0, sizeof(message)); - message.event = LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED; - message.device = dev; dev->attached = 1; usbi_mutex_lock(&dev->ctx->usb_devs_lock); @@ -697,25 +693,17 @@ void usbi_connect_device(struct libusb_device *dev) usbi_mutex_unlock(&dev->ctx->usb_devs_lock); /* Signal that an event has occurred for this device if we support hotplug AND - * the hotplug pipe is ready. This prevents an event from getting raised during - * initial enumeration. */ - if (libusb_has_capability(LIBUSB_CAP_HAS_HOTPLUG) && dev->ctx->hotplug_pipe[1] > 0) { - ret = usbi_write(dev->ctx->hotplug_pipe[1], &message, sizeof(message)); - if (sizeof (message) != ret) { - usbi_err(DEVICE_CTX(dev), "error writing hotplug message"); - } + * the hotplug message list is ready. This prevents an event from getting raised + * during initial enumeration. */ + if (libusb_has_capability(LIBUSB_CAP_HAS_HOTPLUG) && dev->ctx->hotplug_msgs.next) { + usbi_hotplug_notification(ctx, dev, LIBUSB_HOTPLUG_EVENT_DEVICE_ARRIVED); } } void usbi_disconnect_device(struct libusb_device *dev) { - libusb_hotplug_message message; - struct libusb_context *ctx = dev->ctx; - ssize_t ret; + struct libusb_context *ctx = DEVICE_CTX(dev); - memset(&message, 0, sizeof(message)); - message.event = LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT; - message.device = dev; usbi_mutex_lock(&dev->lock); dev->attached = 0; usbi_mutex_unlock(&dev->lock); @@ -725,14 +713,11 @@ void usbi_disconnect_device(struct libusb_device *dev) usbi_mutex_unlock(&ctx->usb_devs_lock); /* Signal that an event has occurred for this device if we support hotplug AND - * the hotplug pipe is ready. This prevents an event from getting raised during - * initial enumeration. libusb_handle_events will take care of dereferencing the - * device. */ - if (libusb_has_capability(LIBUSB_CAP_HAS_HOTPLUG) && dev->ctx->hotplug_pipe[1] > 0) { - ret = usbi_write(dev->ctx->hotplug_pipe[1], &message, sizeof(message)); - if (sizeof(message) != ret) { - usbi_err(DEVICE_CTX(dev), "error writing hotplug message"); - } + * the hotplug message list is ready. This prevents an event from getting raised + * during initial enumeration. libusb_handle_events will take care of dereferencing + * the device. */ + if (libusb_has_capability(LIBUSB_CAP_HAS_HOTPLUG) && dev->ctx->hotplug_msgs.next) { + usbi_hotplug_notification(ctx, dev, LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT); } } diff --git a/libusb/hotplug.c b/libusb/hotplug.c index 9ecb2c7..9171f2c 100644 --- a/libusb/hotplug.c +++ b/libusb/hotplug.c @@ -203,6 +203,27 @@ void usbi_hotplug_match(struct libusb_context *ctx, struct libusb_device *dev, /* the backend is expected to call the callback for each active transfer */ } +void usbi_hotplug_notification(struct libusb_context *ctx, struct libusb_device *dev, + libusb_hotplug_event event) +{ + libusb_hotplug_message *message = calloc(1, sizeof(*message)); + + if (!message) { + usbi_err(ctx, "error allocating hotplug message"); + return; + } + + message->event = event; + message->device = dev; + + /* Take the event data lock and add this message to the list. */ + usbi_mutex_lock(&ctx->event_data_lock); + list_add_tail(&message->list, &ctx->hotplug_msgs); + usbi_mutex_unlock(&ctx->event_data_lock); + + usbi_signal_event(ctx); +} + int API_EXPORTED libusb_hotplug_register_callback(libusb_context *ctx, libusb_hotplug_event events, libusb_hotplug_flag flags, int vendor_id, int product_id, int dev_class, @@ -285,8 +306,6 @@ void API_EXPORTED libusb_hotplug_deregister_callback (struct libusb_context *ctx libusb_hotplug_callback_handle handle) { struct libusb_hotplug_callback *hotplug_cb; - libusb_hotplug_message message; - ssize_t ret; /* check for hotplug support */ if (!libusb_has_capability(LIBUSB_CAP_HAS_HOTPLUG)) { @@ -305,12 +324,7 @@ void API_EXPORTED libusb_hotplug_deregister_callback (struct libusb_context *ctx } usbi_mutex_unlock(&ctx->hotplug_cbs_lock); - /* wakeup handle_events to do the actual free */ - memset(&message, 0, sizeof(message)); - ret = usbi_write(ctx->hotplug_pipe[1], &message, sizeof(message)); - if (sizeof(message) != ret) { - usbi_err(ctx, "error writing hotplug message"); - } + usbi_hotplug_notification(ctx, NULL, 0); } void usbi_hotplug_deregister_all(struct libusb_context *ctx) { diff --git a/libusb/hotplug.h b/libusb/hotplug.h index 321a0a8..2bec81b 100644 --- a/libusb/hotplug.h +++ b/libusb/hotplug.h @@ -69,8 +69,14 @@ struct libusb_hotplug_callback { typedef struct libusb_hotplug_callback libusb_hotplug_callback; struct libusb_hotplug_message { + /** The hotplug event that occurred */ libusb_hotplug_event event; + + /** The device for which this hotplug event occurred */ struct libusb_device *device; + + /** List this message is contained in (ctx->hotplug_msgs) */ + struct list_head list; }; typedef struct libusb_hotplug_message libusb_hotplug_message; @@ -78,5 +84,7 @@ typedef struct libusb_hotplug_message libusb_hotplug_message; void usbi_hotplug_deregister_all(struct libusb_context *ctx); void usbi_hotplug_match(struct libusb_context *ctx, struct libusb_device *dev, libusb_hotplug_event event); +void usbi_hotplug_notification(struct libusb_context *ctx, struct libusb_device *dev, + libusb_hotplug_event event); #endif diff --git a/libusb/io.c b/libusb/io.c index f00af14..6ae4f8d 100644 --- a/libusb/io.c +++ b/libusb/io.c @@ -1118,6 +1118,7 @@ int usbi_io_init(struct libusb_context *ctx) usbi_cond_init(&ctx->event_waiters_cond, NULL); list_init(&ctx->flying_transfers); list_init(&ctx->ipollfds); + list_init(&ctx->hotplug_msgs); /* FIXME should use an eventfd on kernels that support it */ r = usbi_pipe(ctx->event_pipe); @@ -1130,17 +1131,6 @@ int usbi_io_init(struct libusb_context *ctx) if (r < 0) goto err_close_pipe; - /* create hotplug pipe */ - r = usbi_pipe(ctx->hotplug_pipe); - if (r < 0) { - r = LIBUSB_ERROR_OTHER; - goto err_remove_pipe; - } - - r = usbi_add_pollfd(ctx, ctx->hotplug_pipe[0], POLLIN); - if (r < 0) - goto err_close_hp_pipe; - #ifdef USBI_TIMERFD_AVAILABLE ctx->timerfd = timerfd_create(usbi_backend->get_timerfd_clockid(), TFD_NONBLOCK); @@ -1160,13 +1150,8 @@ int usbi_io_init(struct libusb_context *ctx) #ifdef USBI_TIMERFD_AVAILABLE err_close_timerfd: close(ctx->timerfd); - usbi_remove_pollfd(ctx, ctx->hotplug_pipe[0]); -#endif -err_close_hp_pipe: - usbi_close(ctx->hotplug_pipe[0]); - usbi_close(ctx->hotplug_pipe[1]); -err_remove_pipe: usbi_remove_pollfd(ctx, ctx->event_pipe[0]); +#endif err_close_pipe: usbi_close(ctx->event_pipe[0]); usbi_close(ctx->event_pipe[1]); @@ -1185,9 +1170,6 @@ void usbi_io_exit(struct libusb_context *ctx) usbi_remove_pollfd(ctx, ctx->event_pipe[0]); usbi_close(ctx->event_pipe[0]); usbi_close(ctx->event_pipe[1]); - usbi_remove_pollfd(ctx, ctx->hotplug_pipe[0]); - usbi_close(ctx->hotplug_pipe[0]); - usbi_close(ctx->hotplug_pipe[1]); #ifdef USBI_TIMERFD_AVAILABLE if (usbi_using_timerfd(ctx)) { usbi_remove_pollfd(ctx, ctx->timerfd); @@ -1978,17 +1960,16 @@ static int handle_events(struct libusb_context *ctx, struct timeval *tv) /* there are certain fds that libusb uses internally, currently: * * 1) event pipe - * 2) hotplug pipe - * 3) timerfd + * 2) timerfd * * the backend will never need to attempt to handle events on these fds, so * we determine how many fds are in use internally for this context and when * handle_events() is called in the backend, the pollfd list and count will * be adjusted to skip over these internal fds */ if (usbi_using_timerfd(ctx)) - internal_nfds = 3; - else internal_nfds = 2; + else + internal_nfds = 1; /* only reallocate the poll fds when the list of poll fds has been modified * since the last poll, otherwise reuse them to save the additional overhead */ @@ -2048,58 +2029,52 @@ redo_poll: /* fds[0] is always the event pipe */ if (fds[0].revents) { - /* another thread wanted to interrupt event handling, and it succeeded! - * handle any other events that cropped up at the same time, and - * simply return */ unsigned int ru; + libusb_hotplug_message *message = NULL; usbi_dbg("caught a fish on the event pipe"); - /* read the dummy data from the event pipe unless someone is closing - * a device */ + /* take the the event data lock while processing events */ usbi_mutex_lock(&ctx->event_data_lock); + + /* check if someone is closing a device */ ru = ctx->device_close; - usbi_mutex_unlock(&ctx->event_data_lock); - if (!ru) { - r = usbi_clear_event(ctx); - if (r) - goto handled; + + /* check for any pending hotplug messages */ + if (!list_empty(&ctx->hotplug_msgs)) { + usbi_dbg("hotplug message received"); + special_event = 1; + message = list_first_entry(&ctx->hotplug_msgs, libusb_hotplug_message, list); + list_del(&message->list); } - if (0 == --r) - goto handled; - } + usbi_mutex_unlock(&ctx->event_data_lock); - /* fd[1] is always the hotplug pipe */ - if (libusb_has_capability(LIBUSB_CAP_HAS_HOTPLUG) && fds[1].revents) { - libusb_hotplug_message message; - ssize_t ret; + /* process the hotplug message, if any */ + if (message) { + usbi_hotplug_match(ctx, message->device, message->event); - usbi_dbg("caught a fish on the hotplug pipe"); - special_event = 1; + /* the device left, dereference the device */ + if (LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT == message->event) + libusb_unref_device(message->device); - /* read the message from the hotplug thread */ - ret = usbi_read(ctx->hotplug_pipe[0], &message, sizeof (message)); - if (ret != sizeof(message)) { - usbi_err(ctx, "hotplug pipe read error %d != %u", - ret, sizeof(message)); - r = LIBUSB_ERROR_OTHER; - goto handled; + free(message); } - usbi_hotplug_match(ctx, message.device, message.event); - - /* the device left. dereference the device */ - if (LIBUSB_HOTPLUG_EVENT_DEVICE_LEFT == message.event) - libusb_unref_device(message.device); + /* clear the event pipe if this was an fd or hotplug notification */ + if (!ru || message) { + r = usbi_clear_event(ctx); + if (r) + goto handled; + } if (0 == --r) goto handled; - } /* else there shouldn't be anything on this pipe */ + } #ifdef USBI_TIMERFD_AVAILABLE - /* on timerfd configurations, fds[2] is the timerfd */ - if (usbi_using_timerfd(ctx) && fds[2].revents) { + /* on timerfd configurations, fds[1] is the timerfd */ + if (usbi_using_timerfd(ctx) && fds[1].revents) { /* timerfd indicates that a timeout has expired */ int ret; usbi_dbg("timerfd triggered"); diff --git a/libusb/libusbi.h b/libusb/libusbi.h index cb24cf9..c94c5a2 100644 --- a/libusb/libusbi.h +++ b/libusb/libusbi.h @@ -87,6 +87,9 @@ struct list_head { #define list_entry(ptr, type, member) \ ((type *)((uintptr_t)(ptr) - (uintptr_t)offsetof(type, member))) +#define list_first_entry(ptr, type, member) \ + list_entry((ptr)->next, type, member) + /* Get each entry from a list * pos - A structure pointer has a "member" element * head - list head @@ -258,7 +261,6 @@ struct libusb_context { /* A list of registered hotplug callbacks */ struct list_head hotplug_cbs; usbi_mutex_t hotplug_cbs_lock; - int hotplug_pipe[2]; /* this is a list of in-flight transfer handles, sorted by timeout * expiration. URBs to timeout the soonest are placed at the beginning of @@ -298,6 +300,9 @@ struct libusb_context { * pick up a new fd for polling. Protected by event_data_lock. */ unsigned int fd_notify; + /* A list of pending hotplug messages. Protected by event_data_lock. */ + struct list_head hotplug_msgs; + /* used to wait for event completion in threads other than the one that is * event handling */ usbi_mutex_t event_waiters_lock; diff --git a/libusb/version_nano.h b/libusb/version_nano.h index 9624091..90d77c5 100644 --- a/libusb/version_nano.h +++ b/libusb/version_nano.h @@ -1 +1 @@ -#define LIBUSB_NANO 10937 +#define LIBUSB_NANO 10938 -- cgit v1.2.3