From 3c3390532736cfb5198f863d0d2b218e21fcf76d Mon Sep 17 00:00:00 2001 From: Francis Deslauriers Date: Tue, 18 Jan 2022 17:25:06 -0500 Subject: [PATCH] Remove ht-cleanup thread MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The hashtable cleanup thread was introduced to prevent deadlocks happening when the `cds_lfht_destroy()` function was called concurrently with userspace-rcu hashtable resizes. This was fixed in the userspace-rcu project in commit: commit d0ec0ed2fcb5d67a28587dcb778606e64f5b7b83 Author: Mathieu Desnoyers Date: Tue May 30 15:51:45 2017 -0400 Use workqueue in rculfhash That commit makes it so that the `cds_lfht_destroy()` function can safely be called within RCU read-side critical sections. This commit is included in the 0.10 release of urcu. The LTTng-Tools project now has a minimum version dependency on urcu 0.11. Because it's now safe to call `cds_lfht_destroy()` within RCU critical sections, the need for the hash table cleanup thread disappears. This commit replaces all uses of `ht_cleanup_push()` by `lttng_ht_destroy()` and remove all uses and mentions of the ht_cleanup thread. Signed-off-by: Francis Deslauriers Signed-off-by: Jérémie Galarneau Change-Id: I163a281b64a6b3eed62c58515932f71f3b52fea6 --- src/bin/lttng-sessiond/Makefile.am | 2 +- src/bin/lttng-sessiond/agent.cpp | 2 +- src/bin/lttng-sessiond/buffer-registry.cpp | 6 +- src/bin/lttng-sessiond/consumer.cpp | 2 +- src/bin/lttng-sessiond/health-sessiond.h | 1 - src/bin/lttng-sessiond/ht-cleanup.cpp | 267 ------------------- src/bin/lttng-sessiond/ht-cleanup.h | 16 -- src/bin/lttng-sessiond/lttng-sessiond.h | 9 - src/bin/lttng-sessiond/lttng-syscall.cpp | 2 +- src/bin/lttng-sessiond/main.cpp | 30 +-- src/bin/lttng-sessiond/session.cpp | 4 +- src/bin/lttng-sessiond/snapshot.cpp | 2 +- src/bin/lttng-sessiond/testpoint.h | 1 - src/bin/lttng-sessiond/trace-ust.cpp | 14 +- src/bin/lttng-sessiond/ust-app.cpp | 32 +-- src/bin/lttng-sessiond/ust-registry.cpp | 12 +- src/bin/lttng-sessiond/utils.cpp | 27 -- src/bin/lttng-sessiond/utils.h | 1 - src/lib/lttng-ctl/lttng-ctl-health.cpp | 2 - tests/regression/tools/health/health_fail.c | 12 - tests/regression/tools/health/health_stall.c | 12 - tests/regression/tools/health/test_health.sh | 2 - tests/unit/test_session.cpp | 5 - 23 files changed, 32 insertions(+), 431 deletions(-) delete mode 100644 src/bin/lttng-sessiond/ht-cleanup.cpp delete mode 100644 src/bin/lttng-sessiond/ht-cleanup.h diff --git a/src/bin/lttng-sessiond/Makefile.am b/src/bin/lttng-sessiond/Makefile.am index 1ab26e0a0..85f9cea2c 100644 --- a/src/bin/lttng-sessiond/Makefile.am +++ b/src/bin/lttng-sessiond/Makefile.am @@ -29,7 +29,7 @@ liblttng_sessiond_common_la_SOURCES = utils.cpp utils.h \ health-sessiond.h \ cmd.cpp cmd.h \ buffer-registry.cpp buffer-registry.h \ - testpoint.h ht-cleanup.cpp ht-cleanup.h \ + testpoint.h \ snapshot.cpp snapshot.h \ agent.cpp agent.h \ save.h save.cpp \ diff --git a/src/bin/lttng-sessiond/agent.cpp b/src/bin/lttng-sessiond/agent.cpp index 5be61dc36..620ec787f 100644 --- a/src/bin/lttng-sessiond/agent.cpp +++ b/src/bin/lttng-sessiond/agent.cpp @@ -1448,7 +1448,7 @@ void agent_destroy(struct agent *agt) call_rcu(&ctx->rcu_node, destroy_app_ctx_rcu); } rcu_read_unlock(); - ht_cleanup_push(agt->events); + lttng_ht_destroy(agt->events); free(agt); } diff --git a/src/bin/lttng-sessiond/buffer-registry.cpp b/src/bin/lttng-sessiond/buffer-registry.cpp index 392881827..3daca3c95 100644 --- a/src/bin/lttng-sessiond/buffer-registry.cpp +++ b/src/bin/lttng-sessiond/buffer-registry.cpp @@ -594,7 +594,7 @@ static void buffer_reg_session_destroy(struct buffer_reg_session *regp, } rcu_read_unlock(); - ht_cleanup_push(regp->channels); + lttng_ht_destroy(regp->channels); switch (domain) { case LTTNG_DOMAIN_UST: @@ -737,6 +737,6 @@ void buffer_reg_pid_destroy(struct buffer_reg_pid *regp) void buffer_reg_destroy_registries(void) { DBG3("Buffer registry destroy all registry"); - ht_cleanup_push(buffer_registry_uid); - ht_cleanup_push(buffer_registry_pid); + lttng_ht_destroy(buffer_registry_uid); + lttng_ht_destroy(buffer_registry_pid); } diff --git a/src/bin/lttng-sessiond/consumer.cpp b/src/bin/lttng-sessiond/consumer.cpp index ba4e2470f..95fc2c348 100644 --- a/src/bin/lttng-sessiond/consumer.cpp +++ b/src/bin/lttng-sessiond/consumer.cpp @@ -564,7 +564,7 @@ static void consumer_release_output(struct urcu_ref *ref) if (obj->socks) { /* Finally destroy HT */ - ht_cleanup_push(obj->socks); + lttng_ht_destroy(obj->socks); } free(obj); diff --git a/src/bin/lttng-sessiond/health-sessiond.h b/src/bin/lttng-sessiond/health-sessiond.h index af950b134..163676aae 100644 --- a/src/bin/lttng-sessiond/health-sessiond.h +++ b/src/bin/lttng-sessiond/health-sessiond.h @@ -17,7 +17,6 @@ enum health_type_sessiond { HEALTH_SESSIOND_TYPE_APP_REG = 2, HEALTH_SESSIOND_TYPE_KERNEL = 3, HEALTH_SESSIOND_TYPE_CONSUMER = 4, - HEALTH_SESSIOND_TYPE_HT_CLEANUP = 5, HEALTH_SESSIOND_TYPE_APP_MANAGE_NOTIFY = 6, HEALTH_SESSIOND_TYPE_APP_REG_DISPATCH = 7, HEALTH_SESSIOND_TYPE_NOTIFICATION = 8, diff --git a/src/bin/lttng-sessiond/ht-cleanup.cpp b/src/bin/lttng-sessiond/ht-cleanup.cpp deleted file mode 100644 index 050f18e2b..000000000 --- a/src/bin/lttng-sessiond/ht-cleanup.cpp +++ /dev/null @@ -1,267 +0,0 @@ -/* - * Copyright (C) 2013 Mathieu Desnoyers - * - * SPDX-License-Identifier: GPL-2.0-only - * - */ - -#define _LGPL_SOURCE - -#include -#include -#include -#include - -#include "lttng-sessiond.h" -#include "health-sessiond.h" -#include "testpoint.h" -#include "utils.h" -#include "ht-cleanup.h" - -static int ht_cleanup_quit_pipe[2] = { -1, -1 }; - -/* - * Check if the ht_cleanup thread quit pipe was triggered. - * - * Return true if it was triggered else false; - */ -static bool check_quit_pipe(int fd, uint32_t events) -{ - return (fd == ht_cleanup_quit_pipe[0] && (events & LPOLLIN)); -} - -static int init_pipe(int *pipe_fds) -{ - int ret, i; - - ret = pipe(pipe_fds); - if (ret < 0) { - PERROR("ht_cleanup thread quit pipe"); - goto error; - } - - for (i = 0; i < 2; i++) { - ret = fcntl(pipe_fds[i], F_SETFD, FD_CLOEXEC); - if (ret < 0) { - PERROR("fcntl ht_cleanup_quit_pipe"); - goto error; - } - } -error: - return ret; -} - -/* - * Create a poll set with O_CLOEXEC and add the thread quit pipe to the set. - */ -static int set_pollset(struct lttng_poll_event *events, size_t size) -{ - int ret; - - ret = lttng_poll_create(events, size, LTTNG_CLOEXEC); - if (ret < 0) { - goto error; - } - - ret = lttng_poll_add(events, ht_cleanup_quit_pipe[0], - LPOLLIN | LPOLLERR); - if (ret < 0) { - goto error; - } - - ret = lttng_poll_add(events, the_ht_cleanup_pipe[0], LPOLLIN | LPOLLERR); - if (ret < 0) { - DBG("lttng_poll_add error %d.", ret); - goto error; - } - - return 0; - -error: - return ret; -} - -static void cleanup_ht_cleanup_thread(void *data) -{ - utils_close_pipe(ht_cleanup_quit_pipe); - utils_close_pipe(the_ht_cleanup_pipe); -} - -static void *thread_ht_cleanup(void *data) -{ - int ret, i, pollfd, err = -1; - ssize_t size_ret; - uint32_t revents, nb_fd; - struct lttng_poll_event events; - - DBG("startup."); - - rcu_register_thread(); - rcu_thread_online(); - - health_register(the_health_sessiond, HEALTH_SESSIOND_TYPE_HT_CLEANUP); - - if (testpoint(sessiond_thread_ht_cleanup)) { - DBG("testpoint."); - goto error_testpoint; - } - - health_code_update(); - - ret = set_pollset(&events, 2); - if (ret < 0) { - DBG("sessiond_set_ht_cleanup_thread_pollset error %d.", ret); - goto error_poll_create; - } - - health_code_update(); - - while (1) { - restart: - DBG3("Polling."); - health_poll_entry(); - ret = lttng_poll_wait(&events, -1); - DBG3("Returning from poll on %d fds.", - LTTNG_POLL_GETNB(&events)); - health_poll_exit(); - if (ret < 0) { - /* - * Restart interrupted system call. - */ - if (errno == EINTR) { - continue; - } - goto error; - } - - nb_fd = ret; - for (i = 0; i < nb_fd; i++) { - struct lttng_ht *ht; - - health_code_update(); - - /* Fetch once the poll data */ - revents = LTTNG_POLL_GETEV(&events, i); - pollfd = LTTNG_POLL_GETFD(&events, i); - - if (pollfd != the_ht_cleanup_pipe[0]) { - continue; - } - - if (revents & LPOLLIN) { - /* Get socket from dispatch thread. */ - size_ret = lttng_read(the_ht_cleanup_pipe[0], - &ht, sizeof(ht)); - if (size_ret < sizeof(ht)) { - PERROR("ht cleanup notify pipe"); - goto error; - } - health_code_update(); - /* - * The whole point of this thread is to call - * lttng_ht_destroy from a context that is NOT: - * 1) a read-side RCU lock, - * 2) a call_rcu thread. - */ - lttng_ht_destroy(ht); - - health_code_update(); - - /* - * Ensure that we never process the quit pipe - * event while there is still data available - * on the ht clean pipe. - */ - goto restart; - } else if (revents & (LPOLLERR | LPOLLHUP | LPOLLRDHUP)) { - ERR("ht cleanup pipe error"); - goto error; - } else { - ERR("Unexpected poll events %u for sock %d", revents, pollfd); - goto error; - } - } - - for (i = 0; i < nb_fd; i++) { - health_code_update(); - - /* Fetch once the poll data */ - revents = LTTNG_POLL_GETEV(&events, i); - pollfd = LTTNG_POLL_GETFD(&events, i); - - if (!revents) { - /* No activity for this FD (poll implementation). */ - continue; - } - - if (pollfd == the_ht_cleanup_pipe[0]) { - continue; - } - - /* Thread quit pipe has been closed. Killing thread. */ - ret = check_quit_pipe(pollfd, revents); - if (ret) { - err = 0; - DBG("[ht-cleanup] quit."); - goto exit; - } - } - } - -exit: -error: - lttng_poll_clean(&events); -error_poll_create: -error_testpoint: - DBG("[ht-cleanup] Thread terminates."); - if (err) { - health_error(); - ERR("Health error occurred in %s", __func__); - } - health_unregister(the_health_sessiond); - rcu_thread_offline(); - rcu_unregister_thread(); - return NULL; -} - -static bool shutdown_ht_cleanup_thread(void *data) -{ - int ret; - - ret = notify_thread_pipe(ht_cleanup_quit_pipe[1]); - if (ret < 0) { - ERR("write error on ht_cleanup quit pipe"); - goto end; - } -end: - return ret; -} - -struct lttng_thread *launch_ht_cleanup_thread(void) -{ - int ret; - struct lttng_thread *thread; - - ret = init_pipe(the_ht_cleanup_pipe); - if (ret) { - goto error; - } - - ret = init_pipe(ht_cleanup_quit_pipe); - if (ret) { - goto error; - } - - thread = lttng_thread_create("HT cleanup", - thread_ht_cleanup, - shutdown_ht_cleanup_thread, - cleanup_ht_cleanup_thread, - NULL); - if (!thread) { - goto error; - } - return thread; -error: - cleanup_ht_cleanup_thread(NULL); - return NULL; -} diff --git a/src/bin/lttng-sessiond/ht-cleanup.h b/src/bin/lttng-sessiond/ht-cleanup.h deleted file mode 100644 index 52cca8325..000000000 --- a/src/bin/lttng-sessiond/ht-cleanup.h +++ /dev/null @@ -1,16 +0,0 @@ -/* - * Copyright (C) 2016 Jérémie Galarneau - * - * SPDX-License-Identifier: GPL-2.0-only - * - */ - -#ifndef _LTTNG_HT_CLEANUP_H -#define _LTTNG_HT_CLEANUP_H - -#include -#include "thread.h" - -struct lttng_thread *launch_ht_cleanup_thread(void); - -#endif /* _LTTNG_HT_CLEANUP_H */ diff --git a/src/bin/lttng-sessiond/lttng-sessiond.h b/src/bin/lttng-sessiond/lttng-sessiond.h index 65f4c372c..5dad32fc9 100644 --- a/src/bin/lttng-sessiond/lttng-sessiond.h +++ b/src/bin/lttng-sessiond/lttng-sessiond.h @@ -115,15 +115,6 @@ struct ust_reg_wait_node { struct cds_list_head head; }; -/* - * Used to notify that a hash table needs to be destroyed by dedicated - * thread. Required by design because we don't want to move destroy - * paths outside of large RCU read-side lock paths, and destroy cannot - * be called by call_rcu thread, because it may hang (waiting for - * call_rcu completion). - */ -extern int the_ht_cleanup_pipe[2]; - extern int the_kernel_poll_pipe[2]; /* diff --git a/src/bin/lttng-sessiond/lttng-syscall.cpp b/src/bin/lttng-sessiond/lttng-syscall.cpp index f14563457..7dae12d9f 100644 --- a/src/bin/lttng-sessiond/lttng-syscall.cpp +++ b/src/bin/lttng-sessiond/lttng-syscall.cpp @@ -169,7 +169,7 @@ static void destroy_syscall_ht(struct lttng_ht *ht) LTTNG_ASSERT(!ret); free(ksyscall); } - ht_cleanup_push(ht); + lttng_ht_destroy(ht); } /* diff --git a/src/bin/lttng-sessiond/main.cpp b/src/bin/lttng-sessiond/main.cpp index 58cf092ea..d8d2caf8b 100644 --- a/src/bin/lttng-sessiond/main.cpp +++ b/src/bin/lttng-sessiond/main.cpp @@ -67,7 +67,6 @@ #include "notification-thread-commands.h" #include "rotation-thread.h" #include "agent.h" -#include "ht-cleanup.h" #include "sessiond-config.h" #include "timer.h" #include "thread.h" @@ -1448,7 +1447,6 @@ int main(int argc, char **argv) struct lttng_pipe *ust32_channel_monitor_pipe = NULL, *ust64_channel_monitor_pipe = NULL, *kernel_channel_monitor_pipe = NULL; - struct lttng_thread *ht_cleanup_thread = NULL; struct timer_thread_parameters timer_thread_parameters; /* Rotation thread handle. */ struct rotation_thread_handle *rotation_thread_handle = NULL; @@ -1497,9 +1495,7 @@ int main(int argc, char **argv) * Parse arguments and load the daemon configuration file. * * We have an exit_options exit path to free memory reserved by - * set_options. This is needed because the rest of sessiond_cleanup() - * depends on ht_cleanup_thread, which depends on lttng_daemonize, which - * depends on set_options. + * set_options. */ progname = argv[0]; if (set_options(argc, argv)) { @@ -1600,13 +1596,6 @@ int main(int argc, char **argv) goto stop_threads; } - /* Create thread to clean up RCU hash tables */ - ht_cleanup_thread = launch_ht_cleanup_thread(); - if (!ht_cleanup_thread) { - retval = -1; - goto stop_threads; - } - /* Create thread quit pipe */ if (sessiond_init_thread_quit_pipe()) { retval = -1; @@ -1961,10 +1950,7 @@ stop_threads: * perform lookups in those structures. */ rcu_barrier(); - /* - * sessiond_cleanup() is called when no other thread is running, except - * the ht_cleanup thread, which is needed to destroy the hash tables. - */ + rcu_thread_online(); sessiond_cleanup(); @@ -2001,18 +1987,6 @@ stop_threads: modprobe_remove_lttng_all(); } - /* - * Ensure all prior call_rcu are done. call_rcu callbacks may push - * hash tables to the ht_cleanup thread. Therefore, we ensure that - * the queue is empty before shutting down the clean-up thread. - */ - rcu_barrier(); - - if (ht_cleanup_thread) { - lttng_thread_shutdown(ht_cleanup_thread); - lttng_thread_put(ht_cleanup_thread); - } - rcu_thread_offline(); rcu_unregister_thread(); diff --git a/src/bin/lttng-sessiond/session.cpp b/src/bin/lttng-sessiond/session.cpp index fbf8ce55c..b97f6eb2c 100644 --- a/src/bin/lttng-sessiond/session.cpp +++ b/src/bin/lttng-sessiond/session.cpp @@ -331,12 +331,12 @@ end: static void ltt_sessions_ht_destroy(void) { if (ltt_sessions_ht_by_id) { - ht_cleanup_push(ltt_sessions_ht_by_id); + lttng_ht_destroy(ltt_sessions_ht_by_id); ltt_sessions_ht_by_id = NULL; } if (ltt_sessions_ht_by_name) { - ht_cleanup_push(ltt_sessions_ht_by_name); + lttng_ht_destroy(ltt_sessions_ht_by_name); ltt_sessions_ht_by_name = NULL; } diff --git a/src/bin/lttng-sessiond/snapshot.cpp b/src/bin/lttng-sessiond/snapshot.cpp index 03472201a..44928edb0 100644 --- a/src/bin/lttng-sessiond/snapshot.cpp +++ b/src/bin/lttng-sessiond/snapshot.cpp @@ -325,5 +325,5 @@ void snapshot_destroy(struct snapshot *obj) snapshot_output_destroy(output); } rcu_read_unlock(); - ht_cleanup_push(obj->output_ht); + lttng_ht_destroy(obj->output_ht); } diff --git a/src/bin/lttng-sessiond/testpoint.h b/src/bin/lttng-sessiond/testpoint.h index 2ddd66283..6c35d29fc 100644 --- a/src/bin/lttng-sessiond/testpoint.h +++ b/src/bin/lttng-sessiond/testpoint.h @@ -19,7 +19,6 @@ TESTPOINT_DECL(sessiond_thread_manage_apps_before_loop); TESTPOINT_DECL(sessiond_thread_manage_kernel); TESTPOINT_DECL(sessiond_thread_manage_kernel_before_loop); TESTPOINT_DECL(sessiond_thread_manage_consumer); -TESTPOINT_DECL(sessiond_thread_ht_cleanup); TESTPOINT_DECL(sessiond_thread_app_manage_notify); TESTPOINT_DECL(sessiond_thread_app_reg_dispatch); TESTPOINT_DECL(sessiond_thread_notification); diff --git a/src/bin/lttng-sessiond/trace-ust.cpp b/src/bin/lttng-sessiond/trace-ust.cpp index ec62eef09..4be27f897 100644 --- a/src/bin/lttng-sessiond/trace-ust.cpp +++ b/src/bin/lttng-sessiond/trace-ust.cpp @@ -329,8 +329,8 @@ error: process_attr_tracker_destroy(lus->tracker_vpid); process_attr_tracker_destroy(lus->tracker_vuid); process_attr_tracker_destroy(lus->tracker_vgid); - ht_cleanup_push(lus->domain_global.channels); - ht_cleanup_push(lus->agents); + lttng_ht_destroy(lus->domain_global.channels); + lttng_ht_destroy(lus->agents); free(lus); error_alloc: return NULL; @@ -774,7 +774,7 @@ static void fini_id_tracker(struct ust_id_tracker *id_tracker) destroy_id_tracker_node(tracker_node); } rcu_read_unlock(); - ht_cleanup_push(id_tracker->ht); + lttng_ht_destroy(id_tracker->ht); id_tracker->ht = NULL; } @@ -1231,7 +1231,7 @@ static void destroy_contexts(struct lttng_ht *ht) } rcu_read_unlock(); - ht_cleanup_push(ht); + lttng_ht_destroy(ht); } /* @@ -1294,7 +1294,7 @@ static void destroy_events(struct lttng_ht *events) } rcu_read_unlock(); - ht_cleanup_push(events); + lttng_ht_destroy(events); } /* @@ -1371,7 +1371,7 @@ static void destroy_channels(struct lttng_ht *channels) } rcu_read_unlock(); - ht_cleanup_push(channels); + lttng_ht_destroy(channels); } /* @@ -1410,7 +1410,7 @@ void trace_ust_destroy_session(struct ltt_ust_session *session) } rcu_read_unlock(); - ht_cleanup_push(session->agents); + lttng_ht_destroy(session->agents); /* Cleanup UID buffer registry object(s). */ cds_list_for_each_entry_safe(reg, sreg, &session->buffer_reg_uid_list, diff --git a/src/bin/lttng-sessiond/ust-app.cpp b/src/bin/lttng-sessiond/ust-app.cpp index 570d011a7..08089e77e 100644 --- a/src/bin/lttng-sessiond/ust-app.cpp +++ b/src/bin/lttng-sessiond/ust-app.cpp @@ -448,20 +448,14 @@ void delete_ust_app_stream(int sock, struct ust_app_stream *stream, free(stream); } -/* - * We need to execute ht_destroy outside of RCU read-side critical - * section and outside of call_rcu thread, so we postpone its execution - * using ht_cleanup_push. It is simpler than to change the semantic of - * the many callers of delete_ust_app_session(). - */ static void delete_ust_app_channel_rcu(struct rcu_head *head) { struct ust_app_channel *ua_chan = caa_container_of(head, struct ust_app_channel, rcu_head); - ht_cleanup_push(ua_chan->ctx); - ht_cleanup_push(ua_chan->events); + lttng_ht_destroy(ua_chan->ctx); + lttng_ht_destroy(ua_chan->events); free(ua_chan); } @@ -895,19 +889,13 @@ end: return ret; } -/* - * We need to execute ht_destroy outside of RCU read-side critical - * section and outside of call_rcu thread, so we postpone its execution - * using ht_cleanup_push. It is simpler than to change the semantic of - * the many callers of delete_ust_app_session(). - */ static void delete_ust_app_session_rcu(struct rcu_head *head) { struct ust_app_session *ua_sess = caa_container_of(head, struct ust_app_session, rcu_head); - ht_cleanup_push(ua_sess->channels); + lttng_ht_destroy(ua_sess->channels); free(ua_sess); } @@ -1045,10 +1033,10 @@ void delete_ust_app(struct ust_app *app) rcu_read_unlock(); - ht_cleanup_push(app->sessions); - ht_cleanup_push(app->ust_sessions_objd); - ht_cleanup_push(app->ust_objd); - ht_cleanup_push(app->token_to_event_notifier_rule_ht); + lttng_ht_destroy(app->sessions); + lttng_ht_destroy(app->ust_sessions_objd); + lttng_ht_destroy(app->ust_objd); + lttng_ht_destroy(app->token_to_event_notifier_rule_ht); /* * This could be NULL if the event notifier setup failed (e.g the app @@ -4662,13 +4650,13 @@ void ust_app_clean_list(void) /* Destroy is done only when the ht is empty */ if (ust_app_ht) { - ht_cleanup_push(ust_app_ht); + lttng_ht_destroy(ust_app_ht); } if (ust_app_ht_by_sock) { - ht_cleanup_push(ust_app_ht_by_sock); + lttng_ht_destroy(ust_app_ht_by_sock); } if (ust_app_ht_by_notify_sock) { - ht_cleanup_push(ust_app_ht_by_notify_sock); + lttng_ht_destroy(ust_app_ht_by_notify_sock); } } diff --git a/src/bin/lttng-sessiond/ust-registry.cpp b/src/bin/lttng-sessiond/ust-registry.cpp index 088e8d64f..baa58d731 100644 --- a/src/bin/lttng-sessiond/ust-registry.cpp +++ b/src/bin/lttng-sessiond/ust-registry.cpp @@ -689,12 +689,6 @@ static void ust_registry_destroy_enum(struct ust_registry_session *reg_session, call_rcu(®_enum->rcu_head, destroy_enum_rcu); } -/* - * We need to execute ht_destroy outside of RCU read-side critical - * section and outside of call_rcu thread, so we postpone its execution - * using ht_cleanup_push. It is simpler than to change the semantic of - * the many callers of delete_ust_app_session(). - */ static void destroy_channel_rcu(struct rcu_head *head) { @@ -702,7 +696,7 @@ void destroy_channel_rcu(struct rcu_head *head) caa_container_of(head, struct ust_registry_channel, rcu_head); if (chan->ht) { - ht_cleanup_push(chan->ht); + lttng_ht_destroy(chan->ht); } free(chan->ctx_fields); free(chan); @@ -1011,7 +1005,7 @@ void ust_registry_session_destroy(struct ust_registry_session *reg) destroy_channel(chan, true); } rcu_read_unlock(); - ht_cleanup_push(reg->channels); + lttng_ht_destroy(reg->channels); } free(reg->metadata); @@ -1043,6 +1037,6 @@ void ust_registry_session_destroy(struct ust_registry_session *reg) ust_registry_destroy_enum(reg, reg_enum); } rcu_read_unlock(); - ht_cleanup_push(reg->enums); + lttng_ht_destroy(reg->enums); } } diff --git a/src/bin/lttng-sessiond/utils.cpp b/src/bin/lttng-sessiond/utils.cpp index 5b33a8df8..20b223288 100644 --- a/src/bin/lttng-sessiond/utils.cpp +++ b/src/bin/lttng-sessiond/utils.cpp @@ -16,8 +16,6 @@ #include "snapshot.h" #include "lttng-sessiond.h" -int the_ht_cleanup_pipe[2] = {-1, -1}; - /* * Write to writable pipe used to notify a thread. */ @@ -38,31 +36,6 @@ int notify_thread_pipe(int wpipe) return (int) ret; } -void ht_cleanup_push(struct lttng_ht *ht) -{ - ssize_t ret; - int fd = the_ht_cleanup_pipe[1]; - - if (!ht) { - return; - } - if (fd < 0) - return; - ret = lttng_write(fd, &ht, sizeof(ht)); - if (ret < sizeof(ht)) { - PERROR("write ht cleanup pipe %d", fd); - if (ret < 0) { - ret = -errno; - } - goto error; - } - - /* All good. Don't send back the write positive ret value. */ - ret = 0; -error: - LTTNG_ASSERT(!ret); -} - int loglevels_match(int a_loglevel_type, int a_loglevel_value, int b_loglevel_type, int b_loglevel_value, int loglevel_all_type) { diff --git a/src/bin/lttng-sessiond/utils.h b/src/bin/lttng-sessiond/utils.h index b7e0a4fde..5cbf7487f 100644 --- a/src/bin/lttng-sessiond/utils.h +++ b/src/bin/lttng-sessiond/utils.h @@ -14,7 +14,6 @@ struct consumer_output; const char *get_home_dir(void); int notify_thread_pipe(int wpipe); -void ht_cleanup_push(struct lttng_ht *ht); int loglevels_match(int a_loglevel_type, int a_loglevel_value, int b_loglevel_type, int b_loglevel_value, int loglevel_all_type); const char *session_get_base_path(const struct ltt_session *session); diff --git a/src/lib/lttng-ctl/lttng-ctl-health.cpp b/src/lib/lttng-ctl/lttng-ctl-health.cpp index 8981c7619..d061d9bea 100644 --- a/src/lib/lttng-ctl/lttng-ctl-health.cpp +++ b/src/lib/lttng-ctl/lttng-ctl-health.cpp @@ -64,8 +64,6 @@ const char *get_sessiond_thread_name(health_type_sessiond type) { return "Session daemon kernel"; case HEALTH_SESSIOND_TYPE_CONSUMER: return "Session daemon consumer manager"; - case HEALTH_SESSIOND_TYPE_HT_CLEANUP: - return "Session daemon hash table cleanup"; case HEALTH_SESSIOND_TYPE_APP_MANAGE_NOTIFY: return "Session daemon application notification manager"; case HEALTH_SESSIOND_TYPE_APP_REG_DISPATCH: diff --git a/tests/regression/tools/health/health_fail.c b/tests/regression/tools/health/health_fail.c index 6a4c8f08a..6520337e4 100644 --- a/tests/regression/tools/health/health_fail.c +++ b/tests/regression/tools/health/health_fail.c @@ -91,18 +91,6 @@ int __testpoint_sessiond_thread_manage_consumer(void) return 0; } -LTTNG_EXPORT int __testpoint_sessiond_thread_ht_cleanup(void); -int __testpoint_sessiond_thread_ht_cleanup(void) -{ - const char *var = "LTTNG_SESSIOND_THREAD_HT_CLEANUP_TP_FAIL"; - - if (check_env_var(var)) { - return 1; - } - - return 0; -} - LTTNG_EXPORT int __testpoint_sessiond_thread_app_manage_notify(void); int __testpoint_sessiond_thread_app_manage_notify(void) { diff --git a/tests/regression/tools/health/health_stall.c b/tests/regression/tools/health/health_stall.c index e89966315..d01ad4cb9 100644 --- a/tests/regression/tools/health/health_stall.c +++ b/tests/regression/tools/health/health_stall.c @@ -104,18 +104,6 @@ int __testpoint_sessiond_thread_manage_consumer(void) return 0; } -LTTNG_EXPORT int __testpoint_sessiond_thread_ht_cleanup(void); -int __testpoint_sessiond_thread_ht_cleanup(void) -{ - const char *var = "LTTNG_SESSIOND_THREAD_HT_CLEANUP_STALL"; - - if (check_env_var(var)) { - do_stall(); - } - - return 0; -} - LTTNG_EXPORT int __testpoint_sessiond_thread_app_manage_notify(void); int __testpoint_sessiond_thread_app_manage_notify(void) { diff --git a/tests/regression/tools/health/test_health.sh b/tests/regression/tools/health/test_health.sh index b3d6419d2..f586a299d 100644 --- a/tests/regression/tools/health/test_health.sh +++ b/tests/regression/tools/health/test_health.sh @@ -154,7 +154,6 @@ skip $foundobj "No shared object generated. Skipping all tests." $NUM_TESTS && e THREAD=("LTTNG_SESSIOND_THREAD_MANAGE_CLIENTS" "LTTNG_SESSIOND_THREAD_MANAGE_APPS" "LTTNG_SESSIOND_THREAD_REG_APPS" - "LTTNG_SESSIOND_THREAD_HT_CLEANUP" "LTTNG_SESSIOND_THREAD_APP_MANAGE_NOTIFY" "LTTNG_SESSIOND_THREAD_APP_REG_DISPATCH" "LTTNG_SESSIOND_THREAD_MANAGE_KERNEL" @@ -175,7 +174,6 @@ ERROR_STRING=( "Thread \"Session daemon command\" is not responding in component \"sessiond\"." "Thread \"Session daemon application manager\" is not responding in component \"sessiond\"." "Thread \"Session daemon application registration\" is not responding in component \"sessiond\"." - "Thread \"Session daemon hash table cleanup\" is not responding in component \"sessiond\"." "Thread \"Session daemon application notification manager\" is not responding in component \"sessiond\"." "Thread \"Session daemon application registration dispatcher\" is not responding in component \"sessiond\"." "Thread \"Session daemon kernel\" is not responding in component \"sessiond\"." diff --git a/tests/unit/test_session.cpp b/tests/unit/test_session.cpp index ee0549906..5a549a593 100644 --- a/tests/unit/test_session.cpp +++ b/tests/unit/test_session.cpp @@ -18,7 +18,6 @@ #include #include #include -#include #include #include #include @@ -325,14 +324,10 @@ static void test_large_session_number(void) int main(int argc, char **argv) { - struct lttng_thread *ht_cleanup_thread; plan_tests(NUM_TESTS); the_health_sessiond = health_app_create(NR_HEALTH_SESSIOND_TYPES); - ht_cleanup_thread = launch_ht_cleanup_thread(); - LTTNG_ASSERT(ht_cleanup_thread); - lttng_thread_put(ht_cleanup_thread); diag("Sessions unit tests"); -- 2.34.1