From: Francis Deslauriers <francis.deslauriers@efficios.com>
Date: Fri, 26 Mar 2021 21:06:12 +0000 (-0400)
Subject: Fix: sessiond: kernel error accounting fd still open when unloading modules
X-Git-Tag: v2.13.0-rc1~178
X-Git-Url: https://git.lttng.org./?a=commitdiff_plain;h=1d25334cf088d661bac0643ccb50d1acc6d02e05;p=lttng-tools.git

Fix: sessiond: kernel error accounting fd still open when unloading modules

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 <francis.deslauriers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: Ibb51425ef3deedf6e982084ef879bc83ac823ba3
---

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);
 		}
 	}
 }