View Bug Activity | Format For Printing
*** Bug 4301 has been marked as a duplicate of this bug. ***
*** Bug 1322 has been marked as a duplicate of this bug. ***
Created an attachment (id=2120) Fix markers. I've had to modify the code in tapsets.cxx to work correctly with the markers functionality in Linux 2.6.24-rc3. Based on git logs from the kernel, this seems to be the same interface of 2.6.22, so I assume the code has always been broken. This patch fixes markers for me.
Maybe I should elaborate a bit more on how it is broken and what the patch does. Whenever I build a script that uses kernel.mark() (and I have the kernel patched to generate a Module.makers file), the resulting code does not build against a recent kernel. This is, basically, because two problems: 1. marker_probe_register is passed an incompatible function pointer to its third argument. 2. marker_probe_unregister is called with an incorrect number of arguments. The first of the two is specially problematic because the registered function does not follow the expected calling conversions of the maker's code. But that is only raised as a warning by the compiler. Then, the code does not use marker_arm to activate the registered marker, so even fixing the above does not put the marker code to use. What I have done is, first, adjust the two calls that caused build problems to their expected usage. I also fixed the generated enter_marker_probe function to properly get the marker-specific data using marker_get_private_data and to properly deal with its variable number of arguments. And second, I added calls to marker_arm and marker_disarm to respectively activate and deactivate the markers. (According to some sample code in the kernel's samples/markers directory, the calls to marker_disarm do not seem to be needed, but I felt they were good to add for consistency.)
I think what's going on here is that dsmith had already updated the code to the next anticipated version of the marker api - supporting multiple handlers, and removing the "arm/disarm" functions. This means it is unfortunately incompatible with the current linus tree. Perhaps what should happen is that the code should support both versions of the API, and switch between automatically them with a runtime/autoconf-* style check.
(In reply to comment #5) > I think what's going on here is that dsmith had already updated the code to the > next anticipated version of the marker api - supporting multiple handlers, and > removing the "arm/disarm" functions. This means it is unfortunately incompatible > with the current linus tree. That is correct. Systemtap supports the latest markers patches from the lttng project (<http://ltt.polymtl.ca/>). I've tested the latest cvs systemtap with <http://ltt.polymtl.ca/lttng/patch-2.6.24-rc2-lttng-0.10-pre23.tar.bz2> and <http://ltt.polymtl.ca/lttng/patch-2.6.24-rc3-git1-lttng-0.10-pre33.tar.bz2> The current version appears to be: <http://ltt.polymtl.ca/lttng/patch-2.6.24-rc3-git3-lttng-0.10-pre36.tar.bz2> > Perhaps what should happen is that the code should support both versions of the > API, and switch between automatically them with a runtime/autoconf-* style check. It would be possible to support multiple versions of the API with enough work. However, I'm not sure it would be wise with the amount of change markers seem to go through. I'm also not sure we want to do this for a prerelease kernel.
> It would be possible to support multiple versions of the API with enough work. > However, I'm not sure it would be wise with the amount of change markers seem to > go through. I'm also not sure we want to do this for a prerelease kernel. If it's only a day or two's work, it's IMO worth it. A preprelease kernel is a fine support target. That's how early adopters outside the ltt/systemtap developer community get to use this stuff.
Linux 2.6.24 went out the door, and systemtap's marker support doesn't work with it but rather with the -mm (2.6.25-bound?) extensions. One extra feature worth adding in the translator would be to expose the formatting string associated with the marker, perhaps as "$format". Even if it could not be used directly as a printf* string literal, it could be useful self-documentation. (See also bug #5679.)
> One extra feature worth adding in the translator would be to > expose the formatting string associated with the marker, perhaps > as "$format". Mathieu implied that for those marker format strings that include a name before a %FOO format code, we could also expose those names for the same parameters. So parameters trace_mark (something, "foo %p bar %d", a, b) could show up in systemtap with two names: kernel.mark("something") { $arg1 == $foo; $arg2 == $bar }
(In reply to comment #9) > Mathieu implied that for those marker format strings that include a > name before a %FOO format code, we could also expose those names > for the same parameters. That's an interesting idea. Parsing the format string could get tricky, since it is only by convention that a string before a %FOO format code is a name. For instance, the following is a perfectly valid marker: trace_mark (something, "foo[] %p 3bar %d", a, b); However, '$foo[]' and (perhaps) '$3bar' wouldn't be valid systemtap identifiers. In addition, you can also do stuff like these: // first "name" has an embedded space trace_mark (something, "foo pointer %p bar %d", a, b); // duplicated "name" trace_mark (something, "foo %p foo %d", a, b); To support this, we'd have to come up with a set of rules to handle odd situations.
FYI, there is a markers wiki page at: <http://sourceware.org/systemtap/wiki/UsingMarkers>
(In reply to comment #8) > One extra feature worth adding in the translator would be to > expose the formatting string associated with the marker, perhaps > as "$format". Even if it could not be used directly as a printf* > string literal, it could be useful self-documentation. (See also > bug #5679.) I've added support for "$format".
already done