From d21b0d71990ac6ec4272c1f80f0ca544103628b3 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Mon, 17 Dec 2012 15:46:28 -0500 Subject: [PATCH] Fix: poll and epoll fd set reallocation Acked-by: Mathieu Desnoyers Signed-off-by: David Goulet --- src/bin/lttng-sessiond/main.c | 34 +++--- src/common/compat/compat-epoll.c | 76 +++++++++--- src/common/compat/compat-poll.c | 191 ++++++++++++++++++++++++------- src/common/compat/poll.h | 37 ++++-- src/common/consumer.c | 4 +- 5 files changed, 255 insertions(+), 87 deletions(-) diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c index 91fba2643..6f8f1486c 100644 --- a/src/bin/lttng-sessiond/main.c +++ b/src/bin/lttng-sessiond/main.c @@ -697,30 +697,28 @@ static void *thread_manage_kernel(void *data) health_code_update(&health_thread_kernel); - ret = create_thread_poll_set(&events, 2); - if (ret < 0) { - goto error_poll_create; - } - - ret = lttng_poll_add(&events, kernel_poll_pipe[0], LPOLLIN); - if (ret < 0) { - goto error; - } - if (testpoint(thread_manage_kernel_before_loop)) { - goto error; + goto error_testpoint; } while (1) { health_code_update(&health_thread_kernel); if (update_poll_flag == 1) { - /* - * Reset number of fd in the poll set. Always 2 since there is the thread - * quit pipe and the kernel pipe. - */ - events.nb_fd = 2; + /* Clean events object. We are about to populate it again. */ + lttng_poll_clean(&events); + + ret = create_thread_poll_set(&events, 2); + if (ret < 0) { + goto error_poll_create; + } + + ret = lttng_poll_add(&events, kernel_poll_pipe[0], LPOLLIN); + if (ret < 0) { + goto error; + } + /* This will add the available kernel channel if any. */ ret = update_kernel_poll(&events); if (ret < 0) { goto error; @@ -728,7 +726,7 @@ static void *thread_manage_kernel(void *data) update_poll_flag = 0; } - DBG("Thread kernel polling on %d fds", events.nb_fd); + DBG("Thread kernel polling on %d fds", LTTNG_POLL_GETNB(&events)); /* Poll infinite value of time */ restart: @@ -1122,7 +1120,7 @@ static void *thread_manage_apps(void *data) health_code_update(&health_thread_app_manage); while (1) { - DBG("Apps thread polling on %d fds", events.nb_fd); + DBG("Apps thread polling on %d fds", LTTNG_POLL_GETNB(&events)); /* Inifinite blocking call, waiting for transmission */ restart: diff --git a/src/common/compat/compat-epoll.c b/src/common/compat/compat-epoll.c index e1672c4c3..40451b3d2 100644 --- a/src/common/compat/compat-epoll.c +++ b/src/common/compat/compat-epoll.c @@ -16,6 +16,7 @@ */ #define _GNU_SOURCE +#include #include #include #include @@ -31,6 +32,32 @@ unsigned int poll_max_size; +/* + * Resize the epoll events structure of the new size. + * + * Return 0 on success or else -1 with the current events pointer untouched. + */ +static int resize_poll_event(struct lttng_poll_event *events, + uint32_t new_size) +{ + struct epoll_event *ptr; + + assert(events); + + ptr = realloc(events->events, new_size * sizeof(*ptr)); + if (ptr == NULL) { + PERROR("realloc epoll add"); + goto error; + } + events->events = ptr; + events->alloc_size = new_size; + + return 0; + +error: + return -1; +} + /* * Create epoll set and allocate returned events structure. */ @@ -63,7 +90,7 @@ int compat_epoll_create(struct lttng_poll_event *events, int size, int flags) goto error_close; } - events->events_size = size; + events->alloc_size = events->init_size = size; events->nb_fd = 0; return 0; @@ -82,8 +109,8 @@ error: */ int compat_epoll_add(struct lttng_poll_event *events, int fd, uint32_t req_events) { - int ret, new_size; - struct epoll_event ev, *ptr; + int ret; + struct epoll_event ev; if (events == NULL || events->events == NULL || fd < 0) { ERR("Bad compat epoll add arguments"); @@ -112,17 +139,6 @@ int compat_epoll_add(struct lttng_poll_event *events, int fd, uint32_t req_event events->nb_fd++; - if (events->nb_fd >= events->events_size) { - new_size = 2 * events->events_size; - ptr = realloc(events->events, new_size * sizeof(struct epoll_event)); - if (ptr == NULL) { - PERROR("realloc epoll add"); - goto error; - } - events->events = ptr; - events->events_size = new_size; - } - end: return 0; @@ -172,13 +188,39 @@ error: int compat_epoll_wait(struct lttng_poll_event *events, int timeout) { int ret; + uint32_t new_size; - if (events == NULL || events->events == NULL || - events->events_size < events->nb_fd) { + if (events == NULL || events->events == NULL) { ERR("Wrong arguments in compat_epoll_wait"); goto error; } + /* + * Resize if needed before waiting. We could either expand the array or + * shrink it down. It's important to note that after this step, we are + * ensured that the events argument of the epoll_wait call will be large + * enough to hold every possible returned events. + */ + if (events->nb_fd > events->alloc_size) { + /* Expand if the nb_fd is higher than the actual size. */ + new_size = events->alloc_size << 1UL; + } else if ((events->nb_fd << 1UL) <= events->alloc_size && + events->nb_fd >= events->init_size) { + /* Shrink if nb_fd multiplied by two is <= than the actual size. */ + new_size = events->alloc_size >> 1UL; + } else { + /* Indicate that we don't want to resize. */ + new_size = 0; + } + + if (new_size) { + ret = resize_poll_event(events, new_size); + if (ret < 0) { + /* ENOMEM problem at this point. */ + goto error; + } + } + do { ret = epoll_wait(events->epfd, events->events, events->nb_fd, timeout); } while (ret == -1 && errno == EINTR); @@ -220,7 +262,7 @@ void compat_epoll_set_max_size(void) } poll_max_size = atoi(buf); - if (poll_max_size <= 0) { + if (poll_max_size == 0) { /* Extra precaution */ poll_max_size = DEFAULT_POLL_SIZE; } diff --git a/src/common/compat/compat-poll.c b/src/common/compat/compat-poll.c index bacd96e84..cff9f44b8 100644 --- a/src/common/compat/compat-poll.c +++ b/src/common/compat/compat-poll.c @@ -16,6 +16,7 @@ */ #define _GNU_SOURCE +#include #include #include #include @@ -27,11 +28,72 @@ unsigned int poll_max_size; +/* + * Resize the epoll events structure of the new size. + * + * Return 0 on success or else -1 with the current events pointer untouched. + */ +static int resize_poll_event(struct compat_poll_event_array *array, + uint32_t new_size) +{ + struct pollfd *ptr; + + assert(array); + + ptr = realloc(array->events, new_size * sizeof(*ptr)); + if (ptr == NULL) { + PERROR("realloc epoll add"); + goto error; + } + array->events = ptr; + array->alloc_size = new_size; + + return 0; + +error: + return -1; +} + +/* + * Update events with the current events object. + */ +static int update_current_events(struct lttng_poll_event *events) +{ + int ret; + struct compat_poll_event_array *current, *wait; + + assert(events); + + current = &events->current; + wait = &events->wait; + + wait->nb_fd = current->nb_fd; + if (events->need_realloc) { + ret = resize_poll_event(wait, current->alloc_size); + if (ret < 0) { + goto error; + } + } + memcpy(wait->events, current->events, + current->nb_fd * sizeof(*current->events)); + + /* Update is done and realloc as well. */ + events->need_update = 0; + events->need_realloc = 0; + + return 0; + +error: + return -1; +} + /* * Create pollfd data structure. */ int compat_poll_create(struct lttng_poll_event *events, int size) { + struct compat_poll_event_array *current, *wait; + if (events == NULL || size <= 0) { ERR("Wrong arguments for poll create"); goto error; @@ -42,15 +104,29 @@ int compat_poll_create(struct lttng_poll_event *events, int size) size = poll_max_size; } + /* Reset everything before begining the allocation. */ + memset(events, 0, sizeof(struct lttng_poll_event)); + + /* Ease our life a bit. */ + current = &events->current; + wait = &events->wait; + /* This *must* be freed by using lttng_poll_free() */ - events->events = zmalloc(size * sizeof(struct pollfd)); - if (events->events == NULL) { + wait->events = zmalloc(size * sizeof(struct pollfd)); + if (wait->events == NULL) { perror("zmalloc struct pollfd"); goto error; } - events->events_size = size; - events->nb_fd = 0; + wait->alloc_size = wait->init_size = size; + + current->events = zmalloc(size * sizeof(struct pollfd)); + if (current->events == NULL) { + perror("zmalloc struct current pollfd"); + goto error; + } + + current->alloc_size = current->init_size = size; return 0; @@ -64,31 +140,43 @@ error: int compat_poll_add(struct lttng_poll_event *events, int fd, uint32_t req_events) { - int new_size; - struct pollfd *ptr; + int new_size, ret, i; + struct compat_poll_event_array *current; - if (events == NULL || events->events == NULL || fd < 0) { + if (events == NULL || events->current.events == NULL || fd < 0) { ERR("Bad compat poll add arguments"); goto error; } - /* Reallocate pollfd structure by a factor of 2 if needed. */ - if (events->nb_fd >= events->events_size) { - new_size = 2 * events->events_size; - ptr = realloc(events->events, new_size * sizeof(struct pollfd)); - if (ptr == NULL) { - perror("realloc poll add"); + /* Ease our life a bit. */ + current = &events->current; + + /* Check if fd we are trying to add is already there. */ + for (i = 0; i < current->nb_fd; i++) { + /* Don't put back the fd we want to delete */ + if (current->events[i].fd == fd) { + errno = EEXIST; goto error; } - events->events = ptr; - events->events_size = new_size; } - events->events[events->nb_fd].fd = fd; - events->events[events->nb_fd].events = req_events; - events->nb_fd++; + /* Check for a needed resize of the array. */ + if (current->nb_fd > current->alloc_size) { + /* Expand it by a power of two of the current size. */ + new_size = current->alloc_size << 1UL; + ret = resize_poll_event(current, new_size); + if (ret < 0) { + goto error; + } + events->need_realloc = 1; + } - DBG("fd %d of %d added to pollfd", fd, events->nb_fd); + current->events[current->nb_fd].fd = fd; + current->events[current->nb_fd].events = req_events; + current->nb_fd++; + events->need_update = 1; + + DBG("fd %d of %d added to pollfd", fd, current->nb_fd); return 0; @@ -101,42 +189,48 @@ error: */ int compat_poll_del(struct lttng_poll_event *events, int fd) { - int new_size, i, count = 0; - struct pollfd *old = NULL, *new = NULL; + int new_size, i, count = 0, ret; + struct compat_poll_event_array *current; - if (events == NULL || events->events == NULL || fd < 0) { + if (events == NULL || events->current.events == NULL || fd < 0) { ERR("Wrong arguments for poll del"); goto error; } - old = events->events; - new_size = events->events_size - 1; + /* Ease our life a bit. */ + current = &events->current; /* Safety check on size */ if (new_size > poll_max_size) { new_size = poll_max_size; } - new = zmalloc(new_size * sizeof(struct pollfd)); - if (new == NULL) { - perror("zmalloc poll del"); - goto error; + /* Check if we need to shrink it down. */ + if ((current->nb_fd << 1UL) <= current->alloc_size && + current->nb_fd >= current->init_size) { + /* + * Shrink if nb_fd multiplied by two is <= than the actual size and we + * are above the initial size. + */ + new_size = current->alloc_size >> 1UL; + ret = resize_poll_event(current, new_size); + if (ret < 0) { + goto error; + } + events->need_realloc = 1; } - for (i = 0; i < events->events_size; i++) { + for (i = 0; i < current->nb_fd; i++) { /* Don't put back the fd we want to delete */ - if (old[i].fd != fd) { - new[count].fd = old[i].fd; - new[count].events = old[i].events; + if (current->events[i].fd != fd) { + current->events[count].fd = current->events[i].fd; + current->events[count].events = current->events[i].events; count++; } } - events->events_size = new_size; - events->events = new; - events->nb_fd--; - - free(old); + current->nb_fd--; + events->need_update = 1; return 0; @@ -151,13 +245,26 @@ int compat_poll_wait(struct lttng_poll_event *events, int timeout) { int ret; - if (events == NULL || events->events == NULL || - events->events_size < events->nb_fd) { + if (events == NULL || events->current.events == NULL) { ERR("poll wait arguments error"); goto error; } - ret = poll(events->events, events->nb_fd, timeout); + if (events->current.nb_fd == 0) { + /* Return an invalid error to be consistent with epoll. */ + errno = EINVAL; + goto error; + } + + if (events->need_update) { + ret = update_current_events(events); + if (ret < 0) { + errno = ENOMEM; + goto error; + } + } + + ret = poll(events->wait.events, events->wait.nb_fd, timeout); if (ret < 0) { /* At this point, every error is fatal */ perror("poll wait"); @@ -168,7 +275,7 @@ int compat_poll_wait(struct lttng_poll_event *events, int timeout) * poll() should always iterate on all FDs since we handle the pollset in * user space and after poll returns, we have to try every fd for a match. */ - return events->nb_fd; + return events->wait.nb_fd; error: return -1; @@ -192,7 +299,7 @@ void compat_poll_set_max_size(void) } poll_max_size = lim.rlim_cur; - if (poll_max_size <= 0) { + if (poll_max_size == 0) { /* Extra precaution */ poll_max_size = DEFAULT_POLL_SIZE; } diff --git a/src/common/compat/poll.h b/src/common/compat/poll.h index cfde4fc83..2cfad9a25 100644 --- a/src/common/compat/poll.h +++ b/src/common/compat/poll.h @@ -78,7 +78,8 @@ enum { struct compat_epoll_event { int epfd; uint32_t nb_fd; /* Current number of fd in events */ - uint32_t events_size; /* Size of events array */ + uint32_t alloc_size; /* Size of events array */ + uint32_t init_size; /* Initial size of events array */ struct epoll_event *events; }; #define lttng_poll_event compat_epoll_event @@ -202,21 +203,40 @@ enum { LTTNG_CLOEXEC = 0xdead, }; -struct compat_poll_event { +struct compat_poll_event_array { uint32_t nb_fd; /* Current number of fd in events */ - uint32_t events_size; /* Size of events array */ + uint32_t alloc_size; /* Size of events array */ + /* Initial size of the pollset. We never shrink below that. */ + uint32_t init_size; struct pollfd *events; }; + +struct compat_poll_event { + /* + * Modified by the wait action. Updated using current fields if the + * need_update flag is set. + */ + struct compat_poll_event_array wait; + /* + * This is modified by add/del actions being the _current_ flow of + * execution before a poll wait is done. + */ + struct compat_poll_event_array current; + /* Indicate if wait.events needs to be realloc. */ + int need_realloc:1; + /* Indicate if wait.events need to be updated from current. */ + int need_update:1; +}; #define lttng_poll_event compat_poll_event /* * For the following calls, consider 'e' to be a lttng_poll_event pointer and i * being the index of the events array. */ -#define LTTNG_POLL_GETFD(e, i) LTTNG_REF(e)->events[i].fd -#define LTTNG_POLL_GETEV(e, i) LTTNG_REF(e)->events[i].revents -#define LTTNG_POLL_GETNB(e) LTTNG_REF(e)->nb_fd -#define LTTNG_POLL_GETSZ(e) LTTNG_REF(e)->events_size +#define LTTNG_POLL_GETFD(e, i) LTTNG_REF(e)->wait.events[i].fd +#define LTTNG_POLL_GETEV(e, i) LTTNG_REF(e)->wait.events[i].revents +#define LTTNG_POLL_GETNB(e) LTTNG_REF(e)->wait.nb_fd +#define LTTNG_POLL_GETSZ(e) LTTNG_REF(e)->wait.events_size /* * Create a pollfd structure of size 'size'. @@ -270,7 +290,8 @@ static inline void lttng_poll_reset(struct lttng_poll_event *events) static inline void lttng_poll_clean(struct lttng_poll_event *events) { if (events) { - __lttng_poll_free((void *) events->events); + __lttng_poll_free((void *) events->wait.events); + __lttng_poll_free((void *) events->current.events); } } diff --git a/src/common/consumer.c b/src/common/consumer.c index 1de9cbadb..af99333c8 100644 --- a/src/common/consumer.c +++ b/src/common/consumer.c @@ -2102,12 +2102,12 @@ void *consumer_thread_metadata_poll(void *data) while (1) { /* Only the metadata pipe is set */ - if (events.nb_fd == 0 && consumer_quit == 1) { + if (LTTNG_POLL_GETNB(&events) == 0 && consumer_quit == 1) { goto end; } restart: - DBG("Metadata poll wait with %d fd(s)", events.nb_fd); + DBG("Metadata poll wait with %d fd(s)", LTTNG_POLL_GETNB(&events)); ret = lttng_poll_wait(&events, -1); DBG("Metadata event catched in thread"); if (ret < 0) { -- 2.34.1