From 1d25334cf088d661bac0643ccb50d1acc6d02e05 Mon Sep 17 00:00:00 2001 From: Francis Deslauriers Date: Fri, 26 Mar 2021 17:06:12 -0400 Subject: [PATCH] Fix: sessiond: kernel error accounting fd still open when unloading modules MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Issue ===== I noticed that some kernel modules were not unloading properly when shutting down the sessiond. Namely those modules: lttng_counter_client_percpu_64_modular lttng_ring_buffer_event_notifier_client The call to `modprobe -r` for those modules is failing because the kernel error accounting fd has not been closed. Fix === Only call `modprobe -r` after all kernel resources (file descriptors) have been released. Side note ========= The `modprobe_remove_lttng()` is currently printing that the removal of a _OPTIONAL module was a success even if it failed. I rework this function to print a debug message in such situation. I used the `modprobe_lttng()` as a template for this function. Signed-off-by: Francis Deslauriers Signed-off-by: Jérémie Galarneau Change-Id: Ibb51425ef3deedf6e982084ef879bc83ac823ba3 --- src/bin/lttng-sessiond/kernel.c | 2 -- src/bin/lttng-sessiond/main.c | 23 +++++++++++++++++++---- src/bin/lttng-sessiond/modprobe.c | 23 ++++++++++++++++------- 3 files changed, 35 insertions(+), 13 deletions(-) diff --git a/src/bin/lttng-sessiond/kernel.c b/src/bin/lttng-sessiond/kernel.c index afeb181c9..eebe119d2 100644 --- a/src/bin/lttng-sessiond/kernel.c +++ b/src/bin/lttng-sessiond/kernel.c @@ -2124,8 +2124,6 @@ void cleanup_kernel_tracer(void) kernel_tracer_fd = -1; } - DBG("Unloading kernel modules"); - modprobe_remove_lttng_all(); free(syscall_table); } diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c index bb84be1ad..e0b5595f3 100644 --- a/src/bin/lttng-sessiond/main.c +++ b/src/bin/lttng-sessiond/main.c @@ -74,6 +74,7 @@ #include "register.h" #include "manage-apps.h" #include "manage-kernel.h" +#include "modprobe.h" static const char *help_msg = #ifdef LTTNG_EMBED_HELP @@ -1887,7 +1888,7 @@ stop_threads: sessiond_cleanup(); /* - * Wait for all pending call_rcu work to complete tearing shutting down + * Wait for all pending call_rcu work to complete before shutting down * the notification thread. This call_rcu work includes shutting down * UST apps and event notifier pipes. */ @@ -1899,12 +1900,26 @@ stop_threads: } /* - * Teardown of error accounting needs be done after the teardown of the - * notification thread as all error buckets must have been released by - * their users (conditions). + * Error accounting teardown has to be done after the teardown of all + * event notifier pipes to ensure that no tracer may try to use the + * error accounting facilities. */ event_notifier_error_accounting_fini(); + /* + * Unloading the kernel modules needs to be done after all kernel + * ressources have been released. In our case, this includes the + * notification fd, the event notifier group fd, error accounting fd, + * all event and event notifier fds, etc. + * + * In short, at this point, we need to have called close() on all fds + * received from the kernel tracer. + */ + if (is_root && !config.no_kernel) { + DBG("Unloading kernel modules"); + 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 diff --git a/src/bin/lttng-sessiond/modprobe.c b/src/bin/lttng-sessiond/modprobe.c index bc2a86d75..8d162857f 100644 --- a/src/bin/lttng-sessiond/modprobe.c +++ b/src/bin/lttng-sessiond/modprobe.c @@ -514,14 +514,23 @@ static void modprobe_remove_lttng(const struct kern_modules_param *modules, modprobe[sizeof(modprobe) - 1] = '\0'; ret = system(modprobe); if (ret == -1) { - ERR("Unable to launch modprobe -r for module %s", - modules[i].name); - } else if (modules[i].load_policy == KERNEL_MODULE_PROPERTY_LOAD_POLICY_REQUIRED && WEXITSTATUS(ret) != 0) { - ERR("Unable to remove module %s", - modules[i].name); + if (modules[i].load_policy == KERNEL_MODULE_PROPERTY_LOAD_POLICY_REQUIRED) { + ERR("Unable to launch modprobe -r for required module %s", + modules[i].name); + } else { + DBG("Unable to launch modprobe -r for optional module %s", + modules[i].name); + } + } else if (WEXITSTATUS(ret) != 0) { + if (modules[i].load_policy == KERNEL_MODULE_PROPERTY_LOAD_POLICY_REQUIRED) { + ERR("Unable to remove required module %s", + modules[i].name); + } else { + DBG("Unable to remove optional module %s", + modules[i].name); + } } else { - DBG("Modprobe removal successful %s", - modules[i].name); + DBG("Modprobe removal successful %s", modules[i].name); } } } -- 2.34.1