Skip to contentSkip to conversation

Markers and Tracepoints Updates for -tip

PATCH In review Mathieu Desnoyers 5 participants 30 messages Nov 14, 2008 – Nov 25, 2008

Subsystems (7)
lore
Share & tools
lore
#0 lore raw

cover Markers and Tracepoints Updates for -tip

Hi Ingo,

Those are the markers and tracepoint updates for -tip. I have put the "bugfix" patches at the beginning (Impact: bugfix) followed by cleanups and by API changes.

Mathieu

Signature · 2 lines
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#1 lore raw

PATCH 1/16 RCU read sched notrace

Add _notrace version to rcu_read_*_sched().

Impact: new API, useful for tracepoints and markers.

CC Peter Zijlstra <[email protected]>
CC Paul E McKenney <[email protected]>
CC 'Ingo Molnar' <[email protected]>
---
 include/linux/rcupdate.h |    2 ++
 1 file changed, 2 insertions(+)
include/linux/rcupdate.h +2 -0
Index: linux-2.6-lttng/include/linux/rcupdate.h
===================================================================
@@ -142,6 +142,7 @@ struct rcu_head {
  * on the write-side to insure proper synchronization.
  */
 #define rcu_read_lock_sched() preempt_disable()
+#define rcu_read_lock_sched_notrace() preempt_disable_notrace()
 
 /*
  * rcu_read_unlock_sched - marks the end of a RCU-classic critical section
@@ -149,6 +150,7 @@ struct rcu_head {
  * See rcu_read_lock_sched for more information.
  */
 #define rcu_read_unlock_sched() preempt_enable()
+#define rcu_read_unlock_sched_notrace() preempt_enable_notrace()
 
 
 
Signature · 2 lines
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
Reviewed-by on v1 1/16
#2 lore raw

Re: [patch 01/16] RCU read sched notrace

Quoted 3 lines · Mathieu Desnoyers

Add _notrace version to rcu_read_*_sched().

Impact: new API, useful for tracepoints and markers.

Looks good to me. Just don't try it with rcu_read_lock(), given preemptable RCU. ;-)

Quoted 33 lines
CC Peter Zijlstra <[email protected]>
CC Paul E McKenney <[email protected]>
CC 'Ingo Molnar' <[email protected]>
---
 include/linux/rcupdate.h |    2 ++
 1 file changed, 2 insertions(+)

Index: linux-2.6-lttng/include/linux/rcupdate.h
===================================================================

include/linux/rcupdate.h +2 -0
@@ -142,6 +142,7 @@ struct rcu_head {
  * on the write-side to insure proper synchronization.
  */
 #define rcu_read_lock_sched() preempt_disable()
+#define rcu_read_lock_sched_notrace() preempt_disable_notrace()
 
 /*
  * rcu_read_unlock_sched - marks the end of a RCU-classic critical section
@@ -149,6 +150,7 @@ struct rcu_head {
  * See rcu_read_lock_sched for more information.
  */
 #define rcu_read_unlock_sched() preempt_enable()
+#define rcu_read_unlock_sched_notrace() preempt_enable_notrace()
 
 
 
Signature · 3 lines
-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#3 lore raw

PATCH 2/16 Marker fix unregister

In reply to Mathieu Desnoyers

get_marker() can return a NULL entry because the mutex is released in the middle of those functions. Make sure we check to see if it has been concurrently removed.

Impact: Bugfix.

---
 kernel/marker.c |   12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
kernel/marker.c +7 -5
Index: linux.trees.git/kernel/marker.c
===================================================================
@@ -653,10 +653,11 @@ int marker_probe_register(const char *na
 		goto end;
 	}
 	mutex_unlock(&markers_mutex);
-	marker_update_probes();		/* may update entry */
+	marker_update_probes();
 	mutex_lock(&markers_mutex);
 	entry = get_marker(name);
-	WARN_ON(!entry);
+	if (!entry)
+		goto end;
 	if (entry->rcu_pending)
 		rcu_barrier_sched();
 	entry->oldptr = old;
@@ -697,7 +698,7 @@ int marker_probe_unregister(const char *
 		rcu_barrier_sched();
 	old = marker_entry_remove_probe(entry, probe, probe_private);
 	mutex_unlock(&markers_mutex);
-	marker_update_probes();		/* may update entry */
+	marker_update_probes();
 	mutex_lock(&markers_mutex);
 	entry = get_marker(name);
 	if (!entry)
@@ -778,10 +779,11 @@ int marker_probe_unregister_private_data
 		rcu_barrier_sched();
 	old = marker_entry_remove_probe(entry, NULL, probe_private);
 	mutex_unlock(&markers_mutex);
-	marker_update_probes();		/* may update entry */
+	marker_update_probes();
 	mutex_lock(&markers_mutex);
 	entry = get_marker_from_private_data(probe, probe_private);
-	WARN_ON(!entry);
+	if (!entry)
+		goto end;
 	if (entry->rcu_pending)
 		rcu_barrier_sched();
 	entry->oldptr = old;
Signature · 2 lines
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#4 lore raw

PATCH 3/16 markers: Add missing stdargs.h include, needed due to va_list usage

In reply to Mathieu Desnoyers

That seemed to cause built issue when marker.h is included early, even though stdargs.h is included in kernel.h.

Impact: bugfix.

From: Arnaldo Carvalho de Melo <[email protected]>

CC 'Ingo Molnar' <[email protected]>
CC Lai Jiangshan <[email protected]>
---
 include/linux/marker.h |    1 +
 1 file changed, 1 insertion(+)
include/linux/marker.h +1 -0
Index: linux-2.6-lttng/include/linux/marker.h
===================================================================
@@ -12,6 +12,7 @@
  * See the file COPYING for more details.
  */
 
+#include <stdarg.h>
 #include <linux/types.h>
 
 struct module;
Signature · 2 lines
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#5 lore raw

PATCH 4/16 Markers use rcu_*_sched_notrace and notrace

In reply to Mathieu Desnoyers

Make marker critical code use notrace (__attribute__((no_instrument_function)) to make sure they can be used as an ftrace callback.

CC Andrew Morton <[email protected]>
CC Masami Hiramatsu <[email protected]>
CC 'Peter Zijlstra' <[email protected]>
CC "Frank Ch. Eigler" <[email protected]>
CC 'Ingo Molnar' <[email protected]>
CC Lai Jiangshan <[email protected]>
CC 'Hideo AOKI' <[email protected]>
CC Takashi Nishiie <[email protected]>
CC 'Steven Rostedt' <[email protected]>
---
 kernel/marker.c |   16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)
kernel/marker.c +9 -7
Index: linux-2.6-lttng/kernel/marker.c
===================================================================
@@ -81,7 +81,7 @@ struct marker_entry {
  * though the function pointer change and the marker enabling are two distinct
  * operations that modifies the execution flow of preemptible code.
  */
-void __mark_empty_function(void *probe_private, void *call_private,
+notrace void __mark_empty_function(void *probe_private, void *call_private,
 	const char *fmt, va_list *args)
 {
 }
@@ -97,7 +97,8 @@ EXPORT_SYMBOL_GPL(__mark_empty_function)
  * need to put a full smp_rmb() in this branch. This is why we do not use
  * rcu_dereference() for the pointer read.
  */
-void marker_probe_cb(const struct marker *mdata, void *call_private, ...)
+notrace void marker_probe_cb(const struct marker *mdata,
+		void *call_private, ...)
 {
 	va_list args;
 	char ptype;
@@ -107,7 +108,7 @@ void marker_probe_cb(const struct marker
 	 * sure the teardown of the callbacks can be done correctly when they
 	 * are in modules and they insure RCU read coherency.
 	 */
-	rcu_read_lock_sched();
+	rcu_read_lock_sched_notrace();
 	ptype = mdata->ptype;
 	if (likely(!ptype)) {
 		marker_probe_func *func;
@@ -145,7 +146,7 @@ void marker_probe_cb(const struct marker
 			va_end(args);
 		}
 	}
-	rcu_read_unlock_sched();
+	rcu_read_unlock_sched_notrace();
 }
 EXPORT_SYMBOL_GPL(marker_probe_cb);
 
@@ -157,12 +158,13 @@ EXPORT_SYMBOL_GPL(marker_probe_cb);
  *
  * Should be connected to markers "MARK_NOARGS".
  */
-static void marker_probe_cb_noarg(const struct marker *mdata, void *call_private, ...)
+static notrace void marker_probe_cb_noarg(const struct marker *mdata,
+		void *call_private, ...)
 {
 	va_list args;	/* not initialized */
 	char ptype;
 
-	rcu_read_lock_sched();
+	rcu_read_lock_sched_notrace();
 	ptype = mdata->ptype;
 	if (likely(!ptype)) {
 		marker_probe_func *func;
@@ -195,7 +197,7 @@ static void marker_probe_cb_noarg(const 
 			multi[i].func(multi[i].probe_private, call_private,
 				mdata->format, &args);
 	}
-	rcu_read_unlock_sched();
+	rcu_read_unlock_sched_notrace();
 }
 
 static void free_old_closure(struct rcu_head *head)
Signature · 2 lines
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#6 lore raw

PATCH 5/16 Markers use module notifier

In reply to Mathieu Desnoyers

Use module notifiers instead of adding a hook in module.c.

Impact: cleanup.

CC 'Ingo Molnar' <[email protected]>
CC Lai Jiangshan <[email protected]>
---
 kernel/marker.c |   29 +++++++++++++++++++++++++++++
 kernel/module.c |    4 ----
 2 files changed, 29 insertions(+), 4 deletions(-)
kernel/marker.c +29 -0
Index: linux.trees.git/kernel/marker.c
===================================================================
@@ -846,3 +846,32 @@ void *marker_get_private_data(const char
 	return ERR_PTR(-ENOENT);
 }
 EXPORT_SYMBOL_GPL(marker_get_private_data);
+
+int marker_module_notify(struct notifier_block *self,
+			 unsigned long val, void *data)
+{
+	struct module *mod = data;
+
+	switch (val) {
+	case MODULE_STATE_COMING:
+		marker_update_probe_range(mod->markers,
+			mod->markers + mod->num_markers);
+		break;
+	case MODULE_STATE_GOING:
+		marker_update_probe_range(mod->markers,
+			mod->markers + mod->num_markers);
+		break;
+	}
+	return 0;
+}
+
+struct notifier_block marker_module_nb = {
+	.notifier_call = marker_module_notify,
+	.priority = 0,
+};
+
+static int init_markers(void)
+{
+	return register_module_notifier(&marker_module_nb);
+}
+__initcall(init_markers);
kernel/module.c +0 -4
Index: linux.trees.git/kernel/module.c
===================================================================
@@ -2185,10 +2185,6 @@ static noinline struct module *load_modu
 		struct mod_debug *debug;
 		unsigned int num_debug;
 
-#ifdef CONFIG_MARKERS
-		marker_update_probe_range(mod->markers,
-			mod->markers + mod->num_markers);
-#endif
 		debug = section_objs(hdr, sechdrs, secstrings, "__verbose",
 				     sizeof(*debug), &num_debug);
 		dynamic_printk_setup(debug, num_debug);
Signature · 2 lines
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#7 lore raw

PATCH 1/1 markers/tracpoints: fix non-modular build

Quoted 7 lines · Mathieu Desnoyers

Use module notifiers instead of adding a hook in module.c.

Impact: cleanup.

CC 'Ingo Molnar' <[email protected]>
CC Lai Jiangshan <[email protected]>
Quoted 6 lines
---
 kernel/marker.c |   29 +++++++++++++++++++++++++++++
 kernel/module.c |    4 ----
 2 files changed, 29 insertions(+), 4 deletions(-)

From 227a837567e339c74d9d4243d03a29bd943a018c Mon Sep 17 00:00:00 2001

From: Ingo Molnar <[email protected]>
Date: Sun, 16 Nov 2008 09:50:34 +0100
Subject: [PATCH] markers/tracpoints: fix non-modular build

fix:

kernel/marker.c: In function 'marker_module_notify':
kernel/marker.c:905: error: 'MODULE_STATE_COMING' undeclared (first use in this function) [...]

---
 kernel/marker.c     |    4 ++++
 kernel/tracepoint.c |    4 ++++
 2 files changed, 8 insertions(+), 0 deletions(-)
kernel/marker.c +4 -0
index c14ec26..ea54f26 100644
@@ -896,6 +896,8 @@ void *marker_get_private_data(const char *name, marker_probe_func *probe,
 }
 EXPORT_SYMBOL_GPL(marker_get_private_data);
 
+#ifdef CONFIG_MODULES
+
 int marker_module_notify(struct notifier_block *self,
 			 unsigned long val, void *data)
 {
@@ -924,3 +926,5 @@ static int init_markers(void)
 	return register_module_notifier(&marker_module_nb);
 }
 __initcall(init_markers);
+
+#endif /* CONFIG_MODULES */
kernel/tracepoint.c +4 -0
index 94ac4e3..7960274 100644
@@ -542,6 +542,8 @@ void tracepoint_iter_reset(struct tracepoint_iter *iter)
 }
 EXPORT_SYMBOL_GPL(tracepoint_iter_reset);
 
+#ifdef CONFIG_MODULES
+
 int tracepoint_module_notify(struct notifier_block *self,
 			     unsigned long val, void *data)
 {
@@ -570,3 +572,5 @@ static int init_tracepoints(void)
 	return register_module_notifier(&tracepoint_module_nb);
 }
 __initcall(init_tracepoints);
+
+#endif /* CONFIG_MODULES */
author Acked-by on v1 1/1
#8 lore raw

Re: [PATCH] markers/tracpoints: fix non-modular build

Quoted 20 lines · * Ingo Molnar ([email protected]) wrote:
Quoted 11 lines · Mathieu Desnoyers

Use module notifiers instead of adding a hook in module.c.

Impact: cleanup.

CC 'Ingo Molnar' <[email protected]>
CC Lai Jiangshan <[email protected]>
---
 kernel/marker.c |   29 +++++++++++++++++++++++++++++
 kernel/module.c |    4 ----
 2 files changed, 29 insertions(+), 4 deletions(-)

From 227a837567e339c74d9d4243d03a29bd943a018c Mon Sep 17 00:00:00 2001
From: Ingo Molnar <[email protected]>
Date: Sun, 16 Nov 2008 09:50:34 +0100
Subject: [PATCH] markers/tracpoints: fix non-modular build

Quoted 7 lines

fix:

kernel/marker.c: In function 'marker_module_notify':
kernel/marker.c:905: error: 'MODULE_STATE_COMING' undeclared (first use in this function) [...]

Yes, that's needed. Thanks for preparing this patch.

Quoted 43 lines
---
 kernel/marker.c     |    4 ++++
 kernel/tracepoint.c |    4 ++++
 2 files changed, 8 insertions(+), 0 deletions(-)
kernel/marker.c +4 -0
index c14ec26..ea54f26 100644
@@ -896,6 +896,8 @@ void *marker_get_private_data(const char *name, marker_probe_func *probe,
 }
 EXPORT_SYMBOL_GPL(marker_get_private_data);
 
+#ifdef CONFIG_MODULES
+
 int marker_module_notify(struct notifier_block *self,
 			 unsigned long val, void *data)
 {
@@ -924,3 +926,5 @@ static int init_markers(void)
 	return register_module_notifier(&marker_module_nb);
 }
 __initcall(init_markers);
+
+#endif /* CONFIG_MODULES */
kernel/tracepoint.c +4 -0
index 94ac4e3..7960274 100644
@@ -542,6 +542,8 @@ void tracepoint_iter_reset(struct tracepoint_iter *iter)
 }
 EXPORT_SYMBOL_GPL(tracepoint_iter_reset);
 
+#ifdef CONFIG_MODULES
+
 int tracepoint_module_notify(struct notifier_block *self,
 			     unsigned long val, void *data)
 {
@@ -570,3 +572,5 @@ static int init_tracepoints(void)
 	return register_module_notifier(&tracepoint_module_nb);
 }
 __initcall(init_tracepoints);
+
+#endif /* CONFIG_MODULES */
Signature · 2 lines
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#9 lore raw

PATCH 6/16 Markers auto enable tracepoints (new API : trace_mark_tp())

In reply to Mathieu Desnoyers

Add a new API trace_mark_tp(), which declares a marker within a tracepoint probe. When the marker is activated, the tracepoint is automatically enabled.

No branch test is used at the marker site, because it would be a duplicate of the branch already present in the tracepoint.

Impact: new API.

CC 'Ingo Molnar' <[email protected]>
CC Lai Jiangshan <[email protected]>
---
 include/linux/marker.h |   45 ++++++++++++++++++++++++++++++++++++++++-
 init/Kconfig           |    1 
 kernel/marker.c        |   53 +++++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 96 insertions(+), 3 deletions(-)
include/linux/marker.h +44 -1
Index: linux.trees.git/include/linux/marker.h
===================================================================
@@ -49,6 +49,8 @@ struct marker {
 	void (*call)(const struct marker *mdata, void *call_private, ...);
 	struct marker_probe_closure single;
 	struct marker_probe_closure *multi;
+	const char *tp_name;	/* Optional tracepoint name */
+	void *tp_cb;		/* Optional tracepoint callback */
 } __attribute__((aligned(8)));
 
 #ifdef CONFIG_MARKERS
@@ -73,7 +75,7 @@ struct marker {
 		__attribute__((section("__markers"), aligned(8))) =	\
 		{ __mstrtab_##name, &__mstrtab_##name[sizeof(#name)],	\
 		0, 0, marker_probe_cb,					\
-		{ __mark_empty_function, NULL}, NULL };			\
+		{ __mark_empty_function, NULL}, NULL, NULL, NULL };	\
 		__mark_check_format(format, ## args);			\
 		if (unlikely(__mark_##name.state)) {			\
 			(*__mark_##name.call)				\
@@ -81,11 +83,38 @@ struct marker {
 		}							\
 	} while (0)
 
+#define __trace_mark_tp(name, call_private, tp_name, tp_cb, format, args...) \
+	do {								\
+		void __check_tp_type(void)				\
+		{							\
+			register_trace_##tp_name(tp_cb);		\
+		}							\
+		static const char __mstrtab_##name[]			\
+		__attribute__((section("__markers_strings")))		\
+		= #name "\0" format;					\
+		static struct marker __mark_##name			\
+		__attribute__((section("__markers"), aligned(8))) =	\
+		{ __mstrtab_##name, &__mstrtab_##name[sizeof(#name)],	\
+		0, 0, marker_probe_cb,					\
+		{ __mark_empty_function, NULL}, NULL, #tp_name, tp_cb };\
+		__mark_check_format(format, ## args);			\
+		(*__mark_##name.call)(&__mark_##name, call_private,	\
+					## args);			\
+	} while (0)
+
 extern void marker_update_probe_range(struct marker *begin,
 	struct marker *end);
 #else /* !CONFIG_MARKERS */
 #define __trace_mark(generic, name, call_private, format, args...) \
 		__mark_check_format(format, ## args)
+#define __trace_mark_tp(name, call_private, tp_name, tp_cb, format, args...) \
+	do {								\
+		void __check_tp_type(void)				\
+		{							\
+			register_trace_##tp_name(tp_cb);		\
+		}							\
+		__mark_check_format(format, ## args);			\
+	} while (0)
 static inline void marker_update_probe_range(struct marker *begin,
 	struct marker *end)
 { }
@@ -118,6 +147,20 @@ static inline void marker_update_probe_r
 	__trace_mark(1, name, NULL, format, ## args)
 
 /**
+ * trace_mark_tp - Marker in a tracepoint callback
+ * @name: marker name, not quoted.
+ * @tp_name: tracepoint name, not quoted.
+ * @tp_cb: tracepoint callback. Should have an associated global symbol so it
+ *         is not optimized away by the compiler (should not be static).
+ * @format: format string
+ * @args...: variable argument list
+ *
+ * Places a marker in a tracepoint callback.
+ */
+#define trace_mark_tp(name, tp_name, tp_cb, format, args...)	\
+	__trace_mark_tp(name, NULL, tp_name, tp_cb, format, ## args)
+
+/**
  * MARK_NOARGS - Format string for a marker with no argument.
  */
 #define MARK_NOARGS " "
kernel/marker.c +51 -2

80 lines not on this page · raw message

init/Kconfig +1 -0
Index: linux.trees.git/init/Kconfig
===================================================================
@@ -808,6 +808,7 @@ config TRACEPOINTS
 
 config MARKERS
 	bool "Activate markers"
+	depends on TRACEPOINTS
 	help
 	  Place an empty function call at each marker site. Can be
 	  dynamically changed for a probe function.
Signature · 2 lines
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#10 lore raw

Re: [patch 06/16] Markers auto enable tracepoints (new API : trace_mark_tp())

Quoted 8 lines · Mathieu Desnoyers

Add a new API trace_mark_tp(), which declares a marker within a tracepoint probe. When the marker is activated, the tracepoint is automatically enabled.

No branch test is used at the marker site, because it would be a duplicate of the branch already present in the tracepoint.

Impact: new API.

i dont know.

I was actually hoping for markers (the in-kernel API) to go away completely - and be replaced with tracepoints.

Markers are the wrong design on several levels. They couple the kernel dynamically with unknown (to the kernel) entities - and that is causing complexity all around the place, clearly expressed in these patches of yours.

Tracepoints are much more specific - typed and enumerated function call callback points in essence - with some politeness that allows external instrumentation entities to attach to those callbacks.

Is there any usecase of markers that should not be converted to either tracepoints or to ftrace_printk() ?

Ingo

#11 lore raw

Re: [patch 06/16] Markers auto enable tracepoints (new API : trace_mark_tp())

Quoted 22 lines · * Ingo Molnar ([email protected]) wrote:
Quoted 8 lines · Mathieu Desnoyers

Add a new API trace_mark_tp(), which declares a marker within a tracepoint probe. When the marker is activated, the tracepoint is automatically enabled.

No branch test is used at the marker site, because it would be a duplicate of the branch already present in the tracepoint.

Impact: new API.

i dont know.

I was actually hoping for markers (the in-kernel API) to go away completely - and be replaced with tracepoints.

Markers are the wrong design on several levels. They couple the kernel dynamically with unknown (to the kernel) entities - and that is causing complexity all around the place, clearly expressed in these patches of yours.

Quoted 8 lines

Tracepoints are much more specific - typed and enumerated function call callback points in essence - with some politeness that allows external instrumentation entities to attach to those callbacks.

Is there any usecase of markers that should not be converted to either tracepoints or to ftrace_printk() ?

Ingo

I'll try to explain the common use-case I have in mind. I think starting from that we'll be able to better understand what pieces of tracepoints and markers are useful, and which purpose they fulfill. I have other use-cases in mind too, but for sake of clarity, let's begin with this subset.

Tracepoints instrument the kernel. They identify code locations and extract the state of pre-identified interesting variables at these locations. They are built into the kernel. They are closely tied to kernel internals, and hence only provide an in-kernel API.

Markers identify the name (and therefore numeric ID) to attach to an "event" and the data types to export into trace buffers for this specific event type. These data types are fully expressed in a marker format-string table recorded in a "metadata" channel. The size of the various basic types and the endianness is recorded in the buffer header. Therefore, the binary trace buffers are self-described.

Data is exported through binary trace buffers out of kernel-space, either by writing directly to disk, sending data over the network, crash dump extraction, etc.

User-space applications, which run on an architecture with potentially different endianness and different type sizes, reads the binary buffers. Therefore, those buffers must be fully self-described so the application can read them portably (or just on 32-bits userspace running under a 64-bits kernel).

In debug-mode tracing (where the developer wants to add ftrace_printk()-like statements in its kernel and recompile), the extracted information should be made available through the same binary buffers where the information extracted from the tracepoints is saved. Adding printk-like statements to the kernel should not suffer from the time and buffer space penality of expanding the raw data to text. However, ftrace_printk() requires to pretty-print the data in text. It would be too cumbersome to ask developer to deploy their own tracepoints whenever they want to create ad-hoc debug-style tracing statements. This is where trace_mark() statements do better than ftrace_printk() by allowing to dump the information to trace buffers in binary format through a simple to use format-string based macro. Note that each marker format string is dumped in the metadata table to identify the new event types.

So that was my use-case which I think cannot be converted to tracepoints nor ftrace_print(). If things are not as clear as they should be, don't hesitate to ask for clarifications.

I also think we might consider removing the "multiple callback support" from markers if we tie them more closely to the data extraction code, given this multiple-callback role is already fulfilled by tracepoints. OTOH, given my use-case can use markers as information source, thus bypassing tracepoints for debug-style tracing, we may still need to connect more than a single probe to the markers. I becomes very useful for use-cases such as : dumping the kernel or userspace stack, reading the performance counters and activating/de-activating the function tracer at specific instrumentation sites.

Mathieu

Signature · 2 lines
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#12 lore raw

Re: [patch 06/16] Markers auto enable tracepoints (new API : trace_mark_tp())

Quoted 11 lines · Mathieu Desnoyers

Markers identify the name (and therefore numeric ID) to attach to an "event" and the data types to export into trace buffers for this specific event type. These data types are fully expressed in a marker format-string table recorded in a "metadata" channel. The size of the various basic types and the endianness is recorded in the buffer header. Therefore, the binary trace buffers are self-described.

Data is exported through binary trace buffers out of kernel-space, either by writing directly to disk, sending data over the network, crash dump extraction, etc.

Streaming gigabytes of data is really mostly only done when we know _nothing_ useful about a failure mode and are _forced_ into logging gobs and gobs of data at great expense.

And thus in reality this is a rather uninteresting usecase.

We do recognize and support it as it's a valid "last line of defense" for system and application failure analysis, but we should also put it all into proper perspective: it's the rare and abnormal exception, not the design target.

Note that we support this mode of tracing today already: we can already stream binary data via the ftrace channel - the ring buffer gives the infrastructure for that. Just do:

  # echo bin > /debug/tracing/trace_options

... and you'll get the trace data streamed to user-space in an efficient, raw, binary data format!

This works here and today - and if you'd like it to become more efficient within the ftrace framework, we are all for it. (It's obviously not the default mode of output, because humans prefer ASCII and scriptable output formats by a _wide_ margin.)

Almost by definition anything opaque and binary-only that goes from the kernel to user-space has fundamental limitations: it just doesnt actively interact with the kernel for us to be able to form a useful and flexible filter of information around it.

The _real_ solution to tracing in 99% of the cases is to intelligently limit information - it's not like the user will read and parse gigabytes of data ...

Look at the myriads of rather useful ftrace plugins we have already and that sprung out of nothing. Compare it to the _10 years_ of inaction that more static tracing concepts created. Those plugins work and spread because it all lives and breathes within the kernel, and almost none of that could be achieved via the 'stream binary data to user-space' model you are concentrating on.

So in the conceptual space i can see little use for markers in the kernel that are not tracepoints (i.e. not actively used by a real tracer). We had markers in the scheduler initially, then we moved to tracepoints - and tracepoints are much nicer.

[ And you wrote both markers and tracepoints, so it's not like i risk
degenerating this discussion into a flamewar by advocating one of your solutions over the other one ;-) ]

... and in that sense i'd love to see lttng become a "super ftrace plugin", and be merged upstream ASAP.

We could even split it up into multiple bits as its merged: for example syscall tracing would be a nice touch that a couple of other plugins would adapt as well. But every tracepoint should have some active role and active connection to a tracer.

And we'd keep all those tracepoints open for external kprobes use as well - for the dynamic tracers, as a low-cost courtesy. (no long-term API guarantees though.)

Hm?

Ingo

#13 lore raw

Re: [patch 06/16] Markers auto enable tracepoints (new API : trace_mark_tp())

Quoted 19 lines · * Ingo Molnar ([email protected]) wrote:
Quoted 11 lines · Mathieu Desnoyers

Markers identify the name (and therefore numeric ID) to attach to an "event" and the data types to export into trace buffers for this specific event type. These data types are fully expressed in a marker format-string table recorded in a "metadata" channel. The size of the various basic types and the endianness is recorded in the buffer header. Therefore, the binary trace buffers are self-described.

Data is exported through binary trace buffers out of kernel-space, either by writing directly to disk, sending data over the network, crash dump extraction, etc.

Streaming gigabytes of data is really mostly only done when we know _nothing_ useful about a failure mode and are _forced_ into logging gobs and gobs of data at great expense.

Quoted 7 lines

And thus in reality this is a rather uninteresting usecase.

We do recognize and support it as it's a valid "last line of defense" for system and application failure analysis, but we should also put it all into proper perspective: it's the rare and abnormal exception, not the design target.

Hrm, the fact that you assume that large data-throughput recording is seldom used shows you have not been in contact with the same user-base I have been interacting with. A few examples of successful LTTng users :

- Google are deploying LTTng on their servers. They want to use it to monitor their production servers (with flight recorder mode tracing) and to help them solve hard to reproduce problems. They have had success with such tracing approach to fix "rare disk delay" issues and VM-related issues presented in this article :

* "Linux Kernel Debugging on Google-sized clusters at Ottawa Linux

    Symposium 2007"
    http://ltt.polymtl.ca/papers/bligh-Reprint.pdf

- IBM Research have had problems with Commercial Scale-out applications, which are being an increasing trend to split large server workloads. They used LTTng successfully to solve a distributed filesystem-related issue. It's presented in the same paper above.

- Autodesk, in the development of their next-generation of Linux audio/video edition applications, used LTTng extensively to solve soft real-time issues they had. Also presented in the same paper.

- Wind River included LTTng in their Linux distribution so their clients, already familiar to Wind River own tracing solution in VxWorks, car have the same kind of feature they have relied on for a long time.

- Montavista have integrated LTTng in their distribution for the same reasons. It's used by Sony amongst others.

- SuSE are currently integrating LTTng in their next SLES distribution, because their clients asking for solutions which supports a kernel closer to real-time need such tools to debug their problems.

- A project between Ericsson, the Canadian Defense, NSERC and various universities is just starting. It aims at monitoring and debugging multi-core systems and provide automated and help user system behavior analysis.

- Siemens have been using LTTng internally for quite some time now.

The wide user-base I have been interacting with, which range from expert developers to lead OS researchers, all agree on the strong need for a tool streaming gigabytes of data, as LTTng does, to help analysing the problem offline. I think that Linux kernel developers might be a bit biased in this aspect, because they happen to know what they are looking for. But users, even experts, often have very few clue where the problem might be in large applications they are developing. And as the number of cores grows and applications are getting larger and more complex, this problem is not likely to lessen.

Quoted 14 lines

Note that we support this mode of tracing today already: we can already stream binary data via the ftrace channel - the ring buffer gives the infrastructure for that. Just do:

  # echo bin > /debug/tracing/trace_options

... and you'll get the trace data streamed to user-space in an efficient, raw, binary data format!

This works here and today - and if you'd like it to become more efficient within the ftrace framework, we are all for it. (It's obviously not the default mode of output, because humans prefer ASCII and scriptable output formats by a _wide_ margin.)

Quoted 5 lines

Almost by definition anything opaque and binary-only that goes from the kernel to user-space has fundamental limitations: it just doesnt actively interact with the kernel for us to be able to form a useful and flexible filter of information around it.

This is the exact reason why I have an elaborated scheme to export binary data to userspace in LTTng. LTTng buffer format is binary-only, but is *not* opaque. It is self-described and portable, and a simple user-space tool can format it to text without much effort.

Quoted 4 lines

The _real_ solution to tracing in 99% of the cases is to intelligently limit information - it's not like the user will read and parse gigabytes of data ...

Yes, limiting the information flow is sometimes required. e.g. we don't want to export lockdep-rate information all the time. However, having enough information within the trace to understand the surroundings of a problematic behavior can greatly help identifying the root cause of the problem. Ideally, having tools which automatically finds the interesting spots in those gigabytes of data (which we have in lttv), and helps representing the information in graphical form (which helps users find execution patterns much more easily.. it's impressive to see how good the human brain can be at pattern-recognition), and lets the user dig in the detailed information located near the problematic execution scenario is of inestimable value.

It has, in many of the cases explained above, led to fix the problematic situation after a few hours with a tracer rather than a few weeks of painful trial-and-error debugging (involving many developers).

Quoted 6 lines

Look at the myriads of rather useful ftrace plugins we have already and that sprung out of nothing. Compare it to the _10 years_ of inaction that more static tracing concepts created. Those plugins work and spread because it all lives and breathes within the kernel, and almost none of that could be achieved via the 'stream binary data to user-space' model you are concentrating on.

It's great that you have such momentum for ftrace, and yes, there is a need for in-kernel analysis, because some workloads might be better suited for in-kernel analysis (potentially because they generate a too-high tracing throughput for the available system resources).

However, the comparison you are doing here is simply unfair. Ftrace has this momentum simply because it happens to be shipped with the Linux kernel. LTTng has been an out-of-tree patch for about 4 years now and has generated a great deal of interest in users which can afford to deploy their own kernel. Therefore, the real reason why ftrace has such popularity is just because, as you say, it "all lives and breathes within the kernel". It has nothing to do with in-kernel vs post-processing analysis or with ascii vs binary data streaming.

Also, doing the analysis part within the kernel has a downside : it adds complexity to the kernel itself. It adds analysis which are sometimes complex and require additionnal data structures within the kernel. The advantage of streaming the data out of the kernel is that it makes the kernel-side of tracing trivially simple : we get the data out to a memory buffer. Period. This helps being less intrusive, minimizes the risks of distupting the normal system behavior, etc.

Quoted 6 lines

So in the conceptual space i can see little use for markers in the kernel that are not tracepoints (i.e. not actively used by a real tracer). We had markers in the scheduler initially, then we moved to tracepoints - and tracepoints are much nicer.

I am open to changes to the markers API, and we may need to do some, but in LTTng scheme, they fulfill a very important requirement : they turn what would otherwise be "opaque binary data" as you call it into fully described, parseable binary data.

We can then think of LTTng binary data as a very efficient data reprensentation wich can be automatically, and generically, transformed into text with a simple binary-to-ascii parser (think of it as a printk-like parser).

Quoted 4 lines

[ And you wrote both markers and tracepoints, so it's not like i risk
degenerating this discussion into a flamewar by advocating one of your solutions over the other one ;-) ]

That's ok.. I'm just trying to show which design space markers currently fulfill.

Quoted 3 lines

... and in that sense i'd love to see lttng become a "super ftrace plugin", and be merged upstream ASAP.

Hrm, a big part of LTTng is its optimized buffering mechanism, which has been more tested and is more flexible than the one currently found in ftrace (it has supported lockless NMI-safe tracing for over 2 years). It also separates the various tracing layers in separated modules, which helps not mixing the buffer management layer with the timestamping layer and with the memory backend layer.. I am opened to try to merge ftrace and lttng together, but I think it will be more than a simple "make lttng a ftrace plugin".

Quoted 9 lines

We could even split it up into multiple bits as its merged: for example syscall tracing would be a nice touch that a couple of other plugins would adapt as well. But every tracepoint should have some active role and active connection to a tracer.

And we'd keep all those tracepoints open for external kprobes use as well - for the dynamic tracers, as a low-cost courtesy. (no long-term API guarantees though.)

Ah, I see.. when you speak of LTTng as a "super ftrace plugin", you refer to the LTTng tracepoints only (the instrumentation). In that sense, yes, we could add the LTTng tracepoints into the Linux kernel and make ftrace a user of those without any problem. And as you say, no guaranteed API (this is in-kernel only).

Quoted 1 line

Hm?

Well, given that I currently have :

- trace_clock() infrastructure for timestamping (which I could submit for -tip, I think it's ready)

- LTTng instrumentation, which could be used by many tracers.

- LTTng buffer management, trace control, etc, which we might want to get through in a review phase and try to do a mix and match of the best features between LTTng and ftrace.

I think we could end up with a tracer which would be faster, more solid and would support both what ftrace is currently doing and what LTTng is doing. But if we want to do that, we have to both recognise that the use-cases filled by ftrace and by LTTng are complementary and are all needed by the community overall.

Mathieu

Quoted 2 lines

Ingo

Signature · 2 lines
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#14 lore raw

Re: [patch 06/16] Markers auto enable tracepoints (new API : trace_mark_tp())

Quoted 11 lines · Mathieu Desnoyers
Quoted 2 lines

... and in that sense i'd love to see lttng become a "super ftrace plugin", and be merged upstream ASAP.

Hrm, a big part of LTTng is its optimized buffering mechanism, which has been more tested and is more flexible than the one currently found in ftrace (it has supported lockless NMI-safe tracing for over 2 years). It also separates the various tracing layers in separated modules, which helps not mixing the buffer management layer with the timestamping layer and with the memory backend layer.. I am opened to try to merge ftrace and lttng together, but I think it will be more than a simple "make lttng a ftrace plugin".

Sure, it will certainly be a fair bit of (interesting and useful!) work.

But it's something we should do iteratively, instead of merging a parallel framework. Steve's ring-buffer should be a fair step in the right direction. Could we improve on that?

i see nothing in the feature list of LTTng that we wouldnt want to have for ftrace, agreed? And we could do an ftrace tracer named 'LTTng' which would interact with lttng userspace in the customary way.

Ingo

#15 lore raw

Re: [patch 06/16] Markers auto enable tracepoints (new API : trace_mark_tp())

Quoted 15 lines · * Ingo Molnar ([email protected]) wrote:
Quoted 11 lines · Mathieu Desnoyers
Quoted 2 lines

... and in that sense i'd love to see lttng become a "super ftrace plugin", and be merged upstream ASAP.

Hrm, a big part of LTTng is its optimized buffering mechanism, which has been more tested and is more flexible than the one currently found in ftrace (it has supported lockless NMI-safe tracing for over 2 years). It also separates the various tracing layers in separated modules, which helps not mixing the buffer management layer with the timestamping layer and with the memory backend layer.. I am opened to try to merge ftrace and lttng together, but I think it will be more than a simple "make lttng a ftrace plugin".

Quoted 7 lines

Sure, it will certainly be a fair bit of (interesting and useful!) work.

But it's something we should do iteratively, instead of merging a parallel framework. Steve's ring-buffer should be a fair step in the right direction. Could we improve on that?

I think it's a very good thing that both Steve and me went through the effort of creating such buffering mechanism, because it created healthy competition and we have been able to cross-check each other code (well, I actually pointed out some bugs he had in his tracer code just because I ran into the same issues).

The only thing is that ftrace needs are a subset of what LTTng needs and provides. As Steve stated very clearly in the past weeks, he doesn't care about exporting binary data in an organized self-described manner. This is actually a key component of LTTng. Therefore, there seems to be no will nor need to self-describe the information in the trace buffers from Steve's point of view.

However, even if such self-description is not used by ftrace-type tracers within the kernel, it could just be *there* so we can support high-throughput streaming of organized (non-opaque) binary data when needed. A ftrace tracer could then participate to this information flow and make the data available for high-speed extraction. I think this would be very beneficial to the LTTng use-cases and would not hurt ftrace tracers at all.

Quoted 2 lines

i see nothing in the feature list of LTTng that we wouldnt want to have for ftrace, agreed?

Yes, the only thing is that LTTng might be doing a little bit more than what ftrace needs, because it needs to export the information to userspace.

Quoted 3 lines

And we could do an ftrace tracer named
'LTTng' which would interact with lttng userspace in the customary way.

Or we could think of making ftrace be a LTTng information source, so it would be able to export all its information to userspace through the high-speed self-described binary trace buffers (channels) and also manage to keep all of its current features. The way I see it, because ftrace has simpler requirements due to the fact it does not export binary data across the kernel boundary, ftrace buffering can be seen as a subset of the LTTng requirements.

Besides, looking at LWN reviews of Steve's ring buffer implementation "A casual perusal of the patch might well leave a reader confused; 2000 lines of relatively complex code to implement what is, in the end, just a circular buffer. This circular buffer is not even suitable for use by tracing frameworks yet; a separate "tracing" layer is to be added for that."

I achieve all the core work of atomic-ops based buffering, self-description of traces and all I said above with a design easier to review in a few more lines, but distributed in smaller files :

850 ltt-marker-control.c (marker userspace control, event ID
management)
179 ltt-relay.h
701 ltt-relay-alloc.c (buffer memory backend, vmap-less,
contiguous memory range emulated by software)
1648 ltt-relay.c (atomic buffer writer/reader
synchronization)
780 ltt-tracer.h (header definitions, prototypes, inlines)

Compared to :
2241 ring_buffer.c
130 ring_buffer.h

So in the end, my argumentation is not based on a simple "I prefer my code to someone else's code", but simply that I think it could be easier to integrate the tracer with the fewer constraints into the tracer with the more constraints (principally exporting data to userspace) than to do the opposite.

And all ftrace could need that LTTng currently lacks (like in-kernel binary-to-text converter) is currently on the LTTng roadmap.

Mathieu

Quoted 2 lines

Ingo

Signature · 2 lines
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#16 lore raw

Re: [patch 06/16] Markers auto enable tracepoints (new API : trace_mark_tp())

In reply to Ingo Molnar

Hi

Sorry for very late responce.
However, if you do marker removing disucussion, I hope CC to Frank Eigler. because SystemTap also use marker and tracepoint.
and this patch also improvement SystemTap marker support, I think.

IIRC, Currently, Systemtap also have marker/tracepoint on/off syncronization problem.

Quoted 19 lines
Quoted 8 lines

Add a new API trace_mark_tp(), which declares a marker within a tracepoint probe. When the marker is activated, the tracepoint is automatically enabled.

No branch test is used at the marker site, because it would be a duplicate of the branch already present in the tracepoint.

Impact: new API.

i dont know.

I was actually hoping for markers (the in-kernel API) to go away completely - and be replaced with tracepoints.

Markers are the wrong design on several levels. They couple the kernel dynamically with unknown (to the kernel) entities - and that is causing complexity all around the place, clearly expressed in these patches of yours.

Quoted 6 lines

Tracepoints are much more specific - typed and enumerated function call callback points in essence - with some politeness that allows external instrumentation entities to attach to those callbacks.

Is there any usecase of markers that should not be converted to either tracepoints or to ftrace_printk() ?

Frank, Could you please read this thread and give us your comment?

#17 lore raw

Re: [patch 06/16] Markers auto enable tracepoints (new API : trace_mark_tp())

Hi -

Quoted 2 lines · KOSAKI Motohiro

Sorry for very late response.
However, if you do marker removing discussion, I hope CC to Frank Eigler.

Thanks!

Quoted 2 lines

because SystemTap also use marker and tracepoint. and this patch also improvement SystemTap marker support, I think.

I believe that this trace_mark_tp variant would operate at a level below systemtap's. It would mainly improve the kernel performance and simplify kernel code.

Quoted 2 lines

IIRC, Currently, Systemtap also have marker/tracepoint on/off synchronization problem.

Right, to the extent that systemtap can currently connect to markers and not yet tracepoints, so tracepoints need to be converted to markers or another systemtap-visible hooking mechanism at some point. This is a temporary state of affairs though, as we hope to talk to tracepoints directly before too long.

Quoted 2 lines · mingo
Quoted 2 lines

I was actually hoping for markers (the in-kernel API) to go away completely - and be replaced with tracepoints.

(If you're seriously contemplating outright removal of this API, then the earlier worries about how markers would somehow tie its users' hands to keep them around forever was surely exaggerated.)

Quoted 4 lines
Quoted 4 lines

Markers are the wrong design on several levels. They couple the kernel dynamically with unknown (to the kernel) entities - [...] Tracepoints are much more specific - typed and enumerated function call callback points in essence [...]

You are confusing type safety and "unknown (to the kernel)"-ness.

Markers provide some type safety via the format string; tracepoints via individual C declarations.

Markers can connect to "unknown (to the kernel)" clients since their API is EXPORT_SYMBOL_GPL'd. Tracepoints do exactly the same thing.

There are certainly differences between them, but these two particular ones are immaterial.

- FChE

#18 lore raw

PATCH 7/16 Markers : create DEFINE_MARKER and GET_MARKER (new API)

In reply to Mathieu Desnoyers

Allow markers to be used only for declaration, without function call associated. Useful to create specialized probes.

The problem we had is that two function calls were required when one wanted to put a marker in a tracepoint probe. Now the marker can be used simply for trace data type declaration, leaving the trace write work within the tracepoint probe without any additional function call.

Impact: new API.

CC 'Ingo Molnar' <[email protected]>
CC Lai Jiangshan <[email protected]>
---
 Documentation/markers.txt |   14 ++++++++++++++
 include/linux/marker.h    |   39 +++++++++++++++++++++++----------------
 2 files changed, 37 insertions(+), 16 deletions(-)
include/linux/marker.h +23 -16
Index: linux-2.6-lttng/include/linux/marker.h
===================================================================
@@ -55,6 +55,22 @@ struct marker {
 
 #ifdef CONFIG_MARKERS
 
+#define _DEFINE_MARKER(name, tp_name_str, tp_cb, format)		\
+		static const char __mstrtab_##name[]			\
+		__attribute__((section("__markers_strings")))		\
+		= #name "\0" format;					\
+		static struct marker __mark_##name			\
+		__attribute__((section("__markers"), aligned(8))) =	\
+		{ __mstrtab_##name, &__mstrtab_##name[sizeof(#name)],	\
+		  0, 0, marker_probe_cb, { __mark_empty_function, NULL},\
+		  NULL, tp_name_str, tp_cb }
+
+#define DEFINE_MARKER(name, format)					\
+		_DEFINE_MARKER(name, NULL, NULL, format)
+
+#define DEFINE_MARKER_TP(name, tp_name, tp_cb, format)			\
+		_DEFINE_MARKER(name, #tp_name, tp_cb, format)
+
 /*
  * Note : the empty asm volatile with read constraint is used here instead of a
  * "used" attribute to fix a gcc 4.1.x bug.
@@ -68,14 +84,7 @@ struct marker {
  */
 #define __trace_mark(generic, name, call_private, format, args...)	\
 	do {								\
-		static const char __mstrtab_##name[]			\
-		__attribute__((section("__markers_strings")))		\
-		= #name "\0" format;					\
-		static struct marker __mark_##name			\
-		__attribute__((section("__markers"), aligned(8))) =	\
-		{ __mstrtab_##name, &__mstrtab_##name[sizeof(#name)],	\
-		0, 0, marker_probe_cb,					\
-		{ __mark_empty_function, NULL}, NULL, NULL, NULL };	\
+		DEFINE_MARKER(name, format);				\
 		__mark_check_format(format, ## args);			\
 		if (unlikely(__mark_##name.state)) {			\
 			(*__mark_##name.call)				\
@@ -89,14 +98,7 @@ struct marker {
 		{							\
 			register_trace_##tp_name(tp_cb);		\
 		}							\
-		static const char __mstrtab_##name[]			\
-		__attribute__((section("__markers_strings")))		\
-		= #name "\0" format;					\
-		static struct marker __mark_##name			\
-		__attribute__((section("__markers"), aligned(8))) =	\
-		{ __mstrtab_##name, &__mstrtab_##name[sizeof(#name)],	\
-		0, 0, marker_probe_cb,					\
-		{ __mark_empty_function, NULL}, NULL, #tp_name, tp_cb };\
+		DEFINE_MARKER_TP(name, tp_name, tp_cb, format);		\
 		__mark_check_format(format, ## args);			\
 		(*__mark_##name.call)(&__mark_##name, call_private,	\
 					## args);			\
@@ -104,7 +106,11 @@ struct marker {
 
 extern void marker_update_probe_range(struct marker *begin,
 	struct marker *end);
+
+#define GET_MARKER(name)	(__mark_##name)
+
 #else /* !CONFIG_MARKERS */
+#define DEFINE_MARKER(name, tp_name, tp_cb, format)
 #define __trace_mark(generic, name, call_private, format, args...) \
 		__mark_check_format(format, ## args)
 #define __trace_mark_tp(name, call_private, tp_name, tp_cb, format, args...) \
@@ -118,6 +124,7 @@ extern void marker_update_probe_range(st
 static inline void marker_update_probe_range(struct marker *begin,
 	struct marker *end)
 { }
+#define GET_MARKER(name)
 #endif /* CONFIG_MARKERS */
 
 /**
Documentation/markers.txt +14 -0
Index: linux-2.6-lttng/Documentation/markers.txt
===================================================================
@@ -70,6 +70,20 @@ a printk warning which identifies the in
 
 "Format mismatch for probe probe_name (format), marker (format)"
 
+Another way to use markers is to simply define the marker without generating any
+function call to actually call into the marker. This is useful in combination
+with tracepoint probes in a scheme like this :
+
+void probe_tracepoint_name(unsigned int arg1, struct task_struct *tsk);
+
+DEFINE_MARKER_TP(marker_eventname, tracepoint_name, probe_tracepoint_name,
+	"arg1 %u pid %d");
+
+notrace void probe_tracepoint_name(unsigned int arg1, struct task_struct *tsk)
+{
+	struct marker *marker = &GET_MARKER(kernel_irq_entry);
+	/* write data to trace buffers ... */
+}
 
 * Probe / marker example
 
Signature · 2 lines
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#19 lore raw

PATCH 8/16 Tracepoints : Samples fix teardown

In reply to Mathieu Desnoyers

Need a tracepoint_synchronize_unregister() before the end of exit() to make sure every probe callers have exited the non preemptible section and thus are not executing the probe code anymore.

Impact: bugfix.

CC Rusty Russell <[email protected]>
CC "Frank Ch. Eigler" <[email protected]>
CC 'Ingo Molnar' <[email protected]>
CC Lai Jiangshan <[email protected]>
CC Peter Zijlstra <[email protected]>
CC Steven Rostedt <[email protected]>
---
 samples/tracepoints/tracepoint-probe-sample.c  |    1 +
 samples/tracepoints/tracepoint-probe-sample2.c |    1 +
 2 files changed, 2 insertions(+)
samples/tracepoints/tracepoint-probe-sample.c +1 -0
Index: linux-2.6-lttng/samples/tracepoints/tracepoint-probe-sample.c
===================================================================
@@ -46,6 +46,7 @@ void __exit tp_sample_trace_exit(void)
 {
 	unregister_trace_subsys_eventb(probe_subsys_eventb);
 	unregister_trace_subsys_event(probe_subsys_event);
+	tracepoint_synchronize_unregister();
 }
 
 module_exit(tp_sample_trace_exit);
samples/tracepoints/tracepoint-probe-sample2.c +1 -0
Index: linux-2.6-lttng/samples/tracepoints/tracepoint-probe-sample2.c
===================================================================
@@ -33,6 +33,7 @@ module_init(tp_sample_trace_init);
 void __exit tp_sample_trace_exit(void)
 {
 	unregister_trace_subsys_event(probe_subsys_event);
+	tracepoint_synchronize_unregister();
 }
 
 module_exit(tp_sample_trace_exit);
Signature · 2 lines
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#20 lore raw

PATCH 9/16 Tracepoint fix disable

In reply to Mathieu Desnoyers

Set the probe array pointer to NULL when the tracepoint is disabled. The probe array point not being NULL could generate a race condition where the reader would dereference a freed pointer.

Impact: bugfix.

CC 'Ingo Molnar' <[email protected]>
CC Lai Jiangshan <[email protected]>
---
 kernel/tracepoint.c |    1 +
 1 file changed, 1 insertion(+)
kernel/tracepoint.c +1 -0
Index: linux.trees.git/kernel/tracepoint.c
===================================================================
@@ -262,6 +262,7 @@ static void set_tracepoint(struct tracep
 static void disable_tracepoint(struct tracepoint *elem)
 {
 	elem->state = 0;
+	rcu_assign_pointer(elem->funcs, NULL);
 }
 
 /**
Signature · 2 lines
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#21 lore raw

PATCH 10/16 Tracepoints use rcu_*_sched_notrace

In reply to Mathieu Desnoyers

Make sure tracepoints can be called within ftrace callbacks.

CC Andrew Morton <[email protected]>
CC Masami Hiramatsu <[email protected]>
CC 'Peter Zijlstra' <[email protected]>
CC "Frank Ch. Eigler" <[email protected]>
CC 'Ingo Molnar' <[email protected]>
CC Lai Jiangshan <[email protected]>
CC 'Hideo AOKI' <[email protected]>
CC Takashi Nishiie <[email protected]>
CC 'Steven Rostedt' <[email protected]>
---
 include/linux/tracepoint.h |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
include/linux/tracepoint.h +2 -2
Index: linux-2.6-lttng/include/linux/tracepoint.h
===================================================================
@@ -40,14 +40,14 @@ struct tracepoint {
 	do {								\
 		void **it_func;						\
 									\
-		rcu_read_lock_sched();					\
+		rcu_read_lock_sched_notrace();				\
 		it_func = rcu_dereference((tp)->funcs);			\
 		if (it_func) {						\
 			do {						\
 				((void(*)(proto))(*it_func))(args);	\
 			} while (*(++it_func));				\
 		}							\
-		rcu_read_unlock_sched();				\
+		rcu_read_unlock_sched_notrace();			\
 	} while (0)
 
 /*
Signature · 2 lines
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#22 lore raw

PATCH 11/16 Tracepoint Use Unregister Return Value

In reply to Mathieu Desnoyers

Unregistering a tracepoint can fail. Return the error value.

Impact: bugfix.

CC 'Ingo Molnar' <[email protected]>
CC Lai Jiangshan <[email protected]>
---
 include/linux/tracepoint.h |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
include/linux/tracepoint.h +6 -4
Index: linux-2.6-lttng/include/linux/tracepoint.h
===================================================================
@@ -73,9 +73,9 @@ struct tracepoint {
 		return tracepoint_probe_register(#name ":" #proto,	\
 			(void *)probe);					\
 	}								\
-	static inline void unregister_trace_##name(void (*probe)(proto))\
+	static inline int unregister_trace_##name(void (*probe)(proto))	\
 	{								\
-		tracepoint_probe_unregister(#name ":" #proto,		\
+		return tracepoint_probe_unregister(#name ":" #proto,	\
 			(void *)probe);					\
 	}
 
@@ -92,8 +92,10 @@ extern void tracepoint_update_probe_rang
 	{								\
 		return -ENOSYS;						\
 	}								\
-	static inline void unregister_trace_##name(void (*probe)(proto))\
-	{ }
+	static inline int unregister_trace_##name(void (*probe)(proto))	\
+	{								\
+		return -ENOSYS;						\
+	}
 
 static inline void tracepoint_update_probe_range(struct tracepoint *begin,
 	struct tracepoint *end)
Signature · 2 lines
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#23 lore raw

PATCH 12/16 Tracepoint do not put arguments in name

In reply to Mathieu Desnoyers

That's overkill, takes space. We have a global tracepoint registery in header files anyway.

Impact: cleanup.

CC 'Ingo Molnar' <[email protected]>
CC Lai Jiangshan <[email protected]>
---
 include/linux/tracepoint.h |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)
include/linux/tracepoint.h +3 -5
Index: linux.trees.git/include/linux/tracepoint.h
===================================================================
@@ -60,7 +60,7 @@ struct tracepoint {
 	{								\
 		static const char __tpstrtab_##name[]			\
 		__attribute__((section("__tracepoints_strings")))	\
-		= #name ":" #proto;					\
+		= #name;						\
 		static struct tracepoint __tracepoint_##name		\
 		__attribute__((section("__tracepoints"), aligned(8))) =	\
 		{ __tpstrtab_##name, 0, NULL };				\
@@ -70,13 +70,11 @@ struct tracepoint {
 	}								\
 	static inline int register_trace_##name(void (*probe)(proto))	\
 	{								\
-		return tracepoint_probe_register(#name ":" #proto,	\
-			(void *)probe);					\
+		return tracepoint_probe_register(#name, (void *)probe);	\
 	}								\
 	static inline int unregister_trace_##name(void (*probe)(proto))	\
 	{								\
-		return tracepoint_probe_unregister(#name ":" #proto,	\
-			(void *)probe);					\
+		return tracepoint_probe_unregister(#name, (void *)probe);\
 	}
 
 extern void tracepoint_update_probe_range(struct tracepoint *begin,
Signature · 2 lines
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#24 lore raw

PATCH 13/16 Tracepoints : use modules notifiers

In reply to Mathieu Desnoyers

Use module notifiers for tracepoint updates rather than adding a hook in module.c.

Impact: cleanup.

CC 'Ingo Molnar' <[email protected]>
CC Lai Jiangshan <[email protected]>
---
 kernel/module.c     |    5 -----
 kernel/tracepoint.c |   29 +++++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 5 deletions(-)
kernel/tracepoint.c +29 -0
Index: linux.trees.git/kernel/tracepoint.c
===================================================================
@@ -541,3 +541,32 @@ void tracepoint_iter_reset(struct tracep
 	iter->tracepoint = NULL;
 }
 EXPORT_SYMBOL_GPL(tracepoint_iter_reset);
+
+int tracepoint_module_notify(struct notifier_block *self,
+			     unsigned long val, void *data)
+{
+	struct module *mod = data;
+
+	switch (val) {
+	case MODULE_STATE_COMING:
+		tracepoint_update_probe_range(mod->tracepoints,
+			mod->tracepoints + mod->num_tracepoints);
+		break;
+	case MODULE_STATE_GOING:
+		tracepoint_update_probe_range(mod->tracepoints,
+			mod->tracepoints + mod->num_tracepoints);
+		break;
+	}
+	return 0;
+}
+
+struct notifier_block tracepoint_module_nb = {
+	.notifier_call = tracepoint_module_notify,
+	.priority = 0,
+};
+
+static int init_tracepoints(void)
+{
+	return register_module_notifier(&tracepoint_module_nb);
+}
+__initcall(init_tracepoints);
kernel/module.c +0 -5
Index: linux.trees.git/kernel/module.c
===================================================================
@@ -2188,11 +2188,6 @@ static noinline struct module *load_modu
 		debug = section_objs(hdr, sechdrs, secstrings, "__verbose",
 				     sizeof(*debug), &num_debug);
 		dynamic_printk_setup(debug, num_debug);
-
-#ifdef CONFIG_TRACEPOINTS
-		tracepoint_update_probe_range(mod->tracepoints,
-			mod->tracepoints + mod->num_tracepoints);
-#endif
 	}
 
 	/* sechdrs[0].sh_size is always zero */
Signature · 2 lines
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#25 lore raw

PATCH 14/16 Tracepoints : add DECLARE_TRACE() and DEFINE_TRACE()

In reply to Mathieu Desnoyers

Add DEFINE_TRACE() to tracepoints to let them declare the tracepoint structure in a single spot for all the kernel. It helps reducing memory consumption, especially when declaring a lot of tracepoints, e.g. for kmalloc tracing.

*API CHANGE WARNING* : now, DECLARE_TRACE() must be used in headers for tracepoint declarations rather than DEFINE_TRACE(). This is the sane way to do it. The name previously used was misleading.

Updates scheduler instrumentation to follow this API change.

Impact: API *CHANGE*. Must update all tracepoint users.

CC 'Ingo Molnar' <[email protected]>
CC Lai Jiangshan <[email protected]>
---
 Documentation/tracepoints.txt           |    7 +++++-
 include/asm-generic/vmlinux.lds.h       |    1 
 include/linux/tracepoint.h              |   35 ++++++++++++++++++++++----------
 include/trace/sched.h                   |   24 ++++++++++-----------
 kernel/exit.c                           |    4 +++
 kernel/fork.c                           |    2 +
 kernel/kthread.c                        |    3 ++
 kernel/sched.c                          |    6 +++++
 kernel/signal.c                         |    2 +
 samples/tracepoints/tp-samples-trace.h  |    4 +--
 samples/tracepoints/tracepoint-sample.c |    3 ++
 11 files changed, 66 insertions(+), 25 deletions(-)
include/linux/tracepoint.h +25 -10
Index: linux.trees.git/include/linux/tracepoint.h
===================================================================
@@ -24,8 +24,12 @@ struct tracepoint {
 	const char *name;		/* Tracepoint name */
 	int state;			/* State. */
 	void **funcs;
-} __attribute__((aligned(8)));
-
+} __attribute__((aligned(32)));		/*
+					 * Aligned on 32 bytes because it is
+					 * globally visible and gcc happily
+					 * align these on the structure size.
+					 * Keep in sync with vmlinux.lds.h.
+					 */
 
 #define TPPROTO(args...)	args
 #define TPARGS(args...)		args
@@ -55,15 +59,10 @@ struct tracepoint {
  * not add unwanted padding between the beginning of the section and the
  * structure. Force alignment to the same alignment as the section start.
  */
-#define DEFINE_TRACE(name, proto, args)					\
+#define DECLARE_TRACE(name, proto, args)				\
+	extern struct tracepoint __tracepoint_##name;			\
 	static inline void trace_##name(proto)				\
 	{								\
-		static const char __tpstrtab_##name[]			\
-		__attribute__((section("__tracepoints_strings")))	\
-		= #name;						\
-		static struct tracepoint __tracepoint_##name		\
-		__attribute__((section("__tracepoints"), aligned(8))) =	\
-		{ __tpstrtab_##name, 0, NULL };				\
 		if (unlikely(__tracepoint_##name.state))		\
 			__DO_TRACE(&__tracepoint_##name,		\
 				TPPROTO(proto), TPARGS(args));		\
@@ -77,11 +76,23 @@ struct tracepoint {
 		return tracepoint_probe_unregister(#name, (void *)probe);\
 	}
 
+#define DEFINE_TRACE(name)						\
+	static const char __tpstrtab_##name[]				\
+	__attribute__((section("__tracepoints_strings"))) = #name;	\
+	struct tracepoint __tracepoint_##name				\
+	__attribute__((section("__tracepoints"), aligned(32))) =	\
+		{ __tpstrtab_##name, 0, NULL }
+
+#define EXPORT_TRACEPOINT_SYMBOL_GPL(name)				\
+	EXPORT_SYMBOL_GPL(__tracepoint_##name)
+#define EXPORT_TRACEPOINT_SYMBOL(name)					\
+	EXPORT_SYMBOL(__tracepoint_##name)
+
 extern void tracepoint_update_probe_range(struct tracepoint *begin,
 	struct tracepoint *end);
 
 #else /* !CONFIG_TRACEPOINTS */
-#define DEFINE_TRACE(name, proto, args)			\
+#define DECLARE_TRACE(name, proto, args)				\
 	static inline void _do_trace_##name(struct tracepoint *tp, proto) \
 	{ }								\
 	static inline void trace_##name(proto)				\
@@ -95,6 +106,10 @@ extern void tracepoint_update_probe_rang
 		return -ENOSYS;						\
 	}
 
+#define DEFINE_TRACE(name)
+#define EXPORT_TRACEPOINT_SYMBOL_GPL(name)
+#define EXPORT_TRACEPOINT_SYMBOL(name)
+
 static inline void tracepoint_update_probe_range(struct tracepoint *begin,
 	struct tracepoint *end)
 { }
Documentation/tracepoints.txt +6 -1
Index: linux.trees.git/Documentation/tracepoints.txt
===================================================================
@@ -42,7 +42,7 @@ In include/trace/subsys.h :
 
 #include <linux/tracepoint.h>
 
-DEFINE_TRACE(subsys_eventname,
+DECLARE_TRACE(subsys_eventname,
 	TPPTOTO(int firstarg, struct task_struct *p),
 	TPARGS(firstarg, p));
 
@@ -50,6 +50,8 @@ In subsys/file.c (where the tracing stat
 
 #include <trace/subsys.h>
 
+DEFINE_TRACE(subsys_eventname);
+
 void somefct(void)
 {
 	...
@@ -86,6 +88,9 @@ to limit collisions. Tracepoint names ar
 considered as being the same whether they are in the core kernel image or in
 modules.
 
+If the tracepoint has to be used in kernel modules, an
+EXPORT_TRACEPOINT_SYMBOL_GPL() or EXPORT_TRACEPOINT_SYMBOL() can be used to
+export the defined tracepoints.
 
 * Probe / tracepoint example
 
samples/tracepoints/tracepoint-sample.c +3 -0
Index: linux.trees.git/samples/tracepoints/tracepoint-sample.c
===================================================================
@@ -13,6 +13,9 @@
 #include <linux/proc_fs.h>
 #include "tp-samples-trace.h"
 
+DEFINE_TRACE(subsys_event);
+DEFINE_TRACE(subsys_eventb);
+
 struct proc_dir_entry *pentry_example;
 
 static int my_open(struct inode *inode, struct file *file)
samples/tracepoints/tp-samples-trace.h +2 -2
Index: linux.trees.git/samples/tracepoints/tp-samples-trace.h
===================================================================
@@ -4,10 +4,10 @@
 #include <linux/proc_fs.h>	/* for struct inode and struct file */
 #include <linux/tracepoint.h>
 
-DEFINE_TRACE(subsys_event,
+DECLARE_TRACE(subsys_event,
 	TPPROTO(struct inode *inode, struct file *file),
 	TPARGS(inode, file));
-DEFINE_TRACE(subsys_eventb,
+DECLARE_TRACE(subsys_eventb,
 	TPPROTO(void),
 	TPARGS());
 #endif
include/asm-generic/vmlinux.lds.h +1 -0
Index: linux.trees.git/include/asm-generic/vmlinux.lds.h
===================================================================
@@ -71,6 +71,7 @@
 	VMLINUX_SYMBOL(__start___markers) = .;				\
 	*(__markers)							\
 	VMLINUX_SYMBOL(__stop___markers) = .;				\
+	. = ALIGN(32);							\
 	VMLINUX_SYMBOL(__start___tracepoints) = .;			\
 	*(__tracepoints)						\
 	VMLINUX_SYMBOL(__stop___tracepoints) = .;			\
include/trace/sched.h +12 -12
Index: linux.trees.git/include/trace/sched.h
===================================================================
@@ -4,52 +4,52 @@
 #include <linux/sched.h>
 #include <linux/tracepoint.h>
 
-DEFINE_TRACE(sched_kthread_stop,
+DECLARE_TRACE(sched_kthread_stop,
 	TPPROTO(struct task_struct *t),
 		TPARGS(t));
 
-DEFINE_TRACE(sched_kthread_stop_ret,
+DECLARE_TRACE(sched_kthread_stop_ret,
 	TPPROTO(int ret),
 		TPARGS(ret));
 
-DEFINE_TRACE(sched_wait_task,
+DECLARE_TRACE(sched_wait_task,
 	TPPROTO(struct rq *rq, struct task_struct *p),
 		TPARGS(rq, p));
 
-DEFINE_TRACE(sched_wakeup,
+DECLARE_TRACE(sched_wakeup,
 	TPPROTO(struct rq *rq, struct task_struct *p),
 		TPARGS(rq, p));
 
-DEFINE_TRACE(sched_wakeup_new,
+DECLARE_TRACE(sched_wakeup_new,
 	TPPROTO(struct rq *rq, struct task_struct *p),
 		TPARGS(rq, p));
 
-DEFINE_TRACE(sched_switch,
+DECLARE_TRACE(sched_switch,
 	TPPROTO(struct rq *rq, struct task_struct *prev,
 		struct task_struct *next),
 		TPARGS(rq, prev, next));
 
-DEFINE_TRACE(sched_migrate_task,
+DECLARE_TRACE(sched_migrate_task,
 	TPPROTO(struct rq *rq, struct task_struct *p, int dest_cpu),
 		TPARGS(rq, p, dest_cpu));
 
-DEFINE_TRACE(sched_process_free,
+DECLARE_TRACE(sched_process_free,
 	TPPROTO(struct task_struct *p),
 		TPARGS(p));
 
-DEFINE_TRACE(sched_process_exit,
+DECLARE_TRACE(sched_process_exit,
 	TPPROTO(struct task_struct *p),
 		TPARGS(p));
 
-DEFINE_TRACE(sched_process_wait,
+DECLARE_TRACE(sched_process_wait,
 	TPPROTO(struct pid *pid),
 		TPARGS(pid));
 
-DEFINE_TRACE(sched_process_fork,
+DECLARE_TRACE(sched_process_fork,
 	TPPROTO(struct task_struct *parent, struct task_struct *child),
 		TPARGS(parent, child));
 
-DEFINE_TRACE(sched_signal_send,
+DECLARE_TRACE(sched_signal_send,
 	TPPROTO(int sig, struct task_struct *p),
 		TPARGS(sig, p));
 
kernel/sched.c +6 -0
Index: linux.trees.git/kernel/sched.c
===================================================================
@@ -118,6 +118,12 @@
  */
 #define RUNTIME_INF	((u64)~0ULL)
 
+DEFINE_TRACE(sched_wait_task);
+DEFINE_TRACE(sched_wakeup);
+DEFINE_TRACE(sched_wakeup_new);
+DEFINE_TRACE(sched_switch);
+DEFINE_TRACE(sched_migrate_task);
+
 #ifdef CONFIG_SMP
 /*
  * Divide a load by a sched group cpu_power : (load / sg->__cpu_power)
kernel/exit.c +4 -0
Index: linux.trees.git/kernel/exit.c
===================================================================
@@ -54,6 +54,10 @@
 #include <asm/pgtable.h>
 #include <asm/mmu_context.h>
 
+DEFINE_TRACE(sched_process_free);
+DEFINE_TRACE(sched_process_exit);
+DEFINE_TRACE(sched_process_wait);
+
 static void exit_mm(struct task_struct * tsk);
 
 static inline int task_detached(struct task_struct *p)
kernel/fork.c +2 -0
Index: linux.trees.git/kernel/fork.c
===================================================================
@@ -80,6 +80,8 @@ DEFINE_PER_CPU(unsigned long, process_co
 
 __cacheline_aligned DEFINE_RWLOCK(tasklist_lock);  /* outer */
 
+DEFINE_TRACE(sched_process_fork);
+
 int nr_processes(void)
 {
 	int cpu;
kernel/kthread.c +3 -0
Index: linux.trees.git/kernel/kthread.c
===================================================================
@@ -21,6 +21,9 @@ static DEFINE_SPINLOCK(kthread_create_lo
 static LIST_HEAD(kthread_create_list);
 struct task_struct *kthreadd_task;
 
+DEFINE_TRACE(sched_kthread_stop);
+DEFINE_TRACE(sched_kthread_stop_ret);
+
 struct kthread_create_info
 {
 	/* Information passed to kthread() from kthreadd. */
kernel/signal.c +2 -0
Index: linux.trees.git/kernel/signal.c
===================================================================
@@ -41,6 +41,8 @@
 
 static struct kmem_cache *sigqueue_cachep;
 
+DEFINE_TRACE(sched_signal_send);
+
 static void __user *sig_handler(struct task_struct *t, int sig)
 {
 	return t->sighand->action[sig - 1].sa.sa_handler;
Signature · 2 lines
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#26 lore raw

PATCH 15/16 Tracepoints : documentation fix teardown

In reply to Mathieu Desnoyers

Need a tracepoint_synchronize_unregister() before the end of exit() to make sure every probe callers have exited the non preemptible section and thus are not executing the probe code anymore.

Impact: documentation update.

CC Rusty Russell <[email protected]>
CC "Frank Ch. Eigler" <[email protected]>
CC Ingo Molnar <[email protected]>
CC Lai Jiangshan <[email protected]>
CC Peter Zijlstra <[email protected]>
CC Steven Rostedt <[email protected]>
---
 Documentation/tracepoints.txt |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
Documentation/tracepoints.txt +6 -4
Index: linux-2.6-lttng/Documentation/tracepoints.txt
===================================================================
@@ -70,10 +70,12 @@ Where :
 Connecting a function (probe) to a tracepoint is done by providing a probe
 (function to call) for the specific tracepoint through
 register_trace_subsys_eventname().  Removing a probe is done through
-unregister_trace_subsys_eventname(); it will remove the probe sure there is no
-caller left using the probe when it returns. Probe removal is preempt-safe
-because preemption is disabled around the probe call. See the "Probe example"
-section below for a sample probe module.
+unregister_trace_subsys_eventname(); it will remove the probe.
+marker_synchronize_unregister() must be called before the end of the module exit
+function to make sure there is no caller left using the probe. This, and the
+fact that preemption is disabled around the probe call, make sure that probe
+removal and module unload are safe. See the "Probe example" section below for a
+sample probe module.
 
 The tracepoint mechanism supports inserting multiple instances of the same
 tracepoint, but a single definition must be made of a given tracepoint name over
Signature · 2 lines
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#27 lore raw

PATCH 16/16 marker_synchronize_unregister->tracepoint_synchronize_unregister

In reply to Mathieu Desnoyers

Impact: documentation update.

---
 Documentation/tracepoints.txt |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Documentation/tracepoints.txt +1 -1
Index: linux-2.6-lttng/Documentation/tracepoints.txt
===================================================================
@@ -71,7 +71,7 @@ Connecting a function (probe) to a trace
 (function to call) for the specific tracepoint through
 register_trace_subsys_eventname().  Removing a probe is done through
 unregister_trace_subsys_eventname(); it will remove the probe.
-marker_synchronize_unregister() must be called before the end of the module exit
+tracepoint_synchronize_unregister() must be called before the end of the module exit
 function to make sure there is no caller left using the probe. This, and the
 fact that preemption is disabled around the probe call, make sure that probe
 removal and module unload are safe. See the "Probe example" section below for a
Signature · 2 lines
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
#29 lore raw

Re: [patch 16/16] marker_synchronize_unregister->tracepoint_synchronize_unregister

Quoted 10 lines · * Ingo Molnar ([email protected]) wrote:
Quoted 6 lines · Mathieu Desnoyers

Impact: documentation update.

Quoted 8 lines

This should have been:

From: Zhaolei <[email protected]>

correct?

I updated the patch accordingly.

Yes, that's correct. I forgot to add this line. Thanks !

Mathieu

Quoted 1 line

Ingo

Signature · 2 lines
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68