Linux Kernel Markers for 2.6.23-rc6-mm1

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
===================================================================

include/asm-generic/vmlinux.lds.h +5 -1

@@ -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) = .;			\

Quoted 16 lines

Index: linux-2.6-lttng/include/asm-generic/vmlinux.lds.h
===================================================================

include/asm-generic/vmlinux.lds.h +5 -0

@@ -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 ?

Quoted 18 lines · Mathieu Desnoyers

Quoted 16 lines

Index: linux-2.6-lttng/include/asm-generic/vmlinux.lds.h
===================================================================

include/asm-generic/vmlinux.lds.h +5 -0

@@ -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)					\

[...]

Quoted 1 line

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

Quoted 33 lines · * Denys Vlasenko ([email protected]) wrote:

Quoted 18 lines · Mathieu Desnoyers

Quoted 16 lines

Index: linux-2.6-lttng/include/asm-generic/vmlinux.lds.h
===================================================================

include/asm-generic/vmlinux.lds.h +5 -0

@@ -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)					\

[...]

Quoted 1 line

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.

Quoted 8 lines

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

[...] Do you think I should also remove the __markers_strings section from here ?

Quoted 2 lines · Mathieu Desnoyers

[...] Do you think I should also remove the __markers_strings section from here ?

Current systemtap marker support code relies on the __markers_strings section.

Quoted 7 lines · Frank Ch. Eigler

Quoted 2 lines · Mathieu Desnoyers

[...] 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

Ok, I am changing it to:

[...]

Quoted 3 lines

Quoted 2 lines

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) */                      \

[...]

Hi -

Quoted 7 lines · Mathieu Desnoyers

[...]

Quoted 3 lines

Quoted 2 lines

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?

[...]

Quoted 5 lines

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. [...]

Hi -

Quoted 6 lines · I wrote:

[...]

Quoted 5 lines

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

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.

Quoted 2 lines

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.

Quoted 4 lines

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

[...]
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?

I would rather prefer not to implement superfluous built-time data extraction in the kernel build system just to make userspace simpler. [...]

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(-)

scripts/Makefile.modpost +11 -0

index d988f5d..6321870 100644
@@ -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
 

scripts/mod/modpost.c +173 -1

index 93ac52a..df80bfc 100644
@@ -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;
 }

scripts/mod/modpost.h +3 -0

index 0ffed17..175301a 100644
@@ -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;

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 ?

Quoted 4 lines

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.

Quoted 3 lines

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

Quoted 4 lines · * Roland McGrath ([email protected]) wrote:

Quoted 4 lines

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.

Quoted 8 lines

Quoted 3 lines

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

Quoted 14 lines · Mathieu Desnoyers

Quoted 4 lines · * Roland McGrath ([email protected]) wrote:

Quoted 4 lines

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.

Quoted 8 lines

Quoted 3 lines

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.

Quoted 6 lines

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(-)

scripts/Makefile.modpost +11 -0

index d988f5d..6321870 100644
@@ -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
 

scripts/mod/modpost.c +183 -1

index 93ac52a..bbaf26d 100644
@@ -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;
 }

scripts/mod/modpost.h +3 -0

index 0ffed17..175301a 100644
@@ -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;

Quoted 19 lines · * David Smith ([email protected]) wrote:

Quoted 18 lines · Mathieu Desnoyers

Quoted 4 lines · * Roland McGrath ([email protected]) wrote:

Quoted 4 lines

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.

Quoted 8 lines

Quoted 3 lines

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.

Quoted 9 lines

Quoted 1 line

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 ?

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 ?

Quoted 3 lines · Mathieu Desnoyers

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(-)

include/linux/marker.h +3 -6

index 5f36cf9..b978bbe 100644
@@ -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) {						\

scripts/Makefile.modpost +11 -0

index d988f5d..6321870 100644
@@ -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
 

scripts/mod/modpost.c +162 -1

index 93ac52a..53887e8 100644
@@ -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;
 }

scripts/mod/modpost.h +3 -0

index 0ffed17..175301a 100644
@@ -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;

kernel/marker.c new file +21 -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;
+}

kernel/marker.c line 214

Quoted 8 lines · Mathieu Desnoyers

kernel/marker.c new file +8 -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);

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

Quoted 21 lines

kernel/marker.c new file +21 -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;
+}

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

---

 Documentation/markers/markers.txt          |   93 +++++++++++++++++++++++++++
 Documentation/markers/src/Makefile         |    7 ++
 Documentation/markers/src/marker-example.c |   55 ++++++++++++++++
 Documentation/markers/src/probe-example.c  |   98 +++++++++++++++++++++++++++++
 4 files changed, 253 insertions(+)

Index: linux-2.6-lttng/Documentation/markers/markers.txt
===================================================================

Quoted 16 lines · On Tue, 18 Sep 2007 17:13:27 -0400 Mathieu Desnoyers wrote:

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

---

 Documentation/markers/markers.txt          |   93 +++++++++++++++++++++++++++
 Documentation/markers/src/Makefile         |    7 ++
 Documentation/markers/src/marker-example.c |   55 ++++++++++++++++
 Documentation/markers/src/probe-example.c  |   98 +++++++++++++++++++++++++++++
 4 files changed, 253 insertions(+)

Index: linux-2.6-lttng/Documentation/markers/markers.txt
===================================================================

Documentation/markers/markers.txt new file +0 -0

@@ -0,0 +1,93 @@

Documentation/markers/markers.txt line 60

Quoted 3 lines

Documentation/markers/markers.txt new file +3 -0

+The marker mechanism supports inserting multiple instances of the same marker.
+Markers can be put in inline functions, inlined static functions, and
+unrolled loops.

as well as regular functions ?

Documentation/markers/markers.txt line 84

Quoted 3 lines

Documentation/markers/markers.txt new file +3 -0

+* Probe / marker example
+
+See the example provided in Documentation/markers/markers/src

drop one of ^^^ "markers/"

Documentation/markers/markers.txt line 93

Quoted 8 lines

Documentation/markers/markers.txt new file +8 -0

+Run, as root :
+
+make
+insmod marker-example.ko (insmod order is not important)
+insmod probe-example.ko
+cat /proc/marker-example (returns an expected error)
+rmmod marker-example probe-example
+dmesg

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

block/blktrace.c static void blk_trace_set_ht_offsets(voi · line 889

Quoted 11 lines · Mathieu Desnoyers

block/blktrace.c +11 -0

+void blk_probe_disarm(void)
+{
+	int i, err;
+
+	for (i = 0; i < ARRAY_SIZE(probe_array); i++) {
+		err = marker_disarm(probe_array[i].name);
+		BUG_ON(err);
+		err = IS_ERR(marker_probe_unregister(probe_array[i].name));
+		BUG_ON(err);
+	}
+}

As well as changing these to WARN_ON.

-- Steve