[patch 0/6] Linux Kernel Markers for 2.6.23-rc4-mm1

[patch 0/6] Linux Kernel Markers for 2.6.23-rc4-mm1

[Mathieu Desnoyers]

Hi Andrew,

Here are the Linux Kernel Markers for 2.6.23-rc4-mm1. They depend on the sorted
module list and on the immediate values.

Mathieu

-- 
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 1/6] Linux Kernel Markers - Architecture Independent Code

[Mathieu Desnoyers]

[-- Attachment #1: linux-kernel-markers-architecture-independent-code.patch --]
[-- Type: text/plain, Size: 28464 bytes --]

The marker activation functions sits in kernel/marker.c. A hash table is used
to keep track of the registered probes and armed markers, so the markers within
a newly loaded module that should be active can be activated at module load
time.

marker_query has been removed. marker_get_first, marker_get_next and
marker_release should be used as iterators on the markers.

Changelog:
- markers_mutex now nests inside module_mutex rather than the opposite.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
Reviewed-by: Christoph Hellwig <hch@infradead.org>
Reviewed-by: Rusty Russell <rusty@rustcorp.com.au>
Reviewed-by: "Frank Ch. Eigler" <fche@redhat.com>
---

 include/asm-generic/vmlinux.lds.h |   11 
 include/linux/marker.h            |  169 +++++++++
 include/linux/module.h            |    5 
 kernel/marker.c                   |  699 ++++++++++++++++++++++++++++++++++++++
 kernel/module.c                   |   11 
 5 files changed, 894 insertions(+), 1 deletion(-)

Index: linux-2.6-lttng/include/asm-generic/vmlinux.lds.h
===================================================================
--- linux-2.6-lttng.orig/include/asm-generic/vmlinux.lds.h	2007-09-06 15:06:36.000000000 -0400
+++ linux-2.6-lttng/include/asm-generic/vmlinux.lds.h	2007-09-06 15:07:28.000000000 -0400
@@ -12,7 +12,11 @@
 /* .data section */
 #define DATA_DATA							\
 	*(.data)							\
-	*(.data.init.refok)
+	*(.data.init.refok)						\
+	. = ALIGN(8);							\
+	VMLINUX_SYMBOL(__start___markers) = .;				\
+	*(__markers)							\
+	VMLINUX_SYMBOL(__stop___markers) = .;
 
 #define RO_DATA(align)							\
 	. = ALIGN((align));						\
@@ -129,6 +133,11 @@
 		VMLINUX_SYMBOL(__stop___immediate) = .;			\
 	}								\
 									\
+	/* Markers: strings */						\
+        __markers_strings : AT(ADDR(__markers_strings) - LOAD_OFFSET) {	\
+		*(__markers_strings)					\
+ 	}								\
+									\
 	/* Kernel symbol table: strings */				\
         __ksymtab_strings : AT(ADDR(__ksymtab_strings) - LOAD_OFFSET) {	\
 		*(__ksymtab_strings)					\
Index: linux-2.6-lttng/include/linux/marker.h
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6-lttng/include/linux/marker.h	2007-09-06 15:07:28.000000000 -0400
@@ -0,0 +1,169 @@
+#ifndef _LINUX_MARKER_H
+#define _LINUX_MARKER_H
+
+/*
+ * Code markup for dynamic and static tracing.
+ *
+ * See Documentation/marker.txt.
+ *
+ * (C) Copyright 2006 Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
+ *
+ * This file is released under the GPLv2.
+ * See the file COPYING for more details.
+ */
+
+#include <linux/immediate.h>
+#include <linux/types.h>
+
+struct module;
+struct __mark_marker;
+
+/**
+ * marker_probe_func - Type of a marker probe function
+ * @mdata: pointer of type struct __mark_marker
+ * @private_data: caller site private data
+ * @fmt: format string
+ * @...: variable argument list
+ *
+ * Type of marker probe functions. They receive the mdata and need to parse the
+ * format string to recover the variable argument list.
+ */
+typedef void marker_probe_func(const struct __mark_marker *mdata,
+	void *private_data, const char *fmt, ...);
+
+struct __mark_marker {
+	const char *name;	/* Marker name */
+	const char *format;	/* Marker format string, describing the
+				 * variable argument list.
+				 */
+	const char *args;	/* List of arguments litteraly transformed
+				 * into a string: "arg1, arg2, arg3".
+				 */
+	immediate_char_t state;	/* Immediate value state. */
+	marker_probe_func *call;/* Probe handler function pointer */
+	void *pdata;		/* Private probe data */
+} __attribute__((aligned(8)));
+
+#ifdef CONFIG_MARKERS
+
+/*
+ * Generic marker flavor always available.
+ * Note : the empty asm volatile with read constraint is used here instead of a
+ * "used" attribute to fix a gcc 4.1.x bug.
+ * Make sure the alignment of the structure in the __markers section will
+ * not add unwanted padding between the beginning of the section and the
+ * structure. Force alignment to the same alignment as the section start.
+ */
+#define __trace_mark(generic, name, call_data, format, args...)		\
+	do {								\
+		static const char __mstrtab_name_##name[]		\
+		__attribute__((section("__markers_strings")))		\
+		= #name;						\
+		static const char __mstrtab_format_##name[]		\
+		__attribute__((section("__markers_strings")))		\
+		= format;						\
+		static const char __mstrtab_args_##name[]		\
+		__attribute__((section("__markers_strings")))		\
+		= #args;						\
+		static struct __mark_marker __mark_##name		\
+		__attribute__((section("__markers"))) =			\
+		{ __mstrtab_name_##name, __mstrtab_format_##name,	\
+		__mstrtab_args_##name, { 0 },				\
+		__mark_empty_function, NULL };				\
+		asm volatile ( "" : : "i" (&__mark_##name));		\
+		__mark_check_format(format, ## args);			\
+		if (!generic) {						\
+			immediate_if (&__mark_##name.state) { 		\
+				preempt_disable();			\
+				(*__mark_##name.call)			\
+					(&__mark_##name, call_data,	\
+					format, ## args);		\
+				preempt_enable();			\
+			}						\
+		} else {						\
+			_immediate_if (&__mark_##name.state) { 		\
+				preempt_disable();			\
+				(*__mark_##name.call)			\
+					(&__mark_##name, call_data,	\
+					format, ## args);		\
+				preempt_enable();			\
+			}						\
+		}							\
+	} while (0)
+
+extern void module_marker_update(struct module *mod);
+#else /* !CONFIG_MARKERS */
+#define __trace_mark(generic, name, call_data, format, args...) \
+		__mark_check_format(format, ## args)
+static inline void module_marker_update(struct module *mod) { }
+#endif /* CONFIG_MARKERS */
+
+/**
+ * trace_mark - Marker using code patching
+ * @name: marker name, not quoted.
+ * @format: format string
+ * @args...: variable argument list
+ *
+ * Places a marker using optimized code patching technique (immediate_if ())
+ * to be enabled.
+ */
+#define trace_mark(name, format, args...) \
+	__trace_mark(0, name, NULL, format, ## args)
+
+/**
+ * _trace_mark - Marker using variable read
+ * @name: marker name, not quoted.
+ * @format: format string
+ * @args...: variable argument list
+ *
+ * Places a marker using a standard memory read (_immediate_if ()) to be
+ * enabled. Should be used for markers in __init and __exit functions and in
+ * lockdep code.
+ */
+#define _trace_mark(name, format, args...) \
+	__trace_mark(1, name, NULL, format, ## args)
+
+#define MARK_MAX_FORMAT_LEN	1024
+
+/**
+ * MARK_NOARGS - Format string for a marker with no argument.
+ */
+#define MARK_NOARGS " "
+
+/* To be used for string format validity checking with gcc */
+static inline void __attribute__ ((format (printf, 1, 2)))
+	__mark_check_format(const char *fmt, ...) { }
+
+extern marker_probe_func __mark_empty_function;
+
+/*
+ * Connect a probe to a markers.
+ * pdata must be a valid allocated memory address, or NULL.
+ */
+extern int marker_probe_register(const char *name, const char *format,
+				marker_probe_func *probe, void *pdata);
+
+/*
+ * Returns the pdata given to marker_probe_register.
+ */
+extern void *marker_probe_unregister(const char *name);
+/*
+ * Unregister a marker by providing the registered pdata.
+ */
+extern void *marker_probe_unregister_pdata(void *pdata);
+
+extern int marker_arm(const char *name);
+extern int marker_disarm(const char *name);
+
+struct marker_iter {
+	struct module *module;
+	struct __mark_marker *marker;
+};
+
+extern void marker_iter_start(struct marker_iter *iter);
+extern void marker_iter_next(struct marker_iter *iter);
+extern void marker_iter_stop(struct marker_iter *iter);
+extern void marker_iter_reset(struct marker_iter *iter);
+extern void *marker_get_pdata(const char *name);
+
+#endif
Index: linux-2.6-lttng/include/linux/module.h
===================================================================
--- linux-2.6-lttng.orig/include/linux/module.h	2007-09-06 15:06:36.000000000 -0400
+++ linux-2.6-lttng/include/linux/module.h	2007-09-06 15:07:28.000000000 -0400
@@ -16,6 +16,7 @@
 #include <linux/kobject.h>
 #include <linux/moduleparam.h>
 #include <linux/immediate.h>
+#include <linux/marker.h>
 #include <asm/local.h>
 
 #include <asm/module.h>
@@ -380,6 +381,10 @@ struct module
 	const struct __immediate *immediate;
 	unsigned int num_immediate;
 #endif
+#ifdef CONFIG_MARKERS
+	struct __mark_marker *markers;
+	unsigned int num_markers;
+#endif
 };
 #ifndef MODULE_ARCH_INIT
 #define MODULE_ARCH_INIT {}
Index: linux-2.6-lttng/kernel/module.c
===================================================================
--- linux-2.6-lttng.orig/kernel/module.c	2007-09-06 15:07:19.000000000 -0400
+++ linux-2.6-lttng/kernel/module.c	2007-09-06 15:07:59.000000000 -0400
@@ -1721,6 +1721,8 @@ static struct module *load_module(void _
 	unsigned int unusedgplindex;
 	unsigned int unusedgplcrcindex;
 	unsigned int immediateindex;
+	unsigned int markersindex;
+	unsigned int markersstringsindex;
 	struct module *mod;
 	long err = 0;
 	void *percpu = NULL, *ptr = NULL; /* Stops spurious gcc warning */
@@ -1973,6 +1975,8 @@ static struct module *load_module(void _
 	mod->num_immediate =
 		sechdrs[immediateindex].sh_size / sizeof(*mod->immediate);
 #endif
+	markersindex = find_sec(hdr, sechdrs, secstrings, "__markers");
+ 	markersstringsindex = find_sec(hdr, sechdrs, secstrings, "__markers_strings");
 
 	mod->unused_syms = (void *)sechdrs[unusedindex].sh_addr;
 	if (unusedcrcindex)
@@ -2014,6 +2018,11 @@ static struct module *load_module(void _
 		if (err < 0)
 			goto cleanup;
 	}
+#ifdef CONFIG_MARKERS
+	mod->markers = (void *)sechdrs[markersindex].sh_addr;
+	mod->num_markers =
+		sechdrs[markersindex].sh_size / sizeof(*mod->markers);
+#endif
 
         /* Find duplicate symbols */
 	err = verify_export_symbols(mod);
@@ -2038,6 +2047,8 @@ static struct module *load_module(void _
 		 goto nomodsectinfo;
 #endif
 
+	module_marker_update(mod);
+
 	module_immediate_setup(mod);
 
 	err = module_finalize(hdr, sechdrs, mod);
Index: linux-2.6-lttng/kernel/marker.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6-lttng/kernel/marker.c	2007-09-06 15:07:28.000000000 -0400
@@ -0,0 +1,699 @@
+/*
+ * Copyright (C) 2007 Mathieu Desnoyers
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ */
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/types.h>
+#include <linux/jhash.h>
+#include <linux/list.h>
+#include <linux/rcupdate.h>
+#include <linux/marker.h>
+#include <linux/err.h>
+#include <linux/immediate.h>
+
+extern struct __mark_marker __start___markers[];
+extern struct __mark_marker __stop___markers[];
+
+/*
+ * markers_mutex nests inside module_mutex. Markers mutex protects the builtin
+ * and module markers, the hash table and deferred_sync.
+ */
+DEFINE_MUTEX(markers_mutex);
+
+/*
+ * Marker deferred synchronization.
+ * Upon marker probe_unregister, we delay call to synchronize_sched() to
+ * accelerate mass unregistration (only when there is no more reference to a
+ * give module do we call synchronize_sched()). However, we need to make sure
+ * every critical region have ended before we re-arm a marker that has been
+ * unregistered and then registered back with a different probe data.
+ */
+static int deferred_sync;
+
+/*
+ * Marker hash table, containing the active markers.
+ * Protected by module_mutex.
+ */
+#define MARKER_HASH_BITS 6
+#define MARKER_TABLE_SIZE (1 << MARKER_HASH_BITS)
+
+struct marker_entry {
+	struct hlist_node hlist;
+	char *format;
+	marker_probe_func *probe;
+	void *pdata;
+	int refcount;	/* Number of times armed. 0 if disarmed. */
+	char name[0];	/* Contains name'\0'format'\0' */
+};
+
+static struct hlist_head marker_table[MARKER_TABLE_SIZE];
+
+/**
+ * __mark_empty_function - Empty probe callback
+ * @mdata: pointer of type const struct __mark_marker
+ * @fmt: format string
+ * @...: variable argument list
+ *
+ * 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 *mdata,
+	void *private_data,
+	const char *fmt, ...)
+{ }
+EXPORT_SYMBOL_GPL(__mark_empty_function);
+
+/*
+ * Get marker if the marker is present in the marker hash table.
+ * Must be called with markers_mutex held.
+ * Returns NULL if not present.
+ */
+static struct marker_entry *get_marker(const char *name)
+{
+	struct hlist_head *head;
+	struct hlist_node *node;
+	struct marker_entry *e;
+	u32 hash = jhash(name, strlen(name), 0);
+
+	head = &marker_table[hash & ((1 << MARKER_HASH_BITS)-1)];
+	hlist_for_each_entry(e, node, head, hlist) {
+		if (!strcmp(name, e->name))
+			return e;
+	}
+	return NULL;
+}
+
+/*
+ * Add the marker to the marker hash table. Must be called with markers_mutex
+ * held.
+ */
+static int add_marker(const char *name,
+	const char *format, marker_probe_func *probe, void *pdata)
+{
+	struct hlist_head *head;
+	struct hlist_node *node;
+	struct marker_entry *e;
+	size_t name_len = strlen(name) + 1;
+	size_t format_len = 0;
+	u32 hash = jhash(name, name_len-1, 0);
+
+	if (format)
+		format_len = strlen(format) + 1;
+	head = &marker_table[hash & ((1 << MARKER_HASH_BITS)-1)];
+	hlist_for_each_entry(e, node, head, hlist) {
+		if (!strcmp(name, e->name)) {
+			printk(KERN_NOTICE
+				"Marker %s busy, probe %p already installed\n",
+				name, e->probe);
+			return -EBUSY;	/* Already there */
+		}
+	}
+	/*
+	 * Using kmalloc here to allocate a variable length element. Could
+	 * cause some memory fragmentation if overused.
+	 */
+	e = kmalloc(sizeof(struct marker_entry) + name_len + format_len,
+			GFP_KERNEL);
+	if (!e)
+		return -ENOMEM;
+	memcpy(&e->name[0], name, name_len);
+	if (format) {
+		e->format = &e->name[name_len];
+		memcpy(e->format, format, format_len);
+		trace_mark(core_marker_format, "name %s format %s",
+				e->name, e->format);
+	} else
+		e->format = NULL;
+	e->probe = probe;
+	e->pdata = pdata;
+	e->refcount = 0;
+	hlist_add_head(&e->hlist, head);
+	return 0;
+}
+
+/*
+ * Remove the marker from the marker hash table. Must be called with mutex_lock
+ * held.
+ */
+static void *remove_marker(const char *name)
+{
+	struct hlist_head *head;
+	struct hlist_node *node;
+	struct marker_entry *e;
+	int found = 0;
+	size_t len = strlen(name) + 1;
+	void *pdata = NULL;
+	u32 hash = jhash(name, len-1, 0);
+
+	head = &marker_table[hash & ((1 << MARKER_HASH_BITS)-1)];
+	hlist_for_each_entry(e, node, head, hlist) {
+		if (!strcmp(name, e->name)) {
+			found = 1;
+			break;
+		}
+	}
+	if (found) {
+		pdata = e->pdata;
+		hlist_del(&e->hlist);
+		kfree(e);
+	}
+	return pdata;
+}
+
+/*
+ * Set the mark_entry format to the format found in the element.
+ */
+static int marker_set_format(struct marker_entry **entry, const char *format)
+{
+	struct marker_entry *e;
+	size_t name_len = strlen((*entry)->name) + 1;
+	size_t format_len = strlen(format) + 1;
+
+	e = kmalloc(sizeof(struct marker_entry) + name_len + format_len,
+			GFP_KERNEL);
+	if (!e)
+		return -ENOMEM;
+	memcpy(&e->name[0], (*entry)->name, name_len);
+	e->format = &e->name[name_len];
+	memcpy(e->format, format, format_len);
+	e->probe = (*entry)->probe;
+	e->pdata = (*entry)->pdata;
+	e->refcount = (*entry)->refcount;
+	hlist_add_before(&e->hlist, &(*entry)->hlist);
+	hlist_del(&(*entry)->hlist);
+	kfree(*entry);
+	*entry = e;
+	trace_mark(core_marker_format, "name %s format %s",
+			e->name, e->format);
+	return 0;
+}
+
+/*
+ * Sets the probe callback corresponding to one marker.
+ */
+static int set_marker(struct marker_entry **entry,
+			struct __mark_marker *elem)
+{
+	int ret;
+	BUG_ON(strcmp((*entry)->name, elem->name) != 0);
+
+	if ((*entry)->format) {
+		if (strcmp((*entry)->format, elem->format) != 0) {
+			printk(KERN_NOTICE
+				"Format mismatch for probe %s "
+				"(%s), marker (%s)\n",
+				(*entry)->name,
+				(*entry)->format,
+				elem->format);
+			return -EPERM;
+		}
+	} else {
+		ret = marker_set_format(entry, elem->format);
+		if (ret)
+			return ret;
+	}
+	elem->call = (*entry)->probe;
+	elem->pdata = (*entry)->pdata;
+	_immediate_set(&elem->state, 1);
+	return 0;
+}
+
+/*
+ * Disable a marker and its probe callback.
+ * Note: only after a synchronize_sched() issued after setting elem->call to the
+ * empty function insures that the original callback is not used anymore. This
+ * insured by preemption disabling around the call site.
+ */
+static void disable_marker(struct __mark_marker *elem)
+{
+	_immediate_set(&elem->state, 0);
+	elem->call = __mark_empty_function;
+	/*
+	 * Leave the pdata and id there, because removal is racy and should be
+	 * done only after a synchronize_sched(). There are never used until
+	 * the next initialization anyway.
+	 */
+}
+
+/*
+ * Updates the probe callback corresponding to a range of markers.
+ * Must be called with markers_mutex held.
+ */
+static void marker_update_probe_range(
+	struct __mark_marker *begin,
+	struct __mark_marker *end,
+	struct module *probe_module,
+	int *refcount)
+{
+	struct __mark_marker *iter;
+	struct marker_entry *mark_entry;
+
+	for (iter = begin; iter < end; iter++) {
+		mark_entry = get_marker(iter->name);
+		if (mark_entry && mark_entry->refcount) {
+			set_marker(&mark_entry, iter);
+			/*
+			 * ignore error, continue
+			 */
+			if (probe_module)
+				if (probe_module ==
+			__module_text_address((unsigned long)mark_entry->probe))
+					(*refcount)++;
+		} else {
+			disable_marker(iter);
+		}
+	}
+}
+
+#ifdef CONFIG_MODULES
+/*
+ * Update module probes.
+ * Must be called with markers_mutex held.
+ */
+static inline void marker_update_probes_modules(struct module *probe_module,
+	int *refcount)
+{
+	struct module *mod;
+
+	list_for_each_entry(mod, &modules, list) {
+		if (!mod->taints) {
+			marker_update_probe_range(mod->markers,
+				mod->markers+mod->num_markers,
+				probe_module, refcount);
+		}
+	}
+}
+#else
+static inline void marker_update_probes_modules(struct module *probe_module,
+	int *refcount)
+{ }
+#endif
+
+/*
+ * Update probes, removing the faulty probes.
+ * Issues a synchronize_sched() when no reference to the module passed
+ * as parameter is found in the probes so the probe module can be
+ * safely unloaded from now on.
+ */
+static inline void _marker_update_probes(struct module *probe_module)
+{
+	int refcount = 0;
+
+	/* Core kernel markers */
+	marker_update_probe_range(__start___markers,
+			__stop___markers, probe_module, &refcount);
+	/* Markers in modules. */
+	marker_update_probes_modules(probe_module, &refcount);
+	if (probe_module && refcount == 0) {
+		synchronize_sched();
+		deferred_sync = 0;
+	}
+}
+
+#ifdef CONFIG_MODULES
+/**
+ * module_marker_update - Update module's markers
+ * @mod: pointer of type struct module identifying the target module
+ *
+ * Setup the marker according to the data present in the marker hash table
+ * upon module load. Must be called with module_mutex held.  Since the
+ * probe_module parameter is NULL, it is safe for refcount to be NULL.
+ */
+void module_marker_update(struct module *mod)
+{
+	if (!mod->taints) {
+		mutex_lock(&markers_mutex);
+		marker_update_probe_range(mod->markers,
+			mod->markers+mod->num_markers, NULL, NULL);
+		mutex_unlock(&markers_mutex);
+	}
+}
+
+/*
+ * Update the system wide probes, with modules.
+ * Called with module_mutex taken.
+ */
+static inline void marker_update_probes(struct module *probe_module)
+{
+	mutex_lock(&module_mutex);
+	mutex_lock(&markers_mutex);
+	_marker_update_probes(probe_module);
+	mutex_unlock(&markers_mutex);
+	mutex_unlock(&module_mutex);
+}
+#else
+/*
+ * Update the system wide probes, without modules.
+ */
+static inline void marker_update_probes(struct module *probe_module)
+{
+	mutex_lock(&markers_mutex);
+	_marker_update_probes(probe_module);
+	mutex_unlock(&markers_mutex);
+}
+#endif
+
+/**
+ * marker_probe_register -  Connect a probe to a marker
+ * @name: marker name
+ * @format: format string
+ * @probe: probe handler
+ * @pdata: probe private data
+ *
+ * pdata must be a valid allocated memory address, or NULL.
+ * Returns 0 if ok, error value on error.
+ */
+int marker_probe_register(const char *name, const char *format,
+			marker_probe_func *probe, void *pdata)
+{
+	struct marker_entry *entry;
+	int ret = 0, need_update = 0;
+
+	mutex_lock(&markers_mutex);
+	entry = get_marker(name);
+	if (entry && entry->refcount) {
+		ret = -EBUSY;
+		goto end;
+	}
+	if (deferred_sync) {
+		synchronize_sched();
+		deferred_sync = 0;
+	}
+	ret = add_marker(name, format, probe, pdata);
+	if (ret)
+		goto end;
+	need_update = 1;
+end:
+	mutex_unlock(&markers_mutex);
+	if (need_update)
+		marker_update_probes(NULL);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(marker_probe_register);
+
+/**
+ * marker_probe_unregister -  Disconnect a probe from a marker
+ * @name: marker name
+ *
+ * Returns the pdata given to marker_probe_register, or an ERR_PTR().
+ */
+void *marker_probe_unregister(const char *name)
+{
+	struct module *probe_module;
+	struct marker_entry *entry;
+	void *pdata;
+	int need_update = 0;
+
+	mutex_lock(&markers_mutex);
+	entry = get_marker(name);
+	if (!entry) {
+		pdata = ERR_PTR(-ENOENT);
+		goto end;
+	}
+	entry->refcount = 0;
+	/* In what module is the probe handler ? */
+	probe_module = __module_text_address((unsigned long)entry->probe);
+	pdata = remove_marker(name);
+	deferred_sync = 1;
+	need_update = 1;
+end:
+	mutex_unlock(&markers_mutex);
+	if (need_update)
+		marker_update_probes(probe_module);
+	return pdata;
+}
+EXPORT_SYMBOL_GPL(marker_probe_unregister);
+
+/**
+ * marker_probe_unregister -  Disconnect a probe from a marker
+ * @pdata: probe private data
+ *
+ * Unregister a marker by providing the registered pdata.
+ * Returns the pdata given to marker_probe_register, or an ERR_PTR().
+ */
+void *marker_probe_unregister_pdata(void *pdata)
+{
+	struct module *probe_module;
+	struct hlist_head *head;
+	struct hlist_node *node;
+	struct marker_entry *entry;
+	int found = 0;
+	unsigned int i;
+	int need_update = 0;
+
+	mutex_lock(&markers_mutex);
+	for (i = 0; i < MARKER_TABLE_SIZE; i++) {
+		head = &marker_table[i];
+		hlist_for_each_entry(entry, node, head, hlist) {
+			if (entry->pdata == pdata) {
+				found = 1;
+				goto iter_end;
+			}
+		}
+	}
+iter_end:
+	if (!found) {
+		pdata = ERR_PTR(-ENOENT);
+		goto end;
+	}
+	entry->refcount = 0;
+	/* In what module is the probe handler ? */
+	probe_module = __module_text_address((unsigned long)entry->probe);
+	pdata = remove_marker(entry->name);
+	deferred_sync = 1;
+	need_update = 1;
+end:
+	mutex_unlock(&markers_mutex);
+	if (need_update)
+		marker_update_probes(probe_module);
+	return pdata;
+}
+EXPORT_SYMBOL_GPL(marker_probe_unregister_pdata);
+
+/**
+ * marker_arm - Arm a marker
+ * @name: marker name
+ *
+ * Activate a marker. It keeps a reference count of the number of
+ * arming/disarming done.
+ * Returns 0 if ok, error value on error.
+ */
+int marker_arm(const char *name)
+{
+	struct marker_entry * entry;
+	int ret = 0, need_update = 0;
+
+	mutex_lock(&markers_mutex);
+	entry = get_marker(name);
+	if (!entry) {
+		ret = -ENOENT;
+		goto end;
+	}
+	/*
+	 * Only need to update probes when refcount passes from 0 to 1.
+	 */
+	if (entry->refcount++)
+		goto end;
+	need_update = 1;
+end:
+	mutex_unlock(&markers_mutex);
+	if (need_update)
+		marker_update_probes(NULL);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(marker_arm);
+
+/**
+ * marker_disarm - Disarm a marker
+ * @name: marker name
+ *
+ * Disarm a marker. It keeps a reference count of the number of arming/disarming
+ * done.
+ * Returns 0 if ok, error value on error.
+ */
+int marker_disarm(const char *name)
+{
+	struct marker_entry * entry;
+	int ret = 0, need_update = 0;
+
+	mutex_lock(&markers_mutex);
+	entry = get_marker(name);
+	if (!entry) {
+		ret = -ENOENT;
+		goto end;
+	}
+	/*
+	 * Only permit decrement refcount if higher than 0.
+	 * Do probe update only on 1 -> 0 transition.
+	 */
+	if (entry->refcount) {
+		if (--entry->refcount)
+			goto end;
+	} else {
+		ret = -EPERM;
+		goto end;
+	}
+	need_update = 1;
+end:
+	mutex_unlock(&markers_mutex);
+	if (need_update)
+		marker_update_probes(NULL);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(marker_disarm);
+
+/**
+ * marker_get_pdata - Get a marker's probe private data
+ * @name: marker name
+ *
+ * Returns the pdata pointer, or an ERR_PTR.
+ * The pdata pointer should _only_ be dereferenced if the caller is the owner of
+ * the data, or its content could vanish. This is mostly used to confirm that a
+ * caller is the owner of a registered probe.
+ */
+void *marker_get_pdata(const char *name)
+{
+	struct hlist_head *head;
+	struct hlist_node *node;
+	struct marker_entry *e;
+	size_t name_len = strlen(name) + 1;
+	u32 hash = jhash(name, name_len-1, 0);
+	int found = 0;
+
+	head = &marker_table[hash & ((1 << MARKER_HASH_BITS)-1)];
+	hlist_for_each_entry(e, node, head, hlist) {
+		if (!strcmp(name, e->name)) {
+			found = 1;
+			return e->pdata;
+		}
+	}
+	return ERR_PTR(-ENOENT);
+}
+EXPORT_SYMBOL_GPL(marker_get_pdata);
+
+static inline int marker_get_iter_range(struct __mark_marker **marker,
+	struct __mark_marker *begin,
+	struct __mark_marker *end)
+{
+	int found = 0;
+
+	if (!*marker && begin != end) {
+		found = 1;
+		*marker = begin;
+	} else if (*marker >= begin && *marker < end) {
+		found = 1;
+		/*
+		 * *marker is known to be a valid marker from now on.
+		 */
+	}
+	return found;
+}
+
+#ifdef CONFIG_MODULES
+/*
+ * Returns 0 if current not found.
+ * Returns 1 if current found.
+ * Should be called with module_mutex taken.
+ */
+static inline int marker_get_iter_modules(struct marker_iter *iter)
+{
+	struct module *iter_mod;
+	int found = 0;
+
+	list_for_each_entry(iter_mod, &modules, list) {
+		if (!iter_mod->taints) {
+			/*
+			 * Sorted module list
+			 */
+			if (iter_mod < iter->module) {
+				continue;
+			} else if (iter_mod > iter->module) {
+				iter->marker = NULL;
+			} else {
+			}
+			found = marker_get_iter_range(&iter->marker,
+				iter_mod->markers,
+				iter_mod->markers + iter_mod->num_markers);
+			if (found) {
+				iter->module = iter_mod;
+				break;
+			}
+
+		}
+	}
+	return found;
+}
+#else
+static inline int marker_get_iter_modules(struct marker_iter *iter)
+{
+	return 0;
+}
+#endif
+
+static inline void marker_get_iter(struct marker_iter *iter)
+{
+	int found = 0;
+
+	/* Core kernel markers */
+	if (!iter->module) {
+		found = marker_get_iter_range(&iter->marker,
+				__start___markers, __stop___markers);
+		if (found)
+			goto end;
+	}
+	/* Markers in modules. */
+	found = marker_get_iter_modules(iter);
+end:
+	if (!found)
+		marker_iter_reset(iter);
+}
+
+void marker_iter_start(struct marker_iter *iter)
+{
+	mutex_lock(&module_mutex);
+	mutex_lock(&markers_mutex);
+	marker_get_iter(iter);
+}
+EXPORT_SYMBOL_GPL(marker_iter_start);
+
+void marker_iter_next(struct marker_iter *iter)
+{
+	iter->marker++;
+	/*
+	 * iter->marker may be invalid because we blindly incremented it.
+	 * Make sure it is valid by marshalling on the markers, getting the
+	 * markers from following modules if necessary.
+	 */
+	marker_get_iter(iter);
+}
+EXPORT_SYMBOL_GPL(marker_iter_next);
+
+void marker_iter_stop(struct marker_iter *iter)
+{
+	mutex_unlock(&markers_mutex);
+	mutex_unlock(&module_mutex);
+}
+EXPORT_SYMBOL_GPL(marker_iter_stop);
+
+void marker_iter_reset(struct marker_iter *iter)
+{
+	iter->module = NULL;
+	iter->marker = NULL;
+}
+EXPORT_SYMBOL_GPL(marker_iter_reset);

-- 
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 1/6] Linux Kernel Markers - Architecture Independent Code

[Randy Dunlap]

On Thu, 06 Sep 2007 16:07:34 -0400 Mathieu Desnoyers wrote:

> Changelog:
> - markers_mutex now nests inside module_mutex rather than the opposite.
> 
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> Reviewed-by: Christoph Hellwig <hch@infradead.org>
> Reviewed-by: Rusty Russell <rusty@rustcorp.com.au>
> Reviewed-by: "Frank Ch. Eigler" <fche@redhat.com>

Documentation/SubmittingPatches recognizes Signed-off-by: and
Acked-by:.  Nothing about Reviewed-by.  Quote:

  Acked-by: is not as formal as Signed-off-by:.  It is a record that the acker
  has at least reviewed the patch and has indicated acceptance.

Reviewed-by: carries no such connotation or indication IMO.
Please use Acked-by if that's what these reviewers have done.


> ---
> 
>  include/asm-generic/vmlinux.lds.h |   11 
>  include/linux/marker.h            |  169 +++++++++
>  include/linux/module.h            |    5 
>  kernel/marker.c                   |  699 ++++++++++++++++++++++++++++++++++++++
>  kernel/module.c                   |   11 
>  5 files changed, 894 insertions(+), 1 deletion(-)


> Index: linux-2.6-lttng/include/linux/marker.h
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6-lttng/include/linux/marker.h	2007-09-06 15:07:28.000000000 -0400
> @@ -0,0 +1,169 @@

> +/*
> + * Connect a probe to a markers.

                           marker.

> + * pdata must be a valid allocated memory address, or NULL.
> + */
> +extern int marker_probe_register(const char *name, const char *format,
> +				marker_probe_func *probe, void *pdata);

> Index: linux-2.6-lttng/kernel/marker.c
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6-lttng/kernel/marker.c	2007-09-06 15:07:28.000000000 -0400
> @@ -0,0 +1,699 @@
> +/*
> + * Marker deferred synchronization.
> + * Upon marker probe_unregister, we delay call to synchronize_sched() to
> + * accelerate mass unregistration (only when there is no more reference to a
> + * give module do we call synchronize_sched()). However, we need to make sure

      given

> + * every critical region have ended before we re-arm a marker that has been

                            has

> + * unregistered and then registered back with a different probe data.
> + */
> +static int deferred_sync;
> +
> +/*
> + * Marker hash table, containing the active markers.
> + * Protected by module_mutex.
> + */
> +#define MARKER_HASH_BITS 6
> +#define MARKER_TABLE_SIZE (1 << MARKER_HASH_BITS)
> +
> +struct marker_entry {
> +	struct hlist_node hlist;
> +	char *format;
> +	marker_probe_func *probe;
> +	void *pdata;
> +	int refcount;	/* Number of times armed. 0 if disarmed. */
> +	char name[0];	/* Contains name'\0'format'\0' */
> +};
> +
> +static struct hlist_head marker_table[MARKER_TABLE_SIZE];
> +
> +/**
> + * __mark_empty_function - Empty probe callback
> + * @mdata: pointer of type const struct __mark_marker
> + * @fmt: format string
> + * @...: variable argument list
> + *
> + * 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

                          make

> + * 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 *mdata,
> +	void *private_data,
> +	const char *fmt, ...)
> +{ }
> +EXPORT_SYMBOL_GPL(__mark_empty_function);


> +/*
> + * Sets the probe callback corresponding to one marker.
> + */
> +static int set_marker(struct marker_entry **entry,
> +			struct __mark_marker *elem)
> +{
> +	int ret;
> +	BUG_ON(strcmp((*entry)->name, elem->name) != 0);

You can't return -ENOENT here?

> +
> +	if ((*entry)->format) {
> +		if (strcmp((*entry)->format, elem->format) != 0) {
> +			printk(KERN_NOTICE
> +				"Format mismatch for probe %s "
> +				"(%s), marker (%s)\n",
> +				(*entry)->name,
> +				(*entry)->format,
> +				elem->format);
> +			return -EPERM;
> +		}
> +	} else {
> +		ret = marker_set_format(entry, elem->format);
> +		if (ret)
> +			return ret;
> +	}
> +	elem->call = (*entry)->probe;
> +	elem->pdata = (*entry)->pdata;
> +	_immediate_set(&elem->state, 1);
> +	return 0;
> +}
> +
> +/*
> + * Disable a marker and its probe callback.
> + * Note: only after a synchronize_sched() issued after setting elem->call to the
> + * empty function insures that the original callback is not used anymore. This
> + * insured by preemption disabling around the call site.
> + */
> +static void disable_marker(struct __mark_marker *elem)
> +{
> +	_immediate_set(&elem->state, 0);
> +	elem->call = __mark_empty_function;
> +	/*
> +	 * Leave the pdata and id there, because removal is racy and should be
> +	 * done only after a synchronize_sched(). There are never used until

                                                  These

> +	 * the next initialization anyway.
> +	 */
> +}
> +

> +#ifdef CONFIG_MODULES
> +/*
> + * Update module probes.
> + * Must be called with markers_mutex held.
> + */
> +static inline void marker_update_probes_modules(struct module *probe_module,
> +	int *refcount)
> +{
> +	struct module *mod;
> +
> +	list_for_each_entry(mod, &modules, list) {
> +		if (!mod->taints) {
> +			marker_update_probe_range(mod->markers,
> +				mod->markers+mod->num_markers,
> +				probe_module, refcount);
> +		}
> +	}
> +}

hm, so if the module is tainted, markers won't be applied to it?
That's OK IMO.  However, I tend to think that something that
modifies code on the fly (like kprobes or text edit) should set some
flag somewhere so that when the system crashes, we can know that
code was modified.  This could be done via taints flags or some other
mechanism.  Has this already been discussed?


> +#else
> +static inline void marker_update_probes_modules(struct module *probe_module,
> +	int *refcount)
> +{ }
> +#endif
> +
> +/*
> + * Update probes, removing the faulty probes.
> + * Issues a synchronize_sched() when no reference to the module passed
> + * as parameter is found in the probes so the probe module can be
> + * safely unloaded from now on.
> + */
> +static inline void _marker_update_probes(struct module *probe_module)
> +{
> +	int refcount = 0;
> +
> +	/* Core kernel markers */
> +	marker_update_probe_range(__start___markers,
> +			__stop___markers, probe_module, &refcount);
> +	/* Markers in modules. */
> +	marker_update_probes_modules(probe_module, &refcount);
> +	if (probe_module && refcount == 0) {
> +		synchronize_sched();
> +		deferred_sync = 0;
> +	}
> +}

> +/**
> + * marker_probe_unregister -  Disconnect a probe from a marker

wrong function name above.

> + * @pdata: probe private data
> + *
> + * Unregister a marker by providing the registered pdata.
> + * Returns the pdata given to marker_probe_register, or an ERR_PTR().
> + */
> +void *marker_probe_unregister_pdata(void *pdata)
> +{
  ...
> +}
> +EXPORT_SYMBOL_GPL(marker_probe_unregister_pdata);

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

Re: [patch 1/6] Linux Kernel Markers - Architecture Independent Code

[Andrew Morton]

> On Thu, 6 Sep 2007 16:00:01 -0700 Randy Dunlap <randy.dunlap@oracle.com> wrote:
> On Thu, 06 Sep 2007 16:07:34 -0400 Mathieu Desnoyers wrote:
> 
> > Changelog:
> > - markers_mutex now nests inside module_mutex rather than the opposite.
> > 
> > Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> > Reviewed-by: Christoph Hellwig <hch@infradead.org>
> > Reviewed-by: Rusty Russell <rusty@rustcorp.com.au>
> > Reviewed-by: "Frank Ch. Eigler" <fche@redhat.com>
> 
> Documentation/SubmittingPatches recognizes Signed-off-by: and
> Acked-by:.  Nothing about Reviewed-by.  Quote:
> 
>   Acked-by: is not as formal as Signed-off-by:.  It is a record that the acker
>   has at least reviewed the patch and has indicated acceptance.
> 
> Reviewed-by: carries no such connotation or indication IMO.
> Please use Acked-by if that's what these reviewers have done.

Yeah.  We will start introducing Reviewed-by: (I haven't yet quite worked
out how yet) but it will be a quite formal thing and it would be something
which the reviewer explicitly provided.  For now, let's please stick with
acked-by (if those individuals did indeed send an acked-by)

Re: [patch 1/6] Linux Kernel Markers - Architecture Independent Code

[Randy Dunlap]

On Thu, 6 Sep 2007 16:04:29 -0700 Andrew Morton wrote:

> > On Thu, 6 Sep 2007 16:00:01 -0700 Randy Dunlap <randy.dunlap@oracle.com> wrote:
> > On Thu, 06 Sep 2007 16:07:34 -0400 Mathieu Desnoyers wrote:
> > 
> > > Changelog:
> > > - markers_mutex now nests inside module_mutex rather than the opposite.
> > > 
> > > Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> > > Reviewed-by: Christoph Hellwig <hch@infradead.org>
> > > Reviewed-by: Rusty Russell <rusty@rustcorp.com.au>
> > > Reviewed-by: "Frank Ch. Eigler" <fche@redhat.com>
> > 
> > Documentation/SubmittingPatches recognizes Signed-off-by: and
> > Acked-by:.  Nothing about Reviewed-by.  Quote:
> > 
> >   Acked-by: is not as formal as Signed-off-by:.  It is a record that the acker
> >   has at least reviewed the patch and has indicated acceptance.
> > 
> > Reviewed-by: carries no such connotation or indication IMO.
> > Please use Acked-by if that's what these reviewers have done.
> 
> Yeah.  We will start introducing Reviewed-by: (I haven't yet quite worked
> out how yet) but it will be a quite formal thing and it would be something
> which the reviewer explicitly provided.  For now, let's please stick with
> acked-by (if those individuals did indeed send an acked-by)
> -

Thanks.  I look forward to the explanation of Reviewed-by, what it
means, and how it differs from Acked-by.


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

Re: [patch 1/6] Linux Kernel Markers - Architecture Independent Code

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 242 bytes --]

On Thu, 06 Sep 2007 16:37:37 PDT, Randy Dunlap said:

> Thanks.  I look forward to the explanation of Reviewed-by, what it
> means, and how it differs from Acked-by.

As long as you're at it,  the only tag *I* seem to need is 'Tested-By:' :)

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: [patch 1/6] Linux Kernel Markers - Architecture Independent Code

[Randy Dunlap]

Valdis.Kletnieks@vt.edu wrote:
> On Thu, 06 Sep 2007 16:37:37 PDT, Randy Dunlap said:
> 
>> Thanks.  I look forward to the explanation of Reviewed-by, what it
>> means, and how it differs from Acked-by.
> 
> As long as you're at it,  the only tag *I* seem to need is 'Tested-By:' :)

Yep, and that's a good one IMO.

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

Re: [patch 1/6] Linux Kernel Markers - Architecture Independent Code

[Theodore Tso]

On Thu, Sep 06, 2007 at 04:37:37PM -0700, Randy Dunlap wrote:
> Thanks.  I look forward to the explanation of Reviewed-by, what it
> means, and how it differs from Acked-by.

This was proposed by Andrew and discussed at the Kernel Summit; the
basic idea is that it is a formal indication that the person has done
a *full* review of the patch (a few random comments from the local
whitespace police don't count), and is willing to vouch that the patch
is correct, safe, extremely unlikely to cause regressions, etc.  If
the patch does need to be reverted or fixed because it was buggy, then
both the original submitter and the reviewer would bear responsibility
and subsystem maintainers might take that into account when assessing
the reputations of the submitter and reviewer in the future when
deciding whether or not to accept a patch.

Basically, some people seem to be using "Acked-by" to mean, "seems
good to me", without necessarily doing a full review of the patch, and
instead of trying to change the meaning of "Acked-by", to have a new
sign off which is a bit more explicitly about what it means.  (Hmmm,
thinking about it afterwards, maybe "Vouched-by:" would be even
better....)

There was some thought about negative attention (i.e., "public
mockery") given to people who sign off on a patch via Reviewed-by:
that subsequently turns out to be buggy or cause a regression, but the
concern with that is that we have enough trouble finding people to
review patches, and we wouldn't want to scare off reviewers.  But it
would be fair to say that the consequences of reviewing patches
successfully or unsuccessfully would naturally impact people's
reputations.

There was also some discussion about whether or not patches would not
be accepted at all without a Reviewed-by, but that probably won't
happen initially.  The general consensus was to gently ease into it
and see how well it works first.

        					- Ted

Re: [patch 1/6] Linux Kernel Markers - Architecture Independent Code

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 826 bytes --]

On Fri, 07 Sep 2007 12:04:45 EDT, Theodore Tso said:
> This was proposed by Andrew and discussed at the Kernel Summit; the
> basic idea is that it is a formal indication that the person has done
> a *full* review of the patch (a few random comments from the local
> whitespace police don't count),

Anybody got a proposed scheme for the case where somebody like myself
who is *not* a member of the Maintainer Cabal has looked at a patch, and
found a valid show-stopper that's bigger than just whitespace (breaks on
64-bit, locking issues, etc), or other commentary that *should* be addressed
before it gets merged?  I'd like *some* way to tag a patch with "I had an
issue with V1, but the author addressed it to my satisfaction in V2"....

(Note that includes "the author convinced me the patch was right and I was
wrong"...)

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: [patch 1/6] Linux Kernel Markers - Architecture Independent Code

[Christoph Hellwig]

On Fri, Sep 07, 2007 at 01:10:54PM -0400, Valdis.Kletnieks@vt.edu wrote:
> Anybody got a proposed scheme for the case where somebody like myself
> who is *not* a member of the Maintainer Cabal has looked at a patch, and
> found a valid show-stopper that's bigger than just whitespace (breaks on
> 64-bit, locking issues, etc), or other commentary that *should* be addressed
> before it gets merged?  I'd like *some* way to tag a patch with "I had an
> issue with V1, but the author addressed it to my satisfaction in V2"....
> 
> (Note that includes "the author convinced me the patch was right and I was
> wrong"...)

I think that'd be Reviewed-By.  While you are not part of the smokey room
cabal you have shown technical expertise in various areas so it seems
perfectly fine to have reviewed-by from you.  The fix vs a previous version
should probably be just in the text with a paragraph ala:

Issue blah in a previous version as found by Valdis Kletnieks has been fixed
by doing foo.

Re: [patch 1/6] Linux Kernel Markers - Architecture Independent Code

[Roland Dreier]

 > > Anybody got a proposed scheme for the case where somebody like myself
 > > who is *not* a member of the Maintainer Cabal has looked at a patch, and
 > > found a valid show-stopper that's bigger than just whitespace (breaks on
 > > 64-bit, locking issues, etc), or other commentary that *should* be addressed
 > > before it gets merged?  I'd like *some* way to tag a patch with "I had an
 > > issue with V1, but the author addressed it to my satisfaction in V2"....

 > I think that'd be Reviewed-By.  While you are not part of the smokey room
 > cabal you have shown technical expertise in various areas so it seems
 > perfectly fine to have reviewed-by from you.  The fix vs a previous version
 > should probably be just in the text with a paragraph ala:

 > Issue blah in a previous version as found by Valdis Kletnieks has been fixed
 > by doing foo.

At ksummit Andrew also mentioned including a link to the relevant
mailing list discussion too, and I think this would be a good example
of when that would be useful.

 - R.

Re: [patch 1/6] Linux Kernel Markers - Architecture Independent Code

[Mathieu Desnoyers]

* Randy Dunlap (randy.dunlap@oracle.com) wrote:
 [snip]
> 
> > +/*
> > + * Sets the probe callback corresponding to one marker.
> > + */
> > +static int set_marker(struct marker_entry **entry,
> > +			struct __mark_marker *elem)
> > +{
> > +	int ret;
> > +	BUG_ON(strcmp((*entry)->name, elem->name) != 0);
> 
> You can't return -ENOENT here?
> 

I could, but it's really a bug: if this condition happens, there would
be an inconsistency in the hash table get_marker() function. Therefore I
prefer to yell if this happens rather than simply telling the caller
that the entry does not exist.

And actually, marker_update_probe_range ignores errors from set_marker
(there is even a comment saying that in the code) :

                if (mark_entry && mark_entry->refcount) {
                        set_marker(&mark_entry, iter);
                        /*
                         * ignore error, continue
                         */

It is done on purpose: if a marker does not have the right arguments, it
will simply be skipped and a printk warning will be emitted. I prefer
this approach rather than stopping marker activation/disactivation as
soon as a bad marker is found.

[...]
 
> > +#ifdef CONFIG_MODULES
> > +/*
> > + * Update module probes.
> > + * Must be called with markers_mutex held.
> > + */
> > +static inline void marker_update_probes_modules(struct module *probe_module,
> > +	int *refcount)
> > +{
> > +	struct module *mod;
> > +
> > +	list_for_each_entry(mod, &modules, list) {
> > +		if (!mod->taints) {
> > +			marker_update_probe_range(mod->markers,
> > +				mod->markers+mod->num_markers,
> > +				probe_module, refcount);
> > +		}
> > +	}
> > +}
> 
> hm, so if the module is tainted, markers won't be applied to it?
> That's OK IMO.

Yep, 

> However, I tend to think that something that
> modifies code on the fly (like kprobes or text edit) should set some
> flag somewhere so that when the system crashes, we can know that
> code was modified.  This could be done via taints flags or some other
> mechanism.  Has this already been discussed?
> 

I am not aware of discussions about this. But then we would have to do
the same for paravirt, alternatives, kprobes and immediate values.

As soon as the kernel code of a crashing kernel may have been modified
in memory, the image of the crashed kernel should be used instead of the
compiled image to analyze the crash. But code patching becomes so common
that we will have to start using core dumps more and more.

Yes, your idea makes sense. printking that kind of information in a OOPS
would be useful IMO.

Mathieu

-- 
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 2/6] Linux Kernel Markers - Add kconfig menus for the marker code

[Mathieu Desnoyers]

[-- Attachment #1: linux-kernel-markers-kconfig-menus.patch --]
[-- Type: text/plain, Size: 22590 bytes --]

With the increasing complexity of today's user-space application and the wide
deployment of SMP systems, the users need an increasing understanding of the
behavior and performance of a system across multiple processes/different
execution contexts/multiple CPUs.  In applications such as large clusters
(Google, IBM), video acquisition (Autodesk), embedded real-time systems (Wind
River, Monta Vista, Sony) or sysadmin/programmer-type tasks (SystemTAP from
Redhat), a tool that permits tracing of kernel-user space interaction becomes
necessary.

Usage of such tools have been made to successfully pinpoint problems such as:
latency issues in a user-space video acquisition application, slowdown
problems in large clusters due to a switch to a different filesystems with a
different cache size, abnormal Linux scheduler latency (just to name a few
that I have personally investigated).

The currently existing solutions does not give a system-wide overview of what
- and when - things are happening on the system.  Ptracing a program works
with few processes, but quickly becomes useless when it comes to keeping track
of many processes.

Bugs occuring because of bad interaction of such complex systems can be very
hard to find due to the fact that they occur rarely (sometimes once a week on
hundreds of machines).  One can therefore only hope at having the best
conditions to statistically reproduce the bug while extracting information
from the system.  Some bugs have been successfully found at Google using their
ktrace tracer only because they could enable it on production machines and
therefore recreate the same context where the bug happened.

Therefore, it makes sense to offer an instrumentation set of the most relevant
events occurring in the Linux that can have the smallest performance cost
possible when not active while not requiring a reboot of a production system
to activate.  This is essentially what the markers are providing.

Since we cannot limit the growth of the Linux kernel, nor can we pre-determine
each and every "interesting" instrumentation within each subsystem and driver,
it is sensible to let this task to the persons who knows the best their code. 
Adding instrumentation should therefore be as easy as adding and maintaining a
"printk" in the kernel code from the developer's point of view.

Towards a complete tracing mechanism in the Linux kernel, the markers are only
one step forward.  The following step is to connect probes to those markers
that will record the tracing information in buffers exported to user-space,
organized in timestamped "events".  Probe callbacks are responsible for
serializing the information passed as parameter to the markers (described by
the format string) into the events.  A control mechanism to activate/stop the
tracing is required, as well as a daemon that maps the buffers to write them
to disk or send them through the network.

Keeping track of the events also requires a centralized infrastructure : the
idea is to assign a unique ID to each event so they can be later recognized in
the trace.  Keeping in mind that recording the complete instrumentation site
name string for each event would be more that inefficient, assigning a numeric
unique identifier makes sense.

Finally, support for gathering events coming from user-space, with a minimal
performance impact, is very useful to see the interaction between the system's
execution contexts.

The last steps are currently implemented in Linux Trace Toolkit Next
Generation (LTTng).

The SystemTAP project could clearly benefit from such an infrastructure for
tracing.  In addition, they would be providing support for dynamic addition of
kernel probes through breakpoints/jumps when possible, with the associated
restrictions (accessing local variables, reentrancy, speed).




This marker infrastructure is a hook-callback mechanism.  It is meant to have
an impact as low as possible on the system performances when no callback
(probe) is connected so markers (hooks) can be compiled into a production
kernel without noticeable slowdown.

Why use the markers instead of kprobes?

The rationale behind this mechanism the following :

1 - It makes sense to have instrumentation (for tracing, profiling)
    within the kernel source tree so that it can follow its evolution.
    Other options, such as kprobes, imply maintaining an external set of
    instrumentation that must be adapted to each kernel version.
    Although it may make sense for distributions, it is not well suited
    for kernel developers, since they rarely work on a major
    distribution image.
2 - kprobes, although being a very good attempt at providing a dynamic
    hooking mechanism that has no impact when disabled, suffers from
    important limitations :
  a - It cannot access local variables of a function at a particular
      point within its body that will be consistent thorough the kernel
      versions without involving a lot of recurrent hair-pulling.
  b - Kprobes is slow, since it involves going though a trap each time
      a probe site is executed. Even though the djprobes project made a
      good effort to make things faster, it cannot currently instrument
      fully-preemptible kernels and does not solve (1), (2a) and (2c).
  c - On the reentrancy side, going though a trap (thus playing with
      interrupt enable/disable) and taking spinlocks are not suited to
      some code paths, i.e. :
      kernel/lockdep.c, printk (within the lockdep_on()/lockdep_off()).
      It must be understood that some code paths interesting for
      instrumentation often present a particular reentrancy challenge.

Some more details :

The probe callback connection to its markers is done dynamically.  A predicted
branch (see the immediate values infrastructure) is used to skip the hook stack
setup and function call when the marker is "disabled" (no probe is connected).
Further optimizations can be implemented for each architecture to make this
branch faster.

Instrumentation of a subsystem becomes therefore a straightforward task.  One
has to add instrumentation within the key locations of the kernel code in the
following form :

trace_mark(subsystem_event, "%d %p", myint, myptr);


Jim Keniston <jkenisto@us.ibm.com> adds:

kprobes remains a vital foundation for SystemTap.  But markers are attactive
as an alternate source of trace/debug info.  Here's why:

1. Markers will live in the kernel and presumably be kept up to date by
   the maintainers of the enclosing code.  We have a growing set of tapsets
   (probe libraries), each of which "knows" the source code for a certain area
   of the kernel.  Whenever the underlying kernel code changes (e.g., a
   function or one of its args disappears or is renamed), there's a chance
   that the tapset will become invalid until we bring it back in sync with the
   kernel.  As you can imagine, maintaining tapsets separate from the kernel
   source is a maintenance headache.  Markers could mitigate this.

2. Because the kernel code is highly optimized, the kernel's dwarf info
   doesn't always accurately reflect which variables have which values on
   which lines (sometimes even upon entry to a function).  A marker is a way
   to ensure that values of interest are available to SystemTap at marked
   points.

3. Sometimes the overhead of a kprobe probepoint is too much (either in
   terms of time or locking) for the particular hotspot we want to probe.


In OLS2006 proceedings, vol. 1
http://www.linuxsymposium.org/2006/linuxsymposium_procv1.pdf

Frank C. Eigler, from SystemTAP, presents its "static probing markers"
(pp. 261-268) in his paper "Problem Solving With Systemtap".

He explains the advantages :

"In exchange for this effort, systemtap marker-based probes are faster and
 more precise than kprobes.  The better precision comes from not having to
 covet the compiler's favours.  Such fickle favours include retaining
 clean boundaries in the instruction stream between interesting statements,
 and precisely describing positions of variables in the stack frame.  Since
 markers don't rely on debugging information, neither favour is required,
 and the compiler can channel its charms into unabated optimization.  The
 speed advantage comes from using direct call instructions rather than int 3
 breakpoints to dispatch to the systemtap handlers.  We will see below just
 how big a difference this makes."

He does a comparison of his "simple" marker solution with kprobes (his simple
solution looks like my generic markers, but with a major race condition).  I
also posted numbers about the markers performance impact a few months ago in
the initial thread.  I can dig into my emails to find them for you if you
consider it important for the Changelog.

He concludes with :

"To the extent that is true, we propose that these groups consider using a
 shared pool of static markers as the basic kernel-side instrumentation
 mechanism.  If they prove to have as low dormant cost and as high active
 performance as initial experience suggests, perhaps this could motivate the
 various tracing efforts and kernel subsystem developers to finally join
 forces.  Let's designate standard trace/probe points once and for all. 
 Tracing backends can attach to these markers the same way systemtap would. 
 There would be no need for them to maintain kernel patches any more. 
 Let's think about it."


This patch:

Add Kconfig menus for the marker code.

[bunk@stusta.de: Never ever select MODULES]
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
Reviewed-by: Christoph Hellwig <hch@infradead.org>
Reviewed-by: "Frank Ch. Eigler" <fche@redhat.com>
CC: Adrian Bunk <bunk@stusta.de>
---

 arch/alpha/Kconfig       |    2 ++
 arch/arm/Kconfig         |    2 ++
 arch/avr32/Kconfig.debug |    7 +++++++
 arch/cris/Kconfig        |    2 ++
 arch/frv/Kconfig         |    2 ++
 arch/h8300/Kconfig       |    2 ++
 arch/i386/Kconfig        |    2 ++
 arch/ia64/Kconfig        |    2 ++
 arch/m32r/Kconfig        |    2 ++
 arch/m68k/Kconfig        |    2 ++
 arch/m68knommu/Kconfig   |    2 ++
 arch/mips/Kconfig        |    2 ++
 arch/parisc/Kconfig      |    2 ++
 arch/powerpc/Kconfig     |    2 ++
 arch/ppc/Kconfig         |    2 ++
 arch/s390/Kconfig        |    2 ++
 arch/sh/Kconfig          |    2 ++
 arch/sh64/Kconfig        |    2 ++
 arch/sparc/Kconfig       |    2 ++
 arch/sparc64/Kconfig     |    2 ++
 arch/um/Kconfig          |    2 ++
 arch/v850/Kconfig        |    2 ++
 arch/x86_64/Kconfig      |    2 ++
 arch/xtensa/Kconfig      |    2 ++
 kernel/Kconfig.marker    |    7 +++++++
 kernel/Makefile          |    1 +
 26 files changed, 61 insertions(+)

Index: linux-2.6-lttng/arch/alpha/Kconfig
===================================================================
--- linux-2.6-lttng.orig/arch/alpha/Kconfig	2007-09-04 12:14:48.000000000 -0400
+++ linux-2.6-lttng/arch/alpha/Kconfig	2007-09-04 12:16:57.000000000 -0400
@@ -646,6 +646,8 @@ config SRM_ENV
 
 source "fs/Kconfig.binfmt"
 
+source "kernel/Kconfig.marker"
+
 endmenu
 
 source "net/Kconfig"
Index: linux-2.6-lttng/arch/arm/Kconfig
===================================================================
--- linux-2.6-lttng.orig/arch/arm/Kconfig	2007-09-04 12:14:48.000000000 -0400
+++ linux-2.6-lttng/arch/arm/Kconfig	2007-09-04 12:16:57.000000000 -0400
@@ -1057,6 +1057,8 @@ source "drivers/rtc/Kconfig"
 
 source "drivers/dma/Kconfig"
 
+source "kernel/Kconfig.marker"
+
 endmenu
 
 source "fs/Kconfig"
Index: linux-2.6-lttng/arch/cris/Kconfig
===================================================================
--- linux-2.6-lttng.orig/arch/cris/Kconfig	2007-09-04 12:14:48.000000000 -0400
+++ linux-2.6-lttng/arch/cris/Kconfig	2007-09-04 12:16:57.000000000 -0400
@@ -151,6 +151,8 @@ source "net/Kconfig"
 menu "Drivers for built-in interfaces"
 source arch/cris/arch-v10/drivers/Kconfig
 
+source "kernel/Kconfig.marker"
+
 endmenu
 
 source "drivers/base/Kconfig"
Index: linux-2.6-lttng/arch/frv/Kconfig
===================================================================
--- linux-2.6-lttng.orig/arch/frv/Kconfig	2007-09-04 12:14:48.000000000 -0400
+++ linux-2.6-lttng/arch/frv/Kconfig	2007-09-04 12:16:57.000000000 -0400
@@ -367,6 +367,8 @@ menu "Executable formats"
 
 source "fs/Kconfig.binfmt"
 
+source "kernel/Kconfig.marker"
+
 endmenu
 
 source "net/Kconfig"
Index: linux-2.6-lttng/arch/h8300/Kconfig
===================================================================
--- linux-2.6-lttng.orig/arch/h8300/Kconfig	2007-09-04 12:14:49.000000000 -0400
+++ linux-2.6-lttng/arch/h8300/Kconfig	2007-09-04 12:16:57.000000000 -0400
@@ -219,6 +219,8 @@ source "drivers/hwmon/Kconfig"
 
 source "drivers/usb/Kconfig"
 
+source "kernel/Kconfig.marker"
+
 endmenu
 
 source "fs/Kconfig"
Index: linux-2.6-lttng/arch/i386/Kconfig
===================================================================
--- linux-2.6-lttng.orig/arch/i386/Kconfig	2007-09-04 12:14:49.000000000 -0400
+++ linux-2.6-lttng/arch/i386/Kconfig	2007-09-04 12:16:57.000000000 -0400
@@ -1263,6 +1263,8 @@ config KPROBES
 	  for kernel debugging, non-intrusive instrumentation and testing.
 	  If in doubt, say "N".
 
+source "kernel/Kconfig.marker"
+
 endif # INSTRUMENTATION
 
 source "arch/i386/Kconfig.debug"
Index: linux-2.6-lttng/arch/ia64/Kconfig
===================================================================
--- linux-2.6-lttng.orig/arch/ia64/Kconfig	2007-09-04 12:14:49.000000000 -0400
+++ linux-2.6-lttng/arch/ia64/Kconfig	2007-09-04 12:16:57.000000000 -0400
@@ -537,6 +537,8 @@ source "drivers/pci/hotplug/Kconfig"
 
 source "drivers/pcmcia/Kconfig"
 
+source "kernel/Kconfig.marker"
+
 endmenu
 
 endif
Index: linux-2.6-lttng/arch/m32r/Kconfig
===================================================================
--- linux-2.6-lttng.orig/arch/m32r/Kconfig	2007-09-04 12:14:49.000000000 -0400
+++ linux-2.6-lttng/arch/m32r/Kconfig	2007-09-04 12:16:57.000000000 -0400
@@ -418,6 +418,8 @@ menu "Executable file formats"
 
 source "fs/Kconfig.binfmt"
 
+source "kernel/Kconfig.marker"
+
 endmenu
 
 source "net/Kconfig"
Index: linux-2.6-lttng/arch/m68k/Kconfig
===================================================================
--- linux-2.6-lttng.orig/arch/m68k/Kconfig	2007-09-04 12:14:49.000000000 -0400
+++ linux-2.6-lttng/arch/m68k/Kconfig	2007-09-04 12:16:57.000000000 -0400
@@ -679,6 +679,8 @@ config SERIAL_CONSOLE
 
 	  If unsure, say N.
 
+source "kernel/Kconfig.marker"
+
 endmenu
 
 source "fs/Kconfig"
Index: linux-2.6-lttng/arch/m68knommu/Kconfig
===================================================================
--- linux-2.6-lttng.orig/arch/m68knommu/Kconfig	2007-09-04 12:14:49.000000000 -0400
+++ linux-2.6-lttng/arch/m68knommu/Kconfig	2007-09-04 12:16:57.000000000 -0400
@@ -673,6 +673,8 @@ source "drivers/pcmcia/Kconfig"
 
 source "drivers/pci/hotplug/Kconfig"
 
+source "kernel/Kconfig.marker"
+
 endmenu
 
 menu "Executable file formats"
Index: linux-2.6-lttng/arch/mips/Kconfig
===================================================================
--- linux-2.6-lttng.orig/arch/mips/Kconfig	2007-09-04 12:14:49.000000000 -0400
+++ linux-2.6-lttng/arch/mips/Kconfig	2007-09-04 12:16:57.000000000 -0400
@@ -1950,6 +1950,8 @@ menu "Power management options"
 
 source "kernel/power/Kconfig"
 
+source "kernel/Kconfig.marker"
+
 endmenu
 
 source "net/Kconfig"
Index: linux-2.6-lttng/arch/parisc/Kconfig
===================================================================
--- linux-2.6-lttng.orig/arch/parisc/Kconfig	2007-09-04 12:14:49.000000000 -0400
+++ linux-2.6-lttng/arch/parisc/Kconfig	2007-09-04 12:16:57.000000000 -0400
@@ -259,6 +259,8 @@ menu "Executable file formats"
 
 source "fs/Kconfig.binfmt"
 
+source "kernel/Kconfig.marker"
+
 endmenu
 
 source "net/Kconfig"
Index: linux-2.6-lttng/arch/powerpc/Kconfig
===================================================================
--- linux-2.6-lttng.orig/arch/powerpc/Kconfig	2007-09-04 12:14:49.000000000 -0400
+++ linux-2.6-lttng/arch/powerpc/Kconfig	2007-09-04 12:16:57.000000000 -0400
@@ -495,6 +495,8 @@ source "drivers/pcmcia/Kconfig"
 
 source "drivers/pci/hotplug/Kconfig"
 
+source "kernel/Kconfig.marker"
+
 endmenu
 
 menu "Advanced setup"
Index: linux-2.6-lttng/arch/ppc/Kconfig
===================================================================
--- linux-2.6-lttng.orig/arch/ppc/Kconfig	2007-09-04 12:14:49.000000000 -0400
+++ linux-2.6-lttng/arch/ppc/Kconfig	2007-09-04 12:16:57.000000000 -0400
@@ -1303,6 +1303,8 @@ config SERIAL_SICC_CONSOLE
 	depends on SERIAL_SICC && UART0_TTYS1
 	default y
 
+source "kernel/Kconfig.marker"
+
 endmenu
 
 source "lib/Kconfig"
Index: linux-2.6-lttng/arch/s390/Kconfig
===================================================================
--- linux-2.6-lttng.orig/arch/s390/Kconfig	2007-09-04 12:14:49.000000000 -0400
+++ linux-2.6-lttng/arch/s390/Kconfig	2007-09-04 12:16:57.000000000 -0400
@@ -543,6 +543,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"
Index: linux-2.6-lttng/arch/sh/Kconfig
===================================================================
--- linux-2.6-lttng.orig/arch/sh/Kconfig	2007-09-04 12:14:49.000000000 -0400
+++ linux-2.6-lttng/arch/sh/Kconfig	2007-09-04 12:16:57.000000000 -0400
@@ -720,6 +720,8 @@ depends on EXPERIMENTAL && SYS_SUPPORTS_
 
 source kernel/power/Kconfig
 
+source "kernel/Kconfig.marker"
+
 endmenu
 
 source "net/Kconfig"
Index: linux-2.6-lttng/arch/sh64/Kconfig
===================================================================
--- linux-2.6-lttng.orig/arch/sh64/Kconfig	2007-09-04 12:14:49.000000000 -0400
+++ linux-2.6-lttng/arch/sh64/Kconfig	2007-09-04 12:16:57.000000000 -0400
@@ -277,6 +277,8 @@ menu "Executable file formats"
 
 source "fs/Kconfig.binfmt"
 
+source "kernel/Kconfig.marker"
+
 endmenu
 
 source "net/Kconfig"
Index: linux-2.6-lttng/arch/sparc/Kconfig
===================================================================
--- linux-2.6-lttng.orig/arch/sparc/Kconfig	2007-09-04 12:14:49.000000000 -0400
+++ linux-2.6-lttng/arch/sparc/Kconfig	2007-09-04 12:16:57.000000000 -0400
@@ -316,6 +316,8 @@ config UNIX98_PTY_COUNT
 	  When not in use, each additional set of 256 PTYs occupy
 	  approximately 8 KB of kernel memory on 32-bit architectures.
 
+source "kernel/Kconfig.marker"
+
 endmenu
 
 source "fs/Kconfig"
Index: linux-2.6-lttng/arch/sparc64/Kconfig
===================================================================
--- linux-2.6-lttng.orig/arch/sparc64/Kconfig	2007-09-04 12:14:49.000000000 -0400
+++ linux-2.6-lttng/arch/sparc64/Kconfig	2007-09-04 12:16:57.000000000 -0400
@@ -412,6 +412,8 @@ config SOLARIS_EMUL
 	  To compile this code as a module, choose M here: the
 	  module will be called solaris.
 
+source "kernel/Kconfig.marker"
+
 endmenu
 
 config SCHED_SMT
Index: linux-2.6-lttng/arch/um/Kconfig
===================================================================
--- linux-2.6-lttng.orig/arch/um/Kconfig	2007-09-04 12:14:49.000000000 -0400
+++ linux-2.6-lttng/arch/um/Kconfig	2007-09-04 12:16:57.000000000 -0400
@@ -253,6 +253,8 @@ config UML_REAL_TIME_CLOCK
 	  This could result in a noticeable lag.  If this is a problem, then
 	  disable this option.
 
+source "kernel/Kconfig.marker"
+
 endmenu
 
 source "init/Kconfig"
Index: linux-2.6-lttng/arch/v850/Kconfig
===================================================================
--- linux-2.6-lttng.orig/arch/v850/Kconfig	2007-09-04 12:14:49.000000000 -0400
+++ linux-2.6-lttng/arch/v850/Kconfig	2007-09-04 12:16:57.000000000 -0400
@@ -294,6 +294,8 @@ source "drivers/ide/Kconfig"
 
 source "drivers/scsi/Kconfig"
 
+source "kernel/Kconfig.marker"
+
 endmenu
 
 #############################################################################
Index: linux-2.6-lttng/arch/x86_64/Kconfig
===================================================================
--- linux-2.6-lttng.orig/arch/x86_64/Kconfig	2007-09-04 12:14:49.000000000 -0400
+++ linux-2.6-lttng/arch/x86_64/Kconfig	2007-09-04 12:16:57.000000000 -0400
@@ -827,6 +827,8 @@ config SYSVIPC_COMPAT
 	depends on COMPAT && SYSVIPC
 	default y
 
+source "kernel/Kconfig.marker"
+
 endmenu
 
 source "net/Kconfig"
Index: linux-2.6-lttng/arch/xtensa/Kconfig
===================================================================
--- linux-2.6-lttng.orig/arch/xtensa/Kconfig	2007-09-04 12:14:49.000000000 -0400
+++ linux-2.6-lttng/arch/xtensa/Kconfig	2007-09-04 12:16:57.000000000 -0400
@@ -226,6 +226,8 @@ config KCORE_ELF
 
 source "fs/Kconfig.binfmt"
 
+source "kernel/Kconfig.marker"
+
 endmenu
 
 source "net/Kconfig"
Index: linux-2.6-lttng/kernel/Kconfig.marker
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6-lttng/kernel/Kconfig.marker	2007-09-04 12:16:57.000000000 -0400
@@ -0,0 +1,7 @@
+# Code markers configuration
+
+config MARKERS
+	bool "Activate markers"
+	help
+	  Place an empty function call at each marker site. Can be
+	  dynamically changed for a probe function.
Index: linux-2.6-lttng/arch/avr32/Kconfig.debug
===================================================================
--- linux-2.6-lttng.orig/arch/avr32/Kconfig.debug	2007-09-04 12:14:49.000000000 -0400
+++ linux-2.6-lttng/arch/avr32/Kconfig.debug	2007-09-04 12:16:57.000000000 -0400
@@ -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
Index: linux-2.6-lttng/kernel/Makefile
===================================================================
--- linux-2.6-lttng.orig/kernel/Makefile	2007-09-04 12:16:40.000000000 -0400
+++ linux-2.6-lttng/kernel/Makefile	2007-09-04 12:16:57.000000000 -0400
@@ -62,6 +62,7 @@ obj-$(CONFIG_TASK_DELAY_ACCT) += delayac
 obj-$(CONFIG_TASKSTATS) += taskstats.o tsacct.o
 obj-$(CONFIG_RESOURCE_COUNTERS) += res_counter.o
 obj-$(CONFIG_IMMEDIATE) += immediate.o
+obj-$(CONFIG_MARKERS) += marker.o
 
 ifneq ($(CONFIG_SCHED_NO_NO_OMIT_FRAME_POINTER),y)
 # According to Alan Modra <alan@linuxcare.com.au>, the -fno-omit-frame-pointer is

-- 
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 2/6] Linux Kernel Markers - Add kconfig menus for the marker code

[Randy Dunlap]

On Thu, 06 Sep 2007 16:07:35 -0400 Mathieu Desnoyers wrote:

[snip]

> This patch:
> 
> Add Kconfig menus for the marker code.
> 
> [bunk@stusta.de: Never ever select MODULES]
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> Reviewed-by: Christoph Hellwig <hch@infradead.org>
> Reviewed-by: "Frank Ch. Eigler" <fche@redhat.com>
> CC: Adrian Bunk <bunk@stusta.de>
> ---
> 
>  arch/alpha/Kconfig       |    2 ++
>  arch/arm/Kconfig         |    2 ++
>  arch/avr32/Kconfig.debug |    7 +++++++
>  arch/cris/Kconfig        |    2 ++
>  arch/frv/Kconfig         |    2 ++
>  arch/h8300/Kconfig       |    2 ++
>  arch/i386/Kconfig        |    2 ++
>  arch/ia64/Kconfig        |    2 ++
>  arch/m32r/Kconfig        |    2 ++
>  arch/m68k/Kconfig        |    2 ++
>  arch/m68knommu/Kconfig   |    2 ++
>  arch/mips/Kconfig        |    2 ++
>  arch/parisc/Kconfig      |    2 ++
>  arch/powerpc/Kconfig     |    2 ++
>  arch/ppc/Kconfig         |    2 ++
>  arch/s390/Kconfig        |    2 ++
>  arch/sh/Kconfig          |    2 ++
>  arch/sh64/Kconfig        |    2 ++
>  arch/sparc/Kconfig       |    2 ++
>  arch/sparc64/Kconfig     |    2 ++
>  arch/um/Kconfig          |    2 ++
>  arch/v850/Kconfig        |    2 ++
>  arch/x86_64/Kconfig      |    2 ++
>  arch/xtensa/Kconfig      |    2 ++
>  kernel/Kconfig.marker    |    7 +++++++
>  kernel/Makefile          |    1 +
>  26 files changed, 61 insertions(+)
> 
> Index: linux-2.6-lttng/arch/alpha/Kconfig
> ===================================================================
> --- linux-2.6-lttng.orig/arch/alpha/Kconfig	2007-09-04 12:14:48.000000000 -0400
> +++ linux-2.6-lttng/arch/alpha/Kconfig	2007-09-04 12:16:57.000000000 -0400
> @@ -646,6 +646,8 @@ config SRM_ENV
>  
>  source "fs/Kconfig.binfmt"
>  
> +source "kernel/Kconfig.marker"
> +
>  endmenu
>  
>  source "net/Kconfig"
> Index: linux-2.6-lttng/kernel/Kconfig.marker
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6-lttng/kernel/Kconfig.marker	2007-09-04 12:16:57.000000000 -0400
> @@ -0,0 +1,7 @@
> +# Code markers configuration
> +
> +config MARKERS
> +	bool "Activate markers"
> +	help
> +	  Place an empty function call at each marker site. Can be
> +	  dynamically changed for a probe function.
> Index: linux-2.6-lttng/arch/avr32/Kconfig.debug
> ===================================================================
> --- linux-2.6-lttng.orig/arch/avr32/Kconfig.debug	2007-09-04 12:14:49.000000000 -0400
> +++ linux-2.6-lttng/arch/avr32/Kconfig.debug	2007-09-04 12:16:57.000000000 -0400
> @@ -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

It seems sad that this patch sources Kconfig.marker, a 7-line file,
20-something times.  Yes, you (we) don't want to put those 7 lines
into 20-something different files, so sourcing is the right thing.

However, what you did for avr32 seems more on the right track to me:
make _one_ Instrumentation support menu that includes PROFILING,
OPROFILE, KPROBES, and MARKERS and then use (source) that in all
of the arches.

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

Re: [patch 2/6] Linux Kernel Markers - Add kconfig menus for the marker code

[Mathieu Desnoyers]

* Randy Dunlap (randy.dunlap@oracle.com) wrote:
> On Thu, 06 Sep 2007 16:07:35 -0400 Mathieu Desnoyers wrote:
> 
> [snip]
> 
> > This patch:
> > 
> > Add Kconfig menus for the marker code.
> > 
> > [bunk@stusta.de: Never ever select MODULES]
> > Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> > Reviewed-by: Christoph Hellwig <hch@infradead.org>
> > Reviewed-by: "Frank Ch. Eigler" <fche@redhat.com>
> > CC: Adrian Bunk <bunk@stusta.de>
> > ---
> > 
> >  arch/alpha/Kconfig       |    2 ++
> >  arch/arm/Kconfig         |    2 ++
> >  arch/avr32/Kconfig.debug |    7 +++++++
> >  arch/cris/Kconfig        |    2 ++
> >  arch/frv/Kconfig         |    2 ++
> >  arch/h8300/Kconfig       |    2 ++
> >  arch/i386/Kconfig        |    2 ++
> >  arch/ia64/Kconfig        |    2 ++
> >  arch/m32r/Kconfig        |    2 ++
> >  arch/m68k/Kconfig        |    2 ++
> >  arch/m68knommu/Kconfig   |    2 ++
> >  arch/mips/Kconfig        |    2 ++
> >  arch/parisc/Kconfig      |    2 ++
> >  arch/powerpc/Kconfig     |    2 ++
> >  arch/ppc/Kconfig         |    2 ++
> >  arch/s390/Kconfig        |    2 ++
> >  arch/sh/Kconfig          |    2 ++
> >  arch/sh64/Kconfig        |    2 ++
> >  arch/sparc/Kconfig       |    2 ++
> >  arch/sparc64/Kconfig     |    2 ++
> >  arch/um/Kconfig          |    2 ++
> >  arch/v850/Kconfig        |    2 ++
> >  arch/x86_64/Kconfig      |    2 ++
> >  arch/xtensa/Kconfig      |    2 ++
> >  kernel/Kconfig.marker    |    7 +++++++
> >  kernel/Makefile          |    1 +
> >  26 files changed, 61 insertions(+)
> > 
> > Index: linux-2.6-lttng/arch/alpha/Kconfig
> > ===================================================================
> > --- linux-2.6-lttng.orig/arch/alpha/Kconfig	2007-09-04 12:14:48.000000000 -0400
> > +++ linux-2.6-lttng/arch/alpha/Kconfig	2007-09-04 12:16:57.000000000 -0400
> > @@ -646,6 +646,8 @@ config SRM_ENV
> >  
> >  source "fs/Kconfig.binfmt"
> >  
> > +source "kernel/Kconfig.marker"
> > +
> >  endmenu
> >  
> >  source "net/Kconfig"
> > Index: linux-2.6-lttng/kernel/Kconfig.marker
> > ===================================================================
> > --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> > +++ linux-2.6-lttng/kernel/Kconfig.marker	2007-09-04 12:16:57.000000000 -0400
> > @@ -0,0 +1,7 @@
> > +# Code markers configuration
> > +
> > +config MARKERS
> > +	bool "Activate markers"
> > +	help
> > +	  Place an empty function call at each marker site. Can be
> > +	  dynamically changed for a probe function.
> > Index: linux-2.6-lttng/arch/avr32/Kconfig.debug
> > ===================================================================
> > --- linux-2.6-lttng.orig/arch/avr32/Kconfig.debug	2007-09-04 12:14:49.000000000 -0400
> > +++ linux-2.6-lttng/arch/avr32/Kconfig.debug	2007-09-04 12:16:57.000000000 -0400
> > @@ -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
> 
> It seems sad that this patch sources Kconfig.marker, a 7-line file,
> 20-something times.  Yes, you (we) don't want to put those 7 lines
> into 20-something different files, so sourcing is the right thing.
> 
> However, what you did for avr32 seems more on the right track to me:
> make _one_ Instrumentation support menu that includes PROFILING,
> OPROFILE, KPROBES, and MARKERS and then use (source) that in all
> of the arches.
> 

Ok, it will be in my next post.

Mathieu

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

-- 
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 3/6] Linux Kernel Markers - Documentation

[Mathieu Desnoyers]

[-- Attachment #1: linux-kernel-markers-documentation.patch --]
[-- Type: text/plain, Size: 8779 bytes --]

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

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
Reviewed-by: Christoph Hellwig <hch@infradead.org>
Reviewed-by: "Frank Ch. Eigler" <fche@redhat.com>
---

 Documentation/marker.txt |  257 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 257 insertions(+)

Index: linux-2.6-lttng/Documentation/marker.txt
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6-lttng/Documentation/marker.txt	2007-08-17 11:56:20.000000000 -0400
@@ -0,0 +1,257 @@
+ 	             Using the Linux Kernel Markers
+
+			    Mathieu Desnoyers
+
+
+This document introduces Linux Kernel Markers and their use. It provides
+examples of how to insert markers in the kernel and connect probe functions to
+them and provides some examples of probe functions.
+
+
+* Purpose of markers
+
+A marker placed in your code provides a hook to call 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). When a marker is "off" it has no effect, except
+for adding a tiny time penality (checking a condition for a branch) and space
+penality (adding a few bytes for the function call at the end of the
+instrumented function and adds a data structure in a separate section). The
+immediate values are used to minimize the impact on data cache, encoding the
+condition in the instruction stream. When a marker is "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 (continuing from the marker site).
+
+You can put markers at important locations in the code. Markers are
+lightweight hooks that can pass an arbitrary number of parameters,
+described in a printk-like format string, to the attached probe function.
+
+They can be used for tracing and performance accounting.
+
+
+* Usage
+
+In order to use the macro trace_mark, you should include linux/marker.h.
+
+#include <linux/marker.h>
+
+Add, in your code :
+
+trace_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" is the formatted string for the serializer.
+- someint is an integer.
+- somestring is a char pointer.
+
+Connecting a function (probe) to a marker is done by providing a probe (function
+to call) for the specific marker through marker_probe_register() and can be
+activated by calling marker_arm(). Marker deactivation can be done by calling
+marker_disarm() as many times as marker_arm() has been called. Removing a probe
+is done through marker_probe_unregister(); it will disarm the probe and make
+sure there is no caller left using the probe when it returns. Probe removal is
+preempt-safe because preemption is disabled around the probe call. See the
+"Probe example" section below for a sample probe module.
+
+The marker mechanism supports inserting multiple instances of the same marker.
+Markers can be put in inline functions, inlined static functions, and
+unrolled loops.
+
+The naming scheme "subsystem_event" is suggested here as a convention intended
+to limit collisions. Marker names are global to the kernel: they are considered
+as being the same whether they are in the core kernel image or in modules.
+Conflicting format strings for markers with the same name will cause the markers
+to be detected to have a different format string not to be armed and will output
+a printk warning which identifies the inconsistency:
+
+"Format mismatch for probe probe_name (format), marker (format)"
+
+
+* Optimization for a given architecture
+
+One can implement optimized markers for a given architecture by replacing
+asm-$ARCH/marker.h.
+
+To force use of a non-optimized version of the markers, _trace_mark() should be
+used. It takes the same parameters as the normal markers, but it does not use
+the immediate values based on code patching.
+
+
+* 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
+ *
+ * Connects two functions to marker call sites.
+ *
+ * (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>
+#include <linux/module.h>
+#include <linux/marker.h>
+#include <asm/atomic.h>
+
+struct probe_data {
+	const char *name;
+	const char *format;
+	marker_probe_func *probe_func;
+};
+
+void probe_subsystem_event(const struct __mark_marker *mdata,
+		const char *format, ...)
+{
+	va_list ap;
+	/* Declare args */
+	unsigned int value;
+	const char *mystr;
+
+	/* Assign args */
+	va_start(ap, format);
+	value = va_arg(ap, typeof(value));
+	mystr = va_arg(ap, typeof(mystr));
+
+	/* Call printk */
+	printk("Value %u, string %s\n", value, mystr);
+
+	/* or count, check rights, serialize data in a buffer */
+
+	va_end(ap);
+}
+
+atomic_t eventb_count = ATOMIC_INIT(0);
+
+void probe_subsystem_eventb(const struct __mark_marker *mdata,
+	const char *format, ...)
+{
+	/* 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;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(probe_array); i++) {
+		result = marker_probe_register(probe_array[i].name,
+				probe_array[i].format,
+				probe_array[i].probe_func, &probe_array[i]);
+		if (result)
+			printk(KERN_INFO "Unable to register probe %s\n",
+				probe_array[i].name);
+		result = marker_arm(probe_array[i].name);
+		if (result)
+			printk(KERN_INFO "Unable to arm probe %s\n",
+				probe_array[i].name);
+	}
+	return 0;
+}
+
+static void __exit probe_fini(void)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(probe_array); i++) {
+		marker_probe_unregister(probe_array[i].name);
+	}
+	printk("Number of event b : %u\n", atomic_read(&eventb_count));
+}
+
+module_init(probe_init);
+module_exit(probe_fini);
+
+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;
+
+	trace_mark(subsystem_event, "%d %s", 123, "example string");
+	for (i=0; i<10; i++) {
+		trace_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 order is not important)
+insmod probe-example.ko
+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

Re: [patch 3/6] Linux Kernel Markers - Documentation

[Randy Dunlap]

On Thu, 06 Sep 2007 16:07:36 -0400 Mathieu Desnoyers wrote:

> Here is some documentation explaining what is/how to use the Linux
> Kernel Markers.
> ---
> 
>  Documentation/marker.txt |  257 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 257 insertions(+)
> 
> Index: linux-2.6-lttng/Documentation/marker.txt
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6-lttng/Documentation/marker.txt	2007-08-17 11:56:20.000000000 -0400
> @@ -0,0 +1,257 @@
> + 	             Using the Linux Kernel Markers
> +
> +			    Mathieu Desnoyers
> +
> +
> +This document introduces Linux Kernel Markers and their use. It provides
> +examples of how to insert markers in the kernel and connect probe functions to
> +them and provides some examples of probe functions.
> +
> +
> +* Purpose of markers
> +
> +A marker placed in your code provides a hook to call a function (probe) that

                      ^^^^ I would drop "your".  It quite likely
	could be someone else's code.

> +you can provide at runtime. A marker can be "on" (a probe is connected to it) or
> +"off" (no probe is attached). When a marker is "off" it has no effect, except
> +for adding a tiny time penality (checking a condition for a branch) and space

                          penalty

> +penality (adding a few bytes for the function call at the end of the

   penalty

> +instrumented function and adds a data structure in a separate section). The
> +immediate values are used to minimize the impact on data cache, encoding the
> +condition in the instruction stream. When a marker is "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 (continuing from the marker site).
> +
> +You can put markers at important locations in the code. Markers are
> +lightweight hooks that can pass an arbitrary number of parameters,
> +described in a printk-like format string, to the attached probe function.
> +
> +They can be used for tracing and performance accounting.
> +
> +
> +* Usage
> +
> +In order to use the macro trace_mark, you should include linux/marker.h.
> +
> +#include <linux/marker.h>
> +
> +Add, in your code :

   And,

> +
> +trace_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" is the formatted string for the serializer.
> +- someint is an integer.
> +- somestring is a char pointer.
> +
> +Connecting a function (probe) to a marker is done by providing a probe (function
> +to call) for the specific marker through marker_probe_register() and can be
> +activated by calling marker_arm(). Marker deactivation can be done by calling
> +marker_disarm() as many times as marker_arm() has been called. Removing a probe
> +is done through marker_probe_unregister(); it will disarm the probe and make
> +sure there is no caller left using the probe when it returns. Probe removal is
> +preempt-safe because preemption is disabled around the probe call. See the
> +"Probe example" section below for a sample probe module.
> +
> +The marker mechanism supports inserting multiple instances of the same marker.
> +Markers can be put in inline functions, inlined static functions, and
> +unrolled loops.
> +
> +The naming scheme "subsystem_event" is suggested here as a convention intended
> +to limit collisions. Marker names are global to the kernel: they are considered
> +as being the same whether they are in the core kernel image or in modules.
> +Conflicting format strings for markers with the same name will cause the markers
> +to be detected to have a different format string not to be armed and will output
> +a printk warning which identifies the inconsistency:
> +
> +"Format mismatch for probe probe_name (format), marker (format)"
> +
> +
> +* Optimization for a given architecture
> +
> +One can implement optimized markers for a given architecture by replacing
> +asm-$ARCH/marker.h.
> +
> +To force use of a non-optimized version of the markers, _trace_mark() should be
> +used. It takes the same parameters as the normal markers, but it does not use
> +the immediate values based on code patching.
> +
> +
> +* Probe example

I would much prefer that you put marker-related docs into
Documentation/marker[s]/ and then put the Makefile and source files
into Documentation/marker[s]/src/.

This make the source files more visible (findable, grepable) to other
developers & users.  It also makes it feasible to use and test them
on a regular basis so that API changes (=> bugs:) can be easily
found and fixed.

And then you can drop those "CUT" lines.  :)


> +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
> + *
> + * Connects two functions to marker call sites.
> + *
> + * (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>
> +#include <linux/module.h>
> +#include <linux/marker.h>
> +#include <asm/atomic.h>
> +
> +struct probe_data {
> +	const char *name;
> +	const char *format;
> +	marker_probe_func *probe_func;
> +};
> +
> +void probe_subsystem_event(const struct __mark_marker *mdata,
> +		const char *format, ...)
> +{
> +	va_list ap;
> +	/* Declare args */
> +	unsigned int value;
> +	const char *mystr;
> +
> +	/* Assign args */
> +	va_start(ap, format);
> +	value = va_arg(ap, typeof(value));
> +	mystr = va_arg(ap, typeof(mystr));
> +
> +	/* Call printk */
> +	printk("Value %u, string %s\n", value, mystr);
> +
> +	/* or count, check rights, serialize data in a buffer */
> +
> +	va_end(ap);
> +}
> +
> +atomic_t eventb_count = ATOMIC_INIT(0);
> +
> +void probe_subsystem_eventb(const struct __mark_marker *mdata,
> +	const char *format, ...)
> +{
> +	/* 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;
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(probe_array); i++) {
> +		result = marker_probe_register(probe_array[i].name,
> +				probe_array[i].format,
> +				probe_array[i].probe_func, &probe_array[i]);
> +		if (result)
> +			printk(KERN_INFO "Unable to register probe %s\n",
> +				probe_array[i].name);
> +		result = marker_arm(probe_array[i].name);
> +		if (result)
> +			printk(KERN_INFO "Unable to arm probe %s\n",
> +				probe_array[i].name);
> +	}
> +	return 0;
> +}
> +
> +static void __exit probe_fini(void)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(probe_array); i++) {
> +		marker_probe_unregister(probe_array[i].name);
> +	}
> +	printk("Number of event b : %u\n", atomic_read(&eventb_count));
> +}
> +
> +module_init(probe_init);
> +module_exit(probe_fini);
> +
> +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;
> +
> +	trace_mark(subsystem_event, "%d %s", 123, "example string");
> +	for (i=0; i<10; i++) {
> +		trace_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 order is not important)
> +insmod probe-example.ko
> +cat /proc/marker-example (returns an expected error)
> +rmmod marker-example probe-example
> +dmesg
> +------------------------------ CUT -------------------------------------

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

[patch 4/6] Linux Kernel Markers - Documentation Fix

[Mathieu Desnoyers]

[-- Attachment #1: linux-kernel-markers-documentation-fix.patch --]
[-- Type: text/plain, Size: 1024 bytes --]

While porting my code from an older markers version I noticed the
marker callbacks have grown a void *private argument.  Add it to
the documentation aswell.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
---
 Documentation/marker.txt |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6-lttng/Documentation/marker.txt
===================================================================
--- linux-2.6-lttng.orig/Documentation/marker.txt	2007-08-28 16:40:16.000000000 -0400
+++ linux-2.6-lttng/Documentation/marker.txt	2007-08-28 17:26:05.000000000 -0400
@@ -115,7 +115,7 @@ struct probe_data {
 };
 
 void probe_subsystem_event(const struct __mark_marker *mdata,
-		const char *format, ...)
+		void *private, const char *format, ...)
 {
 	va_list ap;
 	/* Declare args */

-- 
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 5/6] Port of blktrace to the Linux Kernel Markers.

[Mathieu Desnoyers]

[-- Attachment #1: linux-kernel-markers-port-blktrace-to-markers.patch --]
[-- Type: text/plain, Size: 27087 bytes --]

Here is the first stage of a port of blktrace to the Linux Kernel Markers. The
advantage of this port is that it minimizes the impact on the running when
blktrace is not active.

A few remarks : this patch has the positive effect of removing some code
from the block io tracing hot paths, minimizing the i-cache impact in a
system where the io tracing is compiled in but inactive.

It also moves the blk tracing code from a header (and therefore from the
body of the instrumented functions) to a separate C file.

There, as soon as one device has to be traced, all devices have to
execute the tracing function call when they pass by the instrumentation site.
This is slower than the previous inline function which tested the condition
quickly.

It does not make the code smaller, since I left all the specialized
tracing functions for requests, bio, generic, remap, which would go away
once a generic infrastructure is in place to serialize the information
passed to the marker. This is mostly why I consider it as a step towards the
full improvements that could bring the markers.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
Reviewed-by: "Frank Ch. Eigler" <fche@redhat.com>
CC: Jens Axboe <jens.axboe@oracle.com>
---

 block/Kconfig                |    1 
 block/blktrace.c             |  343 ++++++++++++++++++++++++++++++++++++++++++-
 block/elevator.c             |    6 
 block/ll_rw_blk.c            |   35 ++--
 drivers/block/cciss.c        |    4 
 drivers/md/dm.c              |   14 -
 fs/bio.c                     |    6 
 include/linux/blktrace_api.h |  145 +-----------------
 mm/bounce.c                  |    4 
 mm/highmem.c                 |    2 
 10 files changed, 388 insertions(+), 172 deletions(-)

Index: linux-2.6-lttng/block/elevator.c
===================================================================
--- linux-2.6-lttng.orig/block/elevator.c	2007-09-04 11:52:17.000000000 -0400
+++ linux-2.6-lttng/block/elevator.c	2007-09-04 12:17:25.000000000 -0400
@@ -32,7 +32,7 @@
 #include <linux/init.h>
 #include <linux/compiler.h>
 #include <linux/delay.h>
-#include <linux/blktrace_api.h>
+#include <linux/marker.h>
 #include <linux/hash.h>
 
 #include <asm/uaccess.h>
@@ -548,7 +548,7 @@ void elv_insert(struct request_queue *q,
 	unsigned ordseq;
 	int unplug_it = 1;
 
-	blk_add_trace_rq(q, rq, BLK_TA_INSERT);
+	trace_mark(blk_request_insert, "%p %p", q, rq);
 
 	rq->q = q;
 
@@ -735,7 +735,7 @@ struct request *elv_next_request(struct 
 			 * not be passed by new incoming requests
 			 */
 			rq->cmd_flags |= REQ_STARTED;
-			blk_add_trace_rq(q, rq, BLK_TA_ISSUE);
+			trace_mark(blk_request_issue, "%p %p", q, rq);
 		}
 
 		if (!q->boundary_rq || q->boundary_rq == rq) {
Index: linux-2.6-lttng/block/ll_rw_blk.c
===================================================================
--- linux-2.6-lttng.orig/block/ll_rw_blk.c	2007-09-04 11:54:04.000000000 -0400
+++ linux-2.6-lttng/block/ll_rw_blk.c	2007-09-04 12:18:42.000000000 -0400
@@ -28,6 +28,7 @@
 #include <linux/task_io_accounting_ops.h>
 #include <linux/interrupt.h>
 #include <linux/cpu.h>
+#include <linux/marker.h>
 #include <linux/blktrace_api.h>
 #include <linux/fault-inject.h>
 #include <linux/scatterlist.h>
@@ -1559,7 +1560,7 @@ void blk_plug_device(struct request_queu
 
 	if (!test_and_set_bit(QUEUE_FLAG_PLUGGED, &q->queue_flags)) {
 		mod_timer(&q->unplug_timer, jiffies + q->unplug_delay);
-		blk_add_trace_generic(q, NULL, 0, BLK_TA_PLUG);
+		trace_mark(blk_plug_device, "%p %p %d", q, NULL, 0);
 	}
 }
 
@@ -1625,7 +1626,7 @@ static void blk_backing_dev_unplug(struc
 	 * devices don't necessarily have an ->unplug_fn defined
 	 */
 	if (q->unplug_fn) {
-		blk_add_trace_pdu_int(q, BLK_TA_UNPLUG_IO, NULL,
+		trace_mark(blk_pdu_unplug_io, "%p %p %d", q, NULL,
 					q->rq.count[READ] + q->rq.count[WRITE]);
 
 		q->unplug_fn(q);
@@ -1637,7 +1638,7 @@ static void blk_unplug_work(struct work_
 	struct request_queue *q =
 		container_of(work, struct request_queue, unplug_work);
 
-	blk_add_trace_pdu_int(q, BLK_TA_UNPLUG_IO, NULL,
+	trace_mark(blk_pdu_unplug_io, "%p %p %d", q, NULL,
 				q->rq.count[READ] + q->rq.count[WRITE]);
 
 	q->unplug_fn(q);
@@ -1647,7 +1648,7 @@ static void blk_unplug_timeout(unsigned 
 {
 	struct request_queue *q = (struct request_queue *)data;
 
-	blk_add_trace_pdu_int(q, BLK_TA_UNPLUG_TIMER, NULL,
+	trace_mark(blk_pdu_unplug_timer, "%p %p %d", q, NULL,
 				q->rq.count[READ] + q->rq.count[WRITE]);
 
 	kblockd_schedule_work(&q->unplug_work);
@@ -2160,7 +2161,7 @@ rq_starved:
 	
 	rq_init(q, rq);
 
-	blk_add_trace_generic(q, bio, rw, BLK_TA_GETRQ);
+	trace_mark(blk_get_request, "%p %p %d", q, bio, rw);
 out:
 	return rq;
 }
@@ -2190,7 +2191,7 @@ static struct request *get_request_wait(
 		if (!rq) {
 			struct io_context *ioc;
 
-			blk_add_trace_generic(q, bio, rw, BLK_TA_SLEEPRQ);
+			trace_mark(blk_sleep_request, "%p %p %d", q, bio, rw);
 
 			__generic_unplug_device(q);
 			spin_unlock_irq(q->queue_lock);
@@ -2264,7 +2265,7 @@ EXPORT_SYMBOL(blk_start_queueing);
  */
 void blk_requeue_request(struct request_queue *q, struct request *rq)
 {
-	blk_add_trace_rq(q, rq, BLK_TA_REQUEUE);
+	trace_mark(blk_requeue, "%p %p", q, rq);
 
 	if (blk_rq_tagged(rq))
 		blk_queue_end_tag(q, rq);
@@ -2987,7 +2988,7 @@ static int __make_request(struct request
 			if (!ll_back_merge_fn(q, req, bio))
 				break;
 
-			blk_add_trace_bio(q, bio, BLK_TA_BACKMERGE);
+			trace_mark(blk_bio_backmerge, "%p %p", q, bio);
 
 			req->biotail->bi_next = bio;
 			req->biotail = bio;
@@ -3004,7 +3005,7 @@ static int __make_request(struct request
 			if (!ll_front_merge_fn(q, req, bio))
 				break;
 
-			blk_add_trace_bio(q, bio, BLK_TA_FRONTMERGE);
+			trace_mark(blk_bio_frontmerge, "%p %p", q, bio);
 
 			bio->bi_next = req->bio;
 			req->bio = bio;
@@ -3087,9 +3088,10 @@ static inline void blk_partition_remap(s
 		bio->bi_sector += p->start_sect;
 		bio->bi_bdev = bdev->bd_contains;
 
-		blk_add_trace_remap(bdev_get_queue(bio->bi_bdev), bio,
-				    bdev->bd_dev, bio->bi_sector,
-				    bio->bi_sector - p->start_sect);
+		trace_mark(blk_remap, "%p %p %llu %llu %llu",
+				    bdev_get_queue(bio->bi_bdev), bio,
+				    (u64)bdev->bd_dev, (u64)bio->bi_sector,
+				    (u64)bio->bi_sector - p->start_sect);
 	}
 }
 
@@ -3254,10 +3256,11 @@ end_io:
 		blk_partition_remap(bio);
 
 		if (old_sector != -1)
-			blk_add_trace_remap(q, bio, old_dev, bio->bi_sector,
-					    old_sector);
+			trace_mark(blk_remap, "%p %p %llu %llu %llu",
+				q, bio, (u64)old_dev,
+				(u64)bio->bi_sector, (u64)old_sector);
 
-		blk_add_trace_bio(q, bio, BLK_TA_QUEUE);
+		trace_mark(blk_bio_queue, "%p %p", q, bio);
 
 		old_sector = bio->bi_sector;
 		old_dev = bio->bi_bdev->bd_dev;
@@ -3446,7 +3449,7 @@ static int __end_that_request_first(stru
 	int total_bytes, bio_nbytes, error, next_idx = 0;
 	struct bio *bio;
 
-	blk_add_trace_rq(req->q, req, BLK_TA_COMPLETE);
+	trace_mark(blk_request_complete, "%p %p", req->q, req);
 
 	/*
 	 * extend uptodate bool to allow < 0 value to be direct io error
Index: linux-2.6-lttng/block/Kconfig
===================================================================
--- linux-2.6-lttng.orig/block/Kconfig	2007-09-04 11:49:36.000000000 -0400
+++ linux-2.6-lttng/block/Kconfig	2007-09-04 12:17:25.000000000 -0400
@@ -32,6 +32,7 @@ config BLK_DEV_IO_TRACE
 	depends on SYSFS
 	select RELAY
 	select DEBUG_FS
+	select MARKERS
 	help
 	  Say Y here, if you want to be able to trace the block layer actions
 	  on a given queue. Tracing allows you to see any traffic happening
Index: linux-2.6-lttng/block/blktrace.c
===================================================================
--- linux-2.6-lttng.orig/block/blktrace.c	2007-09-04 11:52:35.000000000 -0400
+++ linux-2.6-lttng/block/blktrace.c	2007-09-04 12:17:25.000000000 -0400
@@ -23,11 +23,19 @@
 #include <linux/mutex.h>
 #include <linux/debugfs.h>
 #include <linux/time.h>
+#include <linux/marker.h>
 #include <asm/uaccess.h>
 
 static DEFINE_PER_CPU(unsigned long long, blk_trace_cpu_offset) = { 0, };
 static unsigned int blktrace_seq __read_mostly = 1;
 
+/* Global reference count of probes */
+static DEFINE_MUTEX(blk_probe_mutex);
+static int blk_probes_ref;
+
+int blk_probe_arm(void);
+void blk_probe_disarm(void);
+
 /*
  * Send out a notify message.
  */
@@ -179,7 +187,7 @@ void __blk_add_trace(struct blk_trace *b
 EXPORT_SYMBOL_GPL(__blk_add_trace);
 
 static struct dentry *blk_tree_root;
-static struct mutex blk_tree_mutex;
+static DEFINE_MUTEX(blk_tree_mutex);
 static unsigned int root_users;
 
 static inline void blk_remove_root(void)
@@ -229,6 +237,10 @@ static void blk_trace_cleanup(struct blk
 	blk_remove_tree(bt->dir);
 	free_percpu(bt->sequence);
 	kfree(bt);
+	mutex_lock(&blk_probe_mutex);
+	if (--blk_probes_ref == 0)
+		blk_probe_disarm();
+	mutex_unlock(&blk_probe_mutex);
 }
 
 static int blk_trace_remove(struct request_queue *q)
@@ -386,6 +398,11 @@ static int blk_trace_setup(struct reques
 		goto err;
 	}
 
+	mutex_lock(&blk_probe_mutex);
+	if (!blk_probes_ref++)
+		blk_probe_arm();
+	mutex_unlock(&blk_probe_mutex);
+
 	return 0;
 err:
 	if (dir)
@@ -549,9 +566,331 @@ static void blk_trace_set_ht_offsets(voi
 #endif
 }
 
+/**
+ * blk_add_trace_rq - Add a trace for a request oriented action
+ * Expected variable arguments :
+ * @q:		queue the io is for
+ * @rq:		the source request
+ *
+ * Description:
+ *     Records an action against a request. Will log the bio offset + size.
+ *
+ **/
+static void blk_add_trace_rq(const struct __mark_marker *mdata,
+	void *private_data, const char *fmt, ...)
+{
+	va_list args;
+	u32 what;
+	struct blk_trace *bt;
+	int rw;
+	struct blk_probe_data *pinfo = mdata->pdata;
+	struct request_queue *q;
+	struct request *rq;
+
+	va_start(args, fmt);
+	q = va_arg(args, struct request_queue *);
+	rq = va_arg(args, struct request *);
+	va_end(args);
+
+	what = pinfo->flags;
+	bt = q->blk_trace;
+	rw = rq->cmd_flags & 0x03;
+
+	if (likely(!bt))
+		return;
+
+	if (blk_pc_request(rq)) {
+		what |= BLK_TC_ACT(BLK_TC_PC);
+		__blk_add_trace(bt, 0, rq->data_len, rw, what, rq->errors, sizeof(rq->cmd), rq->cmd);
+	} else  {
+		what |= BLK_TC_ACT(BLK_TC_FS);
+		__blk_add_trace(bt, rq->hard_sector, rq->hard_nr_sectors << 9, rw, what, rq->errors, 0, NULL);
+	}
+}
+
+/**
+ * blk_add_trace_bio - Add a trace for a bio oriented action
+ * Expected variable arguments :
+ * @q:		queue the io is for
+ * @bio:	the source bio
+ *
+ * Description:
+ *     Records an action against a bio. Will log the bio offset + size.
+ *
+ **/
+static void blk_add_trace_bio(const struct __mark_marker *mdata,
+	void *private_data, const char *fmt, ...)
+{
+	va_list args;
+	u32 what;
+	struct blk_trace *bt;
+	struct blk_probe_data *pinfo = mdata->pdata;
+	struct request_queue *q;
+	struct bio *bio;
+
+	va_start(args, fmt);
+	q = va_arg(args, struct request_queue *);
+	bio = va_arg(args, struct bio *);
+	va_end(args);
+
+	what = pinfo->flags;
+	bt = q->blk_trace;
+
+	if (likely(!bt))
+		return;
+
+	__blk_add_trace(bt, bio->bi_sector, bio->bi_size, bio->bi_rw, what, !bio_flagged(bio, BIO_UPTODATE), 0, NULL);
+}
+
+/**
+ * blk_add_trace_generic - Add a trace for a generic action
+ * Expected variable arguments :
+ * @q:		queue the io is for
+ * @bio:	the source bio
+ * @rw:		the data direction
+ *
+ * Description:
+ *     Records a simple trace
+ *
+ **/
+static void blk_add_trace_generic(const struct __mark_marker *mdata,
+	void *private_data, const char *fmt, ...)
+{
+	va_list args;
+	struct blk_trace *bt;
+	u32 what;
+	struct blk_probe_data *pinfo = mdata->pdata;
+	struct request_queue *q;
+	struct bio *bio;
+	int rw;
+
+	va_start(args, fmt);
+	q = va_arg(args, struct request_queue *);
+	bio = va_arg(args, struct bio *);
+	rw = va_arg(args, int);
+	va_end(args);
+
+	what = pinfo->flags;
+	bt = q->blk_trace;
+
+	if (likely(!bt))
+		return;
+
+	if (bio)
+		blk_add_trace_bio(mdata, "%p %p", NULL, q, bio);
+	else
+		__blk_add_trace(bt, 0, 0, rw, what, 0, 0, NULL);
+}
+
+/**
+ * blk_add_trace_pdu_ll - Add a trace for a bio with any integer payload
+ * Expected variable arguments :
+ * @q:		queue the io is for
+ * @bio:	the source bio
+ * @pdu:	the long long integer payload
+ *
+ **/
+static inline void blk_trace_integer(struct request_queue *q, struct bio *bio, unsigned long long pdu,
+					u32 what)
+{
+	struct blk_trace *bt;
+	__be64 rpdu;
+
+	bt = q->blk_trace;
+	rpdu = cpu_to_be64(pdu);
+
+	if (likely(!bt))
+		return;
+
+	if (bio)
+		__blk_add_trace(bt, bio->bi_sector, bio->bi_size, bio->bi_rw, what,
+					!bio_flagged(bio, BIO_UPTODATE), sizeof(rpdu), &rpdu);
+	else
+		__blk_add_trace(bt, 0, 0, 0, what, 0, sizeof(rpdu), &rpdu);
+}
+
+/**
+ * blk_add_trace_pdu_ll - Add a trace for a bio with an long long integer
+ * payload
+ * Expected variable arguments :
+ * @q:		queue the io is for
+ * @bio:	the source bio
+ * @pdu:	the long long integer payload
+ *
+ * Description:
+ *     Adds a trace with some long long integer payload. This might be an unplug
+ *     option given as the action, with the depth at unplug time given as the
+ *     payload
+ *
+ **/
+static void blk_add_trace_pdu_ll(const struct __mark_marker *mdata,
+	void *private_data, const char *fmt, ...)
+{
+	va_list args;
+	struct blk_probe_data *pinfo = mdata->pdata;
+	struct request_queue *q;
+	struct bio *bio;
+	unsigned long long pdu;
+	u32 what;
+
+	what = pinfo->flags;
+
+	va_start(args, fmt);
+	q = va_arg(args, struct request_queue *);
+	bio = va_arg(args, struct bio *);
+	pdu = va_arg(args, unsigned long long);
+	va_end(args);
+
+	blk_trace_integer(q, bio, pdu, what);
+}
+
+
+/**
+ * blk_add_trace_pdu_int - Add a trace for a bio with an integer payload
+ * Expected variable arguments :
+ * @q:		queue the io is for
+ * @bio:	the source bio
+ * @pdu:	the integer payload
+ *
+ * Description:
+ *     Adds a trace with some integer payload. This might be an unplug
+ *     option given as the action, with the depth at unplug time given
+ *     as the payload
+ *
+ **/
+static void blk_add_trace_pdu_int(const struct __mark_marker *mdata,
+	void *private_data, const char *fmt, ...)
+{
+	va_list args;
+	struct blk_probe_data *pinfo = mdata->pdata;
+	struct request_queue *q;
+	struct bio *bio;
+	unsigned int pdu;
+	u32 what;
+
+	what = pinfo->flags;
+
+	va_start(args, fmt);
+	q = va_arg(args, struct request_queue *);
+	bio = va_arg(args, struct bio *);
+	pdu = va_arg(args, unsigned int);
+	va_end(args);
+
+	blk_trace_integer(q, bio, pdu, what);
+}
+
+/**
+ * blk_add_trace_remap - Add a trace for a remap operation
+ * Expected variable arguments :
+ * @q:		queue the io is for
+ * @bio:	the source bio
+ * @dev:	target device
+ * @from:	source sector
+ * @to:		target sector
+ *
+ * Description:
+ *     Device mapper or raid target sometimes need to split a bio because
+ *     it spans a stripe (or similar). Add a trace for that action.
+ *
+ **/
+static void blk_add_trace_remap(const struct __mark_marker *mdata,
+	void *private_data, const char *fmt, ...)
+{
+	va_list args;
+	struct blk_trace *bt;
+	struct blk_io_trace_remap r;
+	u32 what;
+	struct blk_probe_data *pinfo = mdata->pdata;
+	struct request_queue *q;
+	struct bio *bio;
+	u64 dev, from, to;
+
+	va_start(args, fmt);
+	q = va_arg(args, struct request_queue *);
+	bio = va_arg(args, struct bio *);
+	dev = va_arg(args, u64);
+	from = va_arg(args, u64);
+	to = va_arg(args, u64);
+	va_end(args);
+
+	what = pinfo->flags;
+	bt = q->blk_trace;
+
+	if (likely(!bt))
+		return;
+
+	r.device = cpu_to_be32(dev);
+	r.device_from = cpu_to_be32(bio->bi_bdev->bd_dev);
+	r.sector = cpu_to_be64(to);
+
+	__blk_add_trace(bt, from, bio->bi_size, bio->bi_rw, BLK_TA_REMAP, !bio_flagged(bio, BIO_UPTODATE), sizeof(r), &r);
+}
+
+#define FACILITY_NAME "blk"
+
+static struct blk_probe_data probe_array[] =
+{
+	{ "blk_bio_queue", "%p %p", BLK_TA_QUEUE, blk_add_trace_bio },
+	{ "blk_bio_backmerge", "%p %p", BLK_TA_BACKMERGE, blk_add_trace_bio },
+	{ "blk_bio_frontmerge", "%p %p", BLK_TA_FRONTMERGE, blk_add_trace_bio },
+	{ "blk_get_request", "%p %p %d", BLK_TA_GETRQ, blk_add_trace_generic },
+	{ "blk_sleep_request", "%p %p %d", BLK_TA_SLEEPRQ,
+		blk_add_trace_generic },
+	{ "blk_requeue", "%p %p", BLK_TA_REQUEUE, blk_add_trace_rq },
+	{ "blk_request_issue", "%p %p", BLK_TA_ISSUE, blk_add_trace_rq },
+	{ "blk_request_complete", "%p %p", BLK_TA_COMPLETE, blk_add_trace_rq },
+	{ "blk_plug_device", "%p %p %d", BLK_TA_PLUG, blk_add_trace_generic },
+	{ "blk_pdu_unplug_io", "%p %p %d", BLK_TA_UNPLUG_IO,
+		blk_add_trace_pdu_int },
+	{ "blk_pdu_unplug_timer", "%p %p %d", BLK_TA_UNPLUG_TIMER,
+		blk_add_trace_pdu_int },
+	{ "blk_request_insert", "%p %p", BLK_TA_INSERT,
+		blk_add_trace_rq },
+	{ "blk_pdu_split", "%p %p %llu", BLK_TA_SPLIT,
+		blk_add_trace_pdu_ll },
+	{ "blk_bio_bounce", "%p %p", BLK_TA_BOUNCE, blk_add_trace_bio },
+	{ "blk_remap", "%p %p %llu %llu %llu", BLK_TA_REMAP,
+		blk_add_trace_remap },
+};
+
+
+int blk_probe_arm(void)
+{
+	int result;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(probe_array); i++) {
+		result = marker_probe_register(probe_array[i].name,
+				probe_array[i].format,
+				probe_array[i].callback, &probe_array[i]);
+		if (result)
+			printk(KERN_INFO
+				"blktrace unable to register probe %s\n",
+				probe_array[i].name);
+		result = marker_arm(probe_array[i].name);
+		if (result)
+			printk(KERN_INFO
+				"blktrace unable to arm probe %s\n",
+				probe_array[i].name);
+	}
+	return 0;
+}
+
+void blk_probe_disarm(void)
+{
+	int i, err;
+
+	for (i = 0; i < ARRAY_SIZE(probe_array); i++) {
+		err = marker_disarm(probe_array[i].name);
+		BUG_ON(err);
+		err = IS_ERR(marker_probe_unregister(probe_array[i].name));
+		BUG_ON(err);
+	}
+}
+
+
 static __init int blk_trace_init(void)
 {
-	mutex_init(&blk_tree_mutex);
 	on_each_cpu(blk_trace_check_cpu_time, NULL, 1, 1);
 	blk_trace_set_ht_offsets();
 
Index: linux-2.6-lttng/include/linux/blktrace_api.h
===================================================================
--- linux-2.6-lttng.orig/include/linux/blktrace_api.h	2007-09-04 11:50:18.000000000 -0400
+++ linux-2.6-lttng/include/linux/blktrace_api.h	2007-09-04 12:17:25.000000000 -0400
@@ -3,6 +3,7 @@
 
 #include <linux/blkdev.h>
 #include <linux/relay.h>
+#include <linux/marker.h>
 
 /*
  * Trace categories
@@ -142,150 +143,22 @@ struct blk_user_trace_setup {
 	u32 pid;
 };
 
+/* Probe data used for probe-marker connection */
+struct blk_probe_data {
+	const char *name;
+	const char *format;
+	u32 flags;
+	marker_probe_func *callback;
+};
+
 #if defined(CONFIG_BLK_DEV_IO_TRACE)
 extern int blk_trace_ioctl(struct block_device *, unsigned, char __user *);
 extern void blk_trace_shutdown(struct request_queue *);
 extern void __blk_add_trace(struct blk_trace *, sector_t, int, int, u32, int, int, void *);
 
-/**
- * blk_add_trace_rq - Add a trace for a request oriented action
- * @q:		queue the io is for
- * @rq:		the source request
- * @what:	the action
- *
- * Description:
- *     Records an action against a request. Will log the bio offset + size.
- *
- **/
-static inline void blk_add_trace_rq(struct request_queue *q, struct request *rq,
-				    u32 what)
-{
-	struct blk_trace *bt = q->blk_trace;
-	int rw = rq->cmd_flags & 0x03;
-
-	if (likely(!bt))
-		return;
-
-	if (blk_pc_request(rq)) {
-		what |= BLK_TC_ACT(BLK_TC_PC);
-		__blk_add_trace(bt, 0, rq->data_len, rw, what, rq->errors, sizeof(rq->cmd), rq->cmd);
-	} else  {
-		what |= BLK_TC_ACT(BLK_TC_FS);
-		__blk_add_trace(bt, rq->hard_sector, rq->hard_nr_sectors << 9, rw, what, rq->errors, 0, NULL);
-	}
-}
-
-/**
- * blk_add_trace_bio - Add a trace for a bio oriented action
- * @q:		queue the io is for
- * @bio:	the source bio
- * @what:	the action
- *
- * Description:
- *     Records an action against a bio. Will log the bio offset + size.
- *
- **/
-static inline void blk_add_trace_bio(struct request_queue *q, struct bio *bio,
-				     u32 what)
-{
-	struct blk_trace *bt = q->blk_trace;
-
-	if (likely(!bt))
-		return;
-
-	__blk_add_trace(bt, bio->bi_sector, bio->bi_size, bio->bi_rw, what, !bio_flagged(bio, BIO_UPTODATE), 0, NULL);
-}
-
-/**
- * blk_add_trace_generic - Add a trace for a generic action
- * @q:		queue the io is for
- * @bio:	the source bio
- * @rw:		the data direction
- * @what:	the action
- *
- * Description:
- *     Records a simple trace
- *
- **/
-static inline void blk_add_trace_generic(struct request_queue *q,
-					 struct bio *bio, int rw, u32 what)
-{
-	struct blk_trace *bt = q->blk_trace;
-
-	if (likely(!bt))
-		return;
-
-	if (bio)
-		blk_add_trace_bio(q, bio, what);
-	else
-		__blk_add_trace(bt, 0, 0, rw, what, 0, 0, NULL);
-}
-
-/**
- * blk_add_trace_pdu_int - Add a trace for a bio with an integer payload
- * @q:		queue the io is for
- * @what:	the action
- * @bio:	the source bio
- * @pdu:	the integer payload
- *
- * Description:
- *     Adds a trace with some integer payload. This might be an unplug
- *     option given as the action, with the depth at unplug time given
- *     as the payload
- *
- **/
-static inline void blk_add_trace_pdu_int(struct request_queue *q, u32 what,
-					 struct bio *bio, unsigned int pdu)
-{
-	struct blk_trace *bt = q->blk_trace;
-	__be64 rpdu = cpu_to_be64(pdu);
-
-	if (likely(!bt))
-		return;
-
-	if (bio)
-		__blk_add_trace(bt, bio->bi_sector, bio->bi_size, bio->bi_rw, what, !bio_flagged(bio, BIO_UPTODATE), sizeof(rpdu), &rpdu);
-	else
-		__blk_add_trace(bt, 0, 0, 0, what, 0, sizeof(rpdu), &rpdu);
-}
-
-/**
- * blk_add_trace_remap - Add a trace for a remap operation
- * @q:		queue the io is for
- * @bio:	the source bio
- * @dev:	target device
- * @from:	source sector
- * @to:		target sector
- *
- * Description:
- *     Device mapper or raid target sometimes need to split a bio because
- *     it spans a stripe (or similar). Add a trace for that action.
- *
- **/
-static inline void blk_add_trace_remap(struct request_queue *q, struct bio *bio,
-				       dev_t dev, sector_t from, sector_t to)
-{
-	struct blk_trace *bt = q->blk_trace;
-	struct blk_io_trace_remap r;
-
-	if (likely(!bt))
-		return;
-
-	r.device = cpu_to_be32(dev);
-	r.device_from = cpu_to_be32(bio->bi_bdev->bd_dev);
-	r.sector = cpu_to_be64(to);
-
-	__blk_add_trace(bt, from, bio->bi_size, bio->bi_rw, BLK_TA_REMAP, !bio_flagged(bio, BIO_UPTODATE), sizeof(r), &r);
-}
-
 #else /* !CONFIG_BLK_DEV_IO_TRACE */
 #define blk_trace_ioctl(bdev, cmd, arg)		(-ENOTTY)
 #define blk_trace_shutdown(q)			do { } while (0)
-#define blk_add_trace_rq(q, rq, what)		do { } while (0)
-#define blk_add_trace_bio(q, rq, what)		do { } while (0)
-#define blk_add_trace_generic(q, rq, rw, what)	do { } while (0)
-#define blk_add_trace_pdu_int(q, what, bio, pdu)	do { } while (0)
-#define blk_add_trace_remap(q, bio, dev, f, t)	do {} while (0)
 #endif /* CONFIG_BLK_DEV_IO_TRACE */
 
 #endif
Index: linux-2.6-lttng/mm/bounce.c
===================================================================
--- linux-2.6-lttng.orig/mm/bounce.c	2007-09-04 11:52:18.000000000 -0400
+++ linux-2.6-lttng/mm/bounce.c	2007-09-04 12:17:25.000000000 -0400
@@ -13,7 +13,7 @@
 #include <linux/init.h>
 #include <linux/hash.h>
 #include <linux/highmem.h>
-#include <linux/blktrace_api.h>
+#include <linux/marker.h>
 #include <asm/tlbflush.h>
 
 #define POOL_SIZE	64
@@ -237,7 +237,7 @@ static void __blk_queue_bounce(struct re
 	if (!bio)
 		return;
 
-	blk_add_trace_bio(q, *bio_orig, BLK_TA_BOUNCE);
+	trace_mark(blk_bio_bounce, "%p %p", q, *bio_orig);
 
 	/*
 	 * at least one page was bounced, fill in possible non-highmem
Index: linux-2.6-lttng/mm/highmem.c
===================================================================
--- linux-2.6-lttng.orig/mm/highmem.c	2007-09-04 11:50:21.000000000 -0400
+++ linux-2.6-lttng/mm/highmem.c	2007-09-04 12:17:25.000000000 -0400
@@ -26,7 +26,7 @@
 #include <linux/init.h>
 #include <linux/hash.h>
 #include <linux/highmem.h>
-#include <linux/blktrace_api.h>
+#include <linux/marker.h>
 #include <asm/tlbflush.h>
 
 /*
Index: linux-2.6-lttng/fs/bio.c
===================================================================
--- linux-2.6-lttng.orig/fs/bio.c	2007-09-04 11:52:17.000000000 -0400
+++ linux-2.6-lttng/fs/bio.c	2007-09-04 12:17:25.000000000 -0400
@@ -25,7 +25,7 @@
 #include <linux/module.h>
 #include <linux/mempool.h>
 #include <linux/workqueue.h>
-#include <linux/blktrace_api.h>
+#include <linux/marker.h>
 #include <scsi/sg.h>		/* for struct sg_iovec */
 
 #define BIO_POOL_SIZE 2
@@ -1072,8 +1072,8 @@ struct bio_pair *bio_split(struct bio *b
 	if (!bp)
 		return bp;
 
-	blk_add_trace_pdu_int(bdev_get_queue(bi->bi_bdev), BLK_TA_SPLIT, bi,
-				bi->bi_sector + first_sectors);
+	trace_mark(blk_pdu_split, "%p %p %llu", bdev_get_queue(bi->bi_bdev), bi,
+				(u64)bi->bi_sector + first_sectors);
 
 	BUG_ON(bi->bi_vcnt != 1);
 	BUG_ON(bi->bi_idx != 0);
Index: linux-2.6-lttng/drivers/block/cciss.c
===================================================================
--- linux-2.6-lttng.orig/drivers/block/cciss.c	2007-09-04 11:53:23.000000000 -0400
+++ linux-2.6-lttng/drivers/block/cciss.c	2007-09-04 12:17:25.000000000 -0400
@@ -37,7 +37,7 @@
 #include <linux/hdreg.h>
 #include <linux/spinlock.h>
 #include <linux/compat.h>
-#include <linux/blktrace_api.h>
+#include <linux/marker.h>
 #include <asm/uaccess.h>
 #include <asm/io.h>
 
@@ -2545,7 +2545,7 @@ after_error_processing:
 	}
 	cmd->rq->data_len = 0;
 	cmd->rq->completion_data = cmd;
-	blk_add_trace_rq(cmd->rq->q, cmd->rq, BLK_TA_COMPLETE);
+	trace_mark(blk_request_complete, "%p %p", cmd->rq->q, cmd->rq);
 	blk_complete_request(cmd->rq);
 }
 
Index: linux-2.6-lttng/drivers/md/dm.c
===================================================================
--- linux-2.6-lttng.orig/drivers/md/dm.c	2007-09-04 11:52:17.000000000 -0400
+++ linux-2.6-lttng/drivers/md/dm.c	2007-09-04 12:17:25.000000000 -0400
@@ -19,7 +19,7 @@
 #include <linux/slab.h>
 #include <linux/idr.h>
 #include <linux/hdreg.h>
-#include <linux/blktrace_api.h>
+#include <linux/marker.h>
 #include <linux/smp_lock.h>
 
 #define DM_MSG_PREFIX "core"
@@ -481,8 +481,8 @@ static void dec_pending(struct dm_io *io
 			wake_up(&io->md->wait);
 
 		if (io->error != DM_ENDIO_REQUEUE) {
-			blk_add_trace_bio(io->md->queue, io->bio,
-					  BLK_TA_COMPLETE);
+			trace_mark(blk_request_complete, "%p %p",
+				io->md->queue, io->bio);
 
 			bio_endio(io->bio, io->bio->bi_size, io->error);
 		}
@@ -578,10 +578,10 @@ static void __map_bio(struct dm_target *
 	r = ti->type->map(ti, clone, &tio->info);
 	if (r == DM_MAPIO_REMAPPED) {
 		/* the bio has been remapped so dispatch it */
-
-		blk_add_trace_remap(bdev_get_queue(clone->bi_bdev), clone,
-				    tio->io->bio->bi_bdev->bd_dev,
-				    clone->bi_sector, sector);
+		trace_mark(blk_remap, "%p %p %llu %llu %llu",
+			bdev_get_queue(clone->bi_bdev), clone,
+			(u64)tio->io->bio->bi_bdev->bd_dev, (u64)sector,
+			(u64)clone->bi_sector);
 
 		generic_make_request(clone);
 	} else if (r < 0 || r == DM_MAPIO_REQUEUE) {

-- 
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 6/6] Linux Kernel Markers - Port SPU to markers

[Mathieu Desnoyers]

[-- Attachment #1: linux-kernel-markers-port-spu-to-markers.patch --]
[-- Type: text/plain, Size: 13606 bytes --]

Andrew, any chance to get this into -mm ASAP so we can have it in
2.6.24?

Just in case anyone wonders what this is usefulfor I've ported my
hacking spu tracing code to it, and if markers get in I'd push a cleaned
up version of this in the tree of the benefit of everyone trying to
follow what's going on in the spufs code.  Similarly I'd like to port
the xfs tracing code over to it which is very helpful in trying to
debug filesystem issues.

Note that in this patch the actual logging code is rather nasty
hand-crafted code lifted from the tcp probe in tree which would benefit
of going away in favour of more general tracing code aswell.

Changelog:
- Porting to 2.6.23-rc4-mm1 gave a reject (a trace point location disappeared)
  from spufs_ps_nopfn() : spufs_ps_nopfn__sleep and spufs_ps_nopfn__wake.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
---
 arch/powerpc/platforms/cell/spufs/Makefile   |    2 
 arch/powerpc/platforms/cell/spufs/file.c     |    3 
 arch/powerpc/platforms/cell/spufs/sched.c    |   29 ++-
 arch/powerpc/platforms/cell/spufs/spufs.h    |    5 
 arch/powerpc/platforms/cell/spufs/sputrace.c |  246 +++++++++++++++++++++++++++
 5 files changed, 279 insertions(+), 6 deletions(-)

Index: linux-2.6-lttng/arch/powerpc/platforms/cell/spufs/Makefile
===================================================================
--- linux-2.6-lttng.orig/arch/powerpc/platforms/cell/spufs/Makefile	2007-09-04 11:49:31.000000000 -0400
+++ linux-2.6-lttng/arch/powerpc/platforms/cell/spufs/Makefile	2007-09-04 14:53:32.000000000 -0400
@@ -4,6 +4,8 @@ obj-$(CONFIG_SPU_FS) += spufs.o
 spufs-y += inode.o file.o context.o syscalls.o coredump.o
 spufs-y += sched.o backing_ops.o hw_ops.o run.o gang.o
 
+obj-m += sputrace.o
+
 # Rules to build switch.o with the help of SPU tool chain
 SPU_CROSS	:= spu-
 SPU_CC		:= $(SPU_CROSS)gcc
Index: linux-2.6-lttng/arch/powerpc/platforms/cell/spufs/sched.c
===================================================================
--- linux-2.6-lttng.orig/arch/powerpc/platforms/cell/spufs/sched.c	2007-09-04 11:49:31.000000000 -0400
+++ linux-2.6-lttng/arch/powerpc/platforms/cell/spufs/sched.c	2007-09-04 14:53:32.000000000 -0400
@@ -39,6 +39,7 @@
 #include <linux/pid_namespace.h>
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
+#include <linux/marker.h>
 
 #include <asm/io.h>
 #include <asm/mmu_context.h>
@@ -224,8 +225,8 @@ EXPORT_SYMBOL_GPL(spu_switch_event_unreg
  */
 static void spu_bind_context(struct spu *spu, struct spu_context *ctx)
 {
-	pr_debug("%s: pid=%d SPU=%d NODE=%d\n", __FUNCTION__, current->pid,
-		 spu->number, spu->node);
+	spu_context_trace(spu_bind_context__enter, ctx, spu);
+
 	spuctx_switch_state(ctx, SPU_UTIL_SYSTEM);
 
 	if (ctx->flags & SPU_CREATE_NOSCHED)
@@ -412,8 +413,8 @@ static int has_affinity(struct spu_conte
  */
 static void spu_unbind_context(struct spu *spu, struct spu_context *ctx)
 {
-	pr_debug("%s: unbind pid=%d SPU=%d NODE=%d\n", __FUNCTION__,
-		 spu->pid, spu->number, spu->node);
+	spu_context_trace(spu_unbind_context__enter, ctx, spu);
+
 	spuctx_switch_state(ctx, SPU_UTIL_SYSTEM);
 
  	if (spu->ctx->flags & SPU_CREATE_NOSCHED)
@@ -514,6 +515,8 @@ static struct spu *spu_get_idle(struct s
 	struct spu *spu;
 	int node, n;
 
+	spu_context_nospu_trace(spu_get_idle__enter, ctx);
+
 	if (has_affinity(ctx)) {
 		node = ctx->gang->aff_ref_spu->node;
 
@@ -522,7 +525,7 @@ static struct spu *spu_get_idle(struct s
 		if (spu && spu->alloc_state == SPU_FREE)
 			goto found;
 		mutex_unlock(&cbe_spu_info[node].list_mutex);
-		return NULL;
+		goto not_found;
 	}
 
 	node = cpu_to_node(raw_smp_processor_id());
@@ -539,12 +542,14 @@ static struct spu *spu_get_idle(struct s
 		mutex_unlock(&cbe_spu_info[node].list_mutex);
 	}
 
+ not_found:
+	spu_context_nospu_trace(spu_get_idle__not_found, ctx);
 	return NULL;
 
  found:
 	spu->alloc_state = SPU_USED;
 	mutex_unlock(&cbe_spu_info[node].list_mutex);
-	pr_debug("Got SPU %d %d\n", spu->number, spu->node);
+	spu_context_trace(spu_get_idle__found, ctx, spu);
 	spu_init_channels(spu);
 	return spu;
 }
@@ -561,6 +566,8 @@ static struct spu *find_victim(struct sp
 	struct spu *spu;
 	int node, n;
 
+	spu_context_nospu_trace(spu_find_vitim__enter, ctx);
+
 	/*
 	 * Look for a possible preemption candidate on the local node first.
 	 * If there is no candidate look at the other nodes.  This isn't
@@ -717,6 +724,8 @@ static int __spu_deactivate(struct spu_c
 		if (new || force) {
 			int node = spu->node;
 
+			spu_context_trace(__spu_deactivate__unload, ctx, spu);
+
 			mutex_lock(&cbe_spu_info[node].list_mutex);
 			spu_unbind_context(spu, ctx);
 			spu->alloc_state = SPU_FREE;
@@ -744,6 +753,7 @@ static int __spu_deactivate(struct spu_c
  */
 void spu_deactivate(struct spu_context *ctx)
 {
+	spu_context_nospu_trace(spu_deactivate__enter, ctx);
 	__spu_deactivate(ctx, 1, MAX_PRIO);
 }
 
@@ -757,6 +767,7 @@ void spu_deactivate(struct spu_context *
  */
 void spu_yield(struct spu_context *ctx)
 {
+	spu_context_nospu_trace(spu_yield__enter, ctx);
 	if (!(ctx->flags & SPU_CREATE_NOSCHED)) {
 		mutex_lock(&ctx->state_mutex);
 		__spu_deactivate(ctx, 0, MAX_PRIO);
@@ -783,6 +794,8 @@ static noinline void spusched_tick(struc
 		struct spu *spu = ctx->spu;
 		struct spu_context *new;
 
+		spu_context_trace(spusched_tick__preempt, ctx, spu);
+
 		new = grab_runnable_context(ctx->prio + 1, spu->node);
 		if (new) {
 			spu_unbind_context(spu, ctx);
@@ -797,10 +810,14 @@ static noinline void spusched_tick(struc
 			 * gets put on the runqueue again ASAP.
 			 */
 			wake_up(&ctx->stop_wq);
+		} else {
+			spu_context_trace(spusched_tick__preempt_failed,
+						ctx, spu);
 		}
 		spu_set_timeslice(ctx);
 		mutex_unlock(&ctx->state_mutex);
 	} else {
+		spu_context_nospu_trace(spusched_tick__newslice, ctx);
 		ctx->time_slice++;
 	}
 }
Index: linux-2.6-lttng/arch/powerpc/platforms/cell/spufs/sputrace.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-2.6-lttng/arch/powerpc/platforms/cell/spufs/sputrace.c	2007-09-04 14:53:32.000000000 -0400
@@ -0,0 +1,246 @@
+/*
+ * Copyright (C) 2007 IBM Deutschland Entwicklung GmbH
+ *	Released under GPL v2.
+ *
+ * Partially based on net/ipv4/tcp_probe.c.
+ *
+ * Simple tracing facility for spu contexts.
+ */
+#include <linux/sched.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/marker.h>
+#include <linux/proc_fs.h>
+#include <linux/wait.h>
+#include <asm/atomic.h>
+#include <asm/uaccess.h>
+#include "spufs.h"
+
+struct spu_probe {
+	const char *name;
+	const char *format;
+	marker_probe_func *probe_func;
+};
+
+struct sputrace {
+	ktime_t tstamp;
+	struct spu_context *ctx;
+	const char *name;
+	int tid;
+	int number;
+};
+
+static int bufsize __read_mostly = 4096;
+
+
+static DEFINE_SPINLOCK(sputrace_lock);
+static DECLARE_WAIT_QUEUE_HEAD(sputrace_wait);
+static ktime_t sputrace_start;
+static unsigned long sputrace_head, sputrace_tail;
+static struct sputrace *sputrace_log;
+
+static int sputrace_used(void)
+{
+	return (sputrace_head - sputrace_tail) % bufsize;
+}
+
+static inline int sputrace_avail(void)
+{
+	return bufsize - sputrace_used();
+}
+
+static int sputrace_sprint(char *tbuf, int n)
+{
+        const struct sputrace *t = sputrace_log + sputrace_tail % bufsize;
+	struct timespec tv =
+		ktime_to_timespec(ktime_sub(t->tstamp, sputrace_start));
+
+	return snprintf(tbuf, n,
+		"[%lu.%09lu] %p: %s (tid = %d, spu = %d)\n",
+		(unsigned long) tv.tv_sec,
+		(unsigned long) tv.tv_nsec,
+		t->ctx,
+		t->name,
+		t->tid,
+		t->number);
+}
+
+static ssize_t sputrace_read(struct file *file, char __user *buf,
+		size_t len, loff_t *ppos)
+{
+	int error = 0, cnt = 0;
+
+	if (!buf || len < 0)
+		return -EINVAL;
+
+	while (cnt < len) {
+		char tbuf[128];
+		int width;
+
+		error = wait_event_interruptible(sputrace_wait,
+						 sputrace_used() > 0);
+		if (error)
+			break;
+
+		spin_lock(&sputrace_lock);
+		if (sputrace_head == sputrace_tail) {
+			spin_unlock(&sputrace_lock);
+			continue;
+		}
+
+		width = sputrace_sprint(tbuf, sizeof(tbuf));
+		if (width < len)
+			sputrace_tail = (sputrace_tail + 1) % bufsize;
+		spin_unlock(&sputrace_lock);
+
+		if (width >= len)
+			break;
+
+		error = copy_to_user(buf + cnt, tbuf, width);
+		if (error)
+			break;
+		cnt += width;
+	}
+
+	return cnt == 0 ? error : cnt;
+}
+
+static int tcpprobe_open(struct inode * inode, struct file * file)
+{
+	spin_lock(&sputrace_lock);
+	sputrace_head = sputrace_tail = 0;
+	sputrace_start = ktime_get();
+	spin_unlock(&sputrace_lock);
+
+	return 0;
+}
+
+static const struct file_operations sputrace_fops = {
+	.owner	= THIS_MODULE,
+	.open	= tcpprobe_open,
+	.read	= sputrace_read,
+};
+
+static void sputrace_log_item(const char *name, struct spu_context *ctx,
+		struct spu *spu)
+{
+	spin_lock(&sputrace_lock);
+	if (sputrace_avail() > 1) {
+		struct sputrace *t = sputrace_log + sputrace_head;
+
+		t->tstamp = ktime_get();
+		t->ctx = ctx;
+		t->name = name;
+		t->tid = ctx->tid;
+		t->number = spu ? spu->number : -1;
+
+		sputrace_head = (sputrace_head + 1) % bufsize;
+	} else {
+		printk(KERN_WARNING
+		       "sputrace: lost samples due to full buffer.\n");
+	}
+	spin_unlock(&sputrace_lock);
+
+	wake_up(&sputrace_wait);
+}
+
+static void spu_context_event(const struct __mark_marker *mdata,
+		void *private, const char *format, ...)
+{
+	struct spu_probe *p = mdata->pdata;
+	va_list ap;
+	struct spu_context *ctx;
+	struct spu *spu;
+
+	va_start(ap, format);
+	ctx = va_arg(ap, struct spu_context *);
+	spu = va_arg(ap, struct spu *);
+
+	sputrace_log_item(p->name, ctx, spu);
+	va_end(ap);
+}
+
+static void spu_context_nospu_event(const struct __mark_marker *mdata,
+		void *private, const char *format, ...)
+{
+	struct spu_probe *p = mdata->pdata;
+	va_list ap;
+	struct spu_context *ctx;
+
+	va_start(ap, format);
+	ctx = va_arg(ap, struct spu_context *);
+
+	sputrace_log_item(p->name, ctx, NULL);
+	va_end(ap);
+}
+
+struct spu_probe spu_probes[] = {
+	{ "spu_bind_context__enter", "%p %p", spu_context_event },
+	{ "spu_unbind_context__enter", "%p %p", spu_context_event },
+	{ "spu_get_idle__enter", "%p", spu_context_nospu_event },
+	{ "spu_get_idle__found", "%p %p", spu_context_event },
+	{ "spu_get_idle__not_found", "%p", spu_context_nospu_event },
+	{ "spu_find_victim__enter", "%p", spu_context_nospu_event },
+	{ "spusched_tick__preempt", "%p %p", spu_context_event },
+	{ "spusched_tick__preempt_failed", "%p %p", spu_context_event },
+	{ "spusched_tick__newslice", "%p", spu_context_nospu_event },
+	{ "spu_yield__enter", "%p", spu_context_nospu_event },
+	{ "spu_deactivate__enter", "%p", spu_context_nospu_event },
+	{ "__spu_deactivate__unload", "%p %p", spu_context_event },
+	{ "spufs_ps_nopfn__enter", "%p", spu_context_nospu_event },
+	{ "spufs_ps_nopfn__sleep", "%p", spu_context_nospu_event },
+	{ "spufs_ps_nopfn__wake", "%p", spu_context_nospu_event },
+};
+
+static int __init sputrace_init(void)
+{
+	struct proc_dir_entry *entry;
+	int error = -ENOMEM, i;
+
+	sputrace_log = kcalloc(sizeof(struct sputrace),
+				bufsize, GFP_KERNEL);
+	if (!sputrace_log)
+		goto out;
+
+	entry = create_proc_entry("sputrace", S_IRUSR, NULL);
+	if (!entry)
+		goto out_free_log;
+	entry->proc_fops = &sputrace_fops;
+
+	for (i = 0; i < ARRAY_SIZE(spu_probes); i++) {
+		struct spu_probe *p = &spu_probes[i];
+
+		error = marker_probe_register(p->name, p->format,
+					      p->probe_func, p);
+		if (error)
+			printk(KERN_INFO "Unable to register probe %s\n",
+					p->name);
+
+		error = marker_arm(p->name);
+		if (error)
+			printk(KERN_INFO "Unable to arm probe %s\n", p->name);
+	}
+
+	return 0;
+
+ out_free_log:
+ 	kfree(sputrace_log);
+ out:
+ 	return -ENOMEM;
+}
+
+static void __exit sputrace_exit(void)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(spu_probes); i++)
+		marker_probe_unregister(spu_probes[i].name);
+
+ 	remove_proc_entry("sputrace", NULL);
+	kfree(sputrace_log);
+}
+
+module_init(sputrace_init);
+module_exit(sputrace_exit);
+
+MODULE_LICENSE("GPL");
Index: linux-2.6-lttng/arch/powerpc/platforms/cell/spufs/file.c
===================================================================
--- linux-2.6-lttng.orig/arch/powerpc/platforms/cell/spufs/file.c	2007-09-04 11:49:31.000000000 -0400
+++ linux-2.6-lttng/arch/powerpc/platforms/cell/spufs/file.c	2007-09-04 14:53:32.000000000 -0400
@@ -29,6 +29,7 @@
 #include <linux/poll.h>
 #include <linux/ptrace.h>
 #include <linux/seq_file.h>
+#include <linux/marker.h>
 
 #include <asm/io.h>
 #include <asm/semaphore.h>
@@ -238,6 +239,8 @@ static unsigned long spufs_ps_nopfn(stru
 	unsigned long area, offset = address - vma->vm_start;
 	int ret;
 
+	spu_context_nospu_trace(spufs_ps_nopfn__enter, ctx);
+
 	offset += vma->vm_pgoff << PAGE_SHIFT;
 	if (offset >= ps_size)
 		return NOPFN_SIGBUS;
Index: linux-2.6-lttng/arch/powerpc/platforms/cell/spufs/spufs.h
===================================================================
--- linux-2.6-lttng.orig/arch/powerpc/platforms/cell/spufs/spufs.h	2007-09-04 11:49:31.000000000 -0400
+++ linux-2.6-lttng/arch/powerpc/platforms/cell/spufs/spufs.h	2007-09-04 14:53:32.000000000 -0400
@@ -338,4 +338,9 @@ static inline void spuctx_switch_state(s
 	}
 }
 
+#define spu_context_trace(name, ctx, spu) \
+	trace_mark(name, "%p %p", ctx, spu);
+#define spu_context_nospu_trace(name, ctx) \
+	trace_mark(name, "%p", ctx);
+
 #endif

-- 
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 6/6] Linux Kernel Markers - Port SPU to markers

[Alexey Dobriyan]

On Thu, Sep 06, 2007 at 04:07:39PM -0400, Mathieu Desnoyers wrote:
> Andrew, any chance to get this into -mm ASAP so we can have it in
> 2.6.24?
> 
> Just in case anyone wonders what this is usefulfor I've ported my
> hacking spu tracing code to it, and if markers get in I'd push a cleaned
> up version of this in the tree of the benefit of everyone trying to
> follow what's going on in the spufs code.  Similarly I'd like to port
> the xfs tracing code over to it which is very helpful in trying to
> debug filesystem issues.
> 
> Note that in this patch the actual logging code is rather nasty
> hand-crafted code lifted from the tcp probe in tree which would benefit
> of going away in favour of more general tracing code aswell.
> 
> Changelog:
> - Porting to 2.6.23-rc4-mm1 gave a reject (a trace point location disappeared)
>   from spufs_ps_nopfn() : spufs_ps_nopfn__sleep and spufs_ps_nopfn__wake.
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>

Where are Christoph's attributions?

Re: [patch 6/6] Linux Kernel Markers - Port SPU to markers

[Mathieu Desnoyers]

* Alexey Dobriyan (adobriyan@gmail.com) wrote:
> On Thu, Sep 06, 2007 at 04:07:39PM -0400, Mathieu Desnoyers wrote:
> > Andrew, any chance to get this into -mm ASAP so we can have it in
> > 2.6.24?
> > 
> > Just in case anyone wonders what this is usefulfor I've ported my
> > hacking spu tracing code to it, and if markers get in I'd push a cleaned
> > up version of this in the tree of the benefit of everyone trying to
> > follow what's going on in the spufs code.  Similarly I'd like to port
> > the xfs tracing code over to it which is very helpful in trying to
> > debug filesystem issues.
> > 
> > Note that in this patch the actual logging code is rather nasty
> > hand-crafted code lifted from the tcp probe in tree which would benefit
> > of going away in favour of more general tracing code aswell.
> > 
> > Changelog:
> > - Porting to 2.6.23-rc4-mm1 gave a reject (a trace point location disappeared)
> >   from spufs_ps_nopfn() : spufs_ps_nopfn__sleep and spufs_ps_nopfn__wake.
> > Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> 
> Where are Christoph's attributions?

I realized only after sending this patch set that the 

From: Christoph Hellwig <hch@infradead.org>

Has somehow been stripped off from the patch header.

-- 
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 6/6] Linux Kernel Markers - Port SPU to markers

[Christoph Hellwig]

On Thu, Sep 06, 2007 at 04:07:39PM -0400, Mathieu Desnoyers wrote:
> Andrew, any chance to get this into -mm ASAP so we can have it in
> 2.6.24?
> 
> Just in case anyone wonders what this is usefulfor I've ported my
> hacking spu tracing code to it, and if markers get in I'd push a cleaned
> up version of this in the tree of the benefit of everyone trying to
> follow what's going on in the spufs code.  Similarly I'd like to port
> the xfs tracing code over to it which is very helpful in trying to
> debug filesystem issues.
> 
> Note that in this patch the actual logging code is rather nasty
> hand-crafted code lifted from the tcp probe in tree which would benefit
> of going away in favour of more general tracing code aswell.
> 
> Changelog:
> - Porting to 2.6.23-rc4-mm1 gave a reject (a trace point location disappeared)
>   from spufs_ps_nopfn() : spufs_ps_nopfn__sleep and spufs_ps_nopfn__wake.

The subject is rather confusing.  What I ported to markers is the spu _tracing_
support.  And given it's never been public before adding the port part to the
commit doesn't make much sense.

Nevermind, I have updated version of this and would prefer to submit it myself
through the spufs maintainer once markers make mainline (hopefully soon!)

I'll also need more cleanups like adding a config option for it.