[PATCH 00/05] Linux Kernel Markers - kernel 2.6.20

[PATCH 00/05] Linux Kernel Markers - kernel 2.6.20

[Mathieu Desnoyers]

Linux Kernel Markers - kernel 2.6.20

Hi,

You will find, in the following posts, the latest revision of the Linux Kernel
Markers. Due to the need some tracing projects (LTTng, SystemTAP) has of this
kind of mechanism, it could be nice to consider it for mainstream inclusion.

Recently added : completed correct support for Intel XMC-safe code modification
(erratum 49 of the Pentium III). I am not using the kprobe infrastructure
because it forbids instrumentation of too much core kernel functions (traps
handlers, lockdep.c, ...). I also made the "GEN_MARK" marker available at all
time : it will force using the generic version of the markers at a specific
instrumentation site. It is intended to be used at instrumentation sites that
are not reentrant with respect to the breakpoint handler.
lockdep.c/trace_hardirqs_on/off() is a good example of where it is needed :
the breakpoint handler calls hardirq tracing event when it returns.

The following patches apply on 2.6.20.

These patches compile fine against :

arm
i686
ia64
m68k
mips
mipsel
x86_64
ppc 405
powerpc 970
s390
sparc
sparc64

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

[PATCH 01/05] Linux Kernel Markers : Kconfig menus

[Mathieu Desnoyers]

Linux Kernel Markers : Kconfig menus

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

--- /dev/null
+++ b/kernel/Kconfig.marker
@@ -0,0 +1,17 @@
+# Code markers configuration
+
+config MARKERS
+	bool "Activate markers"
+	select MODULES
+	default n
+	help
+	  Place an empty function call at each marker site. Can be
+	  dynamically changed for a probe function.
+
+config MARKERS_ENABLE_OPTIMIZATION
+	bool "Enable marker optimization"
+	depends on MARKERS
+	default y
+	help
+	  Disable code replacement jump optimisations. Especially useful if your
+	  code is in a read-only rom/flash.
--- a/Makefile
+++ b/Makefile
@@ -308,7 +308,8 @@ AFLAGS_KERNEL	=
 # Needed to be compatible with the O= option
 LINUXINCLUDE    := -Iinclude \
                    $(if $(KBUILD_SRC),-Iinclude2 -I$(srctree)/include) \
-		   -include include/linux/autoconf.h
+		   -include include/linux/autoconf.h \
+		   -include linux/marker.h
 
 CPPFLAGS        := -D__KERNEL__ $(LINUXINCLUDE)
 
--- a/arch/alpha/Kconfig
+++ b/arch/alpha/Kconfig
@@ -638,6 +638,12 @@ source "fs/Kconfig"
 
 source "arch/alpha/oprofile/Kconfig"
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/alpha/Kconfig.debug"
 
 source "security/Kconfig"
--- a/arch/arm26/Kconfig
+++ b/arch/arm26/Kconfig
@@ -240,6 +240,12 @@ source "drivers/misc/Kconfig"
 
 source "drivers/usb/Kconfig"
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/arm26/Kconfig.debug"
 
 source "security/Kconfig"
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -968,6 +968,12 @@ source "fs/Kconfig"
 
 source "arch/arm/oprofile/Kconfig"
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/arm/Kconfig.debug"
 
 source "security/Kconfig"
--- a/arch/cris/Kconfig
+++ b/arch/cris/Kconfig
@@ -191,6 +191,12 @@ source "sound/Kconfig"
 
 source "drivers/usb/Kconfig"
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/cris/Kconfig.debug"
 
 source "security/Kconfig"
--- a/arch/frv/Kconfig
+++ b/arch/frv/Kconfig
@@ -375,6 +375,12 @@ source "drivers/Kconfig"
 
 source "fs/Kconfig"
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/frv/Kconfig.debug"
 
 source "security/Kconfig"
--- a/arch/h8300/Kconfig
+++ b/arch/h8300/Kconfig
@@ -205,6 +205,12 @@ endmenu
 
 source "fs/Kconfig"
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/h8300/Kconfig.debug"
 
 source "security/Kconfig"
--- a/arch/i386/Kconfig
+++ b/arch/i386/Kconfig
@@ -1210,6 +1210,9 @@ config KPROBES
 	  a probepoint and specifies the callback.  Kprobes is useful
 	  for kernel debugging, non-intrusive instrumentation and testing.
 	  If in doubt, say "N".
+
+source "kernel/Kconfig.marker"
+
 endmenu
 
 source "arch/i386/Kconfig.debug"
--- a/arch/ia64/Kconfig
+++ b/arch/ia64/Kconfig
@@ -564,6 +564,9 @@ config KPROBES
 	  a probepoint and specifies the callback.  Kprobes is useful
 	  for kernel debugging, non-intrusive instrumentation and testing.
 	  If in doubt, say "N".
+
+source "kernel/Kconfig.marker"
+
 endmenu
 
 source "arch/ia64/Kconfig.debug"
--- a/arch/m32r/Kconfig
+++ b/arch/m32r/Kconfig
@@ -394,6 +394,12 @@ source "fs/Kconfig"
 
 source "arch/m32r/oprofile/Kconfig"
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/m32r/Kconfig.debug"
 
 source "security/Kconfig"
--- a/arch/m68k/Kconfig
+++ b/arch/m68k/Kconfig
@@ -660,6 +660,12 @@ endmenu
 
 source "fs/Kconfig"
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/m68k/Kconfig.debug"
 
 source "security/Kconfig"
--- a/arch/m68knommu/Kconfig
+++ b/arch/m68knommu/Kconfig
@@ -669,6 +669,12 @@ source "drivers/Kconfig"
 
 source "fs/Kconfig"
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/m68knommu/Kconfig.debug"
 
 source "security/Kconfig"
--- a/arch/ppc/Kconfig
+++ b/arch/ppc/Kconfig
@@ -1441,8 +1441,14 @@ endmenu
 
 source "lib/Kconfig"
 
+menu "Instrumentation Support"
+
 source "arch/powerpc/oprofile/Kconfig"
 
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/ppc/Kconfig.debug"
 
 source "security/Kconfig"
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -1189,6 +1189,9 @@ config KPROBES
 	  a probepoint and specifies the callback.  Kprobes is useful
 	  for kernel debugging, non-intrusive instrumentation and testing.
 	  If in doubt, say "N".
+
+source "kernel/Kconfig.marker"
+
 endmenu
 
 source "arch/powerpc/Kconfig.debug"
--- a/arch/parisc/Kconfig
+++ b/arch/parisc/Kconfig
@@ -263,6 +263,12 @@ source "fs/Kconfig"
 
 source "arch/parisc/oprofile/Kconfig"
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/parisc/Kconfig.debug"
 
 source "security/Kconfig"
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -2082,6 +2082,12 @@ source "fs/Kconfig"
 
 source "arch/mips/oprofile/Kconfig"
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/mips/Kconfig.debug"
 
 source "security/Kconfig"
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -518,6 +518,8 @@ config KPROBES
 	  for kernel debugging, non-intrusive instrumentation and testing.
 	  If in doubt, say "N".
 
+source "kernel/Kconfig.marker"
+
 endmenu
 
 source "arch/s390/Kconfig.debug"
--- a/arch/sh64/Kconfig
+++ b/arch/sh64/Kconfig
@@ -284,6 +284,12 @@ source "fs/Kconfig"
 
 source "arch/sh64/oprofile/Kconfig"
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/sh64/Kconfig.debug"
 
 source "security/Kconfig"
--- a/arch/sh/Kconfig
+++ b/arch/sh/Kconfig
@@ -707,6 +707,12 @@ source "fs/Kconfig"
 
 source "arch/sh/oprofile/Kconfig"
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/sh/Kconfig.debug"
 
 source "security/Kconfig"
--- a/arch/sparc64/Kconfig
+++ b/arch/sparc64/Kconfig
@@ -443,6 +443,9 @@ config KPROBES
 	  a probepoint and specifies the callback.  Kprobes is useful
 	  for kernel debugging, non-intrusive instrumentation and testing.
 	  If in doubt, say "N".
+
+source "kernel/Kconfig.marker"
+
 endmenu
 
 source "arch/sparc64/Kconfig.debug"
--- a/arch/sparc/Kconfig
+++ b/arch/sparc/Kconfig
@@ -302,6 +302,8 @@ menu "Instrumentation Support"
 
 source "arch/sparc/oprofile/Kconfig"
 
+source "kernel/Kconfig.marker"
+
 endmenu
 
 source "arch/sparc/Kconfig.debug"
--- a/arch/um/Kconfig
+++ b/arch/um/Kconfig
@@ -344,4 +344,10 @@ config INPUT
 	bool
 	default n
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/um/Kconfig.debug"
--- a/arch/v850/Kconfig
+++ b/arch/v850/Kconfig
@@ -332,6 +332,12 @@ source "sound/Kconfig"
 
 source "drivers/usb/Kconfig"
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/v850/Kconfig.debug"
 
 source "security/Kconfig"
--- a/arch/xtensa/Kconfig
+++ b/arch/xtensa/Kconfig
@@ -244,6 +244,12 @@ config EMBEDDED_RAMDISK_IMAGE
 	  provide one yourself.
 endmenu
 
+menu "Instrumentation Support"
+
+source "kernel/Kconfig.marker"
+
+endmenu
+
 source "arch/xtensa/Kconfig.debug"
 
 source "security/Kconfig"
--- a/arch/x86_64/Kconfig
+++ b/arch/x86_64/Kconfig
@@ -731,6 +731,9 @@ config KPROBES
 	  a probepoint and specifies the callback.  Kprobes is useful
 	  for kernel debugging, non-intrusive instrumentation and testing.
 	  If in doubt, say "N".
+
+source "kernel/Kconfig.marker"
+
 endmenu
 
 source "arch/x86_64/Kconfig.debug"

[PATCH 02/05] Linux Kernel Markers, architecture independant code.

[Mathieu Desnoyers]

Linux Kernel Markers, architecture independant code.

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

--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -121,6 +121,19 @@
         __ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) {	\
 		*(__ksymtab_strings)					\
 	}								\
+	/* Kernel markers : pointers */					\
+	.markers : AT(ADDR(.markers) - LOAD_OFFSET) {			\
+		VMLINUX_SYMBOL(__start___markers) = .;			\
+		*(.markers)						\
+		VMLINUX_SYMBOL(__stop___markers) = .;			\
+	}								\
+	.markers.c : AT(ADDR(.markers.c) - LOAD_OFFSET) {		\
+		VMLINUX_SYMBOL(__start___markers_c) = .;		\
+		*(.markers.c)						\
+		VMLINUX_SYMBOL(__stop___markers_c) = .;			\
+	}								\
+	__end_rodata = .;						\
+	. = ALIGN(4096);						\
 									\
 	/* Built-in module parameters. */				\
 	__param : AT(ADDR(__param) - LOAD_OFFSET) {			\
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -356,6 +356,9 @@ struct module
 	/* The command line arguments (may be mangled).  People like
 	   keeping pointers to this stuff */
 	char *args;
+
+	const struct __mark_marker *markers;
+	unsigned int num_markers;
 };
 
 /* FIXME: It'd be nice to isolate modules during init, too, so they
@@ -469,6 +472,7 @@ extern void print_modules(void);
 struct device_driver;
 void module_add_driver(struct module *, struct device_driver *);
 void module_remove_driver(struct device_driver *);
+extern void list_modules(void);
 
 #else /* !CONFIG_MODULES... */
 #define EXPORT_SYMBOL(sym)
--- /dev/null
+++ b/include/linux/marker.h
@@ -0,0 +1,97 @@
+#ifndef _LINUX_MARKER_H
+#define _LINUX_MARKER_H
+
+/*
+ * marker.h
+ *
+ * Code markup for dynamic and static tracing.
+ *
+ * Example :
+ *
+ * MARK(subsystem_event, "%d %s %p[struct task_struct]",
+ *   someint, somestring, current);
+ * Where :
+ * - 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.
+ * - someint is an integer.
+ * - somestring is a char pointer.
+ * - current is a pointer to a struct task_struct.
+ *
+ * - Dynamically overridable function call based on marker mechanism
+ *   from Frank Ch. Eigler <fche@redhat.com>.
+ * - Thanks to Jeremy Fitzhardinge <jeremy@goop.org> for his constructive
+ *   criticism about gcc optimization related issues.
+ *
+ * The marker mechanism supports multiple instances of the same marker.
+ * Markers can be put in inline functions, inlined static functions and
+ * unrolled loops.
+ *
+ * Note : It is safe to put markers within preempt-safe code : preempt_enable()
+ * will not call the scheduler due to the tests in preempt_schedule().
+ *
+ * (C) Copyright 2006 Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#ifndef __ASSEMBLY__
+
+typedef void marker_probe_func(const char *fmt, ...);
+
+enum marker_type { MARKER_GENERIC, MARKER_OPTIMIZED };
+
+struct __mark_marker_c {
+	const char *name;
+	marker_probe_func **call;
+	const char *format;
+	enum marker_type type;
+} __attribute__((packed));
+
+struct __mark_marker {
+	const struct __mark_marker_c *cmark;
+	void *enable;
+} __attribute__((packed));
+
+#ifdef CONFIG_MARKERS_ENABLE_OPTIMIZATION
+#include <asm/marker.h>
+#endif
+
+#include <asm-generic/marker.h>
+
+#define MARK_NOARGS " "
+#define MARK_MAX_FORMAT_LEN	1024
+
+#ifndef CONFIG_MARKERS
+#define GEN_MARK(name, format, args...) \
+		__mark_check_format(format, ## args)
+#endif
+
+#ifndef MARK
+#define MARK GEN_MARK
+#define MARK_ENABLE_TYPE GEN_MARK_ENABLE_TYPE
+#define MARK_ENABLE_IMMEDIATE_OFFSET GEN_MARK_ENABLE_IMMEDIATE_OFFSET
+#endif
+
+/* Dereference enable as lvalue from a pointer to its instruction */
+#define MARK_ENABLE(a) \
+	*(MARK_ENABLE_TYPE*)((char*)a+MARK_ENABLE_IMMEDIATE_OFFSET)
+
+#define GEN_MARK_ENABLE(a) \
+	*(GEN_MARK_ENABLE_TYPE*)((char*)a+GEN_MARK_ENABLE_IMMEDIATE_OFFSET)
+
+static inline __attribute__ ((format (printf, 1, 2)))
+void __mark_check_format(const char *fmt, ...)
+{ }
+
+extern marker_probe_func __mark_empty_function;
+
+extern int marker_set_probe(const char *name, const char *format,
+				marker_probe_func *probe);
+
+extern int marker_remove_probe(marker_probe_func *probe);
+extern int marker_list_probe(marker_probe_func *probe);
+
+#endif
+#endif
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -138,6 +138,8 @@ extern const unsigned long __start___kcrctab_gpl[];
 extern const unsigned long __start___kcrctab_gpl_future[];
 extern const unsigned long __start___kcrctab_unused[];
 extern const unsigned long __start___kcrctab_unused_gpl[];
+extern const struct __mark_marker __start___markers[];
+extern const struct __mark_marker __stop___markers[];
 
 #ifndef CONFIG_MODVERSIONS
 #define symversion(base, idx) NULL
@@ -298,6 +300,214 @@ static struct module *find_module(const char *name)
 	return NULL;
 }
 
+#ifdef CONFIG_MARKERS
+void __mark_empty_function(const char *fmt, ...)
+{
+}
+EXPORT_SYMBOL(__mark_empty_function);
+
+#ifdef MARK_POLYMORPHIC
+static int marker_set_ins_enable(void *address, char enable)
+{
+#ifdef CONFIG_X86_32
+	return arch_marker_set_ins_enable(address, enable);
+#else
+	char newi[MARK_ENABLE_IMMEDIATE_OFFSET+1];
+	int size = MARK_ENABLE_IMMEDIATE_OFFSET+sizeof(MARK_ENABLE_TYPE);
+
+	memcpy(newi, address, size);
+	MARK_ENABLE(&newi[0]) = enable;
+	memcpy(address, newi, size);
+	flush_icache_range((unsigned long)address, size);
+	return 0;
+#endif //CONFIG_X86_32
+}
+#else
+static int marker_set_ins_enable(void *address, char enable)
+{
+	return -EPERM;
+}
+#endif //MARK_POLYMORPHIC
+
+static int marker_set_gen_enable(void *address, char enable)
+{
+	GEN_MARK_ENABLE(address) = enable;
+	return 0;
+}
+
+static int marker_set_enable(void *address, char enable, enum marker_type type)
+{
+	if (type == MARKER_OPTIMIZED)
+		return marker_set_ins_enable(address, enable);
+	else
+		return marker_set_gen_enable(address, enable);
+}
+	
+/* enable and function address are set out of order, and it's ok :
+ * the state is always coherent. */
+static int marker_set_probe_range(const char *name, 
+	const char *format,
+	marker_probe_func *probe,
+	const struct __mark_marker *begin,
+	const struct __mark_marker *end)
+{
+	const struct __mark_marker *iter;
+	int found = 0;
+
+	for (iter = begin; iter < end; iter++) {
+		if (strcmp(name, iter->cmark->name) == 0) {
+			if (format
+				&& strcmp(format, iter->cmark->format) != 0) {
+				printk(KERN_NOTICE
+					"Format mismatch for probe %s "
+					"(%s), marker (%s)\n",
+					name,
+					format,
+					iter->cmark->format);
+				continue;
+			}
+			if (probe == __mark_empty_function) {
+				if (*iter->cmark->call
+					!= __mark_empty_function) {
+					*iter->cmark->call =
+						__mark_empty_function;
+				}
+				marker_set_enable(iter->enable, 0,
+					iter->cmark->type);
+			} else {
+				if (*iter->cmark->call
+					!= __mark_empty_function) {
+					if (*iter->cmark->call != probe) {
+						printk(KERN_NOTICE
+							"Marker %s busy, "
+							"probe %p already "
+							"installed\n",
+							name,
+							*iter->cmark->call);
+						continue;
+					}
+				} else {
+					found++;
+					*iter->cmark->call = probe;
+				}
+				/* Can have many enables for one function */
+				marker_set_enable(iter->enable, 1,
+					iter->cmark->type);
+			}
+			found++;
+		}
+	}
+	return found;
+}
+
+static int marker_remove_probe_range(marker_probe_func *probe,
+	const struct __mark_marker *begin,
+	const struct __mark_marker *end)
+{
+	const struct __mark_marker *iter;
+	int found = 0;
+
+	for (iter = begin; iter < end; iter++) {
+		if (*iter->cmark->call == probe) {
+			marker_set_enable(iter->enable, 0,
+				iter->cmark->type);
+			*iter->cmark->call = __mark_empty_function;
+			found++;
+		}
+	}
+	return found;
+}
+
+static int marker_list_probe_range(marker_probe_func *probe,
+	const struct __mark_marker *begin,
+	const struct __mark_marker *end)
+{
+	const struct __mark_marker *iter;
+	int found = 0;
+
+	for (iter = begin; iter < end; iter++) {
+		if (probe)
+			if (probe != *iter->cmark->call) continue;
+		printk("name %s \n", iter->cmark->name);
+		if (iter->cmark->type == MARKER_OPTIMIZED)
+			printk("  enable %u optimized ",
+				MARK_ENABLE(iter->enable));
+		else
+			printk("  enable %u generic ",
+				GEN_MARK_ENABLE(iter->enable));
+		printk("  func 0x%p format \"%s\"\n",
+			*iter->cmark->call, iter->cmark->format);
+		found++;
+	}
+	return found;
+}
+/* markers use the modlist_lock to to synchronise */
+int marker_set_probe(const char *name, const char *format,
+				marker_probe_func *probe)
+{
+	struct module *mod;
+	int found = 0;
+
+	mutex_lock(&module_mutex);
+	/* Core kernel markers */
+	found += marker_set_probe_range(name, format, probe,
+			__start___markers, __stop___markers);
+	/* Markers in modules. */ 
+	list_for_each_entry(mod, &modules, list) {
+		if (!mod->taints)
+			found += marker_set_probe_range(name, format, probe,
+				mod->markers, mod->markers+mod->num_markers);
+	}
+	mutex_unlock(&module_mutex);
+	return found;
+}
+EXPORT_SYMBOL(marker_set_probe);
+
+int marker_remove_probe(marker_probe_func *probe)
+{
+	struct module *mod;
+	int found = 0;
+
+	mutex_lock(&module_mutex);
+	/* Core kernel markers */
+	found += marker_remove_probe_range(probe,
+			__start___markers, __stop___markers);
+	/* Markers in modules. */ 
+	list_for_each_entry(mod, &modules, list) {
+		if (!mod->taints)
+			found += marker_remove_probe_range(probe,
+				mod->markers, mod->markers+mod->num_markers);
+	}
+	mutex_unlock(&module_mutex);
+	return found;
+}
+EXPORT_SYMBOL(marker_remove_probe);
+
+int marker_list_probe(marker_probe_func *probe)
+{
+	struct module *mod;
+	int found = 0;
+
+	mutex_lock(&module_mutex);
+	/* Core kernel markers */
+	printk("Listing kernel markers\n");
+	found += marker_list_probe_range(probe,
+			__start___markers, __stop___markers);
+	/* Markers in modules. */ 
+	printk("Listing module markers\n");
+	list_for_each_entry(mod, &modules, list) {
+		if (!mod->taints) {
+			printk("Listing markers for module %s\n", mod->name);
+			found += marker_list_probe_range(probe,
+				mod->markers, mod->markers+mod->num_markers);
+		}
+	}
+	mutex_unlock(&module_mutex);
+	return found;
+}
+EXPORT_SYMBOL(marker_list_probe);
+#endif
+
 #ifdef CONFIG_SMP
 /* Number of blocks used and allocated. */
 static unsigned int pcpu_num_used, pcpu_num_allocated;
@@ -1561,6 +1773,7 @@ static struct module *load_module(void __user *umod,
 	unsigned int unusedcrcindex;
 	unsigned int unusedgplindex;
 	unsigned int unusedgplcrcindex;
+	unsigned int markersindex;
 	struct module *mod;
 	long err = 0;
 	void *percpu = NULL, *ptr = NULL; /* Stops spurious gcc warning */
@@ -1657,6 +1870,7 @@ 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");
 
 	/* Don't keep modinfo section */
 	sechdrs[infoindex].sh_flags &= ~(unsigned long)SHF_ALLOC;
@@ -1667,6 +1881,13 @@ static struct module *load_module(void __user *umod,
 #endif
 	if (unwindex)
 		sechdrs[unwindex].sh_flags |= SHF_ALLOC;
+#ifdef CONFIG_MARKERS
+	if (markersindex)
+		sechdrs[markersindex].sh_flags |= SHF_ALLOC;
+#else
+	if (markersindex)
+		sechdrs[markersindex].sh_flags &= ~(unsigned long)SHF_ALLOC;
+#endif
 
 	/* Check module struct version now, before we try to use module. */
 	if (!check_modstruct_version(sechdrs, versindex, mod)) {
@@ -1803,6 +2024,11 @@ static struct module *load_module(void __user *umod,
 	mod->gpl_future_syms = (void *)sechdrs[gplfutureindex].sh_addr;
 	if (gplfuturecrcindex)
 		mod->gpl_future_crcs = (void *)sechdrs[gplfuturecrcindex].sh_addr;
+	if (markersindex) {
+		mod->markers = (void *)sechdrs[markersindex].sh_addr;
+		mod->num_markers =
+			sechdrs[markersindex].sh_size / sizeof(*mod->markers);
+	}
 
 	mod->unused_syms = (void *)sechdrs[unusedindex].sh_addr;
 	if (unusedcrcindex)
@@ -2243,6 +2471,26 @@ const struct seq_operations modules_op = {
 	.show	= m_show
 };
 
+void list_modules(void)
+{
+	/* Enumerate loaded modules */
+	struct list_head	*i;
+	struct module		*mod;
+	unsigned long refcount = 0;
+	
+	mutex_lock(&module_mutex);
+	list_for_each(i, &modules) {
+		mod = list_entry(i, struct module, list);
+#ifdef CONFIG_MODULE_UNLOAD
+		refcount = local_read(&mod->ref[0].count);
+#endif //CONFIG_MODULE_UNLOAD
+		MARK(list_modules, "%s %d[enum module_state] %lu",
+				mod->name, mod->state, refcount);
+	}
+	mutex_unlock(&module_mutex);
+}
+EXPORT_SYMBOL(list_modules);
+
 /* Given an address, look for it in the module exception tables. */
 const struct exception_table_entry *search_module_extables(unsigned long addr)
 {

Re: [PATCH 02/05] Linux Kernel Markers, architecture independant code.

[Andrew Morton]

On Sun, 11 Feb 2007 15:03:24 -0500 Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca> wrote:

> Linux Kernel Markers, architecture independant code.
> 
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> 
> ...
>
> +
> +#ifndef MARK
> +#define MARK GEN_MARK
> +#define MARK_ENABLE_TYPE GEN_MARK_ENABLE_TYPE
> +#define MARK_ENABLE_IMMEDIATE_OFFSET GEN_MARK_ENABLE_IMMEDIATE_OFFSET
> +#endif

Also perhaps this nastiness can go away once each architecture has
asm/marker.h?

> +#ifdef MARK_POLYMORPHIC

What's this?  Is it commented somewhere?  (It should be...)

> +static int marker_set_ins_enable(void *address, char enable)
> +{
> +#ifdef CONFIG_X86_32
> +	return arch_marker_set_ins_enable(address, enable);
> +#else
> +	char newi[MARK_ENABLE_IMMEDIATE_OFFSET+1];
> +	int size = MARK_ENABLE_IMMEDIATE_OFFSET+sizeof(MARK_ENABLE_TYPE);
> +
> +	memcpy(newi, address, size);
> +	MARK_ENABLE(&newi[0]) = enable;
> +	memcpy(address, newi, size);
> +	flush_icache_range((unsigned long)address, size);
> +	return 0;
> +#endif //CONFIG_X86_32
> +}

eww.  Can we put a suitable arch_marker_set_ins_enable() into each arch's
marker.h?

> +#else
> +static int marker_set_ins_enable(void *address, char enable)
> +{
> +	return -EPERM;
> +}
> +#endif //MARK_POLYMORPHIC
> +
>
> ...
>

Re: [PATCH 02/05] Linux Kernel Markers, architecture independant code.

[Mathieu Desnoyers]

* Andrew Morton (akpm@linux-foundation.org) wrote:
> On Sun, 11 Feb 2007 15:03:24 -0500 Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca> wrote:
> 
> > Linux Kernel Markers, architecture independant code.
> > 
> > Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> > 
> > ...
> >
> > +
> > +#ifndef MARK
> > +#define MARK GEN_MARK
> > +#define MARK_ENABLE_TYPE GEN_MARK_ENABLE_TYPE
> > +#define MARK_ENABLE_IMMEDIATE_OFFSET GEN_MARK_ENABLE_IMMEDIATE_OFFSET
> > +#endif
> 
> Also perhaps this nastiness can go away once each architecture has
> asm/marker.h?
> 
Yes, absolutely.

> > +#ifdef MARK_POLYMORPHIC
> 
> What's this?  Is it commented somewhere?  (It should be...)
> 

No, but should be : it is defined by the architecture's marker.h when
the marker flavor has to do code motification when the marker state is
modified.


> > +static int marker_set_ins_enable(void *address, char enable)
> > +{
> > +#ifdef CONFIG_X86_32
> > +	return arch_marker_set_ins_enable(address, enable);
> > +#else
> > +	char newi[MARK_ENABLE_IMMEDIATE_OFFSET+1];
> > +	int size = MARK_ENABLE_IMMEDIATE_OFFSET+sizeof(MARK_ENABLE_TYPE);
> > +
> > +	memcpy(newi, address, size);
> > +	MARK_ENABLE(&newi[0]) = enable;
> > +	memcpy(address, newi, size);
> > +	flush_icache_range((unsigned long)address, size);
> > +	return 0;
> > +#endif //CONFIG_X86_32
> > +}
> 
> eww.  Can we put a suitable arch_marker_set_ins_enable() into each arch's
> marker.h?
> 

Or maybe put that in asm-generic so we do not duplicate code.

> > +#else
> > +static int marker_set_ins_enable(void *address, char enable)
> > +{
> > +	return -EPERM;
> > +}
> > +#endif //MARK_POLYMORPHIC
> > +
> >
> > ...
> >
> 

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Candidate, École Polytechnique de Montréal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

[PATCH 03/05] Linux Kernel Markers : powerpc optimization

[Mathieu Desnoyers]

Linux Kernel Markers : powerpc optimization

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

--- /dev/null
+++ b/include/asm-powerpc/marker.h
@@ -0,0 +1,49 @@
+/*
+ * marker.h
+ *
+ * Code markup for dynamic and static tracing. PowerPC architecture
+ * optimisations.
+ *
+ * (C) Copyright 2006 Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include <asm/asm-compat.h>
+
+#ifdef CONFIG_MARKERS
+
+#define MARK(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, \
+			MARKER_OPTIMIZED } ; \
+		char condition; \
+		asm volatile(	".section .markers, \"a\";\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)); \
+		__mark_check_format(format, ## args); \
+		if (unlikely(condition)) { \
+			preempt_disable(); \
+			(*__mark_call_##name)(format, ## args); \
+			preempt_enable(); \
+		} \
+	} while (0)
+
+
+/* Offset of the immediate value from the start of the addi instruction (result
+ * of the li mnemonic), in bytes. */
+#define MARK_ENABLE_IMMEDIATE_OFFSET 2
+#define MARK_ENABLE_TYPE short
+#define MARK_POLYMORPHIC
+
+#endif

[PATCH 04/05] Linux Kernel Markers : i386 optimization

[Mathieu Desnoyers]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 4617 bytes --]

Linux Kernel Markers : i386 optimization

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

--- /dev/null
+++ b/include/asm-i386/marker.h
@@ -0,0 +1,47 @@
+/*
+ * marker.h
+ *
+ * Code markup for dynamic and static tracing. i386 architecture optimisations.
+ *
+ * (C) Copyright 2006 Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+
+#ifdef CONFIG_MARKERS
+#define MARK(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, \
+			MARKER_OPTIMIZED } ; \
+		char condition; \
+		asm volatile(	".section .markers, \"a\";\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)); \
+		__mark_check_format(format, ## args); \
+		if (unlikely(condition)) { \
+			preempt_disable(); \
+			(*__mark_call_##name)(format, ## args); \
+			preempt_enable(); \
+		} \
+	} while (0)
+
+/* Offset of the immediate value from the start of the movb instruction, in
+ * bytes. */
+#define MARK_ENABLE_IMMEDIATE_OFFSET 1
+#define MARK_ENABLE_TYPE char
+#define MARK_POLYMORPHIC
+
+extern int arch_marker_set_ins_enable(void *address, char enable);
+
+#endif
--- /dev/null
+++ b/arch/i386/kernel/marker.c
@@ -0,0 +1,93 @@
+/* marker.c
+ *
+ * Erratum 49 fix for Intel PIII and higher.
+ *
+ * Permits marker activation by XMC with correct serialization.
+ *
+ * Reentrant for NMI and trap handler instrumentation. :-)
+ *
+ * Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
+ */
+
+#include <linux/notifier.h>
+#include <linux/mutex.h>
+#include <linux/preempt.h>
+#include <linux/smp.h>
+#include <linux/notifier.h>
+#include <linux/module.h>
+#include <asm/cacheflush.h>
+#include <asm/kdebug.h>
+
+#define BREAKPOINT_INSTRUCTION  0xcc
+#define BREAKPOINT_INS_LEN 1
+
+static DEFINE_MUTEX(mark_mutex);
+static long target_eip = 0;
+
+static void mark_synchronize_core(void *info)
+{
+	sync_core();	/* use cpuid to stop speculative execution */
+}
+
+/* We simply skip the 2 bytes load immediate here, leaving the register in an
+ * undefined state. We don't care about the content (0 or !0), because we are
+ * changing the value 0->1 or 1->0. This small window of undefined value
+ * doesn't matter.
+ */
+static int mark_notifier(struct notifier_block *nb,
+	unsigned long val, void *data)
+{
+	enum die_val die_val = (enum die_val) val;
+	struct die_args *args = (struct die_args *)data;
+	
+	if (!args->regs || user_mode_vm(args->regs))
+		return NOTIFY_DONE;
+
+	if (die_val == DIE_INT3	&& args->regs->eip == target_eip) {
+		args->regs->eip += 1; /* Skip the next byte of load immediate */
+		return NOTIFY_STOP;
+	}
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block mark_notify = {
+	.notifier_call = mark_notifier,
+	.priority = 0x7fffffff,	/* we need to be notified first */
+};
+
+int arch_marker_set_ins_enable(void *address, char enable)
+{
+	char saved_byte;
+	int ret;
+	char *dest = address;
+
+	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);
+	saved_byte = *dest;
+	*dest = BREAKPOINT_INSTRUCTION;
+	wmb();
+	/* Execute serializing instruction on each CPU.
+	 * Acts as a memory barrier. */
+	ret = on_each_cpu(mark_synchronize_core, NULL, 1, 1);
+	BUG_ON(ret != 0);
+
+	dest[1] = enable;
+	wmb();
+	*dest = saved_byte;
+		/* Wait for all int3 handlers to end
+		   (interrupts are disabled in int3).
+		   This CPU is clearly not in a int3 handler
+		   (not preemptible).
+		   synchronize_sched has memory barriers */
+	synchronize_sched();
+	unregister_die_notifier(&mark_notify);
+	/* unregister_die_notifier has memory barriers */
+	target_eip = 0;
+	mutex_unlock(&mark_mutex);
+	flush_icache_range(address, size);
+	return 0;
+}
+EXPORT_SYMBOL(arch_marker_set_ins_enable);
--- a/arch/i386/kernel/Makefile
+++ b/arch/i386/kernel/Makefile
@@ -39,6 +39,7 @@ obj-$(CONFIG_VM86)		+= vm86.o
 obj-$(CONFIG_EARLY_PRINTK)	+= early_printk.o
 obj-$(CONFIG_HPET_TIMER) 	+= hpet.o
 obj-$(CONFIG_K8_NB)		+= k8.o
+obj-$(CONFIG_MARKERS_ENABLE_OPTIMIZATION)	+= marker.o
 
 # Make sure this is linked after any other paravirt_ops structs: see head.S
 obj-$(CONFIG_PARAVIRT)		+= paravirt.o

[PATCH 05/05] Linux Kernel Markers, non optimized architectures

[Mathieu Desnoyers]

Linux Kernel Markers, non optimized architectures

This patch also includes marker code for non optimized architectures.

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

--- /dev/null
+++ b/include/asm-arm/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-cris/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-frv/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-generic/marker.h
@@ -0,0 +1,40 @@
+/*
+ * marker.h
+ *
+ * Code markup for dynamic and static tracing. Generic header.
+ * 
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ *
+ * Note : the empty asm volatile with read constraint is used here instead of a
+ * "used" attribute to fix a gcc 4.1.x bug.
+ */
+
+#ifdef CONFIG_MARKERS
+
+#define GEN_MARK(name, format, args...) \
+	do { \
+		static marker_probe_func *__mark_call_##name = \
+					__mark_empty_function; \
+		static char __marker_enable_##name = 0; \
+		static const struct __mark_marker_c __mark_c_##name \
+			__attribute__((section(".markers.c"))) = \
+			{ #name, &__mark_call_##name, format, \
+			MARKER_GENERIC } ; \
+		static const struct __mark_marker __mark_##name \
+			__attribute__((section(".markers"))) = \
+			{ &__mark_c_##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); \
+			preempt_enable(); \
+		} \
+	} while (0)
+
+
+#define GEN_MARK_ENABLE_IMMEDIATE_OFFSET 0
+#define GEN_MARK_ENABLE_TYPE char
+
+#endif
--- /dev/null
+++ b/include/asm-h8300/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-ia64/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-m32r/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-m68k/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-m68knommu/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-mips/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-parisc/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-ppc/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-s390/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-sh64/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-sh/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-sparc64/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-sparc/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-um/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-v850/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-x86_64/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-xtensa/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>

Re: [PATCH 05/05] Linux Kernel Markers, non optimized architectures

[Andrew Morton]

On Sun, 11 Feb 2007 15:03:27 -0500 Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca> wrote:

> Linux Kernel Markers, non optimized architectures
> 
> This patch also includes marker code for non optimized architectures.

I think once we've done this we can nuke
CONFIG_MARKERS_ENABLE_OPTIMIZATION?  (Please, let it be true).

Re: [PATCH 05/05] Linux Kernel Markers, non optimized architectures

[Mathieu Desnoyers]

* Andrew Morton (akpm@linux-foundation.org) wrote:
> On Sun, 11 Feb 2007 15:03:27 -0500 Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca> wrote:
> 
> > Linux Kernel Markers, non optimized architectures
> > 
> > This patch also includes marker code for non optimized architectures.
> 
> I think once we've done this we can nuke
> CONFIG_MARKERS_ENABLE_OPTIMIZATION?  (Please, let it be true).
> 

Hi Andrew,

The main goal of this config option is for embedded systems which
doesn't support live code modification. Maybe we can put that under
"embedded sytems" menu ?

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Candidate, École Polytechnique de Montréal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

Re: Re: [PATCH 05/05] Linux Kernel Markers, non optimized architectures

[Karim Yaghmour]

----- KRYPTIVA PACKAGED MESSAGE -----
PACKAGING TYPE: SIGNED

Mathieu Desnoyers wrote:
 > The main goal of this config option is for embedded systems which doesn't support live code modification. Maybe we can put that under "embedded sytems" menu ?

Not sure whether you had had other feedback on this elsewhere in
the rest of the thread, but yes, this would make sense if the
"embedded" angle is the only reason we need this (and not, say,
performance, etc.) Also, having done that, maybe it would make
some sense to have it be a "disable" rather than "enable":
CONFIG_MARKERS_DISABLE_OPTIMIZATION?

Karim


----- KRYPTIVA SIGNED MESSAGE -----
This email claims to have been packaged by Kryptiva.
To process this email and authenticate its origin, get
the free plugin from:
http://www.kryptiva.com/downloads

----- KRYPTIVA SIGNATURE START -----
AvWVqAAAAAIAAAABAAAAAAAATiACAQAAAAC3AQAIAAAAAgAAAAECABTXxT4xHdR4/1uU1hL2
+TaPrqNB0wMAFNa8GHXZWJH5Dz+D76vfh6JhvWLvBAAUpuIZcCAkCC+ldyaBuoAWxK50HiQF
ABRI38gc/foDHQsS6X3W0VP4xTukBwYAFDzvzh+u6zVtolglAZrnE7FOmtZDBwAUTxyTas6N
WLapdnSnAwVHeC06/ioRABgAAAAAAABOIEXWD8AACTdnAAAAAAAAAN8TAAQAAAAAAAAAggP+
K8Gk1SWj+c67jiJerodkr1gntoa9dJVVN6InxB824CfKC6flE4JMWtffw0Dxh0cJ8iOQ8UeC
zoWzTs9Z+K9j1CL11CHkIIit3RK3hnfnby6whr4xoZ9UX/BUUv8FVKZeyRg7SbDKlhEZTwIH
7axjVQJ6MGU7h+0/5dKCDMEtzPY=
----- KRYPTIVA SIGNATURE END -----

Re: [PATCH 05/05] Linux Kernel Markers, non optimized architectures

[Mathieu Desnoyers]

* Karim Yaghmour (karim.yaghmour@kryptiva.com) wrote:
> ----- KRYPTIVA PACKAGED MESSAGE -----
> PACKAGING TYPE: SIGNED
> 
> Mathieu Desnoyers wrote:
> > The main goal of this config option is for embedded systems which doesn't 
> support live code modification. Maybe we can put that under "embedded 
> sytems" menu ?
> 
> Not sure whether you had had other feedback on this elsewhere in
> the rest of the thread, but yes, this would make sense if the
> "embedded" angle is the only reason we need this (and not, say,
> performance, etc.) Also, having done that, maybe it would make
> some sense to have it be a "disable" rather than "enable":
> CONFIG_MARKERS_DISABLE_OPTIMIZATION?
> 

Hi Karim,

Yes, that was indeed the first way I implemented it, as a "disable"
option. One of the main thing we have to figure out before I modify this
is if we want to have the generic version of markers available in a
"forced" manner at the marker site with the GEN_MARK macro instead of
the MARK macro (this is the actual implementation). It has proven to
be useful to instrument lockdep.c irq enable/disable tracing functions.
The reason why is because they are called just before the trap handler
returns and I need it to do XMC on x86 and x86_64. It would therefore
cause a recursive trap.

I think it makes sense to have this kind of support for hard-to-instrument
sites within the marker infrastructure, but the cost is to have two
marker flavors : MARK and GEN_MARK (but really GEN_MARK is only intended
for a few sites).

Mathieu

> Karim
> 
> 
> ----- KRYPTIVA SIGNED MESSAGE -----
> This email claims to have been packaged by Kryptiva.
> To process this email and authenticate its origin, get
> the free plugin from:
> http://www.kryptiva.com/downloads
> 
> ----- KRYPTIVA SIGNATURE START -----
> AvWVqAAAAAIAAAABAAAAAAAATiACAQAAAAC3AQAIAAAAAgAAAAECABTXxT4xHdR4/1uU1hL2
> +TaPrqNB0wMAFNa8GHXZWJH5Dz+D76vfh6JhvWLvBAAUpuIZcCAkCC+ldyaBuoAWxK50HiQF
> ABRI38gc/foDHQsS6X3W0VP4xTukBwYAFDzvzh+u6zVtolglAZrnE7FOmtZDBwAUTxyTas6N
> WLapdnSnAwVHeC06/ioRABgAAAAAAABOIEXWD8AACTdnAAAAAAAAAN8TAAQAAAAAAAAAggP+
> K8Gk1SWj+c67jiJerodkr1gntoa9dJVVN6InxB824CfKC6flE4JMWtffw0Dxh0cJ8iOQ8UeC
> zoWzTs9Z+K9j1CL11CHkIIit3RK3hnfnby6whr4xoZ9UX/BUUv8FVKZeyRg7SbDKlhEZTwIH
> 7axjVQJ6MGU7h+0/5dKCDMEtzPY=
> ----- KRYPTIVA SIGNATURE END -----

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

Re: [PATCH 05/05] Linux Kernel Markers, non optimized architectures

[Karim Yaghmour]

----- KRYPTIVA PACKAGED MESSAGE -----
PACKAGING TYPE: SIGNED

Hello Mathieu,

Mathieu Desnoyers wrote:
 > Yes, that was indeed the first way I implemented it, as a "disable" option. One of the main thing we have to figure out before I modify this is if we want to have the generic version of markers available in a "forced" manner at the marker site with the GEN_MARK macro instead of the MARK macro (this is the actual implementation). It has proven to be useful to instrument lockdep.c irq
 > enable/disable tracing functions. The reason why is because they are called just before the trap handler returns and I need it to do XMC on x86 and x86_64. It would therefore cause a recursive trap.
 >
 > I think it makes sense to have this kind of support for hard-to-instrument sites within the marker infrastructure, but the cost is to have two marker flavors : MARK and GEN_MARK (but really GEN_MARK is only intended for a few sites).

I must admit that I'm unsure about the use of different marker macros.
How about bitwise flags that could be coded as part of the marker
at the marker site? Something like "MARKER_TYPE_FORCED". This would
still allow some form of toplevel control at the macro definition.
Otherwise there's some digging to be done on a per-marker
basis ...

Karim
----- KRYPTIVA SIGNED MESSAGE -----
This email claims to have been packaged by Kryptiva.
To process this email and authenticate its origin, get
the free plugin from:
http://www.kryptiva.com/downloads

----- KRYPTIVA SIGNATURE START -----
AvWVqAAAAAIAAAABAAAAAAAATiACAQAAAAC3AQAIAAAAAgAAAAECABTXxT4xHdR4/1uU1hL2
+TaPrqNB0wMAFNa8GHXZWJH5Dz+D76vfh6JhvWLvBAAUpuIZcCAkCC+ldyaBuoAWxK50HiQF
ABRI38gc/foDHQsS6X3W0VP4xTukBwYAFB0lithGcxNZYBHaLDONjp6eo/LoBwAU6OwGS0m1
IVdBt6tKzhaPW8MOfncRABgAAAAAAABOIEXcozcACATMAAAAAAAAABkTAAQAAAAAAAAAggQA
mHAJeFbYUzxSX+zkI0DtoVKcqqSp2Ztc9GtY7ZtuLBmeqg5pW0rIbkhutQiztTXlJQ0Ye9bV
yzEVWd/m7GhDAgRBmyg3kCOt7g7potr1l5J3X5K8TiqtWXbNo3k6AHRlGZyn0190iIBSvf85
nVh3hKiNPsw8DYs1NKb+KMON+4g=
----- KRYPTIVA SIGNATURE END -----

Re: [PATCH 05/05] Linux Kernel Markers, non optimized architectures

[Mathieu Desnoyers]

* Karim Yaghmour (karim.yaghmour@kryptiva.com) wrote:
> ----- KRYPTIVA PACKAGED MESSAGE -----
> PACKAGING TYPE: SIGNED
> 
> Hello Mathieu,
> 
> Mathieu Desnoyers wrote:
> > Yes, that was indeed the first way I implemented it, as a "disable" 
> option. One of the main thing we have to figure out before I modify this is 
> if we want to have the generic version of markers available in a "forced" 
> manner at the marker site with the GEN_MARK macro instead of the MARK macro 
> (this is the actual implementation). It has proven to be useful to 
> instrument lockdep.c irq
> > enable/disable tracing functions. The reason why is because they are 
> called just before the trap handler returns and I need it to do XMC on x86 
> and x86_64. It would therefore cause a recursive trap.
> >
> > I think it makes sense to have this kind of support for 
> hard-to-instrument sites within the marker infrastructure, but the cost is 
> to have two marker flavors : MARK and GEN_MARK (but really GEN_MARK is only 
> intended for a few sites).
> 
> I must admit that I'm unsure about the use of different marker macros.
> How about bitwise flags that could be coded as part of the marker
> at the marker site? Something like "MARKER_TYPE_FORCED". This would
> still allow some form of toplevel control at the macro definition.
> Otherwise there's some digging to be done on a per-marker
> basis ...
> 

The problem with your proposal, I guess, is that people will have to
add a supplementary parameter to the macro.

It is not uncommon to have two slightly versions of macros/functions in
the kernel (preempt_enable()/preempt_enable_no_resched(), or macros
starting with underscores). Normally, the underscore states that the
macro does not do the proper locking itself (this is not our case).
Therefore, I would suggest using a name that suggests against what the
macro is protected. For instance, a marker pointing to the generic
version is only needed to protect against the debug trap handler and
should only be used on x86 and x86_64.

So, something like MARK_NO_TRAP() would be appropriate : it would be an
optimized version for every architecture except x86 and x86_64. The
meaning of this macro is : "This is a marker that will never generate a
trap because of its activation" (just as a precision : it doesn't say
anything about the probe connected to the marker). It also acts as a
strong suggestion about what *should not* be done within the probe.

Mathieu

> Karim
> ----- KRYPTIVA SIGNED MESSAGE -----
> This email claims to have been packaged by Kryptiva.
> To process this email and authenticate its origin, get
> the free plugin from:
> http://www.kryptiva.com/downloads
> 
> ----- KRYPTIVA SIGNATURE START -----
> AvWVqAAAAAIAAAABAAAAAAAATiACAQAAAAC3AQAIAAAAAgAAAAECABTXxT4xHdR4/1uU1hL2
> +TaPrqNB0wMAFNa8GHXZWJH5Dz+D76vfh6JhvWLvBAAUpuIZcCAkCC+ldyaBuoAWxK50HiQF
> ABRI38gc/foDHQsS6X3W0VP4xTukBwYAFB0lithGcxNZYBHaLDONjp6eo/LoBwAU6OwGS0m1
> IVdBt6tKzhaPW8MOfncRABgAAAAAAABOIEXcozcACATMAAAAAAAAABkTAAQAAAAAAAAAggQA
> mHAJeFbYUzxSX+zkI0DtoVKcqqSp2Ztc9GtY7ZtuLBmeqg5pW0rIbkhutQiztTXlJQ0Ye9bV
> yzEVWd/m7GhDAgRBmyg3kCOt7g7potr1l5J3X5K8TiqtWXbNo3k6AHRlGZyn0190iIBSvf85
> nVh3hKiNPsw8DYs1NKb+KMON+4g=
> ----- KRYPTIVA SIGNATURE END -----

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

Re: [PATCH 05/05] Linux Kernel Markers, non optimized architectures

[Karim Yaghmour]

----- KRYPTIVA PACKAGED MESSAGE -----
PACKAGING TYPE: SIGNED

Mathieu Desnoyers wrote:
 > The problem with your proposal, I guess, is that people will have to add a supplementary parameter to the macro.
 >
 > It is not uncommon to have two slightly versions of macros/functions in the kernel (preempt_enable()/preempt_enable_no_resched(), or macros starting with underscores). Normally, the underscore states that the macro does not do the proper locking itself (this is not our case). Therefore, I would suggest using a name that suggests against what the macro is protected. For instance, a marker
 > pointing to the generic version is only needed to protect against the debug trap handler and should only be used on x86 and x86_64.

I can see your point, to a degree. The difference here is that the
variants you mention are actually macros that do something, they
aren't stubs for code. IOW, you actually know what's happening
underneath a foo() vs. _foo() by its name only. Maybe this applies
the same to markers, I don't know. But maybe we want to make it
easy for those looking at markers that there's a master kill
switch somewhere that all markers go through and through which
they can all be disabled very simply (say by using a "#if 0").
While different names *may* be doing that, a same name *does* that.
But I don't feel too strongly either way, it's really up to those
who maintaining the code to say.

Karim

----- KRYPTIVA SIGNED MESSAGE -----
This email claims to have been packaged by Kryptiva.
To process this email and authenticate its origin, get
the free plugin from:
http://www.kryptiva.com/downloads

----- KRYPTIVA SIGNATURE START -----
AvWVqAAAAAIAAAABAAAAAAAATiACAQAAAAC3AQAIAAAAAgAAAAECABTXxT4xHdR4/1uU1hL2
+TaPrqNB0wMAFNa8GHXZWJH5Dz+D76vfh6JhvWLvBAAUpuIZcCAkCC+ldyaBuoAWxK50HiQF
ABRI38gc/foDHQsS6X3W0VP4xTukBwYAFB0lithGcxNZYBHaLDONjp6eo/LoBwAUpXC6F2jf
nElq3fnZQpGW97Fk/2QRABgAAAAAAABOIEXcvqAADJ5wAAAAAAAAAB4TAAQAAAAAAAAAggP/
RQ/W0H9H9bhrZyC67an//DbWC4D38PgLoeMG6Tjvx7jWTpEh79DeQ/+sbb9aYZvbwYwtaVaJ
VuPEiRnPZX0mqnOFm+GDzE9jB6202lR0Nzczh1WCifbrrXI7CSEjOwI3ve0jcCoGxTEzZRYj
LGxuubV8Hh5HU12zi3Mxgdz031Y=
----- KRYPTIVA SIGNATURE END -----

[PATCH] Linux Kernel Markers - cleanup

[Mathieu Desnoyers]

Linux Kernel Markers - cleanup

- Keep a positive CONFIG_MARKERS_ENABLE_OPTIMIZATION for Makefile.
- Have CONFIG_MARKERS_DISABLE_OPTIMIZATION depending on EMBEDDED shown
  in the menus.
- CONFIG_MARKERS_ENABLE_OPTIMIZATION depends on
       !CONFIG_MARKERS_DISABLE_OPTIMIZATION and defaults to y (hidden)
         (suggested by Martin Bligh)
- GEN_MARK is now declared in linux/marker.h
- asm-generic/marker.h is now only used as a fallback defining MARK as GEN_MARK
- New MARK_NO_TRAP defined for each architecture.
  asm-generic : defined as GEN_MARK
  asm-i386 : defined as GEN_MARK
  asm-powerpc : defined as MARK (because we don't need to use trap for XMC)
- Remove ugly ifdefs SOME_RANDOM_ARCH in architecture agnostic code

It applies on top of the
linux-kernel-markers-architecture-independant-code-license-fix patch.

Compiles on
arm
i686 (markers enabled, markers disabled)
ia64
m68k
mips
mipsel
powerpc 405
powerpc 970
s390
sparc (except link error not related to markers)
sparc64
x86_64

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

--- a/arch/i386/kernel/marker.c
+++ b/arch/i386/kernel/marker.c
@@ -56,7 +56,7 @@ static struct notifier_block mark_notify = {
 	.priority = 0x7fffffff,	/* we need to be notified first */
 };
 
-int arch_marker_set_ins_enable(void *address, char enable)
+int marker_set_ins_enable(void *address, char enable)
 {
 	char saved_byte;
 	int ret;
@@ -91,4 +91,4 @@ int arch_marker_set_ins_enable(void *address, char enable)
 	flush_icache_range(address, size);
 	return 0;
 }
-EXPORT_SYMBOL_GPL(arch_marker_set_ins_enable);
+EXPORT_SYMBOL_GPL(marker_set_ins_enable);
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -92,3 +92,4 @@ obj-$(CONFIG_PPC64)		+= $(obj64-y)
 
 extra-$(CONFIG_PPC_FPU)		+= fpu.o
 extra-$(CONFIG_PPC64)		+= entry_64.o
+obj-$(CONFIG_MARKERS_ENABLE_OPTIMIZATION)	+= marker.o
--- /dev/null
+++ b/arch/powerpc/kernel/marker.c
@@ -0,0 +1,24 @@
+/* marker.c
+ *
+ * Powerpc optimized marker enabling/disabling.
+ *
+ * Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
+ */
+
+#include <linux/module.h>
+#include <linux/marker.h>
+#include <linux/string.h>
+#include <asm/cacheflush.h>
+
+int marker_set_ins_enable(void *address, char enable)
+{
+	char newi[MARK_ENABLE_IMMEDIATE_OFFSET+1];
+	int size = MARK_ENABLE_IMMEDIATE_OFFSET+sizeof(MARK_ENABLE_TYPE);
+
+	memcpy(newi, address, size);
+	MARK_ENABLE(&newi[0]) = enable;
+	memcpy(address, newi, size);
+	flush_icache_range((unsigned long)address, size);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(marker_set_ins_enable);
--- a/include/asm-generic/marker.h
+++ b/include/asm-generic/marker.h
@@ -1,3 +1,6 @@
+#ifndef _ASM_GENERIC_MARKER_H
+#define _ASM_GENERIC_MARKER_H
+
 /*
  * marker.h
  *
@@ -10,31 +13,17 @@
  * "used" attribute to fix a gcc 4.1.x bug.
  */
 
-#ifdef CONFIG_MARKERS
-
-#define GEN_MARK(name, format, args...) \
-	do { \
-		static marker_probe_func *__mark_call_##name = \
-					__mark_empty_function; \
-		static char __marker_enable_##name = 0; \
-		static const struct __mark_marker_c __mark_c_##name \
-			__attribute__((section(".markers.c"))) = \
-			{ #name, &__mark_call_##name, format, \
-			MARKER_GENERIC } ; \
-		static const struct __mark_marker __mark_##name \
-			__attribute__((section(".markers"))) = \
-			{ &__mark_c_##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); \
-			preempt_enable(); \
-		} \
-	} while (0)
+#include <linux/errno.h>
 
+#define MARK				GEN_MARK
+#define MARK_NO_TRAP			GEN_MARK
+#define MARK_ENABLE_TYPE		GEN_MARK_ENABLE_TYPE
+#define MARK_ENABLE_IMMEDIATE_OFFSET	GEN_MARK_ENABLE_IMMEDIATE_OFFSET
+#define MARK_ENABLE			GEN_MARK_ENABLE
 
-#define GEN_MARK_ENABLE_IMMEDIATE_OFFSET 0
-#define GEN_MARK_ENABLE_TYPE char
+static inline int marker_set_ins_enable(void *address, char enable)
+{
+	return -ENOSYS;
+}
 
-#endif
+#endif /* _ASM_GENERIC_MARKER_H */
--- a/include/asm-i386/marker.h
+++ b/include/asm-i386/marker.h
@@ -36,12 +36,15 @@
 		} \
 	} while (0)
 
+#define MARK_NO_TRAP GEN_MARK
 /* Offset of the immediate value from the start of the movb instruction, in
  * bytes. */
 #define MARK_ENABLE_IMMEDIATE_OFFSET 1
 #define MARK_ENABLE_TYPE char
-#define MARK_POLYMORPHIC
+/* Dereference enable as lvalue from a pointer to its instruction */
+#define MARK_ENABLE(a) \
+	*(MARK_ENABLE_TYPE*)((char*)a+MARK_ENABLE_IMMEDIATE_OFFSET)
 
-extern int arch_marker_set_ins_enable(void *address, char enable);
+extern int marker_set_ins_enable(void *address, char enable);
 
 #endif
--- a/include/asm-powerpc/marker.h
+++ b/include/asm-powerpc/marker.h
@@ -40,10 +40,15 @@
 	} while (0)
 
 
+#define MARK_NO_TRAP MARK
 /* Offset of the immediate value from the start of the addi instruction (result
  * of the li mnemonic), in bytes. */
 #define MARK_ENABLE_IMMEDIATE_OFFSET 2
 #define MARK_ENABLE_TYPE short
-#define MARK_POLYMORPHIC
+/* Dereference enable as lvalue from a pointer to its instruction */
+#define MARK_ENABLE(a) \
+	*(MARK_ENABLE_TYPE*)((char*)a+MARK_ENABLE_IMMEDIATE_OFFSET)
+
+extern int marker_set_ins_enable(void *address, char enable);
 
 #endif
--- a/include/linux/marker.h
+++ b/include/linux/marker.h
@@ -6,29 +6,7 @@
  *
  * Code markup for dynamic and static tracing.
  *
- * Example :
- *
- * MARK(subsystem_event, "%d %s %p[struct task_struct]",
- *   someint, somestring, current);
- * Where :
- * - 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.
- * - someint is an integer.
- * - somestring is a char pointer.
- * - current is a pointer to a struct task_struct.
- *
- * - Dynamically overridable function call based on marker mechanism
- *   from Frank Ch. Eigler <fche@redhat.com>.
- * - Thanks to Jeremy Fitzhardinge <jeremy@goop.org> for his constructive
- *   criticism about gcc optimization related issues.
- *
- * The marker mechanism supports multiple instances of the same marker.
- * Markers can be put in inline functions, inlined static functions and
- * unrolled loops.
- *
- * Note : It is safe to put markers within preempt-safe code : preempt_enable()
- * will not call the scheduler due to the tests in preempt_schedule().
+ * See Documentation/marker.txt.
  *
  * (C) Copyright 2006 Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
  *
@@ -36,7 +14,7 @@
  * See the file COPYING for more details.
  */
 
-#ifndef __ASSEMBLY__
+#ifdef __KERNEL__
 
 typedef void marker_probe_func(const char *fmt, ...);
 
@@ -54,32 +32,48 @@ struct __mark_marker {
 	void *enable;
 } __attribute__((packed));
 
-#ifdef CONFIG_MARKERS_ENABLE_OPTIMIZATION
-#include <asm/marker.h>
-#endif
-
-#include <asm-generic/marker.h>
-
-#define MARK_NOARGS " "
-#define MARK_MAX_FORMAT_LEN	1024
+/* Generic marker flavor always available */
+#ifdef CONFIG_MARKERS
+#define GEN_MARK(name, format, args...) \
+	do { \
+		static marker_probe_func *__mark_call_##name = \
+					__mark_empty_function; \
+		static char __marker_enable_##name = 0; \
+		static const struct __mark_marker_c __mark_c_##name \
+			__attribute__((section(".markers.c"))) = \
+			{ #name, &__mark_call_##name, format, \
+			MARKER_GENERIC } ; \
+		static const struct __mark_marker __mark_##name \
+			__attribute__((section(".markers"))) = \
+			{ &__mark_c_##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); \
+			preempt_enable(); \
+		} \
+	} while (0)
+
+#define GEN_MARK_ENABLE_IMMEDIATE_OFFSET 0
+#define GEN_MARK_ENABLE_TYPE char
+/* Dereference enable as lvalue from a pointer to its instruction */
+#define GEN_MARK_ENABLE(a) \
+	*(GEN_MARK_ENABLE_TYPE*)((char*)a+GEN_MARK_ENABLE_IMMEDIATE_OFFSET)
 
-#ifndef CONFIG_MARKERS
+#else /* !CONFIG_MARKERS */
 #define GEN_MARK(name, format, args...) \
 		__mark_check_format(format, ## args)
-#endif
+#endif /* CONFIG_MARKERS */
 
-#ifndef MARK
-#define MARK GEN_MARK
-#define MARK_ENABLE_TYPE GEN_MARK_ENABLE_TYPE
-#define MARK_ENABLE_IMMEDIATE_OFFSET GEN_MARK_ENABLE_IMMEDIATE_OFFSET
+#ifdef CONFIG_MARKERS_ENABLE_OPTIMIZATION
+#include <asm/marker.h>			/* optimized marker flavor */
+#else
+#include <asm-generic/marker.h>		/* fallback on generic markers */
 #endif
 
-/* Dereference enable as lvalue from a pointer to its instruction */
-#define MARK_ENABLE(a) \
-	*(MARK_ENABLE_TYPE*)((char*)a+MARK_ENABLE_IMMEDIATE_OFFSET)
-
-#define GEN_MARK_ENABLE(a) \
-	*(GEN_MARK_ENABLE_TYPE*)((char*)a+GEN_MARK_ENABLE_IMMEDIATE_OFFSET)
+#define MARK_MAX_FORMAT_LEN	1024
+#define MARK_NOARGS " "
 
 static inline __attribute__ ((format (printf, 1, 2)))
 void __mark_check_format(const char *fmt, ...)
@@ -93,5 +87,5 @@ extern int marker_set_probe(const char *name, const char *format,
 extern int marker_remove_probe(marker_probe_func *probe);
 extern int marker_list_probe(marker_probe_func *probe);
 
-#endif
+#endif /* __KERNEL__ */
 #endif
--- a/kernel/Kconfig.marker
+++ b/kernel/Kconfig.marker
@@ -7,10 +7,14 @@ config MARKERS
 	  Place an empty function call at each marker site. Can be
 	  dynamically changed for a probe function.
 
-config MARKERS_ENABLE_OPTIMIZATION
-	bool "Enable marker optimization"
-	depends on MARKERS
-	default y
+config MARKERS_DISABLE_OPTIMIZATION
+	bool "Disable marker optimization"
+	depends on MARKERS && EMBEDDED
+	default n
 	help
 	  Disable code replacement jump optimisations. Especially useful if your
 	  code is in a read-only rom/flash.
+
+config MARKERS_ENABLE_OPTIMIZATION
+	def_bool y
+	depends on MARKERS && !MARKERS_DISABLE_OPTIMIZATION
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -307,29 +307,6 @@ void __mark_empty_function(const char *fmt, ...)
 }
 EXPORT_SYMBOL_GPL(__mark_empty_function);
 
-#ifdef MARK_POLYMORPHIC
-static int marker_set_ins_enable(void *address, char enable)
-{
-#ifdef CONFIG_X86_32
-	return arch_marker_set_ins_enable(address, enable);
-#else
-	char newi[MARK_ENABLE_IMMEDIATE_OFFSET+1];
-	int size = MARK_ENABLE_IMMEDIATE_OFFSET+sizeof(MARK_ENABLE_TYPE);
-
-	memcpy(newi, address, size);
-	MARK_ENABLE(&newi[0]) = enable;
-	memcpy(address, newi, size);
-	flush_icache_range((unsigned long)address, size);
-	return 0;
-#endif //CONFIG_X86_32
-}
-#else
-static int marker_set_ins_enable(void *address, char enable)
-{
-	return -EPERM;
-}
-#endif //MARK_POLYMORPHIC
-
 static int marker_set_gen_enable(void *address, char enable)
 {
 	GEN_MARK_ENABLE(address) = enable;
-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Candidate, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

Re: [PATCH 00/05] Linux Kernel Markers - kernel 2.6.20

[Andrew Morton]

On Sun, 11 Feb 2007 15:03:22 -0500 Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca> wrote:

> You will find, in the following posts, the latest revision of the Linux Kernel
> Markers.

<looks for the documentation>

And what can I do with these markers?

And once I've done it, are there any userspace applications I can use to
get the data out in human-usable form?

It seems to me that these patches aren't sequenced correctly - the kernel won't
build at each step of the patch sequence.  Maybe I'm mistaken in that.  We prefer
it this way so that people don't hit wont-compile points when doing bisection searches.

Re: [PATCH 00/05] Linux Kernel Markers - kernel 2.6.20

[Frank Ch. Eigler]

akpm wrote:

> [...]  And what can I do with these markers?  And once I've done it,
> are there any userspace applications I can use to get the data out
> in human-usable form?  [...]

The LTTng user-space programs use it today.  Systemtap used to support
the earlier marker prototype and will be rapidly ported over to this
new API upon acceptance.

- FChE

Re: [PATCH 00/05] Linux Kernel Markers - kernel 2.6.20

[Andrew Morton]

On 15 Feb 2007 10:28:57 -0500
fche@redhat.com (Frank Ch. Eigler) wrote:

> 
> akpm wrote:
> 
> > [...]  And what can I do with these markers?  And once I've done it,
> > are there any userspace applications I can use to get the data out
> > in human-usable form?  [...]
> 
> The LTTng user-space programs use it today.  Systemtap used to support
> the earlier marker prototype and will be rapidly ported over to this
> new API upon acceptance.
> 

That's good.

It would be beneficial if some people from those projects could spare the
cycles to carefully review and runtime test this code.

Also, I'm not 100% clear on where we ended up with the huge
static-vs-dynamic flamewar.  Did everyone end up happy?  Is this patchset a
reasonable compromise?  Or do we need a rematch?

Re: [PATCH 00/05] Linux Kernel Markers - kernel 2.6.20

[Mathieu Desnoyers]

* Andrew Morton (akpm@linux-foundation.org) wrote:
> On 15 Feb 2007 10:28:57 -0500
> fche@redhat.com (Frank Ch. Eigler) wrote:
> 
> > 
> > akpm wrote:
> > 
> > > [...]  And what can I do with these markers?  And once I've done it,
> > > are there any userspace applications I can use to get the data out
> > > in human-usable form?  [...]
> > 
> > The LTTng user-space programs use it today.  Systemtap used to support
> > the earlier marker prototype and will be rapidly ported over to this
> > new API upon acceptance.
> > 
> 
> That's good.
> 
> It would be beneficial if some people from those projects could spare the
> cycles to carefully review and runtime test this code.
> 
LTTng is using the marker infrastructure since last november. Me and my
users have been very happy with it.

> Also, I'm not 100% clear on where we ended up with the huge
> static-vs-dynamic flamewar.  Did everyone end up happy?  Is this patchset a
> reasonable compromise?  Or do we need a rematch?

I think the final agreement was the need for some kind of code marking
system, which I tried to implement as best as I could. It gives very
good performances while tracing (advantage of static tracing), has a
very very minimal performance and binary size impact when disabled
(advantage of dynamic tracing) and it can be activated dynamically
(advantage of dynamic tracing).

Mathieu

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Candidate, École Polytechnique de Montréal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

Re: [PATCH 00/05] Linux Kernel Markers - kernel 2.6.20

[Vara Prasad]

Andrew Morton wrote:

>On 15 Feb 2007 10:28:57 -0500
>fche@redhat.com (Frank Ch. Eigler) wrote:
>
>  
>
>>akpm wrote:
>>
>>    
>>
>>>[...]  And what can I do with these markers?  And once I've done it,
>>>are there any userspace applications I can use to get the data out
>>>in human-usable form?  [...]
>>>      
>>>
>>The LTTng user-space programs use it today.  Systemtap used to support
>>the earlier marker prototype and will be rapidly ported over to this
>>new API upon acceptance.
>>
>>    
>>
>
>That's good.
>
>It would be beneficial if some people from those projects could spare the
>cycles to carefully review and runtime test this code.
>  
>

Sure, as soon as SystemTap translator supports this new marker mechanism 
we will give it a spin and report results.

>Also, I'm not 100% clear on where we ended up with the huge
>static-vs-dynamic flamewar.  Did everyone end up happy?  Is this patchset a
>reasonable compromise?  Or do we need a rematch?
>  
>
 From my view this is a good compromise. We all realized in that long 
discussion thread that we need a way to mark the code in the middle of 
functions that can later be activated dynamically. Mathieu's current 
implementation meets that goal. I will be happy to see this in.

Thanks,
Vara Prasad

Re: [PATCH 00/05] Linux Kernel Markers - kernel 2.6.20

[Mathieu Desnoyers]

* Andrew Morton (akpm@linux-foundation.org) wrote:
> On Sun, 11 Feb 2007 15:03:22 -0500 Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca> wrote:
> 
> > You will find, in the following posts, the latest revision of the Linux Kernel
> > Markers.
> 
> <looks for the documentation>
> 

I guess the header of include/linux/marker.h should go into
Documentation/marker.txt. Will fix. Will be in the next reply.

> And what can I do with these markers?
> 

You can put markers at important locations in the code. They act as
lightweight hooks that can pass an abitrary number of parameters,
described in a printk-like format string, to a function.

They can be used for tracing (LTTng, LKET over SystemTAP), overall
performance accounting (SystemTAP). They could also be used to implement
efficient hooks for SELinux or any other subsystem the would have this
kind of need.

> And once I've done it, are there any userspace applications I can use to
> get the data out in human-usable form?
> 

LTTng and LTTV, SystemTAP (should be updated to the new API).

> It seems to me that these patches aren't sequenced correctly - the kernel won't
> build at each step of the patch sequence.  Maybe I'm mistaken in that.  We prefer
> it this way so that people don't hit wont-compile points when doing bisection searches.

Will fix for the next complete post.

Mathieu

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

[PATCH] Linux Kernel Markers Documentation

[Mathieu Desnoyers]

Linux Kernel Markers - Documentation

Here is some documentation explaining what is/how to use the Linux
Kernel Markers.

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

--- /dev/null
+++ b/Documentation/marker.txt
@@ -0,0 +1,130 @@
+ 	             Using the Linux Kernel Markers
+
+			    Mathieu Desnoyers
+
+
+	This document discusses the purpose of markers. It shows some usage
+examples of the Linux Kernel Markers : how to insert markers within the kernel
+and how to connect probes to a marker. Finally, it has some probe module
+examples. This is what connects to a marker.
+
+
+* Purpose of markers
+
+You can put markers at important locations in the code. They act as
+lightweight hooks that can pass an arbitrary number of parameters,
+described in a printk-like format string, to a function whenever the marker
+code is reached.
+
+They can be used for tracing (LTTng, LKET over SystemTAP), overall performance
+accounting (SystemTAP). They could also be used to implement
+efficient hooks for SELinux or any other subsystem the would have this
+kind of need.
+
+Using the markers for system audit (SELinux) would require to pass a
+variable by address that would be later checked by the marked routine.
+
+
+* Usage
+
+MARK(subsystem_event, "%d %s %p[struct task_struct]",
+  someint, somestring, current);
+Where :
+- 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).
+- 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 refer.
+
+The marker mechanism supports multiple instances of the same marker.
+Markers can be put in inline functions, inlined static functions and
+unrolled loops.
+
+Note : It is safe to put markers within preempt-safe code : preempt_enable()
+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,
+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.
+
+
+* Probe example
+
+------------------------------ CUT -------------------------------------
+/* probe-example.c
+ *
+ * Loads a function at a marker call site.
+ *
+ * (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/sched.h>
+#include <linux/kernel.h>
+
+#define SUBSYSTEM_EVENT_FORMAT "%d %s %p[struct task_struct]"
+void probe_subsystem_event(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);
+
+	/* or count, check rights... */
+	
+	va_end(ap);
+}
+
+static int __init probe_init(void)
+{
+	int result;
+	result = marker_set_probe("subsystem_event",
+			FS_CLOSE_FORMAT,
+			probe_fs_close);
+	if (!result)
+		goto cleanup;
+	return 0;
+
+cleanup:
+	marker_remove_probe(probe_subsystem_event);
+	return -EPERM;
+}
+
+static void __exit probe_fini(void)
+{	
+	marker_remove_probe(probe_subsystem_event);
+}
+
+module_init(probe_init);
+module_exit(probe_fini);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Mathieu Desnoyers");
+MODULE_DESCRIPTION("SUBSYSTEM Probe");
+------------------------------ CUT -------------------------------------
+
-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Candidate, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

Re: [PATCH] Linux Kernel Markers Documentation

[Randy Dunlap]

On Thu, 15 Feb 2007 20:33:48 -0500 Mathieu Desnoyers wrote:

> Linux Kernel Markers - Documentation
> 
> Here is some documentation explaining what is/how to use the Linux
> Kernel Markers.
> 
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> 
> --- /dev/null
> +++ b/Documentation/marker.txt
> @@ -0,0 +1,130 @@
> + 	             Using the Linux Kernel Markers
> +
> +			    Mathieu Desnoyers
> +
> +
> +	This document discusses the purpose of markers. It shows some usage
> +examples of the Linux Kernel Markers : how to insert markers within the kernel
> +and how to connect probes to a marker. Finally, it has some probe module
> +examples. This is what connects to a marker.
> +
> +
> +* Purpose of markers
> +
> +You can put markers at important locations in the code. They act as
> +lightweight hooks that can pass an arbitrary number of parameters,
> +described in a printk-like format string, to a function whenever the marker
> +code is reached.
> +
> +They can be used for tracing (LTTng, LKET over SystemTAP), overall performance
> +accounting (SystemTAP). They could also be used to implement
> +efficient hooks for SELinux or any other subsystem the would have this
                                                      that

> +kind of need.
> +
> +Using the markers for system audit (SELinux) would require to pass a
> +variable by address that would be later checked by the marked routine.
> +
> +
> +* Usage
> +
> +MARK(subsystem_event, "%d %s %p[struct task_struct]",
> +  someint, somestring, current);
> +Where :
> +- Subsystem is the name of your subsystem.
> +- event is the name of the event to mark.

so is the MARK() supposed to be:
  MARK(subsystem, event, ...

Please make the 2 doc. lines above match the parameters...
or the parameters match the text.

> +- "%d %s %p[struct task_struct]" is the formatted string for (printk-style).
> +- 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 refer.
                                                              refers.

> +
> +The marker mechanism supports multiple instances of the same marker.
> +Markers can be put in inline functions, inlined static functions and
> +unrolled loops.
> +
> +Note : It is safe to put markers within preempt-safe code : preempt_enable()
> +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,
> +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.
> +
> +
> +* Probe example
> +
> +------------------------------ CUT -------------------------------------
> +/* probe-example.c
> + *
> + * Loads a function at a marker call site.
> + *
> + * (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/sched.h>
> +#include <linux/kernel.h>
> +
> +#define SUBSYSTEM_EVENT_FORMAT "%d %s %p[struct task_struct]"

Is SUBSYSTEM_EVENT_FORMAT used implicitly below?  or elsewhere?

> +void probe_subsystem_event(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);
> +
> +	/* or count, check rights... */
> +	
> +	va_end(ap);
> +}
> +
> +static int __init probe_init(void)
> +{
> +	int result;
> +	result = marker_set_probe("subsystem_event",
> +			FS_CLOSE_FORMAT,
> +			probe_fs_close);

Do FS_CLOSE_FORMAT and probe_fs_close() need to be defined here?
I.e., is this a complete example?

> +	if (!result)
> +		goto cleanup;
> +	return 0;
> +
> +cleanup:
> +	marker_remove_probe(probe_subsystem_event);
> +	return -EPERM;
> +}
> +
> +static void __exit probe_fini(void)
> +{	
> +	marker_remove_probe(probe_subsystem_event);
> +}
> +
> +module_init(probe_init);
> +module_exit(probe_fini);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Mathieu Desnoyers");
> +MODULE_DESCRIPTION("SUBSYSTEM Probe");
> +------------------------------ CUT -------------------------------------



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

Re: [PATCH] Linux Kernel Markers Documentation

[Mathieu Desnoyers]

Hi Randy,

* Randy Dunlap (randy.dunlap@oracle.com) wrote:
> so is the MARK() supposed to be:
>   MARK(subsystem, event, ...
> 
> Please make the 2 doc. lines above match the parameters...
> or the parameters match the text.
> 

Fixing the paragraph below.

> > +- "%d %s %p[struct task_struct]" is the formatted string for (printk-style).
> > +- 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 refer.
>                                                               refers.
> 
> > +
> > +#define SUBSYSTEM_EVENT_FORMAT "%d %s %p[struct task_struct]"
> 
> Is SUBSYSTEM_EVENT_FORMAT used implicitly below?  or elsewhere?
> 

Yes, error follows.

> > +void probe_subsystem_event(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);
> > +
> > +	/* or count, check rights... */
> > +	
> > +	va_end(ap);
> > +}
> > +
> > +static int __init probe_init(void)
> > +{
> > +	int result;
> > +	result = marker_set_probe("subsystem_event",
> > +			FS_CLOSE_FORMAT,
> > +			probe_fs_close);
> 
> Do FS_CLOSE_FORMAT and probe_fs_close() need to be defined here?
> I.e., is this a complete example?
> 

should be SUBSYSTEM_EVENT_FORMAT. Will fix, thanks.

Regards,

Mathieu

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

Re: [PATCH] Linux Kernel Markers Documentation - fix

[Mathieu Desnoyers]

Linux Kernel Markers Documentation - fix

Fixes from Randy's comments.

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

--- a/Documentation/marker.txt
+++ b/Documentation/marker.txt
@@ -18,7 +18,7 @@ code is reached.
 
 They can be used for tracing (LTTng, LKET over SystemTAP), overall performance
 accounting (SystemTAP). They could also be used to implement
-efficient hooks for SELinux or any other subsystem the would have this
+efficient hooks for SELinux or any other subsystem that would have this
 kind of need.
 
 Using the markers for system audit (SELinux) would require to pass a
@@ -30,8 +30,9 @@ variable by address that would be later checked by the marked routine.
 MARK(subsystem_event, "%d %s %p[struct task_struct]",
   someint, somestring, current);
 Where :
-- Subsystem is the name of your subsystem.
-- event is the name of the event to mark.
+- 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).
 - someint is an integer.
 - somestring is a char pointer.
@@ -39,7 +40,7 @@ Where :
 
 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 refer.
+sparse. The brackets contain the type to which the pointer refers.
 
 The marker mechanism supports multiple instances of the same marker.
 Markers can be put in inline functions, inlined static functions and
@@ -104,8 +105,8 @@ static int __init probe_init(void)
 {
 	int result;
 	result = marker_set_probe("subsystem_event",
-			FS_CLOSE_FORMAT,
-			probe_fs_close);
+			SUBSYSTEM_EVENT_FORMAT,
+			probe_subsystem_event);
 	if (!result)
 		goto cleanup;
 	return 0;
-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Candidate, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68