[PATCH] markers-generic-doc-section-fix-flags

[PATCH] markers-generic-doc-section-fix-flags

[Mathieu Desnoyers]

Documents the linux/marker.h header. Fix a bitwise flag manipulation mistake
in the MARK_GENERIC macro.

Use the __marker (ro), new __marker_strings (ro) and __marker_data (rw)
sections. It depends on the linker modification which defines this last
section.

It allows the use of a probe "private" data : upon probe connexion, it
can provide a pointer to its own data structure which will be available
at the probe call site as the pdata field of the mdata structure.

With the new __marker_data section, much less data has to be defined in
the kernel .data segment : only the enable flag, in non-optimized
markers, is left in .data because it is used very often.

The marker flags now only define MF_* which are bitmasks. There is no
way to use the bitmask shift value, so it is less error prone.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>

--- a/include/linux/marker.h
+++ b/include/linux/marker.h
@@ -16,48 +16,59 @@
 
 #ifdef __KERNEL__
 
-typedef void marker_probe_func(const char *fmt, ...);
+struct __mark_marker_data;
 
-struct __mark_marker_c {
+typedef void marker_probe_func(const struct __mark_marker_data *mdata,
+	const char *fmt, ...);
+
+struct __mark_marker_data {
 	const char *name;
-	marker_probe_func **call;
 	const char *format;
 	int flags;
+	marker_probe_func *call;
+	void *pdata;
 } __attribute__((packed));
 
 struct __mark_marker {
-	const struct __mark_marker_c *cmark;
+	struct __mark_marker_data *mdata;
 	void *enable;
 } __attribute__((packed));
 
-/* Generic marker flavor always available */
 #ifdef CONFIG_MARKERS
 
-#define MF_OPTIMIZED 1	/* Use optimized markers */
-#define MF_LOCKDEP 2	/* Can call lockdep */
-#define MF_PRINTK 3	/* vprintk can be called in the probe */
+/* Marker flags : selects the mechanism used to connect the probes to the
+ * markers and what can be executed within the probes. This is primarily
+ * used at reentrancy-unfriendly sites. */
+#define MF_OPTIMIZED	(1 << 0)	/* Use optimized markers */
+#define MF_LOCKDEP	(1 << 1)	/* Can call lockdep */
+#define MF_PRINTK	(1 << 2)	/* vprintk can be called in the probe */
+#define _MF_NR		3		/* Number of marker flags */
 
-#define _MF_OPTIMIZED (1 << MF_OPTIMIZED)
-#define _MF_LOCKDEP (1 << MF_LOCKDEP)
-#define _MF_PRINTK (1 << MF_PRINTK)
+#define DECLARE_MARKER_DATA(flags, name, format) \
 
+/* Generic marker flavor always available */
 #define MARK_GENERIC(flags, name, format, args...) \
 	do { \
-		static marker_probe_func *__mark_call_##name = \
-					__mark_empty_function; \
+		static const char __mstrtab_name_##name[] \
+		__attribute__((section("__markers_strings"))) \
+		= #name; \
+		static const char __mstrtab_format_##name[] \
+		__attribute__((section("__markers_strings"))) \
+		= format; \
+		static struct __mark_marker_data __mark_data_##name \
+		__attribute__((section("__markers_data"))) = \
+		{ __mstrtab_name_##name,  __mstrtab_format_##name, \
+		(flags) & ~MF_OPTIMIZED, __mark_empty_function, NULL }; \
 		static char __marker_enable_##name = 0; \
-		static const struct __mark_marker_c __mark_c_##name \
-			__attribute__((section(".markers.c"))) = \
-			{ #name, &__mark_call_##name, format, \
-			(flags) | ~_MF_OPTIMIZED } ; \
 		static const struct __mark_marker __mark_##name \
-			__attribute__((section(".markers"))) = \
-			{ &__mark_c_##name, &__marker_enable_##name } ; \
+			__attribute__((section("__markers"))) = \
+			{ &__mark_data_##name, &__marker_enable_##name } ; \
 		asm volatile ( "" : : "i" (&__mark_##name)); \
 		__mark_check_format(format, ## args); \
 		if (unlikely(__marker_enable_##name)) { \
 			preempt_disable(); \
-			(*__mark_call_##name)(format, ## args); \
+			(*__mark_data_##name.call)(&__mark_data_##name, \
+						format, ## args); \
 			preempt_enable(); \
 		} \
 	} while (0)
@@ -87,21 +98,23 @@ static inline int marker_generic_set_enable(void *address, char enable)
 #endif
 
 #define MARK_MAX_FORMAT_LEN	1024
+/* Pass this as a format string for a marker with no argument */
 #define MARK_NOARGS " "
 
-static inline __attribute__ ((format (printf, 1, 2)))
+/* To be used for string format validity checking with sparse */
+static inline
 void __mark_check_format(const char *fmt, ...)
 { }
 
 extern marker_probe_func __mark_empty_function;
 
 extern int _marker_set_probe(int flags, const char *name, const char *format,
-				marker_probe_func *probe);
+				marker_probe_func *probe, void *pdata);
 
-#define marker_set_probe(name, format, probe) \
-	_marker_set_probe(_MF_DEFAULT, name, format, probe)
+#define marker_set_probe(name, format, probe, pdata) \
+	_marker_set_probe(MF_DEFAULT, name, format, probe, pdata)
 
-extern int marker_remove_probe(marker_probe_func *probe);
+extern int marker_remove_probe(const char *name);
 extern int marker_list_probe(marker_probe_func *probe);
 
 #endif /* __KERNEL__ */
-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

[PATCH] markers-i386-doc-section-flags-optimized-state-change

[Mathieu Desnoyers]

Document the i386 marker header, change it to use the __marker,
__marker_data and __marker_strings sections. Change the flag usage to
use MF_* directly (which is now a bitmask). In marker.c, for the i386
optimized marker enabling, check the a simple test is the value will be
changed. If not, simply return : we would set it to the same value
anyway.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>

--- a/include/asm-i386/marker.h
+++ b/include/asm-i386/marker.h
@@ -1,3 +1,6 @@
+#ifndef _ASM_I386_MARKER_H
+#define _ASM_I386_MARKER_H
+
 /*
  * marker.h
  *
@@ -12,42 +15,54 @@
 
 #ifdef CONFIG_MARKERS
 
-#define _MF_DEFAULT (_MF_OPTIMIZED | _MF_LOCKDEP | _MF_PRINTK)
+#define MF_DEFAULT (MF_OPTIMIZED | MF_LOCKDEP | MF_PRINTK)
 
+/* Optimized version of the markers */
 #define MARK_OPTIMIZED(flags, name, format, args...) \
 	do { \
-		static marker_probe_func *__mark_call_##name = \
-					__mark_empty_function; \
-		static const struct __mark_marker_c __mark_c_##name \
-			__attribute__((section(".markers.c"))) = \
-			{ #name, &__mark_call_##name, format, \
-			(flags) | _MF_OPTIMIZED } ; \
+		static const char __mstrtab_name_##name[] \
+		__attribute__((section("__markers_strings"))) \
+		= #name; \
+		static const char __mstrtab_format_##name[] \
+		__attribute__((section("__markers_strings"))) \
+		= format; \
+		static struct __mark_marker_data __mark_data_##name \
+		__attribute__((section("__markers_data"))) = \
+		{ __mstrtab_name_##name,  __mstrtab_format_##name, \
+		(flags) | MF_OPTIMIZED, __mark_empty_function, NULL }; \
 		char condition; \
-		asm volatile(	".section .markers, \"a\", @progbits;\n\t" \
+		asm volatile(	".section __markers, \"a\", @progbits;\n\t" \
 					".long %1, 0f;\n\t" \
 					".previous;\n\t" \
 					".align 2\n\t" \
 					"0:\n\t" \
 					"movb $0,%0;\n\t" \
 				: "=r" (condition) \
-				: "m" (__mark_c_##name)); \
+				: "m" (__mark_data_##name)); \
 		__mark_check_format(format, ## args); \
 		if (unlikely(condition)) { \
 			preempt_disable(); \
-			(*__mark_call_##name)(format, ## args); \
+			(*__mark_data_##name.call)(&__mark_data_##name, \
+						format, ## args); \
 			preempt_enable(); \
 		} \
 	} while (0)
 
+/* Marker macro selecting the generic or optimized version of marker, depending
+ * on the flags specified. */
 #define _MARK(flags, format, args...) \
 do { \
-	if (((flags) & _MF_LOCKDEP) && ((flags) & _MF_OPTIMIZED)) \
+	if (((flags) & MF_LOCKDEP) && ((flags) & MF_OPTIMIZED)) \
 		MARK_OPTIMIZED(flags, format, ## args); \
 	else \
 		MARK_GENERIC(flags, format, ## args); \
 } while (0)
 
-#define MARK(format, args...) _MARK(_MF_DEFAULT, format, ## args)
+/* Marker with default behavior */
+#define MARK(format, args...) _MARK(MF_DEFAULT, format, ## args)
+
+/* Architecture dependant marker information, used internally for marker
+ * activation. */
 
 /* Offset of the immediate value from the start of the movb instruction, in
  * bytes. */
@@ -61,3 +76,4 @@ do { \
 extern int marker_optimized_set_enable(void *address, char enable);
 
 #endif
+#endif //_ASM_I386_MARKER_H
--- a/arch/i386/kernel/marker.c
+++ b/arch/i386/kernel/marker.c
@@ -4,7 +4,9 @@
  *
  * Permits marker activation by XMC with correct serialization.
  *
- * Reentrant for NMI and trap handler instrumentation. :-)
+ * Reentrant for NMI and trap handler instrumentation. Permits XMC to a
+ * location that has preemption enabled because it involves no temporary or
+ * reused data structure.
  *
  * Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
  */
@@ -63,8 +65,10 @@ int marker_optimized_set_enable(void *address, char enable)
 	int ret;
 	char *dest = address;
 
+	if (!(enable ^ dest[1])) /* Must be a state change 0<->1 to execute */
+		return 0;
+
 	mutex_lock(&mark_mutex);
-	BUG_ON(!(enable ^ dest[1]));	/* Must be a state change 0<->1 */
 	target_eip = (long)address + BREAKPOINT_INS_LEN;
 	/* register_die_notifier has memory barriers */
 	register_die_notifier(&mark_notify);
-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

[PATCH] markers-powerpc-doc-section-flags

[Mathieu Desnoyers]

Changes to the powerpc marker header : Use the new MF_* bitmask, use the
__markers, __markers_data and __markers_strings sections, add
documentation.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>

--- a/include/asm-powerpc/marker.h
+++ b/include/asm-powerpc/marker.h
@@ -1,3 +1,6 @@
+#ifndef _ASM_POWERPC_MARKER_H
+#define _ASM_POWERPC_MARKER_H
+
 /*
  * marker.h
  *
@@ -14,42 +17,54 @@
 
 #ifdef CONFIG_MARKERS
 
-#define _MF_DEFAULT (_MF_OPTIMIZED | _MF_LOCKDEP | _MF_PRINTK)
+#define MF_DEFAULT (MF_OPTIMIZED | MF_LOCKDEP | MF_PRINTK)
 
+/* Optimized version of the markers */
 #define MARK_OPTIMIZED(flags, name, format, args...) \
 	do { \
-		static marker_probe_func *__mark_call_##name = \
-					__mark_empty_function; \
-		static const struct __mark_marker_c __mark_c_##name \
-			__attribute__((section(".markers.c"))) = \
-			{ #name, &__mark_call_##name, format, \
-			(flags) | _MF_OPTIMIZED } ; \
+		static const char __mstrtab_name_##name[] \
+		__attribute__((section("__markers_strings"))) \
+		= #name; \
+		static const char __mstrtab_format_##name[] \
+		__attribute__((section("__markers_strings"))) \
+		= format; \
+		static struct __mark_marker_data __mark_data_##name \
+		__attribute__((section("__markers_data"))) = \
+		{ __mstrtab_name_##name,  __mstrtab_format_##name, \
+		(flags) | MF_OPTIMIZED, __mark_empty_function, NULL }; \
 		char condition; \
-		asm volatile(	".section .markers, \"a\", @progbits;\n\t" \
+		asm volatile(	".section __markers, \"a\", @progbits;\n\t" \
 					PPC_LONG "%1, 0f;\n\t" \
 					".previous;\n\t" \
 					".align 4\n\t" \
 					"0:\n\t" \
 					"li %0,0;\n\t" \
 				: "=r" (condition) \
-				: "i" (&__mark_c_##name)); \
+				: "i" (&__mark_data_##name)); \
 		__mark_check_format(format, ## args); \
 		if (unlikely(condition)) { \
 			preempt_disable(); \
-			(*__mark_call_##name)(format, ## args); \
+			(*__mark_data_##name.call)(&__mark_data_##name, \
+						format, ## args); \
 			preempt_enable(); \
 		} \
 	} while (0)
 
+/* Marker macro selecting the generic or optimized version of marker, depending
+ * on the flags specified. */
 #define _MARK(flags, format, args...) \
 do { \
-	if ((flags) & _MF_OPTIMIZED) \
+	if ((flags) & MF_OPTIMIZED) \
 		MARK_OPTIMIZED(flags, format, ## args); \
 	else \
 		MARK_GENERIC(flags, format, ## args); \
 } while (0)
 
-#define MARK(format, args...) _MARK(_MF_DEFAULT, format, ## args)
+/* Marker with default behavior */
+#define MARK(format, args...) _MARK(MF_DEFAULT, format, ## args)
+
+/* Architecture dependant marker information, used internally for marker
+ * activation. */
 
 /* Offset of the immediate value from the start of the addi instruction (result
  * of the li mnemonic), in bytes. */
@@ -63,3 +78,4 @@ do { \
 extern int marker_optimized_set_enable(void *address, char enable);
 
 #endif
+#endif //_ASM_POWERPC_MARKER_H
-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

[PATCH] markers-non-opt-arch-alpha-arm26-doc-flags

[Mathieu Desnoyers]

Add alpha marker.h, add arm26 marker.h,  use the new MF_* bitmask for
asm-generic/marker.h. Document asm-generic/marker.h.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>

--- /dev/null
+++ b/include/asm-alpha/marker.h
@@ -0,0 +1,13 @@
+/*
+ * marker.h
+ *
+ * Code markup for dynamic and static tracing. Architecture specific
+ * optimisations.
+ * 
+ * No optimisation implemented.
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include <asm-generic/marker.h>
--- /dev/null
+++ b/include/asm-arm26/marker.h
@@ -0,0 +1,13 @@
+/*
+ * marker.h
+ *
+ * Code markup for dynamic and static tracing. Architecture specific
+ * optimisations.
+ * 
+ * No optimisation implemented.
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include <asm-generic/marker.h>
--- a/include/asm-generic/marker.h
+++ b/include/asm-generic/marker.h
@@ -13,11 +13,18 @@
  * "used" attribute to fix a gcc 4.1.x bug.
  */
 
-#define _MF_DEFAULT			(_MF_LOCKDEP | _MF_PRINTK)
+/* Default flags, used by MARK() */
+#define MF_DEFAULT			(MF_LOCKDEP | MF_PRINTK)
 
+/* Fallback on the generic markers, since no optimized version is available */
 #define MARK_OPTIMIZED			MARK_GENERIC
 #define _MARK				MARK_GENERIC
-#define MARK(format, args...)		_MARK(_MF_DEFAULT, format, ## args)
+
+/* Marker with default behavior */
+#define MARK(format, args...)		_MARK(MF_DEFAULT, format, ## args)
+
+/* Architecture dependant marker information, used internally for marker
+ * activation. */
 
 #define MARK_OPTIMIZED_ENABLE_IMMEDIATE_OFFSET \
 		MARK_GENERIC_ENABLE_IMMEDIATE_OFFSET
-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

[PATCH] markers-doc-update-flags-example

[Mathieu Desnoyers]

Update marker Documentation to be in sync with the flag bitmask
change. Give more complete probe example.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>

--- a/Documentation/marker.txt
+++ b/Documentation/marker.txt
@@ -12,11 +12,11 @@ probe module examples. This is what connects to a marker.
 * Purpose of markers
 
 A marker placed in your code provides a hook to a function (probe) that
-you can provide at runtime. A marker can be on (a probe is connected to it)
-or off (no probe is attached). An "off" marker has no effect. When turned on,
+you can provide at runtime. A marker can be "on" (a probe is connected to it)
+or "off" (no probe is attached). An "off" marker has no effect. When turned on,
 the function you provide is called each time the marker is executed in the
 execution context of the caller. When the function provided ends its execution,
-it returns to the caller (probe site).
+it returns to the caller (marker site).
 
 You can put markers at important locations in the code. They act as
 lightweight hooks that can pass an arbitrary number of parameters,
@@ -40,20 +40,14 @@ In order to use the macro MARK, you should include linux/marker.h.
 
 Add, in your code :
 
-MARK(subsystem_event, "%d %s %p[struct task_struct]",
-  someint, somestring, current);
+MARK(subsystem_event, "%d %s", someint, somestring);
 Where :
 - subsystem_event is an identifier unique to your event
     - subsystem is the name of your subsystem.
     - event is the name of the event to mark.
-- "%d %s %p[struct task_struct]" is the formatted string for (printk-style).
+- "%d %s" is the formatted string for the serializer.
 - someint is an integer.
 - somestring is a char pointer.
-- current is a pointer to a struct task_struct.
-
-The expression %p[struct task_struct] is a suggested marker definition
-standard that could eventually be used for pointer type checking in
-sparse. The brackets contain the type to which the pointer refers.
 
 Connecting a function (probe) to a marker is done by providing a probe
 (function to call) for the specific marker through marker_set_probe(). It will
@@ -73,7 +67,7 @@ will not call the scheduler due to the tests in preempt_schedule().
 * Optimization for a given architecture
 
 You will find, in asm-*/marker.h, optimisations for given architectures
-(currently i386 and powerpc). They use a load immediate instead of a data read,
+(currently i386 and powerpc). They use a load immediate instead of a data load,
 which saves a data cache hit, but also requires cross CPU code modification. In
 order to support embedded systems which use read-only memory for their code, the
 optimization can be disabled through menu options.
@@ -81,25 +75,35 @@ optimization can be disabled through menu options.
 The MF_* flags can be used to control the type of marker. See the
 include/marker.h header for the list of flags. They can be specified as the
 first parameter of the _MARK() macro, such as the following example which is
-safe wrt lockdep.c (useful for marking lockdep.c functions).
+safe with respect to lockdep.c (useful for marking lockdep.c and printk
+functions).
 
-_MARK(_MF_DEFAULT | ~_MF_LOCKDEP, subsystem_eventb,
-  MARK_NOARGS);
+_MARK(MF_DEFAULT | ~MF_LOCKDEP, subsystem_eventb, MARK_NOARGS);
 
 Another example is to specify that a specific marker must never call printk :
-_MARK(_MF_DEFAULT | ~_MF_PRINTK, subsystem_eventc,
-  "%d %s %p[struct task_struct]",
-  someint, somestring, current);
+_MARK(MF_DEFAULT | ~MF_PRINTK, subsystem_eventc,
+  "%d %s", someint, somestring,);
 
-Flag compatibility is checked before connecting the probe to the marker.
+Flag compatibility is checked before connecting the probe to the marker : the
+right flags must be given to _marker_set_enable().
 
 
 * Probe example
 
+You can build the kernel modules, probe-example.ko and marker-example.ko,
+using the following Makefile:
+------------------------------ CUT -------------------------------------
+obj-m := probe-example.o marker-example.o
+KDIR := /lib/modules/$(shell uname -r)/build
+PWD := $(shell pwd)
+default:
+	$(MAKE) -C $(KDIR) SUBDIRS=$(PWD) modules
+clean:
+	rm -f *.mod.c *.ko *.o
 ------------------------------ CUT -------------------------------------
 /* probe-example.c
  *
- * Loads a function at a marker call site.
+ * Connects a two functions to marker call sites.
  *
  * (C) Copyright 2007 Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
  *
@@ -109,67 +113,83 @@ Flag compatibility is checked before connecting the probe to the marker.
 
 #include <linux/sched.h>
 #include <linux/kernel.h>
+#include <linux/module.h>
 #include <linux/marker.h>
+#include <asm/atomic.h>
+
+#define NUM_PROBES (sizeof(probe_array) / sizeof(struct probe_data))
 
-#define SUBSYSTEM_EVENT_FORMAT "%d %s %p[struct task_struct]"
-void probe_subsystem_event(const char *format, ...)
+struct probe_data {
+	const char *name;
+	const char *format;
+	marker_probe_func *probe_func;
+};
+
+void probe_subsystem_event(const struct __mark_marker_c *mdata,
+		const char *format, ...)
 {
 	va_list ap;
 	/* Declare args */
 	unsigned int value;
 	const char *mystr;
-	struct task_struct *task;
 
 	/* Assign args */
 	va_start(ap, format);
 	value = va_arg(ap, typeof(value));
 	mystr = va_arg(ap, typeof(mystr));
-	task = va_arg(ap, typeof(task));
-
-	/* Call tracer */
-	trace_subsystem_event(value, mystr, task);
 
-	/* Or call printk */
-	vprintk(format, ap);
+	/* Call printk */
+	printk("Value %u, string %s\n", value, mystr);
 
-	/* or count, check rights... */
+	/* or count, check rights, serialize data in a buffer */
 
 	va_end(ap);
 }
 
-#define SUBSYSTEM_EVENTB_FORMAT MARK_NOARGS
-void probe_subsystem_eventb(const char *format, ...)
+atomic_t eventb_count = ATOMIC_INIT(0);
+
+void probe_subsystem_eventb(const struct __mark_marker_c *mdata,
+	const char *format, ...)
 {
-	/* Increment counters, trace, ... but _never_ generate a call to
-	 * lockdep.c ! */
+	/* Increment counter */
+	atomic_inc(&eventb_count);
 }
 
+static struct probe_data probe_array[] =
+{
+	{	.name = "subsystem_event",
+		.format = "%d %s",
+		.probe_func = probe_subsystem_event },
+	{	.name = "subsystem_eventb",
+		.format = MARK_NOARGS,
+		.probe_func = probe_subsystem_eventb },
+};
+
 static int __init probe_init(void)
 {
 	int result;
-	result = marker_set_probe("subsystem_event",
-			SUBSYSTEM_EVENT_FORMAT,
-			probe_subsystem_event);
-	if (!result)
-		goto cleanup;
-	result = _marker_set_probe(_MF_DEFAULT & ~_MF_LOCKDEP,
-			"subsystem_eventb",
-			SUBSYSTEM_EVENTB_FORMAT,
-			probe_subsystem_eventb);
-	if (!result)
-		goto cleanup;
+	uint8_t eID;
+
+	for (eID = 0; eID < NUM_PROBES; eID++) {
+		result = marker_set_probe(probe_array[eID].name,
+				probe_array[eID].format,
+				probe_array[eID].probe_func, &probe_array[eID]);
+		if (!result)
+			printk(KERN_INFO "Unable to register probe %s\n",
+				probe_array[eID].name);
+	}
 	return 0;
-
-cleanup:
-	marker_remove_probe(probe_subsystem_event);
-	marker_remove_probe(probe_subsystem_eventb);
-	return -EPERM;
 }
 
 static void __exit probe_fini(void)
 {
-	marker_remove_probe(probe_subsystem_event);
-	marker_remove_probe(probe_subsystem_eventb);
+	uint8_t eID;
+
+	for (eID = 0; eID < NUM_PROBES; eID++) {
+		marker_remove_probe(probe_array[eID].name);
+	}
+	synchronize_sched();	/* Wait for probes to finish */
+	printk("Number of event b : %u\n", atomic_read(&eventb_count));
 }
 
 module_init(probe_init);
@@ -179,4 +199,68 @@ MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Mathieu Desnoyers");
 MODULE_DESCRIPTION("SUBSYSTEM Probe");
 ------------------------------ CUT -------------------------------------
+/* marker-example.c
+ *
+ * Executes a marker when /proc/marker-example is opened.
+ *
+ * (C) Copyright 2007 Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/marker.h>
+#include <linux/sched.h>
+#include <linux/proc_fs.h>
+
+struct proc_dir_entry *pentry_example = NULL;
+
+static int my_open(struct inode *inode, struct file *file)
+{
+	int i;
+
+	MARK(subsystem_event, "%d %s", 123, "example string");
+	for (i=0; i<10; i++) {
+		MARK(subsystem_eventb, MARK_NOARGS);
+	}
+	return -EPERM;
+}
+
+static struct file_operations mark_ops = {
+	.open = my_open,
+};
+
+static int example_init(void)
+{
+	printk(KERN_ALERT "example init\n");
+	pentry_example = create_proc_entry("marker-example", 0444, NULL);
+	if (pentry_example)
+		pentry_example->proc_fops = &mark_ops;
+	else
+		return -EPERM;
+	return 0;
+}
+
+static void example_exit(void)
+{
+	printk(KERN_ALERT "example exit\n");
+	remove_proc_entry("marker-example", NULL);
+}
 
+module_init(example_init)
+module_exit(example_exit)
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Mathieu Desnoyers");
+MODULE_DESCRIPTION("Linux Trace Toolkit example");
+------------------------------ CUT -------------------------------------
+Sequence of operations : (as root)
+make
+insmod marker-example.ko
+insmod probe-example.ko
+  (it is important to load the probe after the marked code)
+cat /proc/marker-example (returns an expected error)
+rmmod marker-example probe-example
+dmesg
+------------------------------ CUT -------------------------------------
-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

[PATCH] build-avr32-marker-menu

[Mathieu Desnoyers]

Add Instrumentation/markers menus to avr32.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>

--- a/arch/avr32/Kconfig.debug
+++ b/arch/avr32/Kconfig.debug
@@ -6,6 +6,9 @@ config TRACE_IRQFLAGS_SUPPORT
 
 source "lib/Kconfig.debug"
 
+menu "Instrumentation Support"
+	depends on EXPERIMENTAL
+
 config KPROBES
 	bool "Kprobes"
 	depends on DEBUG_KERNEL
@@ -16,4 +19,8 @@ config KPROBES
           for kernel debugging, non-intrusive instrumentation and testing.
           If in doubt, say "N".
 
+source "kernel/Kconfig.marker"
+
+endmenu
+
 endmenu

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

[PATCH] markers-module.c-doc-flags

[Mathieu Desnoyers]

Add documentation to the module.c marker functions. Update them to
follow the flags modifications.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>

--- a/kernel/module.c
+++ b/kernel/module.c
@@ -303,24 +303,36 @@ static struct module *find_module(const char *name)
 }
 
 #ifdef CONFIG_MARKERS
-void __mark_empty_function(const char *fmt, ...)
+
+/* Empty callback provided as a probe to the markers. By providing this to a
+ * disabled marker, we makes sure the  execution flow is always valid even
+ * 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(const struct __mark_marker_data *mdata,
+	const char *fmt, ...)
 {
 }
 EXPORT_SYMBOL_GPL(__mark_empty_function);
 
+/* Set the enable bit of the marker, choosing the generic or architecture
+ * specific functions depending on the marker's flags.
+ */
 static int marker_set_enable(void *address, char enable, int flags)
 {
-	if (flags & _MF_OPTIMIZED)
+	if (flags & MF_OPTIMIZED)
 		return marker_optimized_set_enable(address, enable);
 	else
 		return marker_generic_set_enable(address, enable);
 }
 
-/* enable and function address are set out of order, and it's ok :
- * the state is always coherent. */
+/* Sets the probe callback and enables the markers corresponding to a range of
+ * markers. The enable bit and function address are set out of order, and it's
+ * ok : the state is always coherent because of the empty callback we provide.
+ */
 static int _marker_set_probe_range(int flags, const char *name,
 	const char *format,
 	marker_probe_func *probe,
+	void *pdata,
 	const struct __mark_marker *begin,
 	const struct __mark_marker *end)
 {
@@ -328,27 +340,27 @@ static int _marker_set_probe_range(int flags, const char *name,
 	int found = 0;
 
 	for (iter = begin; iter < end; iter++) {
-		if (strcmp(name, iter->cmark->name) == 0) {
+		if (strcmp(name, iter->mdata->name) == 0) {
 			if (format
-				&& strcmp(format, iter->cmark->format) != 0) {
+				&& strcmp(format, iter->mdata->format) != 0) {
 				printk(KERN_NOTICE
 					"Format mismatch for probe %s "
 					"(%s), marker (%s)\n",
 					name,
 					format,
-					iter->cmark->format);
+					iter->mdata->format);
 				continue;
 			}
-			if (flags & _MF_LOCKDEP
-				&& !(iter->cmark->flags & _MF_LOCKDEP)) {
+			if (flags & MF_LOCKDEP
+				&& !(iter->mdata->flags & MF_LOCKDEP)) {
 					printk(KERN_NOTICE
 					"Incompatible lockdep flags for "
 					"probe %s\n",
 					name);
 					continue;
 			}
-			if (flags & _MF_PRINTK
-				&& !(iter->cmark->flags & _MF_PRINTK)) {
+			if (flags & MF_PRINTK
+				&& !(iter->mdata->flags & MF_PRINTK)) {
 					printk(KERN_NOTICE
 					"Incompatible printk flags for "
 					"probe %s\n",
@@ -356,32 +368,33 @@ static int _marker_set_probe_range(int flags, const char *name,
 					continue;
 			}
 			if (probe == __mark_empty_function) {
-				if (*iter->cmark->call
+				if (iter->mdata->call
 					!= __mark_empty_function) {
-					*iter->cmark->call =
+					iter->mdata->call =
 						__mark_empty_function;
 				}
 				marker_set_enable(iter->enable, 0,
-					iter->cmark->flags);
+					iter->mdata->flags);
 			} else {
-				if (*iter->cmark->call
+				if (iter->mdata->call
 					!= __mark_empty_function) {
-					if (*iter->cmark->call != probe) {
+					if (iter->mdata->call != probe) {
 						printk(KERN_NOTICE
 							"Marker %s busy, "
 							"probe %p already "
 							"installed\n",
 							name,
-							*iter->cmark->call);
+							iter->mdata->call);
 						continue;
 					}
 				} else {
 					found++;
-					*iter->cmark->call = probe;
+					iter->mdata->call = probe;
 				}
-				/* Can have many enables for one function */
+				iter->mdata->pdata = pdata;
+				smp_wmb();
 				marker_set_enable(iter->enable, 1,
-					iter->cmark->flags);
+					iter->mdata->flags);
 			}
 			found++;
 		}
@@ -389,7 +402,9 @@ static int _marker_set_probe_range(int flags, const char *name,
 	return found;
 }
 
-static int marker_remove_probe_range(marker_probe_func *probe,
+/* Sets a range of markers to a disabled state : unset the enable bit and
+ * provide the empty callback. */
+static int marker_remove_probe_range(const char *name,
 	const struct __mark_marker *begin,
 	const struct __mark_marker *end)
 {
@@ -397,16 +412,19 @@ static int marker_remove_probe_range(marker_probe_func *probe,
 	int found = 0;
 
 	for (iter = begin; iter < end; iter++) {
-		if (*iter->cmark->call == probe) {
+		if (strcmp(name, iter->mdata->name) == 0) {
 			marker_set_enable(iter->enable, 0,
-				iter->cmark->flags);
-			*iter->cmark->call = __mark_empty_function;
+				iter->mdata->flags);
+			iter->mdata->call = __mark_empty_function;
 			found++;
 		}
 	}
 	return found;
 }
 
+/* Provides a listing of the markers present in the kernel with their type
+ * (optimized or generic), state (enabled or disabled), callback and format
+ * string. */
 static int marker_list_probe_range(marker_probe_func *probe,
 	const struct __mark_marker *begin,
 	const struct __mark_marker *end)
@@ -416,23 +434,26 @@ static int marker_list_probe_range(marker_probe_func *probe,
 
 	for (iter = begin; iter < end; iter++) {
 		if (probe)
-			if (probe != *iter->cmark->call) continue;
-		printk("name %s \n", iter->cmark->name);
-		if (iter->cmark->flags & _MF_OPTIMIZED)
+			if (probe != iter->mdata->call) continue;
+		printk("name %s \n", iter->mdata->name);
+		if (iter->mdata->flags & MF_OPTIMIZED)
 			printk("  enable %u optimized ",
 				MARK_OPTIMIZED_ENABLE(iter->enable));
 		else
 			printk("  enable %u generic ",
 				MARK_GENERIC_ENABLE(iter->enable));
 		printk("  func 0x%p format \"%s\"\n",
-			*iter->cmark->call, iter->cmark->format);
+			iter->mdata->call, iter->mdata->format);
 		found++;
 	}
 	return found;
 }
-/* markers use the modlist_lock to to synchronise */
+
+/* Calls _marker_set_probe_range for the core markers and modules markers.
+ * Marker enabling/disabling use the modlist_lock to synchronise. */
 int _marker_set_probe(int flags, const char *name, const char *format,
-				marker_probe_func *probe)
+				marker_probe_func *probe,
+				void *pdata)
 {
 	struct module *mod;
 	int found = 0;
@@ -440,31 +461,35 @@ int _marker_set_probe(int flags, const char *name, const char *format,
 	mutex_lock(&module_mutex);
 	/* Core kernel markers */
 	found += _marker_set_probe_range(flags, name, format, probe,
+			pdata,
 			__start___markers, __stop___markers);
 	/* Markers in modules. */
 	list_for_each_entry(mod, &modules, list) {
 		if (!mod->taints)
 			found += _marker_set_probe_range(flags, name, format,
-			probe, mod->markers, mod->markers+mod->num_markers);
+			probe, pdata,
+			mod->markers, mod->markers+mod->num_markers);
 	}
 	mutex_unlock(&module_mutex);
 	return found;
 }
 EXPORT_SYMBOL_GPL(_marker_set_probe);
 
-int marker_remove_probe(marker_probe_func *probe)
+/* Calls _marker_remove_probe_range for the core markers and modules markers.
+ * Marker enabling/disabling use the modlist_lock to synchronise. */
+int marker_remove_probe(const char *name)
 {
 	struct module *mod;
 	int found = 0;
 
 	mutex_lock(&module_mutex);
 	/* Core kernel markers */
-	found += marker_remove_probe_range(probe,
+	found += marker_remove_probe_range(name,
 			__start___markers, __stop___markers);
 	/* Markers in modules. */
 	list_for_each_entry(mod, &modules, list) {
 		if (!mod->taints)
-			found += marker_remove_probe_range(probe,
+			found += marker_remove_probe_range(name,
 				mod->markers, mod->markers+mod->num_markers);
 	}
 	mutex_unlock(&module_mutex);
@@ -472,6 +497,9 @@ int marker_remove_probe(marker_probe_func *probe)
 }
 EXPORT_SYMBOL_GPL(marker_remove_probe);
 
+/* Calls _marker_list_probe_range for the core markers and modules markers.
+ * Marker listing uses the modlist_lock to synchronise.
+ * TODO : should output this listing to a procfs file. */
 int marker_list_probe(marker_probe_func *probe)
 {
 	struct module *mod;
@@ -1856,6 +1886,8 @@ static struct module *load_module(void __user *umod,
 	unsigned int unusedgplindex;
 	unsigned int unusedgplcrcindex;
 	unsigned int markersindex;
+	unsigned int markersdataindex;
+	unsigned int markersstringsindex;
 	struct module *mod;
 	long err = 0;
 	void *percpu = NULL, *ptr = NULL; /* Stops spurious gcc warning */
@@ -1952,7 +1984,10 @@ static struct module *load_module(void __user *umod,
 #ifdef ARCH_UNWIND_SECTION_NAME
 	unwindex = find_sec(hdr, sechdrs, secstrings, ARCH_UNWIND_SECTION_NAME);
 #endif
-	markersindex = find_sec(hdr, sechdrs, secstrings, ".markers");
+	markersindex = find_sec(hdr, sechdrs, secstrings, "__markers");
+	markersdataindex = find_sec(hdr, sechdrs, secstrings, "__markers_data");
+	markersstringsindex = find_sec(hdr, sechdrs, secstrings,
+				"__markers_strings");
 
 	/* Don't keep modinfo section */
 	sechdrs[infoindex].sh_flags &= ~(unsigned long)SHF_ALLOC;
@@ -1966,9 +2001,18 @@ static struct module *load_module(void __user *umod,
 #ifdef CONFIG_MARKERS
 	if (markersindex)
 		sechdrs[markersindex].sh_flags |= SHF_ALLOC;
+	if (markersdataindex)
+		sechdrs[markersdataindex].sh_flags |= SHF_ALLOC;
+	if (markersstringsindex)
+		sechdrs[markersstringsindex].sh_flags |= SHF_ALLOC;
 #else
 	if (markersindex)
 		sechdrs[markersindex].sh_flags &= ~(unsigned long)SHF_ALLOC;
+	if (markersdataindex)
+		sechdrs[markersdataindex].sh_flags &= ~(unsigned long)SHF_ALLOC;
+	if (markersstringsindex)
+		sechdrs[markersstringsindex].sh_flags
+					&= ~(unsigned long)SHF_ALLOC;
 #endif
 
 	/* Check module struct version now, before we try to use module. */
@@ -2622,7 +2668,7 @@ void list_modules(void)
 #ifdef CONFIG_MODULE_UNLOAD
 		refcount = local_read(&mod->ref[0].count);
 #endif //CONFIG_MODULE_UNLOAD
-		MARK(list_modules, "%s %d[enum module_state] %lu",
+		MARK(list_module, "%s %d %lu",
 				mod->name, mod->state, refcount);
 	}
 	mutex_unlock(&module_mutex);
-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

Re: [PATCH] markers-doc-update-flags-example

[Randy Dunlap]

On Tue, 10 Apr 2007 19:26:20 -0400 Mathieu Desnoyers wrote:


>  ------------------------------ CUT -------------------------------------
>  /* probe-example.c
>   *
> - * Loads a function at a marker call site.
> + * Connects a two functions to marker call sites.

s/a //

>   *
>   * (C) Copyright 2007 Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
>   *
> @@ -109,67 +113,83 @@ Flag compatibility is checked before connecting the probe to the marker.
>  
>  #include <linux/sched.h>
>  #include <linux/kernel.h>
> +#include <linux/module.h>
>  #include <linux/marker.h>
> +#include <asm/atomic.h>
> +
> +#define NUM_PROBES (sizeof(probe_array) / sizeof(struct probe_data))

#define NUM_PROBES	ARRAY_SIZE(probe_array)


> -#define SUBSYSTEM_EVENT_FORMAT "%d %s %p[struct task_struct]"
> -void probe_subsystem_event(const char *format, ...)
> +struct probe_data {
> +	const char *name;
> +	const char *format;
> +	marker_probe_func *probe_func;
> +};
> +
>  
> +static struct probe_data probe_array[] =
> +{
> +	{	.name = "subsystem_event",
> +		.format = "%d %s",
> +		.probe_func = probe_subsystem_event },
> +	{	.name = "subsystem_eventb",
> +		.format = MARK_NOARGS,
> +		.probe_func = probe_subsystem_eventb },
> +};
> +

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

[PATCH] Linux Kernel Markers documentation fix typo and use ARRAY_SIZE

[Mathieu Desnoyers]

Linux Kernel Markers documentation fix typo and use ARRAY_SIZE

Following comments from Randy Dunlap. Applies on top of the 
linux-kernel-markers-documentation-markers-update-documentation patch.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>

--- a/Documentation/marker.txt
+++ b/Documentation/marker.txt
@@ -103,7 +103,7 @@ clean:
 ------------------------------ CUT -------------------------------------
 /* probe-example.c
  *
- * Connects a two functions to marker call sites.
+ * Connects two functions to marker call sites.
  *
  * (C) Copyright 2007 Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
  *
@@ -117,7 +117,7 @@ clean:
 #include <linux/marker.h>
 #include <asm/atomic.h>
 
-#define NUM_PROBES (sizeof(probe_array) / sizeof(struct probe_data))
+#define NUM_PROBES ARRAY_SIZE(probe_array)
 
 struct probe_data {
 	const char *name;
-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68