Remove the sessiond "ready" counter mechanism

This commit replaces the sessiond "ready" counter scheme with
the use of the lttng_thread util. The launch of the threads which
need to be active before the sessiond can signal its parents
(when launched in daemon mode) is now blocking. This means that
their associated "launch" functions wait until the threads mark
themselves as ready (through the use of a "ready" semaphore)
before returning and allowing the initialization of the sessiond
to continue.

The threads which expose externally-visible resources (UNIX and
TCP sockets) which must be fully initialized before marking the
session daemon as ready are:
  - Health thread,
  - Agent thread,
  - Client thread.

Previously, the "load session" thread was part of this group.
However, it is no longer necessary to perform the loading of
session configurations in a dedicated thread. The main thread
performs that operation itself. It is safe to do so since it
is performed after the launch of the client thread. The client
thread has to be fully initialized as the session loading code
"impersonates" a client to initialize the loaded sessions.

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

diff --git a/src/bin/lttng-sessiond/Makefile.am b/src/bin/lttng-sessiond/Makefile.am
index eaf388b015605566db372805929babec64fff526..e1e5a00e8764c230d510835e679ebf29a4ac348d 100644 (file)
--- a/src/bin/lttng-sessiond/Makefile.am
+++ b/src/bin/lttng-sessiond/Makefile.am
@@ -39,7 +39,6 @@ lttng_sessiond_SOURCES = utils.c utils.h \
                        rotate.h rotate.c \
                        rotation-thread.h rotation-thread.c \
                        timer.c timer.h \
-                       ready.c \
                        globals.c \
                        thread-utils.c \
                        process-utils.c \

diff --git a/src/bin/lttng-sessiond/agent-thread.c b/src/bin/lttng-sessiond/agent-thread.c
index 9c98d30ab4e71accd303a442d41b45feeee212b6..3ee383389d5e35927b9787e221fc3e51a0cf2b03 100644 (file)
--- a/src/bin/lttng-sessiond/agent-thread.c
+++ b/src/bin/lttng-sessiond/agent-thread.c
@@ -33,6 +33,11 @@
 #include "utils.h"
 #include "thread.h"
 
+struct thread_notifiers {
+       struct lttng_pipe *quit_pipe;
+       sem_t ready;
+};
+
 static int agent_tracing_enabled = -1;
 
 /*
@@ -293,6 +298,21 @@ static int write_agent_port(uint16_t port)
                        config.agent_port_file_path.value);
 }
 
+static
+void mark_thread_as_ready(struct thread_notifiers *notifiers)
+{
+       DBG("Marking agent management thread as ready");
+       sem_post(&notifiers->ready);
+}
+
+static
+void wait_until_thread_is_ready(struct thread_notifiers *notifiers)
+{
+       DBG("Waiting for agent management thread to be ready");
+       sem_wait(&notifiers->ready);
+       DBG("Agent management thread is ready");
+}
+
 /*
  * This thread manage application notify communication.
  */
@@ -302,8 +322,9 @@ static void *thread_agent_management(void *data)
        uint32_t revents, nb_fd;
        struct lttng_poll_event events;
        struct lttcomm_sock *reg_sock;
-       struct lttng_pipe *quit_pipe = data;
-       const int quit_pipe_read_fd = lttng_pipe_get_readfd(quit_pipe);
+       struct thread_notifiers *notifiers = data;
+       const int quit_pipe_read_fd = lttng_pipe_get_readfd(
+                       notifiers->quit_pipe);
 
        DBG("[agent-thread] Manage agent application registration.");
 
@@ -335,12 +356,12 @@ static void *thread_agent_management(void *data)
                if (ret) {
                        ERR("[agent-thread] Failed to create agent port file: agent tracing will be unavailable");
                        /* Don't prevent the launch of the sessiond on error. */
-                       sessiond_notify_ready();
+                       mark_thread_as_ready(notifiers);
                        goto error;
                }
        } else {
                /* Don't prevent the launch of the sessiond on error. */
-               sessiond_notify_ready();
+               mark_thread_as_ready(notifiers);
                goto error_tcp_socket;
        }
 
@@ -349,7 +370,7 @@ static void *thread_agent_management(void *data)
         * may start to query whether or not agent tracing is enabled.
         */
        uatomic_set(&agent_tracing_enabled, 1);
-       sessiond_notify_ready();
+       mark_thread_as_ready(notifiers);
 
        /* Add TCP socket to poll set. */
        ret = lttng_poll_add(&events, reg_sock->fd,
@@ -461,40 +482,48 @@ error_poll_create:
 
 static bool shutdown_agent_management_thread(void *data)
 {
-       struct lttng_pipe *quit_pipe = data;
-       const int write_fd = lttng_pipe_get_writefd(quit_pipe);
+       struct thread_notifiers *notifiers = data;
+       const int write_fd = lttng_pipe_get_writefd(notifiers->quit_pipe);
 
        return notify_thread_pipe(write_fd) == 1;
 }
 
 static void cleanup_agent_management_thread(void *data)
 {
-       struct lttng_pipe *quit_pipe = data;
+       struct thread_notifiers *notifiers = data;
 
-       lttng_pipe_destroy(quit_pipe);
+       lttng_pipe_destroy(notifiers->quit_pipe);
+       sem_destroy(&notifiers->ready);
+       free(notifiers);
 }
 
 bool launch_agent_management_thread(void)
 {
-       struct lttng_pipe *quit_pipe;
+       struct thread_notifiers *notifiers;
        struct lttng_thread *thread;
 
-       quit_pipe = lttng_pipe_open(FD_CLOEXEC);
-       if (!quit_pipe) {
+       notifiers = zmalloc(sizeof(*notifiers));
+       if (!notifiers) {
+               goto error;
+       }
+
+       sem_init(&notifiers->ready, 0, 0);
+       notifiers->quit_pipe = lttng_pipe_open(FD_CLOEXEC);
+       if (!notifiers->quit_pipe) {
                goto error;
        }
        thread = lttng_thread_create("Agent management",
                        thread_agent_management,
                        shutdown_agent_management_thread,
                        cleanup_agent_management_thread,
-                       quit_pipe);
+                       notifiers);
        if (!thread) {
                goto error;
        }
-
+       wait_until_thread_is_ready(notifiers);
        lttng_thread_put(thread);
        return true;
 error:
-       cleanup_agent_management_thread(quit_pipe);
+       cleanup_agent_management_thread(notifiers);
        return false;
 }

diff --git a/src/bin/lttng-sessiond/health.c b/src/bin/lttng-sessiond/health.c
index e2b8cc5a3e1b3794b75299737f064e9ac3fff220..e8c42e4bd7fa6f2b8b7394e9af43461be4b26fb4 100644 (file)
--- a/src/bin/lttng-sessiond/health.c
+++ b/src/bin/lttng-sessiond/health.c
@@ -27,17 +27,39 @@
 #include "utils.h"
 #include "thread.h"
 
-static void cleanup_health_management_thread(void *thread_data)
+struct thread_notifiers {
+       struct lttng_pipe *quit_pipe;
+       sem_t ready;
+};
+
+static
+void mark_thread_as_ready(struct thread_notifiers *notifiers)
 {
-       struct lttng_pipe *quit_pipe = thread_data;
+       DBG("Marking health management thread as ready");
+       sem_post(&notifiers->ready);
+}
 
-       lttng_pipe_destroy(quit_pipe);
+static
+void wait_until_thread_is_ready(struct thread_notifiers *notifiers)
+{
+       DBG("Waiting for health management thread to be ready");
+       sem_wait(&notifiers->ready);
+       DBG("Health management thread is ready");
+}
+
+static void cleanup_health_management_thread(void *data)
+{
+       struct thread_notifiers *notifiers = data;
+
+       lttng_pipe_destroy(notifiers->quit_pipe);
+       sem_destroy(&notifiers->ready);
+       free(notifiers);
 }
 
 /*
  * Thread managing health check socket.
  */
-static void *thread_manage_health(void *thread_data)
+static void *thread_manage_health(void *data)
 {
        const bool is_root = (getuid() == 0);
        int sock = -1, new_sock = -1, ret, i, pollfd, err = -1;
@@ -46,8 +68,9 @@ static void *thread_manage_health(void *thread_data)
        struct health_comm_msg msg;
        struct health_comm_reply reply;
        /* Thread-specific quit pipe. */
-       struct lttng_pipe *quit_pipe = thread_data;
-       const int quit_pipe_read_fd = lttng_pipe_get_readfd(quit_pipe);
+       struct thread_notifiers *notifiers = data;
+       const int quit_pipe_read_fd = lttng_pipe_get_readfd(
+                       notifiers->quit_pipe);
 
        DBG("[thread] Manage health check started");
 
@@ -111,8 +134,7 @@ static void *thread_manage_health(void *thread_data)
                goto error;
        }
 
-       sessiond_notify_ready();
-
+       mark_thread_as_ready(notifiers);
        while (1) {
                DBG("Health check ready");
 
@@ -228,44 +250,42 @@ error:
        return NULL;
 }
 
-static bool shutdown_health_management_thread(void *thread_data)
+static bool shutdown_health_management_thread(void *data)
 {
-       int ret;
-       int pipe_write_fd;
-       struct lttng_pipe *health_quit_pipe = thread_data;
+       struct thread_notifiers *notifiers = data;
+       const int write_fd = lttng_pipe_get_writefd(notifiers->quit_pipe);
 
-       pipe_write_fd = lttng_pipe_get_writefd(health_quit_pipe);
-       ret = notify_thread_pipe(pipe_write_fd);
-       if (ret < 0) {
-               ERR("Failed to notify Health management thread's quit pipe");
-               goto error;
-       }
-       return true;
-error:
-       return false;
+       return notify_thread_pipe(write_fd) == 1;
 }
 
 bool launch_health_management_thread(void)
 {
+       struct thread_notifiers *notifiers;
        struct lttng_thread *thread;
-       struct lttng_pipe *health_quit_pipe = NULL;
 
-       health_quit_pipe = lttng_pipe_open(FD_CLOEXEC);
-       if (!health_quit_pipe) {
+       notifiers = zmalloc(sizeof(*notifiers));
+       if (!notifiers) {
                goto error;
        }
 
+       sem_init(&notifiers->ready, 0, 0);
+       notifiers->quit_pipe = lttng_pipe_open(FD_CLOEXEC);
+       if (!notifiers->quit_pipe) {
+               goto error;
+       }
        thread = lttng_thread_create("Health management",
                        thread_manage_health,
                        shutdown_health_management_thread,
                        cleanup_health_management_thread,
-                       health_quit_pipe);
+                       notifiers);
        if (!thread) {
                goto error;
        }
+
+       wait_until_thread_is_ready(notifiers);
        lttng_thread_put(thread);
        return true;
 error:
-       cleanup_health_management_thread(health_quit_pipe);
+       cleanup_health_management_thread(notifiers);
        return false;
 }

diff --git a/src/bin/lttng-sessiond/lttng-sessiond.h b/src/bin/lttng-sessiond/lttng-sessiond.h
index ae73507620a8b7ea02c7afdc1d75efa6a30c34bc..d321fceda1de99b870636f6d7b42dc7c57fd19e3 100644 (file)
--- a/src/bin/lttng-sessiond/lttng-sessiond.h
+++ b/src/bin/lttng-sessiond/lttng-sessiond.h
@@ -147,8 +147,6 @@ extern struct health_app *health_sessiond;
 
 extern struct sessiond_config config;
 
-extern int lttng_sessiond_ready;
-
 extern int ust_consumerd64_fd, ust_consumerd32_fd;
 
 /* Parent PID for --sig-parent option */
@@ -168,7 +166,6 @@ int sessiond_notify_quit_pipe(void);
 void sessiond_close_quit_pipe(void);
 
 int sessiond_set_thread_pollset(struct lttng_poll_event *events, size_t size);
-void sessiond_notify_ready(void);
 void sessiond_signal_parents(void);
 
 void sessiond_set_client_thread_state(bool running);

diff --git a/src/bin/lttng-sessiond/ready.c b/src/bin/lttng-sessiond/ready.c
deleted file mode 100644 (file)
index 3a268b0..0000000
--- a/src/bin/lttng-sessiond/ready.c
+++ /dev/null
@@ -1,80 +0,0 @@
-/*
- * Copyright (C) 2011 - David Goulet <david.goulet@polymtl.ca>
- *                      Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
- *               2013 - Jérémie Galarneau <jeremie.galarneau@efficios.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License, version 2 only,
- * as published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write to the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
- */
-
-#include <common/macros.h>
-#include <urcu.h>
-#include "lttng-sessiond.h"
-
-/*
- * 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.
- *
- * 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 four (4) support threads are:
- *  - agent_thread
- *  - notification_thread
- *  - rotation_thread
- *  - health_thread
- */
-#define NR_LTTNG_SESSIOND_SUPPORT_THREADS 4
-int lttng_sessiond_ready = NR_LTTNG_SESSIOND_SUPPORT_THREADS;
-
-LTTNG_HIDDEN
-void sessiond_notify_ready(void)
-{
-       /*
-        * This memory barrier is paired with the one performed by
-        * the client thread after it has seen that 'lttng_sessiond_ready' is 0.
-        *
-        * The purpose of these memory barriers is to ensure that all
-        * initialization operations of the various threads that call this
-        * function to signal that they are ready are commited/published
-        * before the client thread can see the 'lttng_sessiond_ready' counter
-        * reach 0.
-        *
-        * Note that this could be a 'write' memory barrier, but a full barrier
-        * is used in case the code using this utility changes. The performance
-        * implications of this choice are minimal since this is a slow path.
-        */
-       cmm_smp_mb();
-       uatomic_sub(&lttng_sessiond_ready, 1);
-}

diff --git a/src/bin/lttng-sessiond/rotation-thread.c b/src/bin/lttng-sessiond/rotation-thread.c
index 9a1d803b795846a380fa53b157616b370f2d2e05..8e63e160f980c15f8e60af0ea8f74d88a89e758e 100644 (file)
--- a/src/bin/lttng-sessiond/rotation-thread.c
+++ b/src/bin/lttng-sessiond/rotation-thread.c
@@ -950,9 +950,6 @@ void *thread_rotation(void *data)
                goto error;
        }
 
-       /* Ready to handle client connections. */
-       sessiond_notify_ready();
-
        while (true) {
                int fd_count, i;
 

diff --git a/tests/unit/Makefile.am b/tests/unit/Makefile.am
index d8d60c8a510abdca79ec5f9e8bf8a81f36714991..d6a87d5cd412558565e11e2fe872e0b43052eee3 100644 (file)
--- a/tests/unit/Makefile.am
+++ b/tests/unit/Makefile.am
@@ -51,7 +51,6 @@ SESSIOND_OBJS = $(top_builddir)/src/bin/lttng-sessiond/buffer-registry.$(OBJEXT)
         $(top_builddir)/src/bin/lttng-sessiond/lttng-syscall.$(OBJEXT) \
         $(top_builddir)/src/bin/lttng-sessiond/channel.$(OBJEXT) \
         $(top_builddir)/src/bin/lttng-sessiond/agent.$(OBJEXT) \
-        $(top_builddir)/src/bin/lttng-sessiond/ready.$(OBJEXT) \
         $(top_builddir)/src/bin/lttng-sessiond/kernel-consumer.$(OBJEXT) \
         $(top_builddir)/src/bin/lttng-sessiond/trace-kernel.$(OBJEXT) \
         $(top_builddir)/src/bin/lttng-sessiond/rotation-thread.$(OBJEXT) \