[ltt-dev] [PATCH] ltt-tracer : fix timer

Mathieu Desnoyers compudj at krystal.dyndns.org
Fri Aug 15 14:49:46 EDT 2008


François Prenoveau found implementation problems with the timers in LTTng. I
when through the patch he submitted and reviewed the whole timer usage in LTTng
to fix it, both in the heartbeat and the tracer code.

Basically, the timers should be started at the creation of the first trace,
stay untouched until the last trace is destroyed.

The previous use was somewhat inconsistent, with timers being started at trace
_start_ and deleted (without proper sync) at trace destroy.

This patch applies to LTTng 0.16 and prior.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at polymtl.ca>
CC: François Prenoveau <francois.prenoveau at polymtl.ca>
---
 ltt/ltt-heartbeat.c |   11 +++----
 ltt/ltt-tracer.c    |   73 ++++++++++++++++++++++++----------------------------
 2 files changed, 39 insertions(+), 45 deletions(-)

Index: linux-2.6-lttng/ltt/ltt-tracer.c
===================================================================
--- linux-2.6-lttng.orig/ltt/ltt-tracer.c	2008-08-15 12:32:40.000000000 -0400
+++ linux-2.6-lttng/ltt/ltt-tracer.c	2008-08-15 12:34:49.000000000 -0400
@@ -42,7 +42,9 @@
 #include <linux/kref.h>
 #include <asm/atomic.h>
 
-static struct timer_list ltt_async_wakeup_timer;
+static void async_wakeup(unsigned long data);
+
+static DEFINE_TIMER(ltt_async_wakeup_timer, async_wakeup, 0, 0);
 
 /* Default callbacks for modules */
 int ltt_filter_control_default
@@ -249,9 +251,7 @@ static void async_wakeup(unsigned long d
 	}
 	rcu_read_unlock_sched();
 
-	del_timer(&ltt_async_wakeup_timer);
-	ltt_async_wakeup_timer.expires = jiffies + LTT_PERCPU_TIMER_INTERVAL;
-	add_timer(&ltt_async_wakeup_timer);
+	mod_timer(&ltt_async_wakeup_timer, jiffies + LTT_PERCPU_TIMER_INTERVAL);
 }
 
 
@@ -299,12 +299,29 @@ static int _ltt_trace_create(char *trace
 		err = EEXIST;
 		goto traces_error;
 	}
+	if (list_empty(&ltt_traces.head)) {
+		probe_id_defrag();
+#ifdef CONFIG_LTT_HEARTBEAT
+		if (ltt_heartbeat_trigger(LTT_HEARTBEAT_START)) {
+			err = ENODEV;
+			printk(KERN_ERR
+				"LTT : Heartbeat timer module not present.\n");
+			goto ltt_heartbeat_error;
+		}
+#endif
+		mod_timer(&ltt_async_wakeup_timer,
+				jiffies + LTT_PERCPU_TIMER_INTERVAL);
+		set_kernel_trace_flag_all_tasks();
+	}
 	list_add_rcu(&new_trace->list, &ltt_traces.head);
 	synchronize_sched();
 	/* Everything went fine, finish creation */
 	return 0;
 
 	/* Error handling */
+#ifdef CONFIG_LTT_HEARTBEAT
+ltt_heartbeat_error:
+#endif
 traces_error:
 	return err;
 }
@@ -545,6 +562,19 @@ static int _ltt_trace_destroy(struct ltt
 	/* Everything went fine */
 	list_del_rcu(&trace->list);
 	synchronize_sched();
+	if (list_empty(&ltt_traces.head)) {
+		clear_kernel_trace_flag_all_tasks();
+		/*
+		 * We stop the asynchronous delivery of reader wakeup, but
+		 * we must make one last check for reader wakeups pending
+		 * later in __ltt_trace_destroy.
+		 */
+		del_timer_sync(&ltt_async_wakeup_timer);
+#ifdef CONFIG_LTT_HEARTBEAT
+		/* stop the heartbeat if we are the last trace */
+		ltt_heartbeat_trigger(LTT_HEARTBEAT_STOP);
+#endif
+	}
 	return 0;
 
 	/* error handling */
@@ -568,13 +598,6 @@ static void __ltt_trace_destroy(struct l
 
 	flush_scheduled_work();
 
-	if (ltt_traces.num_active_traces == 0) {
-		/*
-		 * We stop the asynchronous delivery of reader wakeup, but
-		 * we must make one last check for reader wakeups pending.
-		 */
-		del_timer(&ltt_async_wakeup_timer);
-	}
 	/*
 	 * The currently destroyed trace is not in the trace list anymore,
 	 * so it's safe to call the async wakeup ourself. It will deliver
@@ -636,23 +659,6 @@ static int _ltt_trace_start(struct ltt_t
 		printk(KERN_ERR "LTT : Can't lock filter module.\n");
 		goto get_ltt_run_filter_error;
 	}
-	if (ltt_traces.num_active_traces == 0) {
-		probe_id_defrag();
-#ifdef CONFIG_LTT_HEARTBEAT
-		if (ltt_heartbeat_trigger(LTT_HEARTBEAT_START)) {
-			err = ENODEV;
-			printk(KERN_ERR
-				"LTT : Heartbeat timer module not present.\n");
-			goto ltt_heartbeat_error;
-		}
-#endif
-		init_timer(&ltt_async_wakeup_timer);
-		ltt_async_wakeup_timer.function = async_wakeup;
-		ltt_async_wakeup_timer.expires =
-			jiffies + LTT_PERCPU_TIMER_INTERVAL;
-		add_timer(&ltt_async_wakeup_timer);
-		set_kernel_trace_flag_all_tasks();
-	}
 	/*
 	 * Write a full 64 bits TSC in each trace. Used for successive
 	 * trace stop/start.
@@ -664,10 +670,6 @@ static int _ltt_trace_start(struct ltt_t
 	return err;
 
 	/* error handling */
-#ifdef CONFIG_LTT_HEARTBEAT
-ltt_heartbeat_error:
-#endif
-	module_put(ltt_run_filter_owner);
 get_ltt_run_filter_error:
 traces_error:
 	return err;
@@ -732,13 +734,6 @@ static int _ltt_trace_stop(struct ltt_tr
 		ltt_traces.num_active_traces--;
 		synchronize_sched(); /* Wait for each tracing to be finished */
 	}
-	if (ltt_traces.num_active_traces == 0) {
-		clear_kernel_trace_flag_all_tasks();
-#ifdef CONFIG_LTT_HEARTBEAT
-	/* stop the heartbeat if we are the last active trace */
-		ltt_heartbeat_trigger(LTT_HEARTBEAT_STOP);
-#endif
-	}
 	module_put(ltt_run_filter_owner);
 	/* Everything went fine */
 	return 0;
Index: linux-2.6-lttng/ltt/ltt-heartbeat.c
===================================================================
--- linux-2.6-lttng.orig/ltt/ltt-heartbeat.c	2008-08-15 12:32:40.000000000 -0400
+++ linux-2.6-lttng/ltt/ltt-heartbeat.c	2008-08-15 12:33:29.000000000 -0400
@@ -31,7 +31,10 @@
 /* Expected maximum interrupt latency in ms : 15ms, *2 for security */
 #define EXPECTED_INTERRUPT_LATENCY	30
 
+static void heartbeat_timer_fct(unsigned long data);
+
 static struct timer_list heartbeat_timer;
+static DEFINE_TIMER(heartbeat_timer, heartbeat_timer_fct, 0, 0);
 static unsigned int precalc_heartbeat_expire;
 
 int ltt_compact_data_shift;
@@ -190,11 +193,7 @@ static void start_heartbeat_timer(void)
 {
 	if (precalc_heartbeat_expire > 0) {
 		printk(KERN_DEBUG "LTT : ltt-heartbeat start\n");
-
-		init_timer(&heartbeat_timer);
-		heartbeat_timer.function = heartbeat_timer_fct;
-		heartbeat_timer.expires = jiffies + precalc_heartbeat_expire;
-		add_timer(&heartbeat_timer);
+		mod_timer(&heartbeat_timer, jiffies + precalc_heartbeat_expire);
 	} else
 		printk(KERN_WARNING
 			"LTT: no tsc for heartbeat timer "
@@ -208,7 +207,7 @@ static void stop_heartbeat_timer(void)
 {
 	if (loops_per_jiffy > 0) {
 		printk(KERN_DEBUG "LTT : ltt-heartbeat stop\n");
-		del_timer(&heartbeat_timer);
+		del_timer_sync(&heartbeat_timer);
 	}
 }
 
-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68




More information about the lttng-dev mailing list