Fix: perform the initialization memory barrier out of loop body

[lttng-tools.git] / src / bin / lttng-sessiond / main.c

diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c
index 0625c11939b958f8b9d88ed3ce48a88a3074d703..3d2cad9361464534128a44f5917d01f1e58899fe 100644 (file)
--- a/src/bin/lttng-sessiond/main.c
+++ b/src/bin/lttng-sessiond/main.c
@@ -332,7 +332,8 @@ struct lttng_ht *agent_apps_ht_by_sock = NULL;
  *  - rotation_thread
  *  - health_thread
  */
-int lttng_sessiond_ready = 4;
+#define NR_LTTNG_SESSIOND_SUPPORT_THREADS 4
+int lttng_sessiond_ready = NR_LTTNG_SESSIOND_SUPPORT_THREADS;
 
 int sessiond_check_thread_quit_pipe(int fd, uint32_t events)
 {
@@ -365,10 +366,21 @@ LTTNG_HIDDEN
 void sessiond_notify_ready(void)
 {
        /*
-        * The _return variant is used since the implied memory barriers are
-        * required.
+        * 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.
         */
-       (void) uatomic_sub_return(&lttng_sessiond_ready, 1);
+       cmm_smp_mb();
+       uatomic_sub(&lttng_sessiond_ready, 1);
 }
 
 static
@@ -4562,8 +4574,17 @@ static void *thread_manage_clients(void *data)
                if (ret > 0 || (ret < 0 && errno != EINTR)) {
                        goto exit;
                }
-               cmm_smp_rmb();
        }
+       /*
+        * This barrier is paired with the one in sessiond_notify_ready() to
+        * ensure that loads accessing data initialized by the other threads,
+        * on which this thread was waiting, are not performed before this point.
+        *
+        * Note that this could be a 'read' memory barrier, but a full barrier
+        * is used in case the code changes. The performance implications of
+        * this choice are minimal since this is a slow path.
+        */
+       cmm_smp_mb();
 
        /* This testpoint is after we signal readiness to the parent. */
        if (testpoint(sessiond_thread_manage_clients)) {