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.
- Iteration on modules is now done in module.c.
- module_mutex is not exported anymore.
Signed-off-by: Mathieu Desnoyers <[email protected]>
Acked-by: "Frank Ch. Eigler" <[email protected]>
CC: Christoph Hellwig <[email protected]>
CC: Rusty Russell <[email protected]>
---
include/asm-generic/vmlinux.lds.h | 11
include/linux/marker.h | 175 ++++++++++
include/linux/module.h | 18 +
kernel/marker.c | 608 ++++++++++++++++++++++++++++++++++++++
kernel/module.c | 66 ++++
5 files changed, 875 insertions(+), 3 deletions(-)
Index: linux-2.6-lttng/include/asm-generic/vmlinux.lds.h
===================================================================
--- linux-2.6-lttng.orig/include/asm-generic/vmlinux.lds.h 2007-09-14 10:11:18.000000000 -0400
+++ linux-2.6-lttng/include/asm-generic/vmlinux.lds.h 2007-09-14 10:11:31.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-17 12:43:54.000000000 -0400
@@ -0,0 +1,175 @@
+#ifndef _LINUX_MARKER_H
+#define _LINUX_MARKER_H
+
+/*
+ * Code markup for dynamic and static tracing.
+ *
+ * See Documentation/marker.txt.
+ *
+ * (C) Copyright 2006 Mathieu Desnoyers <[email protected]>
+ *
+ * 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".
+ */
+ DEFINE_IMMEDIATE(char, 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) { \
+ if (unlikely(immediate_read(__mark_##name.state))) { \
+ preempt_disable(); \
+ (*__mark_##name.call) \
+ (&__mark_##name, call_data, \
+ format, ## args); \
+ preempt_enable(); \
+ } \
+ } else { \
+ if (unlikely(_immediate_read(__mark_##name.state))) { \
+ preempt_disable(); \
+ (*__mark_##name.call) \
+ (&__mark_##name, call_data, \
+ format, ## args); \
+ preempt_enable(); \
+ } \
+ } \
+ } while (0)
+
+extern void marker_update_probe_range(struct __mark_marker *begin,
+ struct __mark_marker *end, struct module *probe_module, int *refcount);
+#else /* !CONFIG_MARKERS */
+#define __trace_mark(generic, name, call_data, format, args...) \
+ __mark_check_format(format, ## args)
+static inline void marker_update_probe_range(struct __mark_marker *begin,
+ struct __mark_marker *end, struct module *probe_module, int *refcount)
+{ }
+#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_read())
+ * 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_read()) 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 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);
+
+/*
+ * 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);
+extern int marker_get_iter_range(struct __mark_marker **marker,
+ struct __mark_marker *begin,
+ struct __mark_marker *end);
+
+#endif
Index: linux-2.6-lttng/include/linux/module.h
===================================================================
--- linux-2.6-lttng.orig/include/linux/module.h 2007-09-14 10:11:18.000000000 -0400
+++ linux-2.6-lttng/include/linux/module.h 2007-09-14 10:11:31.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>
@@ -376,6 +377,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 {}
@@ -482,6 +487,9 @@ extern void print_modules(void);
extern void _module_immediate_update(void);
extern void module_immediate_update(void);
+extern void module_update_markers(struct module *probe_module, int *refcount);
+extern int module_get_iter_markers(struct marker_iter *iter);
+
#else /* !CONFIG_MODULES... */
#define EXPORT_SYMBOL(sym)
#define EXPORT_SYMBOL_GPL(sym)
@@ -589,6 +597,16 @@ static inline void module_immediate_upda
{
}
+static inline void module_update_markers(struct module *probe_module,
+ int *refcount)
+{
+}
+
+static inline int module_get_iter_markers(struct marker_iter *iter)
+{
+ return 0;
+}
+
#endif /* CONFIG_MODULES */
struct device_driver;
Index: linux-2.6-lttng/kernel/module.c
===================================================================
--- linux-2.6-lttng.orig/kernel/module.c 2007-09-14 10:11:30.000000000 -0400
+++ linux-2.6-lttng/kernel/module.c 2007-09-14 10:11:31.000000000 -0400
@@ -1720,6 +1720,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 */
@@ -1972,6 +1974,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)
@@ -2013,6 +2017,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);
@@ -2037,12 +2046,16 @@ static struct module *load_module(void _
goto nomodsectinfo;
#endif
+ if (!mod->taints) {
#ifdef CONFIG_IMMEDIATE
- if (!mod->taints)
immediate_update_range(mod->immediate,
mod->immediate + mod->num_immediate);
#endif
-
+#ifdef CONFIG_MARKERS
+ marker_update_probe_range(mod->markers,
+ mod->markers + mod->num_markers, NULL, NULL);
+#endif
+ }
err = module_finalize(hdr, sechdrs, mod);
if (err < 0)
goto cleanup;
@@ -2693,3 +2706,52 @@ void module_immediate_update(void)
}
EXPORT_SYMBOL_GPL(module_immediate_update);
#endif
+
+#ifdef CONFIG_MARKERS
+void module_update_markers(struct module *probe_module, int *refcount)
+{
+ struct module *mod;
+
+ mutex_lock(&module_mutex);
+ list_for_each_entry(mod, &modules, list)
+ if (!mod->taints)
+ marker_update_probe_range(mod->markers,
+ mod->markers + mod->num_markers,
+ probe_module, refcount);
+ mutex_unlock(&module_mutex);
+}
+EXPORT_SYMBOL_GPL(module_update_markers);
+
+/*
+ * Returns 0 if current not found.
+ * Returns 1 if current found.
+ */
+int module_get_iter_markers(struct marker_iter *iter)
+{
+ struct module *iter_mod;
+ int found = 0;
+
+ mutex_lock(&module_mutex);
+ 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;
+ found = marker_get_iter_range(&iter->marker,
+ iter_mod->markers,
+ iter_mod->markers + iter_mod->num_markers);
+ if (found) {
+ iter->module = iter_mod;
+ break;
+ }
+ }
+ }
+ mutex_unlock(&module_mutex);
+ return found;
+}
+EXPORT_SYMBOL_GPL(module_get_iter_markers);
+#endif
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-14 10:11:31.000000000 -0400
@@ -0,0 +1,608 @@
+/*
+ * 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[];
+
+/*
+ * module_mutex nests inside markers_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
+ * given module do we call synchronize_sched()). However, we need to make sure
+ * every critical region has 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 make 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(). These are never used until
+ * the next initialization anyway.
+ */
+}
+
+/**
+ * marker_update_probe_range - Update a probe range
+ * @begin: beginning of the range
+ * @end: end of the range
+ * @probe_module: module address of the probe being updated
+ * @refcount: number of references left to the given probe_module (out)
+ *
+ * Updates the probe callback corresponding to a range of markers.
+ * Must be called with markers_mutex held.
+ */
+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);
+ }
+ }
+}
+EXPORT_SYMBOL_GPL(marker_update_probe_range);
+
+/*
+ * 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;
+
+ mutex_lock(&markers_mutex);
+ /* Core kernel markers */
+ marker_update_probe_range(__start___markers,
+ __stop___markers, probe_module, &refcount);
+ /* Markers in modules. */
+ module_update_markers(probe_module, &refcount);
+ if (probe_module && refcount == 0) {
+ synchronize_sched();
+ deferred_sync = 0;
+ }
+ mutex_unlock(&markers_mutex);
+}
+
+/**
+ * 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_pdata - 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);
+
+/**
+ * marker_get_iter_range - Get a next marker iterator given a range.
+ * @marker: current markers (in), next marker (out)
+ * @begin: beginning of the range
+ * @end: end of the range
+ *
+ * Returns whether a next marker has been found (1) or not (0).
+ * Will return the first marker in the range if the input marker is NULL.
+ */
+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;
+}
+EXPORT_SYMBOL_GPL(marker_get_iter_range);
+
+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 = module_get_iter_markers(iter);
+end:
+ if (!found)
+ marker_iter_reset(iter);
+}
+
+void marker_iter_start(struct marker_iter *iter)
+{
+ 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);
+}
+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
* Mathieu Desnoyers ([email protected]) wrote:
> 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.
> - Iteration on modules is now done in module.c.
> - module_mutex is not exported anymore.
>
[...]
> Index: linux-2.6-lttng/include/asm-generic/vmlinux.lds.h
> ===================================================================
> --- linux-2.6-lttng.orig/include/asm-generic/vmlinux.lds.h 2007-09-14 10:11:18.000000000 -0400
> +++ linux-2.6-lttng/include/asm-generic/vmlinux.lds.h 2007-09-14 10:11:31.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) \
[...]
Do you think I should also remove the __markers_strings section from here ?
--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
On Wednesday 19 September 2007 12:37, Mathieu Desnoyers wrote:
> > Index: linux-2.6-lttng/include/asm-generic/vmlinux.lds.h
> > ===================================================================
> > --- linux-2.6-lttng.orig/include/asm-generic/vmlinux.lds.h 2007-09-14 10:11:18.000000000 -0400
> > +++ linux-2.6-lttng/include/asm-generic/vmlinux.lds.h 2007-09-14 10:11:31.000000000 -0400
> > @@ -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) \
> [...]
>
> Do you think I should also remove the __markers_strings section from here ?
Yes.
It will be beneficial if one can read include/asm-generic/vmlinux.lds.h and
arch/$ARCH/kernel/vmlinux.lds.S and understand which sections in resulting
vmlinux serve what purpose. A comment atop each section explaining
its role will be nice. Even more so that not many people are fluent
in ld script language.
Currently, one will need to grep around (and not only in kernel tree -
you need to read depmod.c source too) in order to understand the role
of various sections in vmlinux.
There are dearth of comments in ld scripts, and some sections
are created "just because I felt like it". For example, there are
".data.page_aligned" and ".data.percpu" sections - can you
easily tell which one has to be a section, and which does not need
to be one (can be merged with ".data")? Maybe both must be sections?
Or none of them?
--
vda
* Denys Vlasenko ([email protected]) wrote:
> On Wednesday 19 September 2007 12:37, Mathieu Desnoyers wrote:
> > > Index: linux-2.6-lttng/include/asm-generic/vmlinux.lds.h
> > > ===================================================================
> > > --- linux-2.6-lttng.orig/include/asm-generic/vmlinux.lds.h 2007-09-14 10:11:18.000000000 -0400
> > > +++ linux-2.6-lttng/include/asm-generic/vmlinux.lds.h 2007-09-14 10:11:31.000000000 -0400
> > > @@ -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) \
> > [...]
> >
> > Do you think I should also remove the __markers_strings section from here ?
>
> Yes.
>
> It will be beneficial if one can read include/asm-generic/vmlinux.lds.h and
> arch/$ARCH/kernel/vmlinux.lds.S and understand which sections in resulting
> vmlinux serve what purpose. A comment atop each section explaining
> its role will be nice. Even more so that not many people are fluent
> in ld script language.
>
> Currently, one will need to grep around (and not only in kernel tree -
> you need to read depmod.c source too) in order to understand the role
> of various sections in vmlinux.
>
> There are dearth of comments in ld scripts, and some sections
> are created "just because I felt like it". For example, there are
> ".data.page_aligned" and ".data.percpu" sections - can you
> easily tell which one has to be a section, and which does not need
> to be one (can be merged with ".data")? Maybe both must be sections?
> Or none of them?
> --
> vda
Oh, wait.. I need it in module.c:
immediateindex = find_sec(hdr, sechdrs, secstrings, "__immediate");
I'll leave the section there then.
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
* Mathieu Desnoyers ([email protected]) wrote:
> * Denys Vlasenko ([email protected]) wrote:
> > On Wednesday 19 September 2007 12:37, Mathieu Desnoyers wrote:
> > > > Index: linux-2.6-lttng/include/asm-generic/vmlinux.lds.h
> > > > ===================================================================
> > > > --- linux-2.6-lttng.orig/include/asm-generic/vmlinux.lds.h 2007-09-14 10:11:18.000000000 -0400
> > > > +++ linux-2.6-lttng/include/asm-generic/vmlinux.lds.h 2007-09-14 10:11:31.000000000 -0400
> > > > @@ -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) \
> > > [...]
> > >
> > > Do you think I should also remove the __markers_strings section from here ?
> >
> > Yes.
> >
> > It will be beneficial if one can read include/asm-generic/vmlinux.lds.h and
> > arch/$ARCH/kernel/vmlinux.lds.S and understand which sections in resulting
> > vmlinux serve what purpose. A comment atop each section explaining
> > its role will be nice. Even more so that not many people are fluent
> > in ld script language.
> >
> > Currently, one will need to grep around (and not only in kernel tree -
> > you need to read depmod.c source too) in order to understand the role
> > of various sections in vmlinux.
> >
> > There are dearth of comments in ld scripts, and some sections
> > are created "just because I felt like it". For example, there are
> > ".data.page_aligned" and ".data.percpu" sections - can you
> > easily tell which one has to be a section, and which does not need
> > to be one (can be merged with ".data")? Maybe both must be sections?
> > Or none of them?
> > --
> > vda
>
> Oh, wait.. I need it in module.c:
>
> immediateindex = find_sec(hdr, sechdrs, secstrings, "__immediate");
>
> I'll leave the section there then.
>
> Mathieu
Sorry, let me take this back. It applies to what is linked in the core
image, but I believe it does not apply to the kernel modules.
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
Mathieu Desnoyers <[email protected]> writes:
> [...] Do you think I should also remove the __markers_strings
> section from here ?
Current systemtap marker support code relies on the __markers_strings
section.
- FChE
On Wednesday 19 September 2007 14:53, Frank Ch. Eigler wrote:
> Mathieu Desnoyers <[email protected]> writes:
>
> > [...] Do you think I should also remove the __markers_strings
> > section from here ?
>
> Current systemtap marker support code relies on the __markers_strings
> section.
Let users know that in comment above section definition in ld script.
Thanks!
--
vda
On Tue, Sep 18, 2007 at 05:13:25PM -0400, Mathieu Desnoyers wrote:
> +/*
> + * 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);
Can you switch this at least to WARN_ON? Killing a system with X
running where the user just sees a freeze is not that nice. But a nasty
message in dmesg is very noticable.
-- Steve
> +
> + 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;
> +}
* Denys Vlasenko ([email protected]) wrote:
> On Wednesday 19 September 2007 14:53, Frank Ch. Eigler wrote:
> > Mathieu Desnoyers <[email protected]> writes:
> >
> > > [...] Do you think I should also remove the __markers_strings
> > > section from here ?
> >
> > Current systemtap marker support code relies on the __markers_strings
> > section.
>
> Let users know that in comment above section definition in ld script.
>
Ok, I am changing it to:
/* Markers: strings (used by SystemTAP) */ \
__markers_strings : AT(ADDR(__markers_strings) - LOAD_OFFSET) { \
*(__markers_strings) \
} \
Mathieu
> Thanks!
> --
> vda
--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
On Fri, Sep 21, 2007 at 08:58:19AM -0400, Mathieu Desnoyers wrote:
> Ok, I am changing it to:
As I mentioned before pleae just kill this gunk entirely as it's not needed
at all intree. markers are already getting far too complex, I'd rather
want a simple useable version in now than trying to cater for every possible
use-case. Once the systemtap people are ready for merging their stuff into
the kernel tree we can cater towards their needs.
Hi -
On Fri, Sep 21, 2007 at 08:58:19AM -0400, Mathieu Desnoyers wrote:
> [...]
> > > Current systemtap marker support code relies on the __markers_strings
> > > section.
> > Let users know that in comment above section definition in ld script.
> [...]
> /* Markers: strings (used by SystemTAP) */ \
> [...]
I did not mean to imply that this was a necessary state of affairs.
The marker metadata must be stored in at least one place in the kernel
image - this just happens to be a convenient one that David Smith's
recent systemtap code used. Without it, we'd probably have to do a
more complicated search, following the pointers within the __markers
structs. That could work, but it hasn't been built/tested.
So, this proposed change (removal of this section) would break
systemtap, and we have to jump through more hoops to make it work
again. Is the change worth it?
- FChE
* Frank Ch. Eigler ([email protected]) wrote:
> Hi -
>
> On Fri, Sep 21, 2007 at 08:58:19AM -0400, Mathieu Desnoyers wrote:
> > [...]
> > > > Current systemtap marker support code relies on the __markers_strings
> > > > section.
> > > Let users know that in comment above section definition in ld script.
> > [...]
> > /* Markers: strings (used by SystemTAP) */ \
> > [...]
>
> I did not mean to imply that this was a necessary state of affairs.
>
> The marker metadata must be stored in at least one place in the kernel
> image - this just happens to be a convenient one that David Smith's
> recent systemtap code used. Without it, we'd probably have to do a
> more complicated search, following the pointers within the __markers
> structs. That could work, but it hasn't been built/tested.
>
> So, this proposed change (removal of this section) would break
> systemtap, and we have to jump through more hoops to make it work
> again. Is the change worth it?
>
I guess so. Getting the markers as clean as we can is very important for
kernel inclusion.
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
* Steven Rostedt ([email protected]) wrote:
> On Tue, Sep 18, 2007 at 05:13:25PM -0400, Mathieu Desnoyers wrote:
> > +/*
> > + * 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);
>
> Can you switch this at least to WARN_ON? Killing a system with X
> running where the user just sees a freeze is not that nice. But a nasty
> message in dmesg is very noticable.
>
Sure.
> -- Steve
>
> > +
> > + 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;
> > +}
--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
Hi -
I wrote:
> [...]
> > The marker metadata must be stored in at least one place in the kernel
> > image - this just happens to be a convenient one that David Smith's
> > recent systemtap code used. Without it, we'd probably have to do a
> > more complicated search, following the pointers within the __markers
> > structs. [...]
Our team is farther along adapting to this change against 2.6.23-mm1,
and we have run into a complication. It's more of a distribution
issue.
We would prefer to retain systemtap's capability to build
instrumentation for a kernel other than the currently running one.
Such instrumentation can be then copied and run on a distinct machine.
This has meant relying on development data: make install_headers +
Makefiles (as packaged by Fedora/RHEL), and to a lesser extent
separated debugging information.
Markers are attractive partly because they don't require debugging
information, so the data needs to be found in an executable image.
But we prefer not to force the executable image itself to be
installed, for example because /boot is relatively small. So we would
prefer something in between: something small that we can put into the
development package.
If there exists sympathy to this problem, Roland McGrath supposes we
could implement a standardized solution, a file like Module.symvers,
containing the marker names & format strings extracted at build time.
Any opinions?
PS. I wonder why the marker name/format strings are put into a
__markers_strings object section at all, considering that the only
place where that is used again appears to be this code in
kernel/module.c:
markersstringsindex = find_sec(hdr, sechdrs, secstrings,
"__markers_strings");
and the "markersstringsindex" variable is never used.
- FChE
* Frank Ch. Eigler ([email protected]) wrote:
> Hi -
>
> I wrote:
>
> > [...]
> > > The marker metadata must be stored in at least one place in the kernel
> > > image - this just happens to be a convenient one that David Smith's
> > > recent systemtap code used. Without it, we'd probably have to do a
> > > more complicated search, following the pointers within the __markers
> > > structs. [...]
>
> Our team is farther along adapting to this change against 2.6.23-mm1,
> and we have run into a complication. It's more of a distribution
> issue.
>
> We would prefer to retain systemtap's capability to build
> instrumentation for a kernel other than the currently running one.
> Such instrumentation can be then copied and run on a distinct machine.
> This has meant relying on development data: make install_headers +
> Makefiles (as packaged by Fedora/RHEL), and to a lesser extent
> separated debugging information.
>
> Markers are attractive partly because they don't require debugging
> information, so the data needs to be found in an executable image.
> But we prefer not to force the executable image itself to be
> installed, for example because /boot is relatively small. So we would
> prefer something in between: something small that we can put into the
> development package.
>
> If there exists sympathy to this problem, Roland McGrath supposes we
> could implement a standardized solution, a file like Module.symvers,
> containing the marker names & format strings extracted at build time.
> Any opinions?
>
Hi Frank,
I think the main issue with the solution you propose is that it doesn't
deal with markers in modules, am I right ?
I will soon come with a marker iterator and a module that provides a
userspace -and in kernel- interface to enable/disable markers. Actually,
I already have the code ready in my LTTng snapshots. I can provide a
link if you want to have a look.
>
> PS. I wonder why the marker name/format strings are put into a
> __markers_strings object section at all, considering that the only
> place where that is used again appears to be this code in
> kernel/module.c:
>
> markersstringsindex = find_sec(hdr, sechdrs, secstrings,
> "__markers_strings");
>
> and the "markersstringsindex" variable is never used.
>
Considering that I want to minimize the impact on the system, I put the
marker strings in their own memory location rather than clobbering the
memory containing the kernel strings (which will likely be used more
often than markers). It makes sure that I don't pollute cachelines
otherwise containing useful kernel strings.
Mathieu
>
> - FChE
--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
> I think the main issue with the solution you propose is that it doesn't
> deal with markers in modules, am I right ?
My suggestion applies as well to modules as anything else.
What "like Module.symvers" means is something like:
name1 vmlinux %s
name2 fs/nfs/nfs %d
All the modules built by the same kernel build go into this one file.
Modules packaged separately for the same kernel could provide additional
files of the same kind.
> I will soon come with a marker iterator and a module that provides a
> userspace -and in kernel- interface to enable/disable markers. Actually,
> I already have the code ready in my LTTng snapshots. I can provide a
> link if you want to have a look.
That's clearly straightforward to do given the basic markers data structures.
It does not address the need for an offline list of markers available in a
particular kernel build or set of modules that you are not running right now.
The approach now available for that is grovelling through the markers data
structures extracted from vmlinux and .ko ELF files offline. That is more
work than one should have to do, and has lots of problems with coping with
different packaging details, etc.
Thanks,
Roland
* Roland McGrath ([email protected]) wrote:
> > I think the main issue with the solution you propose is that it doesn't
> > deal with markers in modules, am I right ?
>
> My suggestion applies as well to modules as anything else.
> What "like Module.symvers" means is something like:
>
> name1 vmlinux %s
> name2 fs/nfs/nfs %d
>
> All the modules built by the same kernel build go into this one file.
>
> Modules packaged separately for the same kernel could provide additional
> files of the same kind.
>
> > I will soon come with a marker iterator and a module that provides a
> > userspace -and in kernel- interface to enable/disable markers. Actually,
> > I already have the code ready in my LTTng snapshots. I can provide a
> > link if you want to have a look.
>
> That's clearly straightforward to do given the basic markers data structures.
>
> It does not address the need for an offline list of markers available in a
> particular kernel build or set of modules that you are not running right now.
> The approach now available for that is grovelling through the markers data
> structures extracted from vmlinux and .ko ELF files offline. That is more
> work than one should have to do, and has lots of problems with coping with
> different packaging details, etc.
>
Since gcc is required to build the systemtap probes on the development
marchine, I don't see why it would be much harder to also require prople
to install drawf ? Or maybe the "crash" tool ?
I guess you must already need to extract the symbols for your kprobes.
Do you use kallsyms for this ? The way I see it, you could maybe extract
kallsyms symbols corresponding to the markers data structures quite
easily.
I would rather prefer not to implement superfluous built-time data
extraction in the kernel build system just to make userspace simpler. If
we can leverage what currently exists, that would be better.
Mathieu
>
> Thanks,
> Roland
--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
Hi -
On Thu, Oct 25, 2007 at 03:17:22PM -0400, Mathieu Desnoyers wrote:
> [...]
> Since gcc is required to build the systemtap probes on the development
> marchine, I don't see why it would be much harder to also require prople
> to install drawf ? Or maybe the "crash" tool ?
The crash tool requires the dwarf data to work. The dwarf data for an
entire kernel (including all the modules) is on the order of hundreds
of megabytes. The symbol & marker list would be one thousandth the
size. You can see the deployment attractiveness of the latter.
> I guess you must already need to extract the symbols for your kprobes.
> Do you use kallsyms for this?
Nope. /proc/kallsyms is a another run-time-only source of data, and
so is not applicable for off-line (ahead-of-time) mapping.
> I would rather prefer not to implement superfluous built-time data
> extraction in the kernel build system just to make userspace
> simpler. [...]
It is not superfluous, as it would solve a real distribution problem.
- FChE
This adds some new magic in the MODPOST phase for CONFIG_MARKERS.
Analogous to the Module.symvers file, the build will now write a
Module.markers file when CONFIG_MARKERS=y is set. This file lists
the name, defining module, and format string of each marker,
separated by \t characters. This simple text file can be used by
offline build procedures for instrumentation code, analogous to
how System.map and Module.symvers can be useful to have for
kernels other than the one you are running right now.
The method of extracting the strings is somewhat crude, but is very
simple and should work fine in practice for the foreseeable future.
Signed-off-by: Roland McGrath <[email protected]>
---
scripts/Makefile.modpost | 11 +++
scripts/mod/modpost.c | 174 +++++++++++++++++++++++++++++++++++++++++++++-
scripts/mod/modpost.h | 3 +
3 files changed, 187 insertions(+), 1 deletions(-)
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index d988f5d..6321870 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -13,6 +13,7 @@
# 2) modpost is then used to
# 3) create one <module>.mod.c file pr. module
# 4) create one Module.symvers file with CRC for all exported symbols
+# 4a) [CONFIG_MARKERS] create one Module.markers file listing defined markers
# 5) compile all <module>.mod.c files
# 6) final link of the module to a <module.ko> file
@@ -45,6 +46,10 @@ include scripts/Makefile.lib
kernelsymfile := $(objtree)/Module.symvers
modulesymfile := $(firstword $(KBUILD_EXTMOD))/Module.symvers
+kernelmarkersfile := $(objtree)/Module.markers
+modulemarkersfile := $(firstword $(KBUILD_EXTMOD))/Module.markers
+
+markersfile = $(if $(KBUILD_EXTMOD),$(modulemarkersfile),$(kernelmarkersfile))
# Step 1), find all modules listed in $(MODVERDIR)/
__modules := $(sort $(shell grep -h '\.ko' /dev/null $(wildcard $(MODVERDIR)/*.mod)))
@@ -62,6 +67,8 @@ modpost = scripts/mod/modpost \
$(if $(KBUILD_EXTMOD),-i,-o) $(kernelsymfile) \
$(if $(KBUILD_EXTMOD),-I $(modulesymfile)) \
$(if $(KBUILD_EXTMOD),-o $(modulesymfile)) \
+ $(if $(CONFIG_MARKERS),-K $(kernelmarkersfile)) \
+ $(if $(CONFIG_MARKERS),-M $(markersfile)) \
$(if $(KBUILD_EXTMOD)$(KBUILD_MODPOST_WARN),-w)
quiet_cmd_modpost = MODPOST $(words $(filter-out vmlinux FORCE, $^)) modules
@@ -81,6 +88,10 @@ vmlinux.o: FORCE
$(symverfile): __modpost ;
$(modules:.ko=.mod.c): __modpost ;
+ifdef CONFIG_MARKERS
+$(markersfile): __modpost ;
+endif
+
# Step 5), compile all *.mod.c files
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 93ac52a..df80bfc 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -11,6 +11,8 @@
* Usage: modpost vmlinux module1.o module2.o ...
*/
+#define _GNU_SOURCE
+#include <stdio.h>
#include <ctype.h>
#include "modpost.h"
#include "../../include/linux/license.h"
@@ -424,6 +426,8 @@ static int parse_elf(struct elf_info *info, const char *filename)
info->export_unused_gpl_sec = i;
else if (strcmp(secname, "__ksymtab_gpl_future") == 0)
info->export_gpl_future_sec = i;
+ else if (strcmp(secname, "__markers_strings") == 0)
+ info->markers_strings_sec = i;
if (sechdrs[i].sh_type != SHT_SYMTAB)
continue;
@@ -1249,6 +1253,73 @@ static int exit_section_ref_ok(const char *name)
return 0;
}
+static size_t strlen_with_padding(const char *start, const char *limit)
+{
+ const char *p = memchr(start, '\0', limit - start);
+ if (p == NULL)
+ return 0;
+ do
+ ++p;
+ while (p < limit && *p == '\0');
+ return p - start;
+}
+
+static void get_markers(struct elf_info *info, struct module *mod)
+{
+ const Elf_Shdr *sh = &info->sechdrs[info->markers_strings_sec];
+ const char *strings;
+ const char *strings_end;
+ const char *p;
+ size_t n, i;
+
+ if (!info->markers_strings_sec)
+ return;
+
+ strings = (const char *) info->hdr + sh->sh_offset;
+ strings_end = strings + sh->sh_size;
+
+ /*
+ * First count the strings. They come in pairs of name, format.
+ */
+ for (n = 0, p = strings; p < strings_end; ++n) {
+ size_t len = strlen_with_padding(p, strings_end);
+ if (len == 0)
+ break;
+ p += len;
+ }
+ if (n % 2 != 0 || p != strings_end) {
+ warn("%s.ko has bad __markers_strings, ignoring it\n",
+ mod->name);
+ return;
+ }
+
+ if (n == 0)
+ return;
+
+ /*
+ * Now collect each pair into a formatted line for the output.
+ * Lines look like:
+ * marker_name vmlinux marker %s format %d
+ * The format string after the second \t can use whitespace.
+ */
+ mod->markers = NOFAIL(malloc(sizeof mod->markers[0] * n / 2));
+ mod->nmarkers = n / 2;
+
+ p = strings;
+ for (i = 0; i < n; i += 2) {
+ const char *name, *fmt;
+ name = p;
+ p += strlen_with_padding(p, strings_end);
+ fmt = p;
+ p += strlen_with_padding(p, strings_end);
+
+ mod->markers[i / 2] = NULL;
+ asprintf(&mod->markers[i / 2], "%s\t%s\t%s\n",
+ name, mod->name, fmt);
+ NOFAIL(mod->markers[i / 2]);
+ }
+}
+
static void read_symbols(char *modname)
{
const char *symname;
@@ -1301,6 +1372,8 @@ static void read_symbols(char *modname)
get_src_version(modname, mod->srcversion,
sizeof(mod->srcversion)-1);
+ get_markers(&info, mod);
+
parse_elf_finish(&info);
/* Our trick to get versioning for struct_module - it's
@@ -1649,6 +1722,91 @@ static void write_dump(const char *fname)
write_if_changed(&buf, fname);
}
+static void add_marker(struct module *mod, const char *name, const char *fmt)
+{
+ char *line = NULL;
+ asprintf(&line, "%s\t%s\t%s\n", name, mod->name, fmt);
+ NOFAIL(line);
+
+ mod->markers = NOFAIL(realloc(mod->markers, ((mod->nmarkers + 1) *
+ sizeof mod->markers[0])));
+ mod->markers[mod->nmarkers++] = line;
+}
+
+static void read_markers(const char *fname)
+{
+ unsigned long size, pos = 0;
+ void *file = grab_file(fname, &size);
+ char *line;
+
+ if (!file)
+ /* No old markers, silently ignore */
+ return;
+
+ while ((line = get_next_line(&pos, file, size))) {
+ char *marker, *modname, *fmt;
+ struct module *mod;
+
+ marker = line;
+ if (!(modname = strchr(marker, '\t')))
+ goto fail;
+ *modname++ = '\0';
+ if (!(fmt = strchr(modname, '\t')))
+ goto fail;
+ *fmt++ = '\0';
+ if (*marker == '\0' || *modname == '\0')
+ goto fail;
+
+ if (!(mod = find_module(modname))) {
+ if (is_vmlinux(modname)) {
+ have_vmlinux = 1;
+ }
+ mod = new_module(NOFAIL(strdup(modname)));
+ mod->skip = 1;
+ }
+
+ add_marker(mod, marker, fmt);
+ }
+ return;
+fail:
+ fatal("parse error in markers list file\n");
+}
+
+static int compare_strings(const void *a, const void *b)
+{
+ return strcmp(*(const char **) a, *(const char **) b);
+}
+
+static void write_markers(const char *fname)
+{
+ struct buffer buf = { };
+ struct module *mod;
+ size_t i;
+
+ for (mod = modules; mod; mod = mod->next)
+ if ((!external_module || !mod->skip) && mod->markers != NULL) {
+ /*
+ * Sort the strings so we can skip duplicates when
+ * we write them out.
+ */
+ qsort(mod->markers, mod->nmarkers,
+ sizeof mod->markers[0], &compare_strings);
+ for (i = 0; i < mod->nmarkers; ++i) {
+ char *line = mod->markers[i];
+ buf_write(&buf, line, strlen(line));
+ while (i + 1 < mod->nmarkers &&
+ !strcmp(mod->markers[i],
+ mod->markers[i + 1]))
+ free(mod->markers[i++]);
+ free(mod->markers[i]);
+ }
+ free(mod->markers);
+ mod->markers = NULL;
+ }
+
+ write_if_changed(&buf, fname);
+}
+
int main(int argc, char **argv)
{
struct module *mod;
@@ -1656,10 +1814,12 @@ int main(int argc, char **argv)
char fname[SZ];
char *kernel_read = NULL, *module_read = NULL;
char *dump_write = NULL;
+ char *markers_read = NULL;
+ char *markers_write = NULL;
int opt;
int err;
- while ((opt = getopt(argc, argv, "i:I:mso:aw")) != -1) {
+ while ((opt = getopt(argc, argv, "i:I:mso:awM:K:")) != -1) {
switch(opt) {
case 'i':
kernel_read = optarg;
@@ -1683,6 +1843,12 @@ int main(int argc, char **argv)
case 'w':
warn_unresolved = 1;
break;
+ case 'M':
+ markers_write = optarg;
+ break;
+ case 'K':
+ markers_read = optarg;
+ break;
default:
exit(1);
}
@@ -1724,5 +1890,11 @@ int main(int argc, char **argv)
if (dump_write)
write_dump(dump_write);
+ if (markers_read)
+ read_markers(markers_read);
+
+ if (markers_write)
+ write_markers(markers_write);
+
return err;
}
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 0ffed17..175301a 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -110,6 +110,8 @@ struct module {
int has_init;
int has_cleanup;
struct buffer dev_table_buf;
+ char **markers;
+ size_t nmarkers;
char srcversion[25];
};
@@ -124,6 +126,7 @@ struct elf_info {
Elf_Section export_gpl_sec;
Elf_Section export_unused_gpl_sec;
Elf_Section export_gpl_future_sec;
+ Elf_Section markers_strings_sec;
const char *strtab;
char *modinfo;
unsigned int modinfo_len;
* Roland McGrath ([email protected]) wrote:
>
> This adds some new magic in the MODPOST phase for CONFIG_MARKERS.
> Analogous to the Module.symvers file, the build will now write a
> Module.markers file when CONFIG_MARKERS=y is set. This file lists
> the name, defining module, and format string of each marker,
> separated by \t characters. This simple text file can be used by
> offline build procedures for instrumentation code, analogous to
> how System.map and Module.symvers can be useful to have for
> kernels other than the one you are running right now.
>
> The method of extracting the strings is somewhat crude, but is very
> simple and should work fine in practice for the foreseeable future.
>
Hi Roland,
I'm ok with the idea of extracting such information, but I doubt one can
assume the strings will always be layed out in the same order in the
__markers_strings section. We also shouldn't assume that a marker name
will be a neighbor of its format string.
If we want to do it safely, I think we should iterate from
__start___markers to __stop___markers symbols of vmlinux and get the
pointers to the name/format string pairs.
The same can then be done with modules using the __markers section.
Or maybe is there some reason not to do that ?
Mathieu
> Signed-off-by: Roland McGrath <[email protected]>
> ---
> scripts/Makefile.modpost | 11 +++
> scripts/mod/modpost.c | 174 +++++++++++++++++++++++++++++++++++++++++++++-
> scripts/mod/modpost.h | 3 +
> 3 files changed, 187 insertions(+), 1 deletions(-)
>
> diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
> index d988f5d..6321870 100644
> --- a/scripts/Makefile.modpost
> +++ b/scripts/Makefile.modpost
> @@ -13,6 +13,7 @@
> # 2) modpost is then used to
> # 3) create one <module>.mod.c file pr. module
> # 4) create one Module.symvers file with CRC for all exported symbols
> +# 4a) [CONFIG_MARKERS] create one Module.markers file listing defined markers
> # 5) compile all <module>.mod.c files
> # 6) final link of the module to a <module.ko> file
>
> @@ -45,6 +46,10 @@ include scripts/Makefile.lib
>
> kernelsymfile := $(objtree)/Module.symvers
> modulesymfile := $(firstword $(KBUILD_EXTMOD))/Module.symvers
> +kernelmarkersfile := $(objtree)/Module.markers
> +modulemarkersfile := $(firstword $(KBUILD_EXTMOD))/Module.markers
> +
> +markersfile = $(if $(KBUILD_EXTMOD),$(modulemarkersfile),$(kernelmarkersfile))
>
> # Step 1), find all modules listed in $(MODVERDIR)/
> __modules := $(sort $(shell grep -h '\.ko' /dev/null $(wildcard $(MODVERDIR)/*.mod)))
> @@ -62,6 +67,8 @@ modpost = scripts/mod/modpost \
> $(if $(KBUILD_EXTMOD),-i,-o) $(kernelsymfile) \
> $(if $(KBUILD_EXTMOD),-I $(modulesymfile)) \
> $(if $(KBUILD_EXTMOD),-o $(modulesymfile)) \
> + $(if $(CONFIG_MARKERS),-K $(kernelmarkersfile)) \
> + $(if $(CONFIG_MARKERS),-M $(markersfile)) \
> $(if $(KBUILD_EXTMOD)$(KBUILD_MODPOST_WARN),-w)
>
> quiet_cmd_modpost = MODPOST $(words $(filter-out vmlinux FORCE, $^)) modules
> @@ -81,6 +88,10 @@ vmlinux.o: FORCE
> $(symverfile): __modpost ;
> $(modules:.ko=.mod.c): __modpost ;
>
> +ifdef CONFIG_MARKERS
> +$(markersfile): __modpost ;
> +endif
> +
>
> # Step 5), compile all *.mod.c files
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 93ac52a..df80bfc 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -11,6 +11,8 @@
> * Usage: modpost vmlinux module1.o module2.o ...
> */
>
> +#define _GNU_SOURCE
> +#include <stdio.h>
> #include <ctype.h>
> #include "modpost.h"
> #include "../../include/linux/license.h"
> @@ -424,6 +426,8 @@ static int parse_elf(struct elf_info *info, const char *filename)
> info->export_unused_gpl_sec = i;
> else if (strcmp(secname, "__ksymtab_gpl_future") == 0)
> info->export_gpl_future_sec = i;
> + else if (strcmp(secname, "__markers_strings") == 0)
> + info->markers_strings_sec = i;
>
> if (sechdrs[i].sh_type != SHT_SYMTAB)
> continue;
> @@ -1249,6 +1253,73 @@ static int exit_section_ref_ok(const char *name)
> return 0;
> }
>
> +static size_t strlen_with_padding(const char *start, const char *limit)
> +{
> + const char *p = memchr(start, '\0', limit - start);
> + if (p == NULL)
> + return 0;
> + do
> + ++p;
> + while (p < limit && *p == '\0');
> + return p - start;
> +}
> +
> +static void get_markers(struct elf_info *info, struct module *mod)
> +{
> + const Elf_Shdr *sh = &info->sechdrs[info->markers_strings_sec];
> + const char *strings;
> + const char *strings_end;
> + const char *p;
> + size_t n, i;
> +
> + if (!info->markers_strings_sec)
> + return;
> +
> + strings = (const char *) info->hdr + sh->sh_offset;
> + strings_end = strings + sh->sh_size;
> +
> + /*
> + * First count the strings. They come in pairs of name, format.
> + */
> + for (n = 0, p = strings; p < strings_end; ++n) {
> + size_t len = strlen_with_padding(p, strings_end);
> + if (len == 0)
> + break;
> + p += len;
> + }
> + if (n % 2 != 0 || p != strings_end) {
> + warn("%s.ko has bad __markers_strings, ignoring it\n",
> + mod->name);
> + return;
> + }
> +
> + if (n == 0)
> + return;
> +
> + /*
> + * Now collect each pair into a formatted line for the output.
> + * Lines look like:
> + * marker_name vmlinux marker %s format %d
> + * The format string after the second \t can use whitespace.
> + */
> + mod->markers = NOFAIL(malloc(sizeof mod->markers[0] * n / 2));
> + mod->nmarkers = n / 2;
> +
> + p = strings;
> + for (i = 0; i < n; i += 2) {
> + const char *name, *fmt;
> + name = p;
> + p += strlen_with_padding(p, strings_end);
> + fmt = p;
> + p += strlen_with_padding(p, strings_end);
> +
> + mod->markers[i / 2] = NULL;
> + asprintf(&mod->markers[i / 2], "%s\t%s\t%s\n",
> + name, mod->name, fmt);
> + NOFAIL(mod->markers[i / 2]);
> + }
> +}
> +
> static void read_symbols(char *modname)
> {
> const char *symname;
> @@ -1301,6 +1372,8 @@ static void read_symbols(char *modname)
> get_src_version(modname, mod->srcversion,
> sizeof(mod->srcversion)-1);
>
> + get_markers(&info, mod);
> +
> parse_elf_finish(&info);
>
> /* Our trick to get versioning for struct_module - it's
> @@ -1649,6 +1722,91 @@ static void write_dump(const char *fname)
> write_if_changed(&buf, fname);
> }
>
> +static void add_marker(struct module *mod, const char *name, const char *fmt)
> +{
> + char *line = NULL;
> + asprintf(&line, "%s\t%s\t%s\n", name, mod->name, fmt);
> + NOFAIL(line);
> +
> + mod->markers = NOFAIL(realloc(mod->markers, ((mod->nmarkers + 1) *
> + sizeof mod->markers[0])));
> + mod->markers[mod->nmarkers++] = line;
> +}
> +
> +static void read_markers(const char *fname)
> +{
> + unsigned long size, pos = 0;
> + void *file = grab_file(fname, &size);
> + char *line;
> +
> + if (!file)
> + /* No old markers, silently ignore */
> + return;
> +
> + while ((line = get_next_line(&pos, file, size))) {
> + char *marker, *modname, *fmt;
> + struct module *mod;
> +
> + marker = line;
> + if (!(modname = strchr(marker, '\t')))
> + goto fail;
> + *modname++ = '\0';
> + if (!(fmt = strchr(modname, '\t')))
> + goto fail;
> + *fmt++ = '\0';
> + if (*marker == '\0' || *modname == '\0')
> + goto fail;
> +
> + if (!(mod = find_module(modname))) {
> + if (is_vmlinux(modname)) {
> + have_vmlinux = 1;
> + }
> + mod = new_module(NOFAIL(strdup(modname)));
> + mod->skip = 1;
> + }
> +
> + add_marker(mod, marker, fmt);
> + }
> + return;
> +fail:
> + fatal("parse error in markers list file\n");
> +}
> +
> +static int compare_strings(const void *a, const void *b)
> +{
> + return strcmp(*(const char **) a, *(const char **) b);
> +}
> +
> +static void write_markers(const char *fname)
> +{
> + struct buffer buf = { };
> + struct module *mod;
> + size_t i;
> +
> + for (mod = modules; mod; mod = mod->next)
> + if ((!external_module || !mod->skip) && mod->markers != NULL) {
> + /*
> + * Sort the strings so we can skip duplicates when
> + * we write them out.
> + */
> + qsort(mod->markers, mod->nmarkers,
> + sizeof mod->markers[0], &compare_strings);
> + for (i = 0; i < mod->nmarkers; ++i) {
> + char *line = mod->markers[i];
> + buf_write(&buf, line, strlen(line));
> + while (i + 1 < mod->nmarkers &&
> + !strcmp(mod->markers[i],
> + mod->markers[i + 1]))
> + free(mod->markers[i++]);
> + free(mod->markers[i]);
> + }
> + free(mod->markers);
> + mod->markers = NULL;
> + }
> +
> + write_if_changed(&buf, fname);
> +}
> +
> int main(int argc, char **argv)
> {
> struct module *mod;
> @@ -1656,10 +1814,12 @@ int main(int argc, char **argv)
> char fname[SZ];
> char *kernel_read = NULL, *module_read = NULL;
> char *dump_write = NULL;
> + char *markers_read = NULL;
> + char *markers_write = NULL;
> int opt;
> int err;
>
> - while ((opt = getopt(argc, argv, "i:I:mso:aw")) != -1) {
> + while ((opt = getopt(argc, argv, "i:I:mso:awM:K:")) != -1) {
> switch(opt) {
> case 'i':
> kernel_read = optarg;
> @@ -1683,6 +1843,12 @@ int main(int argc, char **argv)
> case 'w':
> warn_unresolved = 1;
> break;
> + case 'M':
> + markers_write = optarg;
> + break;
> + case 'K':
> + markers_read = optarg;
> + break;
> default:
> exit(1);
> }
> @@ -1724,5 +1890,11 @@ int main(int argc, char **argv)
> if (dump_write)
> write_dump(dump_write);
>
> + if (markers_read)
> + read_markers(markers_read);
> +
> + if (markers_write)
> + write_markers(markers_write);
> +
> return err;
> }
> diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
> index 0ffed17..175301a 100644
> --- a/scripts/mod/modpost.h
> +++ b/scripts/mod/modpost.h
> @@ -110,6 +110,8 @@ struct module {
> int has_init;
> int has_cleanup;
> struct buffer dev_table_buf;
> + char **markers;
> + size_t nmarkers;
> char srcversion[25];
> };
>
> @@ -124,6 +126,7 @@ struct elf_info {
> Elf_Section export_gpl_sec;
> Elf_Section export_unused_gpl_sec;
> Elf_Section export_gpl_future_sec;
> + Elf_Section markers_strings_sec;
> const char *strtab;
> char *modinfo;
> unsigned int modinfo_len;
--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
> If we want to do it safely, I think we should iterate from
> __start___markers to __stop___markers symbols of vmlinux and get the
> pointers to the name/format string pairs.
>
> The same can then be done with modules using the __markers section.
>
> Or maybe is there some reason not to do that ?
It's just rather a pain in the ass, a whole lot more fiddly work.
cf "somewhat crude" and "foreseeable future" in my patch's log entry.
Knock yourself out if you're looking for more tedious hacking to do in
modpost.c, but I say fix it when it breaks.
Thanks,
Roland
* Roland McGrath ([email protected]) wrote:
> > If we want to do it safely, I think we should iterate from
> > __start___markers to __stop___markers symbols of vmlinux and get the
> > pointers to the name/format string pairs.
> >
> > The same can then be done with modules using the __markers section.
> >
> > Or maybe is there some reason not to do that ?
>
> It's just rather a pain in the ass, a whole lot more fiddly work.
> cf "somewhat crude" and "foreseeable future" in my patch's log entry.
> Knock yourself out if you're looking for more tedious hacking to do in
> modpost.c, but I say fix it when it breaks.
>
Hmmmm, I have rarely seen code go into mainline without addressing valid
technical criticism first. Please fix.
I'll look into it if I find the time.
Mathieu
>
> Thanks,
> Roland
--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
Mathieu Desnoyers wrote:
> * Roland McGrath ([email protected]) wrote:
>>> If we want to do it safely, I think we should iterate from
>>> __start___markers to __stop___markers symbols of vmlinux and get the
>>> pointers to the name/format string pairs.
>>>
>>> The same can then be done with modules using the __markers section.
>>>
>>> Or maybe is there some reason not to do that ?
>> It's just rather a pain in the ass, a whole lot more fiddly work.
>> cf "somewhat crude" and "foreseeable future" in my patch's log entry.
>> Knock yourself out if you're looking for more tedious hacking to do in
>> modpost.c, but I say fix it when it breaks.
>>
>
> Hmmmm, I have rarely seen code go into mainline without addressing valid
> technical criticism first. Please fix.
>
> I'll look into it if I find the time.
>
> Mathieu
Mathieu,
Here's an updated patch, written by Roland (that I tested for him), that
looks for all marker symbols in the __markers_strings section. It doesn't
get the pointers from the __markers section because that is very difficult
to do in modpost (having to handle the architecture-dependent relocations
applied to those pointers).
See what you think.
---
This adds some new magic in the MODPOST phase for CONFIG_MARKERS.
Analogous to the Module.symvers file, the build will now write a
Module.markers file when CONFIG_MARKERS=y is set. This file lists
the name, defining module, and format string of each marker,
separated by \t characters. This simple text file can be used by
offline build procedures for instrumentation code, analogous to
how System.map and Module.symvers can be useful to have for
kernels other than the one you are running right now.
The method of extracting the strings is somewhat crude, but is pretty
simple and should work fine in practice for the foreseeable future.
Signed-off-by: Roland McGrath <[email protected]>
---
scripts/Makefile.modpost | 11 +++
scripts/mod/modpost.c | 184 +++++++++++++++++++++++++++++++++++++++++++++-
scripts/mod/modpost.h | 3 +
3 files changed, 197 insertions(+), 1 deletions(-)
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index d988f5d..6321870 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -13,6 +13,7 @@
# 2) modpost is then used to
# 3) create one <module>.mod.c file pr. module
# 4) create one Module.symvers file with CRC for all exported symbols
+# 4a) [CONFIG_MARKERS] create one Module.markers file listing defined markers
# 5) compile all <module>.mod.c files
# 6) final link of the module to a <module.ko> file
@@ -45,6 +46,10 @@ include scripts/Makefile.lib
kernelsymfile := $(objtree)/Module.symvers
modulesymfile := $(firstword $(KBUILD_EXTMOD))/Module.symvers
+kernelmarkersfile := $(objtree)/Module.markers
+modulemarkersfile := $(firstword $(KBUILD_EXTMOD))/Module.markers
+
+markersfile = $(if $(KBUILD_EXTMOD),$(modulemarkersfile),$(kernelmarkersfile))
# Step 1), find all modules listed in $(MODVERDIR)/
__modules := $(sort $(shell grep -h '\.ko' /dev/null $(wildcard $(MODVERDIR)/*.mod)))
@@ -62,6 +67,8 @@ modpost = scripts/mod/modpost \
$(if $(KBUILD_EXTMOD),-i,-o) $(kernelsymfile) \
$(if $(KBUILD_EXTMOD),-I $(modulesymfile)) \
$(if $(KBUILD_EXTMOD),-o $(modulesymfile)) \
+ $(if $(CONFIG_MARKERS),-K $(kernelmarkersfile)) \
+ $(if $(CONFIG_MARKERS),-M $(markersfile)) \
$(if $(KBUILD_EXTMOD)$(KBUILD_MODPOST_WARN),-w)
quiet_cmd_modpost = MODPOST $(words $(filter-out vmlinux FORCE, $^)) modules
@@ -81,6 +88,10 @@ vmlinux.o: FORCE
$(symverfile): __modpost ;
$(modules:.ko=.mod.c): __modpost ;
+ifdef CONFIG_MARKERS
+$(markersfile): __modpost ;
+endif
+
# Step 5), compile all *.mod.c files
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 93ac52a..bbaf26d 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -11,6 +11,8 @@
* Usage: modpost vmlinux module1.o module2.o ...
*/
+#define _GNU_SOURCE
+#include <stdio.h>
#include <ctype.h>
#include "modpost.h"
#include "../../include/linux/license.h"
@@ -424,6 +426,8 @@ static int parse_elf(struct elf_info *info, const char *filename)
info->export_unused_gpl_sec = i;
else if (strcmp(secname, "__ksymtab_gpl_future") == 0)
info->export_gpl_future_sec = i;
+ else if (strcmp(secname, "__markers_strings") == 0)
+ info->markers_strings_sec = i;
if (sechdrs[i].sh_type != SHT_SYMTAB)
continue;
@@ -1249,6 +1253,83 @@ static int exit_section_ref_ok(const char *name)
return 0;
}
+static void get_markers(struct elf_info *info, struct module *mod)
+{
+ const Elf_Shdr *sh = &info->sechdrs[info->markers_strings_sec];
+ const char *strings = (const char *) info->hdr + sh->sh_offset;
+ const Elf_Sym *sym, *first_sym, *last_sym;
+ const char *name;
+ size_t n;
+
+ if (!info->markers_strings_sec)
+ return;
+
+ /*
+ * First count the strings. They come in pairs of name, format.
+ * We look for all the symbols defined in the __markers_strings
+ * section. They are named __mstrtab_name_* and __mstrtab_format_*
+ * in matching pairs. For these local names, the compiler puts
+ * a random .NNN suffix on, so the names don't correspond exactly.
+ */
+ first_sym = last_sym = NULL;
+ n = 0;
+ for (sym = info->symtab_start; sym < info->symtab_stop; sym++)
+ if (ELF_ST_TYPE(sym->st_info) == STT_OBJECT &&
+ sym->st_shndx == info->markers_strings_sec) {
+ if (first_sym == NULL)
+ first_sym = sym;
+ last_sym = sym;
+ ++n;
+ name = info->strtab + sym->st_name;
+ if (n % 2 == 0 ?
+ !strncmp(name, "__mstrtab_name_",
+ sizeof "__mstrtab_name_" - 1) :
+ !strncmp(name, "__mstrtab_format_",
+ sizeof "__mstrtab_format_" - 1)) {
+ warn("%s.ko has unexpected symbol \"%s\"\n",
+ mod->name, name);
+ first_sym = NULL;
+ }
+ }
+
+ if (n % 2 != 0 || first_sym == NULL) {
+ warn("%s.ko has bad __markers_strings, ignoring it\n",
+ mod->name);
+ return;
+ }
+
+ if (n == 0)
+ return;
+
+ /*
+ * Now collect each pair into a formatted line for the output.
+ * Lines look like:
+ * marker_name vmlinux marker %s format %d
+ * The format string after the second \t can use whitespace.
+ */
+ n /= 2;
+ mod->markers = NOFAIL(malloc(sizeof mod->markers[0] * n));
+ mod->nmarkers = n;
+
+ name = NULL;
+ n = 0;
+ for (sym = first_sym; sym <= last_sym; sym++)
+ if (ELF_ST_TYPE(sym->st_info) == STT_OBJECT &&
+ sym->st_shndx == info->markers_strings_sec) {
+ const char *str = strings + sym->st_value;
+ if (name == NULL)
+ name = str;
+ else {
+ char *line = NULL;
+ asprintf(&line, "%s\t%s\t%s\n",
+ name, mod->name, str);
+ NOFAIL(line);
+ mod->markers[n++] = line;
+ name = NULL;
+ }
+ }
+}
+
static void read_symbols(char *modname)
{
const char *symname;
@@ -1301,6 +1382,8 @@ static void read_symbols(char *modname)
get_src_version(modname, mod->srcversion,
sizeof(mod->srcversion)-1);
+ get_markers(&info, mod);
+
parse_elf_finish(&info);
/* Our trick to get versioning for struct_module - it's
@@ -1649,6 +1732,91 @@ static void write_dump(const char *fname)
write_if_changed(&buf, fname);
}
+static void add_marker(struct module *mod, const char *name, const char *fmt)
+{
+ char *line = NULL;
+ asprintf(&line, "%s\t%s\t%s\n", name, mod->name, fmt);
+ NOFAIL(line);
+
+ mod->markers = NOFAIL(realloc(mod->markers, ((mod->nmarkers + 1) *
+ sizeof mod->markers[0])));
+ mod->markers[mod->nmarkers++] = line;
+}
+
+static void read_markers(const char *fname)
+{
+ unsigned long size, pos = 0;
+ void *file = grab_file(fname, &size);
+ char *line;
+
+ if (!file)
+ /* No old markers, silently ignore */
+ return;
+
+ while ((line = get_next_line(&pos, file, size))) {
+ char *marker, *modname, *fmt;
+ struct module *mod;
+
+ marker = line;
+ if (!(modname = strchr(marker, '\t')))
+ goto fail;
+ *modname++ = '\0';
+ if (!(fmt = strchr(modname, '\t')))
+ goto fail;
+ *fmt++ = '\0';
+ if (*marker == '\0' || *modname == '\0')
+ goto fail;
+
+ if (!(mod = find_module(modname))) {
+ if (is_vmlinux(modname)) {
+ have_vmlinux = 1;
+ }
+ mod = new_module(NOFAIL(strdup(modname)));
+ mod->skip = 1;
+ }
+
+ add_marker(mod, marker, fmt);
+ }
+ return;
+fail:
+ fatal("parse error in markers list file\n");
+}
+
+static int compare_strings(const void *a, const void *b)
+{
+ return strcmp(*(const char **) a, *(const char **) b);
+}
+
+static void write_markers(const char *fname)
+{
+ struct buffer buf = { };
+ struct module *mod;
+ size_t i;
+
+ for (mod = modules; mod; mod = mod->next)
+ if ((!external_module || !mod->skip) && mod->markers != NULL) {
+ /*
+ * Sort the strings so we can skip duplicates when
+ * we write them out.
+ */
+ qsort(mod->markers, mod->nmarkers,
+ sizeof mod->markers[0], &compare_strings);
+ for (i = 0; i < mod->nmarkers; ++i) {
+ char *line = mod->markers[i];
+ buf_write(&buf, line, strlen(line));
+ while (i + 1 < mod->nmarkers &&
+ !strcmp(mod->markers[i],
+ mod->markers[i + 1]))
+ free(mod->markers[i++]);
+ free(mod->markers[i]);
+ }
+ free(mod->markers);
+ mod->markers = NULL;
+ }
+
+ write_if_changed(&buf, fname);
+}
+
int main(int argc, char **argv)
{
struct module *mod;
@@ -1656,10 +1824,12 @@ int main(int argc, char **argv)
char fname[SZ];
char *kernel_read = NULL, *module_read = NULL;
char *dump_write = NULL;
+ char *markers_read = NULL;
+ char *markers_write = NULL;
int opt;
int err;
- while ((opt = getopt(argc, argv, "i:I:mso:aw")) != -1) {
+ while ((opt = getopt(argc, argv, "i:I:mso:awM:K:")) != -1) {
switch(opt) {
case 'i':
kernel_read = optarg;
@@ -1683,6 +1853,12 @@ int main(int argc, char **argv)
case 'w':
warn_unresolved = 1;
break;
+ case 'M':
+ markers_write = optarg;
+ break;
+ case 'K':
+ markers_read = optarg;
+ break;
default:
exit(1);
}
@@ -1724,5 +1900,11 @@ int main(int argc, char **argv)
if (dump_write)
write_dump(dump_write);
+ if (markers_read)
+ read_markers(markers_read);
+
+ if (markers_write)
+ write_markers(markers_write);
+
return err;
}
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 0ffed17..175301a 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -110,6 +110,8 @@ struct module {
int has_init;
int has_cleanup;
struct buffer dev_table_buf;
+ char **markers;
+ size_t nmarkers;
char srcversion[25];
};
@@ -124,6 +126,7 @@ struct elf_info {
Elf_Section export_gpl_sec;
Elf_Section export_unused_gpl_sec;
Elf_Section export_gpl_future_sec;
+ Elf_Section markers_strings_sec;
const char *strtab;
char *modinfo;
unsigned int modinfo_len;
* David Smith ([email protected]) wrote:
> Mathieu Desnoyers wrote:
> > * Roland McGrath ([email protected]) wrote:
> >>> If we want to do it safely, I think we should iterate from
> >>> __start___markers to __stop___markers symbols of vmlinux and get the
> >>> pointers to the name/format string pairs.
> >>>
> >>> The same can then be done with modules using the __markers section.
> >>>
> >>> Or maybe is there some reason not to do that ?
> >> It's just rather a pain in the ass, a whole lot more fiddly work.
> >> cf "somewhat crude" and "foreseeable future" in my patch's log entry.
> >> Knock yourself out if you're looking for more tedious hacking to do in
> >> modpost.c, but I say fix it when it breaks.
> >>
> >
> > Hmmmm, I have rarely seen code go into mainline without addressing valid
> > technical criticism first. Please fix.
> >
> > I'll look into it if I find the time.
> >
> > Mathieu
>
> Mathieu,
>
> Here's an updated patch, written by Roland (that I tested for him), that
> looks for all marker symbols in the __markers_strings section. It doesn't
> get the pointers from the __markers section because that is very difficult
> to do in modpost (having to handle the architecture-dependent relocations
> applied to those pointers).
>
Hrm, what would happen if a gcc optimization eventually decides to mix
the memory layout of the strings ? Is there something that specifies
that they won't ?
> See what you think.
>
> ---
> This adds some new magic in the MODPOST phase for CONFIG_MARKERS.
> Analogous to the Module.symvers file, the build will now write a
> Module.markers file when CONFIG_MARKERS=y is set. This file lists
> the name, defining module, and format string of each marker,
> separated by \t characters. This simple text file can be used by
> offline build procedures for instrumentation code, analogous to
> how System.map and Module.symvers can be useful to have for
> kernels other than the one you are running right now.
>
> The method of extracting the strings is somewhat crude, but is pretty
> simple and should work fine in practice for the foreseeable future.
>
> Signed-off-by: Roland McGrath <[email protected]>
> ---
> scripts/Makefile.modpost | 11 +++
> scripts/mod/modpost.c | 184 +++++++++++++++++++++++++++++++++++++++++++++-
> scripts/mod/modpost.h | 3 +
> 3 files changed, 197 insertions(+), 1 deletions(-)
>
> diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
> index d988f5d..6321870 100644
> --- a/scripts/Makefile.modpost
> +++ b/scripts/Makefile.modpost
> @@ -13,6 +13,7 @@
> # 2) modpost is then used to
> # 3) create one <module>.mod.c file pr. module
> # 4) create one Module.symvers file with CRC for all exported symbols
> +# 4a) [CONFIG_MARKERS] create one Module.markers file listing defined markers
> # 5) compile all <module>.mod.c files
> # 6) final link of the module to a <module.ko> file
>
> @@ -45,6 +46,10 @@ include scripts/Makefile.lib
>
> kernelsymfile := $(objtree)/Module.symvers
> modulesymfile := $(firstword $(KBUILD_EXTMOD))/Module.symvers
> +kernelmarkersfile := $(objtree)/Module.markers
> +modulemarkersfile := $(firstword $(KBUILD_EXTMOD))/Module.markers
> +
> +markersfile = $(if $(KBUILD_EXTMOD),$(modulemarkersfile),$(kernelmarkersfile))
>
> # Step 1), find all modules listed in $(MODVERDIR)/
> __modules := $(sort $(shell grep -h '\.ko' /dev/null $(wildcard $(MODVERDIR)/*.mod)))
> @@ -62,6 +67,8 @@ modpost = scripts/mod/modpost \
> $(if $(KBUILD_EXTMOD),-i,-o) $(kernelsymfile) \
> $(if $(KBUILD_EXTMOD),-I $(modulesymfile)) \
> $(if $(KBUILD_EXTMOD),-o $(modulesymfile)) \
> + $(if $(CONFIG_MARKERS),-K $(kernelmarkersfile)) \
> + $(if $(CONFIG_MARKERS),-M $(markersfile)) \
> $(if $(KBUILD_EXTMOD)$(KBUILD_MODPOST_WARN),-w)
>
> quiet_cmd_modpost = MODPOST $(words $(filter-out vmlinux FORCE, $^)) modules
> @@ -81,6 +88,10 @@ vmlinux.o: FORCE
> $(symverfile): __modpost ;
> $(modules:.ko=.mod.c): __modpost ;
>
> +ifdef CONFIG_MARKERS
> +$(markersfile): __modpost ;
> +endif
> +
>
> # Step 5), compile all *.mod.c files
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 93ac52a..bbaf26d 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -11,6 +11,8 @@
> * Usage: modpost vmlinux module1.o module2.o ...
> */
>
> +#define _GNU_SOURCE
> +#include <stdio.h>
> #include <ctype.h>
> #include "modpost.h"
> #include "../../include/linux/license.h"
> @@ -424,6 +426,8 @@ static int parse_elf(struct elf_info *info, const char *filename)
> info->export_unused_gpl_sec = i;
> else if (strcmp(secname, "__ksymtab_gpl_future") == 0)
> info->export_gpl_future_sec = i;
> + else if (strcmp(secname, "__markers_strings") == 0)
> + info->markers_strings_sec = i;
>
> if (sechdrs[i].sh_type != SHT_SYMTAB)
> continue;
> @@ -1249,6 +1253,83 @@ static int exit_section_ref_ok(const char *name)
> return 0;
> }
>
> +static void get_markers(struct elf_info *info, struct module *mod)
> +{
> + const Elf_Shdr *sh = &info->sechdrs[info->markers_strings_sec];
> + const char *strings = (const char *) info->hdr + sh->sh_offset;
> + const Elf_Sym *sym, *first_sym, *last_sym;
> + const char *name;
> + size_t n;
> +
> + if (!info->markers_strings_sec)
> + return;
> +
> + /*
> + * First count the strings. They come in pairs of name, format.
> + * We look for all the symbols defined in the __markers_strings
> + * section. They are named __mstrtab_name_* and __mstrtab_format_*
> + * in matching pairs. For these local names, the compiler puts
> + * a random .NNN suffix on, so the names don't correspond exactly.
> + */
> + first_sym = last_sym = NULL;
> + n = 0;
> + for (sym = info->symtab_start; sym < info->symtab_stop; sym++)
> + if (ELF_ST_TYPE(sym->st_info) == STT_OBJECT &&
> + sym->st_shndx == info->markers_strings_sec) {
> + if (first_sym == NULL)
> + first_sym = sym;
> + last_sym = sym;
> + ++n;
> + name = info->strtab + sym->st_name;
> + if (n % 2 == 0 ?
> + !strncmp(name, "__mstrtab_name_",
> + sizeof "__mstrtab_name_" - 1) :
> + !strncmp(name, "__mstrtab_format_",
> + sizeof "__mstrtab_format_" - 1)) {
> + warn("%s.ko has unexpected symbol \"%s\"\n",
> + mod->name, name);
> + first_sym = NULL;
> + }
> + }
> +
> + if (n % 2 != 0 || first_sym == NULL) {
> + warn("%s.ko has bad __markers_strings, ignoring it\n",
> + mod->name);
> + return;
> + }
> +
> + if (n == 0)
> + return;
> +
> + /*
> + * Now collect each pair into a formatted line for the output.
> + * Lines look like:
> + * marker_name vmlinux marker %s format %d
> + * The format string after the second \t can use whitespace.
> + */
> + n /= 2;
> + mod->markers = NOFAIL(malloc(sizeof mod->markers[0] * n));
> + mod->nmarkers = n;
> +
> + name = NULL;
> + n = 0;
> + for (sym = first_sym; sym <= last_sym; sym++)
> + if (ELF_ST_TYPE(sym->st_info) == STT_OBJECT &&
> + sym->st_shndx == info->markers_strings_sec) {
> + const char *str = strings + sym->st_value;
> + if (name == NULL)
> + name = str;
> + else {
> + char *line = NULL;
> + asprintf(&line, "%s\t%s\t%s\n",
> + name, mod->name, str);
> + NOFAIL(line);
> + mod->markers[n++] = line;
> + name = NULL;
> + }
> + }
> +}
> +
> static void read_symbols(char *modname)
> {
> const char *symname;
> @@ -1301,6 +1382,8 @@ static void read_symbols(char *modname)
> get_src_version(modname, mod->srcversion,
> sizeof(mod->srcversion)-1);
>
> + get_markers(&info, mod);
> +
> parse_elf_finish(&info);
>
> /* Our trick to get versioning for struct_module - it's
> @@ -1649,6 +1732,91 @@ static void write_dump(const char *fname)
> write_if_changed(&buf, fname);
> }
>
> +static void add_marker(struct module *mod, const char *name, const char *fmt)
> +{
> + char *line = NULL;
> + asprintf(&line, "%s\t%s\t%s\n", name, mod->name, fmt);
> + NOFAIL(line);
> +
> + mod->markers = NOFAIL(realloc(mod->markers, ((mod->nmarkers + 1) *
> + sizeof mod->markers[0])));
> + mod->markers[mod->nmarkers++] = line;
> +}
> +
> +static void read_markers(const char *fname)
> +{
> + unsigned long size, pos = 0;
> + void *file = grab_file(fname, &size);
> + char *line;
> +
> + if (!file)
> + /* No old markers, silently ignore */
> + return;
> +
> + while ((line = get_next_line(&pos, file, size))) {
> + char *marker, *modname, *fmt;
> + struct module *mod;
> +
> + marker = line;
> + if (!(modname = strchr(marker, '\t')))
> + goto fail;
> + *modname++ = '\0';
> + if (!(fmt = strchr(modname, '\t')))
> + goto fail;
> + *fmt++ = '\0';
> + if (*marker == '\0' || *modname == '\0')
> + goto fail;
> +
> + if (!(mod = find_module(modname))) {
> + if (is_vmlinux(modname)) {
> + have_vmlinux = 1;
> + }
> + mod = new_module(NOFAIL(strdup(modname)));
> + mod->skip = 1;
> + }
> +
> + add_marker(mod, marker, fmt);
> + }
> + return;
> +fail:
> + fatal("parse error in markers list file\n");
> +}
> +
> +static int compare_strings(const void *a, const void *b)
> +{
> + return strcmp(*(const char **) a, *(const char **) b);
> +}
> +
> +static void write_markers(const char *fname)
> +{
> + struct buffer buf = { };
> + struct module *mod;
> + size_t i;
> +
> + for (mod = modules; mod; mod = mod->next)
> + if ((!external_module || !mod->skip) && mod->markers != NULL) {
> + /*
> + * Sort the strings so we can skip duplicates when
> + * we write them out.
> + */
> + qsort(mod->markers, mod->nmarkers,
> + sizeof mod->markers[0], &compare_strings);
> + for (i = 0; i < mod->nmarkers; ++i) {
> + char *line = mod->markers[i];
> + buf_write(&buf, line, strlen(line));
> + while (i + 1 < mod->nmarkers &&
> + !strcmp(mod->markers[i],
> + mod->markers[i + 1]))
> + free(mod->markers[i++]);
> + free(mod->markers[i]);
> + }
> + free(mod->markers);
> + mod->markers = NULL;
> + }
> +
> + write_if_changed(&buf, fname);
> +}
> +
> int main(int argc, char **argv)
> {
> struct module *mod;
> @@ -1656,10 +1824,12 @@ int main(int argc, char **argv)
> char fname[SZ];
> char *kernel_read = NULL, *module_read = NULL;
> char *dump_write = NULL;
> + char *markers_read = NULL;
> + char *markers_write = NULL;
> int opt;
> int err;
>
> - while ((opt = getopt(argc, argv, "i:I:mso:aw")) != -1) {
> + while ((opt = getopt(argc, argv, "i:I:mso:awM:K:")) != -1) {
> switch(opt) {
> case 'i':
> kernel_read = optarg;
> @@ -1683,6 +1853,12 @@ int main(int argc, char **argv)
> case 'w':
> warn_unresolved = 1;
> break;
> + case 'M':
> + markers_write = optarg;
> + break;
> + case 'K':
> + markers_read = optarg;
> + break;
> default:
> exit(1);
> }
> @@ -1724,5 +1900,11 @@ int main(int argc, char **argv)
> if (dump_write)
> write_dump(dump_write);
>
> + if (markers_read)
> + read_markers(markers_read);
> +
> + if (markers_write)
> + write_markers(markers_write);
> +
> return err;
> }
> diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
> index 0ffed17..175301a 100644
> --- a/scripts/mod/modpost.h
> +++ b/scripts/mod/modpost.h
> @@ -110,6 +110,8 @@ struct module {
> int has_init;
> int has_cleanup;
> struct buffer dev_table_buf;
> + char **markers;
> + size_t nmarkers;
> char srcversion[25];
> };
>
> @@ -124,6 +126,7 @@ struct elf_info {
> Elf_Section export_gpl_sec;
> Elf_Section export_unused_gpl_sec;
> Elf_Section export_gpl_future_sec;
> + Elf_Section markers_strings_sec;
> const char *strtab;
> char *modinfo;
> unsigned int modinfo_len;
>
>
>
>
>
--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
Mathieu Desnoyers wrote:
> * David Smith ([email protected]) wrote:
>> Mathieu Desnoyers wrote:
>>> * Roland McGrath ([email protected]) wrote:
>>>>> If we want to do it safely, I think we should iterate from
>>>>> __start___markers to __stop___markers symbols of vmlinux and get the
>>>>> pointers to the name/format string pairs.
>>>>>
>>>>> The same can then be done with modules using the __markers section.
>>>>>
>>>>> Or maybe is there some reason not to do that ?
>>>> It's just rather a pain in the ass, a whole lot more fiddly work.
>>>> cf "somewhat crude" and "foreseeable future" in my patch's log entry.
>>>> Knock yourself out if you're looking for more tedious hacking to do in
>>>> modpost.c, but I say fix it when it breaks.
>>>>
>>> Hmmmm, I have rarely seen code go into mainline without addressing valid
>>> technical criticism first. Please fix.
>>>
>>> I'll look into it if I find the time.
>>>
>>> Mathieu
>> Mathieu,
>>
>> Here's an updated patch, written by Roland (that I tested for him), that
>> looks for all marker symbols in the __markers_strings section. It doesn't
>> get the pointers from the __markers section because that is very difficult
>> to do in modpost (having to handle the architecture-dependent relocations
>> applied to those pointers).
>>
>
> Hrm, what would happen if a gcc optimization eventually decides to mix
> the memory layout of the strings ? Is there something that specifies
> that they won't ?
I don't believe there is anything in gcc that specifies that the strings
won't get mixed around. But, I believe this code is good for the
foreseeable future. We could fix this code if the future breakage does
happen.
--
David Smith
[email protected]
Red Hat
http://www.redhat.com
256.217.0141 (direct)
256.837.0057 (fax)
Mathieu Desnoyers wrote:
> Hrm, what would happen if a gcc optimization eventually decides to mix
> the memory layout of the strings ? Is there something that specifies
> that they won't ?
Here's another patch that Roland wrote and I tested that
attempts to solve the potential problem of string ordering
by merging the name and format strings.
---
This adds some new magic in the MODPOST phase for CONFIG_MARKERS.
Analogous to the Module.symvers file, the build will now write a
Module.markers file when CONFIG_MARKERS=y is set. This file lists
the name, defining module, and format string of each marker,
separated by \t characters. This simple text file can be used by
offline build procedures for instrumentation code, analogous to
how System.map and Module.symvers can be useful to have for
kernels other than the one you are running right now.
The strings are made easy to extract by having the __trace_mark macro
define the name and format together in a single array called __mstrtab_*
in the __markers_strings section. This is straightforward and reliable
as long as the marker structs are always defined by this macro. It is
an unreasonable amount of hairy work to extract the string pointers from
the __markers section structs, which entails handling a relocation type
for every machine under the sun.
Signed-off-by: Roland McGrath <[email protected]>
---
include/linux/marker.h | 9 +--
scripts/Makefile.modpost | 11 +++
scripts/mod/modpost.c | 163 +++++++++++++++++++++++++++++++++++++++++++++-
scripts/mod/modpost.h | 3 +
4 files changed, 179 insertions(+), 7 deletions(-)
diff --git a/include/linux/marker.h b/include/linux/marker.h
index 5f36cf9..b978bbe 100644
--- a/include/linux/marker.h
+++ b/include/linux/marker.h
@@ -51,15 +51,12 @@ struct marker {
*/
#define __trace_mark(generic, name, call_data, format, args...) \
do { \
- static const char __mstrtab_name_##name[] \
+ static const char __mstrtab_##name[] \
__attribute__((section("__markers_strings"))) \
- = #name; \
- static const char __mstrtab_format_##name[] \
- __attribute__((section("__markers_strings"))) \
- = format; \
+ = #name "\0" format; \
static struct marker __mark_##name \
__attribute__((section("__markers"), aligned(8))) = \
- { __mstrtab_name_##name, __mstrtab_format_##name, \
+ { __mstrtab_##name, &__mstrtab_##name[sizeof(#name)], \
0, __mark_empty_function, NULL }; \
__mark_check_format(format, ## args); \
if (!generic) { \
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index d988f5d..6321870 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -13,6 +13,7 @@
# 2) modpost is then used to
# 3) create one <module>.mod.c file pr. module
# 4) create one Module.symvers file with CRC for all exported symbols
+# 4a) [CONFIG_MARKERS] create one Module.markers file listing defined markers
# 5) compile all <module>.mod.c files
# 6) final link of the module to a <module.ko> file
@@ -45,6 +46,10 @@ include scripts/Makefile.lib
kernelsymfile := $(objtree)/Module.symvers
modulesymfile := $(firstword $(KBUILD_EXTMOD))/Module.symvers
+kernelmarkersfile := $(objtree)/Module.markers
+modulemarkersfile := $(firstword $(KBUILD_EXTMOD))/Module.markers
+
+markersfile = $(if $(KBUILD_EXTMOD),$(modulemarkersfile),$(kernelmarkersfile))
# Step 1), find all modules listed in $(MODVERDIR)/
__modules := $(sort $(shell grep -h '\.ko' /dev/null $(wildcard $(MODVERDIR)/*.mod)))
@@ -62,6 +67,8 @@ modpost = scripts/mod/modpost \
$(if $(KBUILD_EXTMOD),-i,-o) $(kernelsymfile) \
$(if $(KBUILD_EXTMOD),-I $(modulesymfile)) \
$(if $(KBUILD_EXTMOD),-o $(modulesymfile)) \
+ $(if $(CONFIG_MARKERS),-K $(kernelmarkersfile)) \
+ $(if $(CONFIG_MARKERS),-M $(markersfile)) \
$(if $(KBUILD_EXTMOD)$(KBUILD_MODPOST_WARN),-w)
quiet_cmd_modpost = MODPOST $(words $(filter-out vmlinux FORCE, $^)) modules
@@ -81,6 +88,10 @@ vmlinux.o: FORCE
$(symverfile): __modpost ;
$(modules:.ko=.mod.c): __modpost ;
+ifdef CONFIG_MARKERS
+$(markersfile): __modpost ;
+endif
+
# Step 5), compile all *.mod.c files
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 93ac52a..53887e8 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -11,6 +11,8 @@
* Usage: modpost vmlinux module1.o module2.o ...
*/
+#define _GNU_SOURCE
+#include <stdio.h>
#include <ctype.h>
#include "modpost.h"
#include "../../include/linux/license.h"
@@ -424,6 +426,8 @@ static int parse_elf(struct elf_info *info, const char *filename)
info->export_unused_gpl_sec = i;
else if (strcmp(secname, "__ksymtab_gpl_future") == 0)
info->export_gpl_future_sec = i;
+ else if (strcmp(secname, "__markers_strings") == 0)
+ info->markers_strings_sec = i;
if (sechdrs[i].sh_type != SHT_SYMTAB)
continue;
@@ -1249,6 +1253,62 @@ static int exit_section_ref_ok(const char *name)
return 0;
}
+static void get_markers(struct elf_info *info, struct module *mod)
+{
+ const Elf_Shdr *sh = &info->sechdrs[info->markers_strings_sec];
+ const char *strings = (const char *) info->hdr + sh->sh_offset;
+ const Elf_Sym *sym, *first_sym, *last_sym;
+ size_t n;
+
+ if (!info->markers_strings_sec)
+ return;
+
+ /*
+ * First count the strings. We look for all the symbols defined
+ * in the __markers_strings section named __mstrtab_*. For
+ * these local names, the compiler puts a random .NNN suffix on,
+ * so the names don't correspond exactly.
+ */
+ first_sym = last_sym = NULL;
+ n = 0;
+ for (sym = info->symtab_start; sym < info->symtab_stop; sym++)
+ if (ELF_ST_TYPE(sym->st_info) == STT_OBJECT &&
+ sym->st_shndx == info->markers_strings_sec &&
+ !strncmp(info->strtab + sym->st_name,
+ "__mstrtab_", sizeof "__mstrtab_" - 1)) {
+ if (first_sym == NULL)
+ first_sym = sym;
+ last_sym = sym;
+ ++n;
+ }
+
+ if (n == 0)
+ return;
+
+ /*
+ * Now collect each name and format into a line for the output.
+ * Lines look like:
+ * marker_name vmlinux marker %s format %d
+ * The format string after the second \t can use whitespace.
+ */
+ mod->markers = NOFAIL(malloc(sizeof mod->markers[0] * n));
+ mod->nmarkers = n;
+
+ n = 0;
+ for (sym = first_sym; sym <= last_sym; sym++)
+ if (ELF_ST_TYPE(sym->st_info) == STT_OBJECT &&
+ sym->st_shndx == info->markers_strings_sec &&
+ !strncmp(info->strtab + sym->st_name,
+ "__mstrtab_", sizeof "__mstrtab_" - 1)) {
+ const char *name = strings + sym->st_value;
+ const char *fmt = strchr(name, '\0') + 1;
+ char *line = NULL;
+ asprintf(&line, "%s\t%s\t%s\n", name, mod->name, fmt);
+ NOFAIL(line);
+ mod->markers[n++] = line;
+ }
+}
+
static void read_symbols(char *modname)
{
const char *symname;
@@ -1301,6 +1361,8 @@ static void read_symbols(char *modname)
get_src_version(modname, mod->srcversion,
sizeof(mod->srcversion)-1);
+ get_markers(&info, mod);
+
parse_elf_finish(&info);
/* Our trick to get versioning for struct_module - it's
@@ -1649,6 +1711,91 @@ static void write_dump(const char *fname)
write_if_changed(&buf, fname);
}
+static void add_marker(struct module *mod, const char *name, const char *fmt)
+{
+ char *line = NULL;
+ asprintf(&line, "%s\t%s\t%s\n", name, mod->name, fmt);
+ NOFAIL(line);
+
+ mod->markers = NOFAIL(realloc(mod->markers, ((mod->nmarkers + 1) *
+ sizeof mod->markers[0])));
+ mod->markers[mod->nmarkers++] = line;
+}
+
+static void read_markers(const char *fname)
+{
+ unsigned long size, pos = 0;
+ void *file = grab_file(fname, &size);
+ char *line;
+
+ if (!file)
+ /* No old markers, silently ignore */
+ return;
+
+ while ((line = get_next_line(&pos, file, size))) {
+ char *marker, *modname, *fmt;
+ struct module *mod;
+
+ marker = line;
+ if (!(modname = strchr(marker, '\t')))
+ goto fail;
+ *modname++ = '\0';
+ if (!(fmt = strchr(modname, '\t')))
+ goto fail;
+ *fmt++ = '\0';
+ if (*marker == '\0' || *modname == '\0')
+ goto fail;
+
+ if (!(mod = find_module(modname))) {
+ if (is_vmlinux(modname)) {
+ have_vmlinux = 1;
+ }
+ mod = new_module(NOFAIL(strdup(modname)));
+ mod->skip = 1;
+ }
+
+ add_marker(mod, marker, fmt);
+ }
+ return;
+fail:
+ fatal("parse error in markers list file\n");
+}
+
+static int compare_strings(const void *a, const void *b)
+{
+ return strcmp(*(const char **) a, *(const char **) b);
+}
+
+static void write_markers(const char *fname)
+{
+ struct buffer buf = { };
+ struct module *mod;
+ size_t i;
+
+ for (mod = modules; mod; mod = mod->next)
+ if ((!external_module || !mod->skip) && mod->markers != NULL) {
+ /*
+ * Sort the strings so we can skip duplicates when
+ * we write them out.
+ */
+ qsort(mod->markers, mod->nmarkers,
+ sizeof mod->markers[0], &compare_strings);
+ for (i = 0; i < mod->nmarkers; ++i) {
+ char *line = mod->markers[i];
+ buf_write(&buf, line, strlen(line));
+ while (i + 1 < mod->nmarkers &&
+ !strcmp(mod->markers[i],
+ mod->markers[i + 1]))
+ free(mod->markers[i++]);
+ free(mod->markers[i]);
+ }
+ free(mod->markers);
+ mod->markers = NULL;
+ }
+
+ write_if_changed(&buf, fname);
+}
+
int main(int argc, char **argv)
{
struct module *mod;
@@ -1656,10 +1803,12 @@ int main(int argc, char **argv)
char fname[SZ];
char *kernel_read = NULL, *module_read = NULL;
char *dump_write = NULL;
+ char *markers_read = NULL;
+ char *markers_write = NULL;
int opt;
int err;
- while ((opt = getopt(argc, argv, "i:I:mso:aw")) != -1) {
+ while ((opt = getopt(argc, argv, "i:I:mso:awM:K:")) != -1) {
switch(opt) {
case 'i':
kernel_read = optarg;
@@ -1683,6 +1832,12 @@ int main(int argc, char **argv)
case 'w':
warn_unresolved = 1;
break;
+ case 'M':
+ markers_write = optarg;
+ break;
+ case 'K':
+ markers_read = optarg;
+ break;
default:
exit(1);
}
@@ -1724,5 +1879,11 @@ int main(int argc, char **argv)
if (dump_write)
write_dump(dump_write);
+ if (markers_read)
+ read_markers(markers_read);
+
+ if (markers_write)
+ write_markers(markers_write);
+
return err;
}
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index 0ffed17..175301a 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -110,6 +110,8 @@ struct module {
int has_init;
int has_cleanup;
struct buffer dev_table_buf;
+ char **markers;
+ size_t nmarkers;
char srcversion[25];
};
@@ -124,6 +126,7 @@ struct elf_info {
Elf_Section export_gpl_sec;
Elf_Section export_unused_gpl_sec;
Elf_Section export_gpl_future_sec;
+ Elf_Section markers_strings_sec;
const char *strtab;
char *modinfo;
unsigned int modinfo_len;
* David Smith ([email protected]) wrote:
> Mathieu Desnoyers wrote:
> > Hrm, what would happen if a gcc optimization eventually decides to mix
> > the memory layout of the strings ? Is there something that specifies
> > that they won't ?
>
> Here's another patch that Roland wrote and I tested that
> attempts to solve the potential problem of string ordering
> by merging the name and format strings.
>
Yup, it looks good. I'll give it a try.
Thanks!
Mathieu
> ---
> This adds some new magic in the MODPOST phase for CONFIG_MARKERS.
> Analogous to the Module.symvers file, the build will now write a
> Module.markers file when CONFIG_MARKERS=y is set. This file lists
> the name, defining module, and format string of each marker,
> separated by \t characters. This simple text file can be used by
> offline build procedures for instrumentation code, analogous to
> how System.map and Module.symvers can be useful to have for
> kernels other than the one you are running right now.
>
> The strings are made easy to extract by having the __trace_mark macro
> define the name and format together in a single array called __mstrtab_*
> in the __markers_strings section. This is straightforward and reliable
> as long as the marker structs are always defined by this macro. It is
> an unreasonable amount of hairy work to extract the string pointers from
> the __markers section structs, which entails handling a relocation type
> for every machine under the sun.
>
> Signed-off-by: Roland McGrath <[email protected]>
> ---
> include/linux/marker.h | 9 +--
> scripts/Makefile.modpost | 11 +++
> scripts/mod/modpost.c | 163 +++++++++++++++++++++++++++++++++++++++++++++-
> scripts/mod/modpost.h | 3 +
> 4 files changed, 179 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/marker.h b/include/linux/marker.h
> index 5f36cf9..b978bbe 100644
> --- a/include/linux/marker.h
> +++ b/include/linux/marker.h
> @@ -51,15 +51,12 @@ struct marker {
> */
> #define __trace_mark(generic, name, call_data, format, args...) \
> do { \
> - static const char __mstrtab_name_##name[] \
> + static const char __mstrtab_##name[] \
> __attribute__((section("__markers_strings"))) \
> - = #name; \
> - static const char __mstrtab_format_##name[] \
> - __attribute__((section("__markers_strings"))) \
> - = format; \
> + = #name "\0" format; \
> static struct marker __mark_##name \
> __attribute__((section("__markers"), aligned(8))) = \
> - { __mstrtab_name_##name, __mstrtab_format_##name, \
> + { __mstrtab_##name, &__mstrtab_##name[sizeof(#name)], \
> 0, __mark_empty_function, NULL }; \
> __mark_check_format(format, ## args); \
> if (!generic) { \
> diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
> index d988f5d..6321870 100644
> --- a/scripts/Makefile.modpost
> +++ b/scripts/Makefile.modpost
> @@ -13,6 +13,7 @@
> # 2) modpost is then used to
> # 3) create one <module>.mod.c file pr. module
> # 4) create one Module.symvers file with CRC for all exported symbols
> +# 4a) [CONFIG_MARKERS] create one Module.markers file listing defined markers
> # 5) compile all <module>.mod.c files
> # 6) final link of the module to a <module.ko> file
>
> @@ -45,6 +46,10 @@ include scripts/Makefile.lib
>
> kernelsymfile := $(objtree)/Module.symvers
> modulesymfile := $(firstword $(KBUILD_EXTMOD))/Module.symvers
> +kernelmarkersfile := $(objtree)/Module.markers
> +modulemarkersfile := $(firstword $(KBUILD_EXTMOD))/Module.markers
> +
> +markersfile = $(if $(KBUILD_EXTMOD),$(modulemarkersfile),$(kernelmarkersfile))
>
> # Step 1), find all modules listed in $(MODVERDIR)/
> __modules := $(sort $(shell grep -h '\.ko' /dev/null $(wildcard $(MODVERDIR)/*.mod)))
> @@ -62,6 +67,8 @@ modpost = scripts/mod/modpost \
> $(if $(KBUILD_EXTMOD),-i,-o) $(kernelsymfile) \
> $(if $(KBUILD_EXTMOD),-I $(modulesymfile)) \
> $(if $(KBUILD_EXTMOD),-o $(modulesymfile)) \
> + $(if $(CONFIG_MARKERS),-K $(kernelmarkersfile)) \
> + $(if $(CONFIG_MARKERS),-M $(markersfile)) \
> $(if $(KBUILD_EXTMOD)$(KBUILD_MODPOST_WARN),-w)
>
> quiet_cmd_modpost = MODPOST $(words $(filter-out vmlinux FORCE, $^)) modules
> @@ -81,6 +88,10 @@ vmlinux.o: FORCE
> $(symverfile): __modpost ;
> $(modules:.ko=.mod.c): __modpost ;
>
> +ifdef CONFIG_MARKERS
> +$(markersfile): __modpost ;
> +endif
> +
>
> # Step 5), compile all *.mod.c files
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 93ac52a..53887e8 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -11,6 +11,8 @@
> * Usage: modpost vmlinux module1.o module2.o ...
> */
>
> +#define _GNU_SOURCE
> +#include <stdio.h>
> #include <ctype.h>
> #include "modpost.h"
> #include "../../include/linux/license.h"
> @@ -424,6 +426,8 @@ static int parse_elf(struct elf_info *info, const char *filename)
> info->export_unused_gpl_sec = i;
> else if (strcmp(secname, "__ksymtab_gpl_future") == 0)
> info->export_gpl_future_sec = i;
> + else if (strcmp(secname, "__markers_strings") == 0)
> + info->markers_strings_sec = i;
>
> if (sechdrs[i].sh_type != SHT_SYMTAB)
> continue;
> @@ -1249,6 +1253,62 @@ static int exit_section_ref_ok(const char *name)
> return 0;
> }
>
> +static void get_markers(struct elf_info *info, struct module *mod)
> +{
> + const Elf_Shdr *sh = &info->sechdrs[info->markers_strings_sec];
> + const char *strings = (const char *) info->hdr + sh->sh_offset;
> + const Elf_Sym *sym, *first_sym, *last_sym;
> + size_t n;
> +
> + if (!info->markers_strings_sec)
> + return;
> +
> + /*
> + * First count the strings. We look for all the symbols defined
> + * in the __markers_strings section named __mstrtab_*. For
> + * these local names, the compiler puts a random .NNN suffix on,
> + * so the names don't correspond exactly.
> + */
> + first_sym = last_sym = NULL;
> + n = 0;
> + for (sym = info->symtab_start; sym < info->symtab_stop; sym++)
> + if (ELF_ST_TYPE(sym->st_info) == STT_OBJECT &&
> + sym->st_shndx == info->markers_strings_sec &&
> + !strncmp(info->strtab + sym->st_name,
> + "__mstrtab_", sizeof "__mstrtab_" - 1)) {
> + if (first_sym == NULL)
> + first_sym = sym;
> + last_sym = sym;
> + ++n;
> + }
> +
> + if (n == 0)
> + return;
> +
> + /*
> + * Now collect each name and format into a line for the output.
> + * Lines look like:
> + * marker_name vmlinux marker %s format %d
> + * The format string after the second \t can use whitespace.
> + */
> + mod->markers = NOFAIL(malloc(sizeof mod->markers[0] * n));
> + mod->nmarkers = n;
> +
> + n = 0;
> + for (sym = first_sym; sym <= last_sym; sym++)
> + if (ELF_ST_TYPE(sym->st_info) == STT_OBJECT &&
> + sym->st_shndx == info->markers_strings_sec &&
> + !strncmp(info->strtab + sym->st_name,
> + "__mstrtab_", sizeof "__mstrtab_" - 1)) {
> + const char *name = strings + sym->st_value;
> + const char *fmt = strchr(name, '\0') + 1;
> + char *line = NULL;
> + asprintf(&line, "%s\t%s\t%s\n", name, mod->name, fmt);
> + NOFAIL(line);
> + mod->markers[n++] = line;
> + }
> +}
> +
> static void read_symbols(char *modname)
> {
> const char *symname;
> @@ -1301,6 +1361,8 @@ static void read_symbols(char *modname)
> get_src_version(modname, mod->srcversion,
> sizeof(mod->srcversion)-1);
>
> + get_markers(&info, mod);
> +
> parse_elf_finish(&info);
>
> /* Our trick to get versioning for struct_module - it's
> @@ -1649,6 +1711,91 @@ static void write_dump(const char *fname)
> write_if_changed(&buf, fname);
> }
>
> +static void add_marker(struct module *mod, const char *name, const char *fmt)
> +{
> + char *line = NULL;
> + asprintf(&line, "%s\t%s\t%s\n", name, mod->name, fmt);
> + NOFAIL(line);
> +
> + mod->markers = NOFAIL(realloc(mod->markers, ((mod->nmarkers + 1) *
> + sizeof mod->markers[0])));
> + mod->markers[mod->nmarkers++] = line;
> +}
> +
> +static void read_markers(const char *fname)
> +{
> + unsigned long size, pos = 0;
> + void *file = grab_file(fname, &size);
> + char *line;
> +
> + if (!file)
> + /* No old markers, silently ignore */
> + return;
> +
> + while ((line = get_next_line(&pos, file, size))) {
> + char *marker, *modname, *fmt;
> + struct module *mod;
> +
> + marker = line;
> + if (!(modname = strchr(marker, '\t')))
> + goto fail;
> + *modname++ = '\0';
> + if (!(fmt = strchr(modname, '\t')))
> + goto fail;
> + *fmt++ = '\0';
> + if (*marker == '\0' || *modname == '\0')
> + goto fail;
> +
> + if (!(mod = find_module(modname))) {
> + if (is_vmlinux(modname)) {
> + have_vmlinux = 1;
> + }
> + mod = new_module(NOFAIL(strdup(modname)));
> + mod->skip = 1;
> + }
> +
> + add_marker(mod, marker, fmt);
> + }
> + return;
> +fail:
> + fatal("parse error in markers list file\n");
> +}
> +
> +static int compare_strings(const void *a, const void *b)
> +{
> + return strcmp(*(const char **) a, *(const char **) b);
> +}
> +
> +static void write_markers(const char *fname)
> +{
> + struct buffer buf = { };
> + struct module *mod;
> + size_t i;
> +
> + for (mod = modules; mod; mod = mod->next)
> + if ((!external_module || !mod->skip) && mod->markers != NULL) {
> + /*
> + * Sort the strings so we can skip duplicates when
> + * we write them out.
> + */
> + qsort(mod->markers, mod->nmarkers,
> + sizeof mod->markers[0], &compare_strings);
> + for (i = 0; i < mod->nmarkers; ++i) {
> + char *line = mod->markers[i];
> + buf_write(&buf, line, strlen(line));
> + while (i + 1 < mod->nmarkers &&
> + !strcmp(mod->markers[i],
> + mod->markers[i + 1]))
> + free(mod->markers[i++]);
> + free(mod->markers[i]);
> + }
> + free(mod->markers);
> + mod->markers = NULL;
> + }
> +
> + write_if_changed(&buf, fname);
> +}
> +
> int main(int argc, char **argv)
> {
> struct module *mod;
> @@ -1656,10 +1803,12 @@ int main(int argc, char **argv)
> char fname[SZ];
> char *kernel_read = NULL, *module_read = NULL;
> char *dump_write = NULL;
> + char *markers_read = NULL;
> + char *markers_write = NULL;
> int opt;
> int err;
>
> - while ((opt = getopt(argc, argv, "i:I:mso:aw")) != -1) {
> + while ((opt = getopt(argc, argv, "i:I:mso:awM:K:")) != -1) {
> switch(opt) {
> case 'i':
> kernel_read = optarg;
> @@ -1683,6 +1832,12 @@ int main(int argc, char **argv)
> case 'w':
> warn_unresolved = 1;
> break;
> + case 'M':
> + markers_write = optarg;
> + break;
> + case 'K':
> + markers_read = optarg;
> + break;
> default:
> exit(1);
> }
> @@ -1724,5 +1879,11 @@ int main(int argc, char **argv)
> if (dump_write)
> write_dump(dump_write);
>
> + if (markers_read)
> + read_markers(markers_read);
> +
> + if (markers_write)
> + write_markers(markers_write);
> +
> return err;
> }
> diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
> index 0ffed17..175301a 100644
> --- a/scripts/mod/modpost.h
> +++ b/scripts/mod/modpost.h
> @@ -110,6 +110,8 @@ struct module {
> int has_init;
> int has_cleanup;
> struct buffer dev_table_buf;
> + char **markers;
> + size_t nmarkers;
> char srcversion[25];
> };
>
> @@ -124,6 +126,7 @@ struct elf_info {
> Elf_Section export_gpl_sec;
> Elf_Section export_unused_gpl_sec;
> Elf_Section export_gpl_future_sec;
> + Elf_Section markers_strings_sec;
> const char *strtab;
> char *modinfo;
> unsigned int modinfo_len;
>
>
--
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68