X-Git-Url: https://git.lttng.org/?a=blobdiff_plain;f=src%2Fcommon%2Fconsumer-timer.c;h=37c9861b906a0a81c6b08a302c73e486e6c6d3dd;hb=e5d1a9b33aa0920bbd9f6948bd0676156da67c61;hp=641478697904e516df1feb7e5c214b814fd38fb0;hpb=4419b4fb187cb896a8e64bb97c5471240abca122;p=lttng-tools.git

diff --git a/src/common/consumer-timer.c b/src/common/consumer-timer.c
index 641478697..37c9861b9 100644
--- a/src/common/consumer-timer.c
+++ b/src/common/consumer-timer.c
@@ -26,7 +26,12 @@
 #include "consumer-timer.h"
 #include "ust-consumer/ust-consumer.h"
 
-static struct timer_signal_data timer_signal;
+static struct timer_signal_data timer_signal = {
+	.tid = 0,
+	.setup_done = 0,
+	.qs_done = 0,
+	.lock = PTHREAD_MUTEX_INITIALIZER,
+};
 
 /*
  * Set custom signal mask to current thread.
@@ -51,6 +56,10 @@ static void setmask(sigset_t *mask)
 
 /*
  * Execute action on a timer switch.
+ *
+ * Beware: metadata_switch_timer() should *never* take a mutex also held
+ * while consumer_timer_switch_stop() is called. It would result in
+ * deadlocks.
  */
 static void metadata_switch_timer(struct lttng_consumer_local_data *ctx,
 		int sig, siginfo_t *si, void *uc)
@@ -69,7 +78,21 @@ static void metadata_switch_timer(struct lttng_consumer_local_data *ctx,
 	switch (ctx->type) {
 	case LTTNG_CONSUMER32_UST:
 	case LTTNG_CONSUMER64_UST:
-		ret = lttng_ustconsumer_request_metadata(ctx, channel);
+		/*
+		 * Locks taken by lttng_ustconsumer_request_metadata():
+		 * - metadata_socket_lock
+		 *   - Calling lttng_ustconsumer_recv_metadata():
+		 *     - channel->metadata_cache->lock
+		 *     - Calling consumer_metadata_cache_flushed():
+		 *       - channel->timer_lock
+		 *         - channel->metadata_cache->lock
+		 *
+		 * Ensure that neither consumer_data.lock nor
+		 * channel->lock are taken within this function, since
+		 * they are held while consumer_timer_switch_stop() is
+		 * called.
+		 */
+		ret = lttng_ustconsumer_request_metadata(ctx, channel, 1);
 		if (ret < 0) {
 			channel->switch_timer_error = 1;
 		}
@@ -81,10 +104,59 @@ static void metadata_switch_timer(struct lttng_consumer_local_data *ctx,
 	}
 }
 
+static
+void consumer_timer_signal_thread_qs(unsigned int signr)
+{
+	sigset_t pending_set;
+	int ret;
+
+	/*
+	 * We need to be the only thread interacting with the thread
+	 * that manages signals for teardown synchronization.
+	 */
+	pthread_mutex_lock(&timer_signal.lock);
+
+	/* Ensure we don't have any signal queued for this channel. */
+	for (;;) {
+		ret = sigemptyset(&pending_set);
+		if (ret == -1) {
+			PERROR("sigemptyset");
+		}
+		ret = sigpending(&pending_set);
+		if (ret == -1) {
+			PERROR("sigpending");
+		}
+		if (!sigismember(&pending_set, LTTNG_CONSUMER_SIG_SWITCH)) {
+			break;
+		}
+		caa_cpu_relax();
+	}
+
+	/*
+	 * From this point, no new signal handler will be fired that would try to
+	 * access "chan". However, we still need to wait for any currently
+	 * executing handler to complete.
+	 */
+	cmm_smp_mb();
+	CMM_STORE_SHARED(timer_signal.qs_done, 0);
+	cmm_smp_mb();
+
+	/*
+	 * Kill with LTTNG_CONSUMER_SIG_TEARDOWN, so signal management thread wakes
+	 * up.
+	 */
+	kill(getpid(), LTTNG_CONSUMER_SIG_TEARDOWN);
+
+	while (!CMM_LOAD_SHARED(timer_signal.qs_done)) {
+		caa_cpu_relax();
+	}
+	cmm_smp_mb();
+
+	pthread_mutex_unlock(&timer_signal.lock);
+}
+
 /*
  * Set the timer for periodical metadata flush.
- * Should be called only from the recv cmd thread (single thread ensures
- * mutual exclusion).
  */
 void consumer_timer_switch_start(struct lttng_consumer_channel *channel,
 		unsigned int switch_timer_interval)
@@ -122,13 +194,10 @@ void consumer_timer_switch_start(struct lttng_consumer_channel *channel,
 
 /*
  * Stop and delete timer.
- * Should be called only from the recv cmd thread (single thread ensures
- * mutual exclusion).
  */
 void consumer_timer_switch_stop(struct lttng_consumer_channel *channel)
 {
 	int ret;
-	sigset_t pending_set;
 
 	assert(channel);
 
@@ -137,41 +206,10 @@ void consumer_timer_switch_stop(struct lttng_consumer_channel *channel)
 		PERROR("timer_delete");
 	}
 
-	/* Ensure we don't have any signal queued for this channel. */
-	for (;;) {
-		ret = sigemptyset(&pending_set);
-		if (ret == -1) {
-			PERROR("sigemptyset");
-		}
-		ret = sigpending(&pending_set);
-		if (ret == -1) {
-			PERROR("sigpending");
-		}
-		if (!sigismember(&pending_set, LTTNG_CONSUMER_SIG_SWITCH)) {
-			break;
-		}
-		caa_cpu_relax();
-	}
-
-	/*
-	 * From this point, no new signal handler will be fired that would try to
-	 * access "chan". However, we still need to wait for any currently
-	 * executing handler to complete.
-	 */
-	cmm_smp_mb();
-	CMM_STORE_SHARED(timer_signal.qs_done, 0);
-	cmm_smp_mb();
-
-	/*
-	 * Kill with LTTNG_CONSUMER_SIG_TEARDOWN, so signal management thread wakes
-	 * up.
-	 */
-	kill(getpid(), LTTNG_CONSUMER_SIG_TEARDOWN);
+	consumer_timer_signal_thread_qs(LTTNG_CONSUMER_SIG_SWITCH);
 
-	while (!CMM_LOAD_SHARED(timer_signal.qs_done)) {
-		caa_cpu_relax();
-	}
-	cmm_smp_mb();
+	channel->switch_timer = 0;
+	channel->switch_timer_enabled = 0;
 }
 
 /*