From 9cc1ba3b1f895beb3ef4b10009372f6a859068dd Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=A9mie=20Galarneau?= Date: Wed, 16 May 2018 17:08:36 -0400 Subject: [PATCH] Fix: don't wait for the load thread before serving client commands MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 --- src/bin/lttng-sessiond/load-session-thread.c | 2 +- src/bin/lttng-sessiond/lttng-sessiond.h | 1 + src/bin/lttng-sessiond/main.c | 82 ++++++++++++++------ 3 files changed, 60 insertions(+), 25 deletions(-) diff --git a/src/bin/lttng-sessiond/load-session-thread.c b/src/bin/lttng-sessiond/load-session-thread.c index b95764d66..a6c393c9f 100644 --- 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 bcc425987..c3fa0f073 100644 --- 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 37c0a9325..0625c1193 100644 --- a/src/bin/lttng-sessiond/main.c +++ b/src/bin/lttng-sessiond/main.c @@ -299,13 +299,40 @@ struct rotation_thread_handle *rotation_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 four (4) support threads are: + * - agent_thread + * - notification_thread + * - rotation_thread + * - health_thread */ -#define NR_LTTNG_SESSIOND_READY 6 -int lttng_sessiond_ready = NR_LTTNG_SESSIOND_READY; +int lttng_sessiond_ready = 4; int sessiond_check_thread_quit_pipe(int fd, uint32_t events) { @@ -314,28 +341,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(<tng_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(<tng_sessiond_ready, 1); +} + static int __sessiond_set_thread_pollset(struct lttng_poll_event *events, size_t size, int *a_pipe) @@ -4495,8 +4530,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"); @@ -4529,6 +4562,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. */ -- 2.34.1