Fix: don't wait for the load thread before serving client commands

Since the session loading thread uses the same communication than
the external clients, it should not be included in the set of
threads that must be launched before the sessiond starts to serve
client commands.

Since the "load session" thread is guaranteed to be the last
essential thread to be initialized, it can explicitly signal
the parents that the sessiond is ready once it is done auto-loading
session configurations.

This commit also adds a lengthy comment explaining the initialization
of the session daemon.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

diff --git a/src/bin/lttng-sessiond/load-session-thread.c b/src/bin/lttng-sessiond/load-session-thread.c
index b95764d6693a76cd902187955362979ccb3c651c..a6c393c9f994a912230d46f5c7f926ee84d040f6 100644 (file)
--- a/src/bin/lttng-sessiond/load-session-thread.c
+++ b/src/bin/lttng-sessiond/load-session-thread.c
@@ -103,6 +103,6 @@ void *thread_load_session(void *data)
        }
 
 end:
-       sessiond_notify_ready();
+       sessiond_signal_parents();
        return NULL;
 }

diff --git a/src/bin/lttng-sessiond/lttng-sessiond.h b/src/bin/lttng-sessiond/lttng-sessiond.h
index bcc4259871c1ad0196799aa1ea1ec1b2db300775..c3fa0f073e2a163e67f42696306333b23daaef01 100644 (file)
--- a/src/bin/lttng-sessiond/lttng-sessiond.h
+++ b/src/bin/lttng-sessiond/lttng-sessiond.h
@@ -125,5 +125,6 @@ extern struct sessiond_config config;
 int sessiond_check_thread_quit_pipe(int fd, uint32_t events);
 int sessiond_set_thread_pollset(struct lttng_poll_event *events, size_t size);
 void sessiond_notify_ready(void);
+void sessiond_signal_parents(void);
 
 #endif /* _LTT_SESSIOND_H */

diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c
index e727888181d8ae1089e967297adec076a6a8585e..ec907628dd5628f0bf77752fc0a5752f2c9911dc 100644 (file)
--- a/src/bin/lttng-sessiond/main.c
+++ b/src/bin/lttng-sessiond/main.c
@@ -286,13 +286,39 @@ struct notification_thread_handle *notification_thread_handle;
 struct lttng_ht *agent_apps_ht_by_sock = NULL;
 
 /*
- * Whether sessiond is ready for commands/notification channel/health check
+ * The initialization of the session daemon is done in multiple phases.
+ *
+ * While all threads are launched near-simultaneously, only some of them
+ * are needed to ensure the session daemon can start to respond to client
  * requests.
- * NR_LTTNG_SESSIOND_READY must match the number of calls to
- * sessiond_notify_ready().
+ *
+ * There are two important guarantees that we wish to offer with respect
+ * to the initialisation of the session daemon:
+ *   - When the daemonize/background launcher process exits, the sessiond
+ *     is fully able to respond to client requests,
+ *   - Auto-loaded sessions are visible to clients.
+ *
+ * In order to achieve this, a number of support threads have to be launched
+ * to allow the "client" thread to function properly. Moreover, since the
+ * "load session" thread needs the client thread, we must provide a way
+ * for the "load session" thread to know that the "client" thread is up
+ * and running.
+ *
+ * Hence, the support threads decrement the lttng_sessiond_ready counter
+ * while the "client" threads waits for it to reach 0. Once the "client" thread
+ * unblocks, it posts the message_thread_ready semaphore which allows the
+ * "load session" thread to progress.
+ *
+ * This implies that the "load session" thread is the last to be initialized
+ * and will explicitly call sessiond_signal_parents(), which signals the parents
+ * that the session daemon is fully initialized.
+ *
+ * The three (3) support threads are:
+ *  - agent_thread
+ *  - notification_thread
+ *  - health_thread
  */
-#define NR_LTTNG_SESSIOND_READY                5
-int lttng_sessiond_ready = NR_LTTNG_SESSIOND_READY;
+int lttng_sessiond_ready = 3;
 
 int sessiond_check_thread_quit_pipe(int fd, uint32_t events)
 {
@@ -301,28 +327,36 @@ int sessiond_check_thread_quit_pipe(int fd, uint32_t events)
 
 /* Notify parents that we are ready for cmd and health check */
 LTTNG_HIDDEN
-void sessiond_notify_ready(void)
+void sessiond_signal_parents(void)
 {
-       if (uatomic_sub_return(&lttng_sessiond_ready, 1) == 0) {
-               /*
-                * Notify parent pid that we are ready to accept command
-                * for client side.  This ppid is the one from the
-                * external process that spawned us.
-                */
-               if (config.sig_parent) {
-                       kill(ppid, SIGUSR1);
-               }
+       /*
+        * Notify parent pid that we are ready to accept command
+        * for client side.  This ppid is the one from the
+        * external process that spawned us.
+        */
+       if (config.sig_parent) {
+               kill(ppid, SIGUSR1);
+       }
 
-               /*
-                * Notify the parent of the fork() process that we are
-                * ready.
-                */
-               if (config.daemonize || config.background) {
-                       kill(child_ppid, SIGUSR1);
-               }
+       /*
+        * Notify the parent of the fork() process that we are
+        * ready.
+        */
+       if (config.daemonize || config.background) {
+               kill(child_ppid, SIGUSR1);
        }
 }
 
+LTTNG_HIDDEN
+void sessiond_notify_ready(void)
+{
+       /*
+        * The _return variant is used since the implied memory barriers are
+        * required.
+        */
+       (void) uatomic_sub_return(&lttng_sessiond_ready, 1);
+}
+
 static
 int __sessiond_set_thread_pollset(struct lttng_poll_event *events, size_t size,
                int *a_pipe)
@@ -4286,8 +4320,6 @@ static void *thread_manage_clients(void *data)
                goto error;
        }
 
-       sessiond_notify_ready();
-
        ret = sem_post(&load_info->message_thread_ready);
        if (ret) {
                PERROR("sem_post message_thread_ready");
@@ -4320,6 +4352,7 @@ static void *thread_manage_clients(void *data)
                if (ret > 0 || (ret < 0 && errno != EINTR)) {
                        goto exit;
                }
+               cmm_smp_rmb();
        }
 
        /* This testpoint is after we signal readiness to the parent. */