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 afeb181c946f90ae2cb6d0cee40276dd841959cd..eebe119d2701dc8e3875c9f5e2f482a0849fd452 100644 (file)
--- 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 bb84be1ad1475ee491ad9d2e807b481f56f522fb..e0b5595f3b25adbd6fb98332420d9cc458435060 100644 (file)
--- 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 bc2a86d7512a3321f20c20b36f828aa36f1fc164..8d162857f6ae88eeec547857d96baf055db99b63 100644 (file)
--- 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);
                }
        }
 }