Fix: poll and epoll fd set reallocation
authorDavid Goulet <dgoulet@efficios.com>
Mon, 17 Dec 2012 20:46:28 +0000 (15:46 -0500)
committerDavid Goulet <dgoulet@efficios.com>
Tue, 18 Dec 2012 17:45:40 +0000 (12:45 -0500)
Acked-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: David Goulet <dgoulet@efficios.com>
src/bin/lttng-sessiond/main.c
src/common/compat/compat-epoll.c
src/common/compat/compat-poll.c
src/common/compat/poll.h
src/common/consumer.c

index 91fba2643fc2ae94acf79850fd0e9aa589cfbf90..6f8f1486ca60de5d958352868b9912ddcbdcf24f 100644 (file)
@@ -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:
index e1672c4c3178afbbce32acddebf2b30cea66fe09..40451b3d21286106430c1322209b21d9fef9579f 100644 (file)
@@ -16,6 +16,7 @@
  */
 
 #define _GNU_SOURCE
+#include <assert.h>
 #include <fcntl.h>
 #include <limits.h>
 #include <stdlib.h>
 
 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;
        }
index bacd96e842be04c0719f740205e42852972b5f8e..cff9f44b8e17c369a4b015866a825044bb43ef48 100644 (file)
@@ -16,6 +16,7 @@
  */
 
 #define _GNU_SOURCE
+#include <assert.h>
 #include <stdlib.h>
 #include <sys/resource.h>
 #include <sys/time.h>
 
 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;
        }
index cfde4fc8381d36786f23683de1ffa7cc3f9e40f8..2cfad9a25fa1c7e0339ff8e810796c58f1b5b552 100644 (file)
@@ -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);
        }
 }
 
index 1de9cbadbd053749baa81d1a800975dd2bb22a8b..af99333c8d2ae1bacc73f7f4af5fcd231e79e104 100644 (file)
@@ -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) {
This page took 0.036276 seconds and 5 git commands to generate.