From 1524f98c04431d04e50796f83a9dd29184b3a8a4 Mon Sep 17 00:00:00 2001 From: Michael Jeanson Date: Wed, 11 Nov 2020 10:38:51 -0500 Subject: [PATCH] Cleanup: remove ignored flags from poll events bitmasks MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The POLLHUP and POLLERR flags are only valid in 'revents', they are implicitly enabled regardless of the fact they were set in 'events' or not. As such remove those flags from all poll events to reduce possible confusion as to which flags can be returned by poll. Change-Id: Id22c78c38257d96dfc47e1337795f13c70dd5f91 Signed-off-by: Michael Jeanson Signed-off-by: Jérémie Galarneau --- src/bin/lttng-relayd/thread-utils.cpp | 2 +- src/bin/lttng-sessiond/agent-thread.cpp | 9 +++------ src/bin/lttng-sessiond/client.cpp | 2 +- src/bin/lttng-sessiond/dispatch.cpp | 3 +-- src/bin/lttng-sessiond/health.cpp | 2 +- src/bin/lttng-sessiond/manage-apps.cpp | 5 ++--- src/bin/lttng-sessiond/manage-consumer.cpp | 2 +- .../lttng-sessiond/notification-thread-events.cpp | 4 ++-- src/bin/lttng-sessiond/notification-thread.cpp | 15 +++++---------- src/bin/lttng-sessiond/notify-apps.cpp | 8 +++----- src/bin/lttng-sessiond/rotation-thread.cpp | 7 +++---- src/bin/lttng-sessiond/thread-utils.cpp | 4 ++-- src/common/consumer/consumer.cpp | 9 ++++----- src/lib/lttng-ctl/channel.cpp | 4 ++-- src/lib/lttng-ctl/clear.cpp | 2 +- src/lib/lttng-ctl/destruction-handle.cpp | 2 +- tests/unit/test_utils_compat_poll.cpp | 4 ++-- 17 files changed, 35 insertions(+), 49 deletions(-) diff --git a/src/bin/lttng-relayd/thread-utils.cpp b/src/bin/lttng-relayd/thread-utils.cpp index d469f90e5..25ea256d5 100644 --- a/src/bin/lttng-relayd/thread-utils.cpp +++ b/src/bin/lttng-relayd/thread-utils.cpp @@ -92,7 +92,7 @@ int create_named_thread_poll_set(struct lttng_poll_event *events, } /* Add thread quit pipe to monitored events. */ - const auto poll_add_ret = lttng_poll_add(events, thread_quit_pipe[0], LPOLLIN | LPOLLERR); + const auto poll_add_ret = lttng_poll_add(events, thread_quit_pipe[0], LPOLLIN); if (poll_add_ret < 0) { return -1; } diff --git a/src/bin/lttng-sessiond/agent-thread.cpp b/src/bin/lttng-sessiond/agent-thread.cpp index dc8cb2961..e54a7cecf 100644 --- a/src/bin/lttng-sessiond/agent-thread.cpp +++ b/src/bin/lttng-sessiond/agent-thread.cpp @@ -377,8 +377,7 @@ static void *thread_agent_management(void *data) goto error_poll_create; } - ret = lttng_poll_add(&events, thread_quit_pipe_fd, - LPOLLIN | LPOLLERR); + ret = lttng_poll_add(&events, thread_quit_pipe_fd, LPOLLIN); if (ret < 0) { goto error_tcp_socket; } @@ -411,8 +410,7 @@ static void *thread_agent_management(void *data) mark_thread_as_ready(notifiers); /* Add TCP socket to the poll set. */ - ret = lttng_poll_add(&events, reg_sock->fd, - LPOLLIN | LPOLLERR | LPOLLHUP | LPOLLRDHUP); + ret = lttng_poll_add(&events, reg_sock->fd, LPOLLIN | LPOLLRDHUP); if (ret < 0) { goto error; } @@ -484,8 +482,7 @@ restart: * read), only add poll error event to only * detect shutdown. */ - ret = lttng_poll_add(&events, new_app_socket_fd, - LPOLLERR | LPOLLHUP | LPOLLRDHUP); + ret = lttng_poll_add(&events, new_app_socket_fd, LPOLLRDHUP); if (ret < 0) { agent_destroy_app(new_app); continue; diff --git a/src/bin/lttng-sessiond/client.cpp b/src/bin/lttng-sessiond/client.cpp index 4c4cdf540..f33648fcc 100644 --- a/src/bin/lttng-sessiond/client.cpp +++ b/src/bin/lttng-sessiond/client.cpp @@ -2501,7 +2501,7 @@ static void *thread_manage_clients(void *data) } /* Add thread quit pipe */ - ret = lttng_poll_add(&events, thread_quit_pipe_fd, LPOLLIN | LPOLLERR); + ret = lttng_poll_add(&events, thread_quit_pipe_fd, LPOLLIN); if (ret < 0) { goto error; } diff --git a/src/bin/lttng-sessiond/dispatch.cpp b/src/bin/lttng-sessiond/dispatch.cpp index 82bf1c2d3..a8e8d6c8d 100644 --- a/src/bin/lttng-sessiond/dispatch.cpp +++ b/src/bin/lttng-sessiond/dispatch.cpp @@ -113,8 +113,7 @@ static void sanitize_wait_queue(struct ust_reg_wait_queue *wait_queue) cds_list_for_each_entry_safe(wait_node, tmp_wait_node, &wait_queue->head, head) { LTTNG_ASSERT(wait_node->app); - ret = lttng_poll_add(&events, wait_node->app->sock, - LPOLLHUP | LPOLLERR); + ret = lttng_poll_add(&events, wait_node->app->sock, LPOLLIN); if (ret < 0) { goto error; } diff --git a/src/bin/lttng-sessiond/health.cpp b/src/bin/lttng-sessiond/health.cpp index 1ba991447..6457cb31b 100644 --- a/src/bin/lttng-sessiond/health.cpp +++ b/src/bin/lttng-sessiond/health.cpp @@ -122,7 +122,7 @@ static void *thread_manage_health(void *data) goto error; } - ret = lttng_poll_add(&events, thread_quit_pipe_fd, LPOLLIN | LPOLLERR); + ret = lttng_poll_add(&events, thread_quit_pipe_fd, LPOLLIN); if (ret < 0) { goto error; } diff --git a/src/bin/lttng-sessiond/manage-apps.cpp b/src/bin/lttng-sessiond/manage-apps.cpp index 7698ea442..ea46f0527 100644 --- a/src/bin/lttng-sessiond/manage-apps.cpp +++ b/src/bin/lttng-sessiond/manage-apps.cpp @@ -75,7 +75,7 @@ static void *thread_application_management(void *data) goto error; } - ret = lttng_poll_add(&events, thread_quit_pipe_fd, LPOLLIN | LPOLLERR); + ret = lttng_poll_add(&events, thread_quit_pipe_fd, LPOLLIN); if (ret < 0) { goto error; } @@ -142,8 +142,7 @@ static void *thread_application_management(void *data) * Since this is a command socket (write then read), * we only monitor the error events of the socket. */ - ret = lttng_poll_add(&events, sock, - LPOLLERR | LPOLLHUP | LPOLLRDHUP); + ret = lttng_poll_add(&events, sock, LPOLLRDHUP); if (ret < 0) { goto error; } diff --git a/src/bin/lttng-sessiond/manage-consumer.cpp b/src/bin/lttng-sessiond/manage-consumer.cpp index d9d1e670e..ae1529e9b 100644 --- a/src/bin/lttng-sessiond/manage-consumer.cpp +++ b/src/bin/lttng-sessiond/manage-consumer.cpp @@ -83,7 +83,7 @@ static void *thread_consumer_management(void *data) goto error_poll; } - ret = lttng_poll_add(&events, thread_quit_pipe_fd, LPOLLIN | LPOLLERR); + ret = lttng_poll_add(&events, thread_quit_pipe_fd, LPOLLIN); if (ret < 0) { mark_thread_intialization_as_failed(notifiers); goto error; diff --git a/src/bin/lttng-sessiond/notification-thread-events.cpp b/src/bin/lttng-sessiond/notification-thread-events.cpp index 6fb691be5..64c3fc4f3 100644 --- a/src/bin/lttng-sessiond/notification-thread-events.cpp +++ b/src/bin/lttng-sessiond/notification-thread-events.cpp @@ -48,7 +48,7 @@ #include "lttng-sessiond.hpp" #include "kernel.hpp" -#define CLIENT_POLL_EVENTS_IN (LPOLLIN | LPOLLERR | LPOLLHUP | LPOLLRDHUP) +#define CLIENT_POLL_EVENTS_IN (LPOLLIN | LPOLLRDHUP) #define CLIENT_POLL_EVENTS_IN_OUT (CLIENT_POLL_EVENTS_IN | LPOLLOUT) /* The tracers currently limit the capture size to PIPE_BUF (4kb on linux). */ @@ -2208,7 +2208,7 @@ int handle_notification_thread_command_add_tracer_event_source( lttng_domain_type_str(domain_type)); /* Adding the read side pipe to the event poll. */ - ret = lttng_poll_add(&state->events, tracer_event_source_fd, LPOLLPRI | LPOLLIN | LPOLLERR); + ret = lttng_poll_add(&state->events, tracer_event_source_fd, LPOLLPRI | LPOLLIN); if (ret < 0) { ERR("Failed to add tracer event source to poll set: tracer_event_source_fd = %d, domain = '%s'", tracer_event_source_fd, diff --git a/src/bin/lttng-sessiond/notification-thread.cpp b/src/bin/lttng-sessiond/notification-thread.cpp index 7d9f4ed12..320e639d0 100644 --- a/src/bin/lttng-sessiond/notification-thread.cpp +++ b/src/bin/lttng-sessiond/notification-thread.cpp @@ -292,28 +292,24 @@ int init_poll_set(struct lttng_poll_event *poll_set, goto end; } - ret = lttng_poll_add(poll_set, notification_channel_socket, - LPOLLIN | LPOLLERR | LPOLLHUP | LPOLLRDHUP); + ret = lttng_poll_add(poll_set, notification_channel_socket, LPOLLIN | LPOLLRDHUP); if (ret < 0) { ERR("Failed to add notification channel socket to pollset"); goto error; } - ret = lttng_poll_add(poll_set, handle->cmd_queue.event_fd, - LPOLLIN | LPOLLERR); + ret = lttng_poll_add(poll_set, handle->cmd_queue.event_fd, LPOLLIN); if (ret < 0) { ERR("Failed to add notification command queue event fd to pollset"); goto error; } ret = lttng_poll_add(poll_set, - handle->channel_monitoring_pipes.ust32_consumer, - LPOLLIN | LPOLLERR); + handle->channel_monitoring_pipes.ust32_consumer, LPOLLIN); if (ret < 0) { ERR("Failed to add ust-32 channel monitoring pipe fd to pollset"); goto error; } ret = lttng_poll_add(poll_set, - handle->channel_monitoring_pipes.ust64_consumer, - LPOLLIN | LPOLLERR); + handle->channel_monitoring_pipes.ust64_consumer, LPOLLIN); if (ret < 0) { ERR("Failed to add ust-64 channel monitoring pipe fd to pollset"); goto error; @@ -322,8 +318,7 @@ int init_poll_set(struct lttng_poll_event *poll_set, goto end; } ret = lttng_poll_add(poll_set, - handle->channel_monitoring_pipes.kernel_consumer, - LPOLLIN | LPOLLERR); + handle->channel_monitoring_pipes.kernel_consumer, LPOLLIN); if (ret < 0) { ERR("Failed to add kernel channel monitoring pipe fd to pollset"); goto error; diff --git a/src/bin/lttng-sessiond/notify-apps.cpp b/src/bin/lttng-sessiond/notify-apps.cpp index 1b83491d1..ff70ac187 100644 --- a/src/bin/lttng-sessiond/notify-apps.cpp +++ b/src/bin/lttng-sessiond/notify-apps.cpp @@ -58,13 +58,12 @@ static void *thread_application_notification(void *data) /* Add notify pipe to the pollset. */ ret = lttng_poll_add(&events, notifiers->apps_cmd_notify_pipe_read_fd, - LPOLLIN | LPOLLERR | LPOLLHUP | LPOLLRDHUP); + LPOLLIN | LPOLLRDHUP); if (ret < 0) { goto error; } - ret = lttng_poll_add(&events, thread_quit_pipe_fd, - LPOLLIN | LPOLLERR); + ret = lttng_poll_add(&events, thread_quit_pipe_fd, LPOLLIN); if (ret < 0) { goto error; } @@ -121,8 +120,7 @@ restart: } health_code_update(); - ret = lttng_poll_add(&events, sock, - LPOLLIN | LPOLLERR | LPOLLHUP | LPOLLRDHUP); + ret = lttng_poll_add(&events, sock, LPOLLIN | LPOLLRDHUP); if (ret < 0) { /* * It's possible we've reached the max poll fd allowed. diff --git a/src/bin/lttng-sessiond/rotation-thread.cpp b/src/bin/lttng-sessiond/rotation-thread.cpp index 22891dd13..aa6ba0780 100644 --- a/src/bin/lttng-sessiond/rotation-thread.cpp +++ b/src/bin/lttng-sessiond/rotation-thread.cpp @@ -257,8 +257,7 @@ int init_poll_set(struct lttng_poll_event *poll_set, } ret = lttng_poll_add(poll_set, - lttng_pipe_get_readfd(handle->quit_pipe), - LPOLLIN | LPOLLERR); + lttng_pipe_get_readfd(handle->quit_pipe), LPOLLIN); if (ret < 0) { ERR("Failed to add quit pipe read fd to poll set"); goto error; @@ -266,7 +265,7 @@ int init_poll_set(struct lttng_poll_event *poll_set, ret = lttng_poll_add(poll_set, lttng_pipe_get_readfd(handle->rotation_timer_queue->event_pipe), - LPOLLIN | LPOLLERR); + LPOLLIN); if (ret < 0) { ERR("Failed to add rotate_pending fd to poll set"); goto error; @@ -310,7 +309,7 @@ int init_thread_state(struct rotation_thread_handle *handle, goto end; } ret = lttng_poll_add(&state->events, rotate_notification_channel->socket, - LPOLLIN | LPOLLERR); + LPOLLIN); if (ret < 0) { ERR("Failed to add notification fd to pollset"); goto end; diff --git a/src/bin/lttng-sessiond/thread-utils.cpp b/src/bin/lttng-sessiond/thread-utils.cpp index 1b8ff0881..3465fc227 100644 --- a/src/bin/lttng-sessiond/thread-utils.cpp +++ b/src/bin/lttng-sessiond/thread-utils.cpp @@ -65,7 +65,7 @@ int sessiond_wait_for_main_quit_pipe(int timeout_ms) ret = -1; goto end; } - ret = lttng_poll_add(&events, main_quit_pipe[0], LPOLLIN | LPOLLERR); + ret = lttng_poll_add(&events, main_quit_pipe[0], LPOLLIN); if (ret < 0) { PERROR("Failed to add file descriptor to poll/epoll set"); ret = -1; @@ -114,7 +114,7 @@ int sessiond_set_thread_pollset(struct lttng_poll_event *events, size_t size) } /* Add main quit pipe */ - ret = lttng_poll_add(events, main_quit_pipe[0], LPOLLIN | LPOLLERR); + ret = lttng_poll_add(events, main_quit_pipe[0], LPOLLIN); if (ret < 0) { goto error; } diff --git a/src/common/consumer/consumer.cpp b/src/common/consumer/consumer.cpp index c1abb58df..f97c11970 100644 --- a/src/common/consumer/consumer.cpp +++ b/src/common/consumer/consumer.cpp @@ -2428,9 +2428,8 @@ restart: stream->wait_fd); /* Add metadata stream to the global poll events list */ - lttng_poll_add(&events, stream->wait_fd, - LPOLLIN | LPOLLPRI | LPOLLHUP); - } else if (revents & (LPOLLERR | LPOLLHUP)) { + lttng_poll_add(&events, stream->wait_fd, LPOLLIN | LPOLLPRI); + }else if (revents & (LPOLLERR | LPOLLHUP)) { DBG("Metadata thread pipe hung up"); /* * Remove the pipe from the poll set and continue the loop @@ -3020,8 +3019,8 @@ restart: &chan->wait_fd_node); rcu_read_unlock(); /* Add channel to the global poll events list */ - lttng_poll_add(&events, chan->wait_fd, - LPOLLERR | LPOLLHUP); + // FIXME: Empty flag on a pipe pollset, this might hang on FreeBSD. + lttng_poll_add(&events, chan->wait_fd, 0); break; case CONSUMER_CHANNEL_DEL: { diff --git a/src/lib/lttng-ctl/channel.cpp b/src/lib/lttng-ctl/channel.cpp index 3613f1b1c..aee38f58b 100644 --- a/src/lib/lttng-ctl/channel.cpp +++ b/src/lib/lttng-ctl/channel.cpp @@ -268,7 +268,7 @@ lttng_notification_channel_get_next_notification( status = LTTNG_NOTIFICATION_CHANNEL_STATUS_ERROR; goto end_unlock; } - ret = lttng_poll_add(&events, channel->socket, LPOLLIN | LPOLLERR); + ret = lttng_poll_add(&events, channel->socket, LPOLLIN); if (ret < 0) { status = LTTNG_NOTIFICATION_CHANNEL_STATUS_ERROR; goto end_clean_poll; @@ -448,7 +448,7 @@ lttng_notification_channel_has_pending_notification( status = LTTNG_NOTIFICATION_CHANNEL_STATUS_ERROR; goto end_unlock; } - ret = lttng_poll_add(&events, channel->socket, LPOLLIN | LPOLLERR); + ret = lttng_poll_add(&events, channel->socket, LPOLLIN); if (ret < 0) { status = LTTNG_NOTIFICATION_CHANNEL_STATUS_ERROR; goto end_clean_poll; diff --git a/src/lib/lttng-ctl/clear.cpp b/src/lib/lttng-ctl/clear.cpp index b205ac8f2..870935bd3 100644 --- a/src/lib/lttng-ctl/clear.cpp +++ b/src/lib/lttng-ctl/clear.cpp @@ -77,7 +77,7 @@ struct lttng_clear_handle *lttng_clear_handle_create(int sessiond_socket) } ret = lttng_poll_add(&handle->communication.events, sessiond_socket, - LPOLLIN | LPOLLHUP | LPOLLRDHUP | LPOLLERR); + LPOLLIN | LPOLLRDHUP); if (ret) { goto error; } diff --git a/src/lib/lttng-ctl/destruction-handle.cpp b/src/lib/lttng-ctl/destruction-handle.cpp index 04cbd92a5..688e4faba 100644 --- a/src/lib/lttng-ctl/destruction-handle.cpp +++ b/src/lib/lttng-ctl/destruction-handle.cpp @@ -82,7 +82,7 @@ struct lttng_destruction_handle *lttng_destruction_handle_create( } ret = lttng_poll_add(&handle->communication.events, sessiond_socket, - LPOLLIN | LPOLLHUP | LPOLLRDHUP | LPOLLERR); + LPOLLIN | LPOLLRDHUP); if (ret) { goto error; } diff --git a/tests/unit/test_utils_compat_poll.cpp b/tests/unit/test_utils_compat_poll.cpp index f34ef5d0e..697440963 100644 --- a/tests/unit/test_utils_compat_poll.cpp +++ b/tests/unit/test_utils_compat_poll.cpp @@ -147,7 +147,7 @@ static void test_mod_wait(void) childok(lttng_poll_create(&cpoll_events, 1, 0) == 0, "Create valid poll set succeeds"); childok(lttng_poll_mod(NULL, infd[0], LPOLLIN) == -1, "lttng_poll_mod with invalid input returns an error"); childok(lttng_poll_mod(&cpoll_events, infd[0], LPOLLIN) == -1, "lttng_poll_mod with invalid input returns an error"); - childok(lttng_poll_add(&cpoll_events, infd[0], LPOLLHUP) == 0, "Add valid FD succeeds"); + childok(lttng_poll_add(&cpoll_events, infd[0], LPOLLIN) == 0, "Add valid FD succeeds"); childok(lttng_poll_mod(&cpoll_events, -1, LPOLLIN) == -1, "lttng_poll_mod with invalid input returns an error"); childok(lttng_poll_mod(&cpoll_events, hupfd[0], LPOLLIN) == 0, "lttng_poll_mod on unincluded FD goes on"); childok(lttng_poll_mod(&cpoll_events, infd[0], LPOLLIN) == 0, "Modify event type succeeds"); @@ -169,7 +169,7 @@ static void test_mod_wait(void) ok(lttng_poll_create(&poll_events, 1, 0) == 0, "Create valid poll set succeeds"); ok(lttng_poll_wait(&poll_events, -1) == -1, "lttng_poll_wait call with invalid input returns error"); - ok(lttng_poll_add(&poll_events, hupfd[0], LPOLLHUP) == 0, "Add valid FD succeeds"); + ok(lttng_poll_add(&poll_events, hupfd[0], LPOLLIN) == 0, "Add valid FD succeeds"); ok(lttng_write(infd[1], &tbuf, 1) == 1, "Write to pipe succeeds"); ok(lttng_poll_wait(&poll_events, -1) == 1, "Wakes up on one event"); ok(lttng_poll_del(&poll_events, hupfd[0]) == 0, "Removing valid FD succeeds"); -- 2.34.1