[ltt-dev] [UST PATCH] Markers: remove channel name from trace_mark()
Nils Carlson
nils.carlson at ericsson.com
Mon Apr 11 07:58:07 EDT 2011
Hi,
This patch looks good. My only thought is, should we rename at the same
time, so maybe trace_mark -> ust_mark ? It would in some ways make more
sense to have an api where everything is prefixed with ust_ and not trace_
?
/Nils
On Mon, 11 Apr 2011, Mathieu Desnoyers wrote:
> *** This is an instrumentation API change ***
>
> Given that UST will gradually move to a scheme where channels are
> dynamically associated with markers on a per tracing session basis (and
> thus associated dynamically rather than fixed statically), it does not
> make sense to specify the "channel name" in addition to the marker name
> in the trace_mark() arguments.
>
> API touched:
>
> GET_MARKER()
> DEFINE_MARKER()
> DEFINE_MARKER_TP()
> trace_mark()
> _trace_mark()
>
> I'm introducing this API change without changing the underlying
> implementation, trying to minimize the impact of API changes by doing
> them sooner than later.
>
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
> ---
> include/ust/marker.h | 35 ++++++--------
> libust/marker.c | 6 +-
> libust/tracectl.c | 2
> libustinstr-malloc/mallocwrap.c | 4 -
> tests/basic/basic.c | 4 -
> tests/basic_long/basic_long.c | 4 -
> tests/benchmark/bench.c | 2
> tests/dlopen/dlopen.c | 4 -
> tests/dlopen/libdummy.c | 2
> tests/fork/fork.c | 10 ++--
> tests/fork/fork2.c | 2
> tests/hello/hello.c | 4 -
> tests/hello2/hello2.c | 4 -
> tests/libustctl_function_tests/libustctl_function_tests.c | 4 -
> tests/make_shared_lib/basic_lib.c | 2
> tests/make_shared_lib/prog.c | 2
> tests/same_line_marker/same_line_marker.c | 2
> tests/test-nevents/prog.c | 4 -
> tests/tracepoint/benchmark/tracepoint_benchmark.c | 4 -
> tests/tracepoint/tracepoint_test.c | 8 +--
>
> diff --git a/include/ust/marker.h b/include/ust/marker.h
> index 29e84cc..da738d1 100644
> --- a/include/ust/marker.h
> +++ b/include/ust/marker.h
> @@ -77,7 +77,7 @@ struct marker {
> void *location; /* Address of marker in code */
> };
>
> -#define GET_MARKER(channel, name) (__mark_##channel##_##name)
> +#define GET_MARKER(name) (__mark_ust_##name)
>
> #define _DEFINE_MARKER(channel, name, tp_name_str, tp_cb, format, unique, m) \
> struct registers __marker_regs; \
> @@ -135,17 +135,17 @@ struct marker {
> save_registers(&__marker_regs)
>
>
> -#define DEFINE_MARKER(channel, name, format, unique, m) \
> - _DEFINE_MARKER(channel, name, NULL, NULL, format, unique, m)
> +#define DEFINE_MARKER(name, format, unique, m) \
> + _DEFINE_MARKER(ust, name, NULL, NULL, format, unique, m)
>
> -#define DEFINE_MARKER_TP(channel, name, tp_name, tp_cb, format) \
> - _DEFINE_MARKER_TP(channel, name, #tp_name, tp_cb, format)
> +#define DEFINE_MARKER_TP(name, tp_name, tp_cb, format) \
> + _DEFINE_MARKER_TP(ust, name, #tp_name, tp_cb, format)
>
> #define _DEFINE_MARKER_TP(channel, name, tp_name_str, tp_cb, format) \
> static const char __mstrtab_##channel##_##name[] \
> __attribute__((section("__markers_strings"))) \
> = #channel "\0" #name "\0" format; \
> - static struct marker GET_MARKER(channel, name) \
> + static struct marker __mark_##channel##_##name \
> __attribute__((section("__markers"))) = \
> { __mstrtab_##channel##_##name, \
> &__mstrtab_##channel##_##name[sizeof(#channel)], \
> @@ -155,7 +155,7 @@ struct marker {
> NULL, tp_name_str, tp_cb }; \
> static struct marker * const __mark_ptr_##channel##_##name \
> __attribute__((used, section("__markers_ptrs"))) = \
> - &GET_MARKER(channel, name);
> + &__mark_##channel##_##name;
>
> /*
> * Make sure the alignment of the structure in the __markers section will
> @@ -173,7 +173,7 @@ struct marker {
> #define __trace_mark_counter(generic, channel, name, unique, call_private, format, args...) \
> do { \
> struct marker *__marker_counter_ptr; \
> - DEFINE_MARKER(channel, name, format, unique, __marker_counter_ptr); \
> + _DEFINE_MARKER(channel, name, NULL, NULL, format, unique, __marker_counter_ptr); \
> __mark_check_format(format, ## args); \
> if (!generic) { \
> if (unlikely(imv_read(__marker_counter_ptr->state))) \
> @@ -194,9 +194,9 @@ struct marker {
> { \
> register_trace_##tp_name(tp_cb, call_private); \
> } \
> - DEFINE_MARKER_TP(channel, name, tp_name, tp_cb, format);\
> + _DEFINE_MARKER_TP(channel, name, #tp_name, tp_cb, format); \
> __mark_check_format(format, ## args); \
> - (*GET_MARKER(channel, name).call)(&GET_MARKER(channel, name), \
> + (*__mark_##channel##_##name.call)(&__mark_##channel##_##name, \
> call_private, &__marker_regs, ## args); \
> } while (0)
>
> @@ -205,7 +205,6 @@ extern void marker_update_probe_range(struct marker * const *begin,
>
> /**
> * trace_mark - Marker using code patching
> - * @channel: marker channel (where to send the data), not quoted.
> * @name: marker name, not quoted.
> * @format: format string
> * @args...: variable argument list
> @@ -213,12 +212,11 @@ extern void marker_update_probe_range(struct marker * const *begin,
> * Places a marker using optimized code patching technique (imv_read())
> * to be enabled when immediate values are present.
> */
> -#define trace_mark(channel, name, format, args...) \
> - __trace_mark(0, channel, name, NULL, format, ## args)
> +#define trace_mark(name, format, args...) \
> + __trace_mark(0, ust, name, NULL, format, ## args)
>
> /**
> * _trace_mark - Marker using variable read
> - * @channel: marker channel (where to send the data), not quoted.
> * @name: marker name, not quoted.
> * @format: format string
> * @args...: variable argument list
> @@ -227,12 +225,11 @@ extern void marker_update_probe_range(struct marker * const *begin,
> * enabled. Should be used for markers in code paths where instruction
> * modification based enabling is not welcome.
> */
> -#define _trace_mark(channel, name, format, args...) \
> - __trace_mark(1, channel, name, NULL, format, ## args)
> +#define _trace_mark(name, format, args...) \
> + __trace_mark(1, ust, name, NULL, format, ## args)
>
> /**
> * trace_mark_tp - Marker in a tracepoint callback
> - * @channel: marker channel (where to send the data), not quoted.
> * @name: marker name, not quoted.
> * @tp_name: tracepoint name, not quoted.
> * @tp_cb: tracepoint callback. Should have an associated global symbol so it
> @@ -242,8 +239,8 @@ extern void marker_update_probe_range(struct marker * const *begin,
> *
> * Places a marker in a tracepoint callback.
> */
> -#define trace_mark_tp(channel, name, tp_name, tp_cb, format, args...) \
> - __trace_mark_tp(channel, name, NULL, tp_name, tp_cb, format, ## args)
> +#define trace_mark_tp(name, tp_name, tp_cb, format, args...) \
> + __trace_mark_tp(ust, name, NULL, tp_name, tp_cb, format, ## args)
>
> /**
> * MARK_NOARGS - Format string for a marker with no argument.
> diff --git a/libust/marker.c b/libust/marker.c
> index a64b46f..4b23e53 100644
> --- a/libust/marker.c
> +++ b/libust/marker.c
> @@ -447,7 +447,7 @@ static struct marker_entry *add_marker(const char *channel, const char *name,
> e->call = marker_probe_cb_noarg;
> else
> e->call = marker_probe_cb;
> - trace_mark(metadata, core_marker_format,
> + __trace_mark(0, metadata, core_marker_format, NULL,
> "channel %s name %s format %s",
> e->channel, e->name, e->format);
> } else {
> @@ -514,7 +514,7 @@ static int marker_set_format(struct marker_entry *entry, const char *format)
> return -ENOMEM;
> entry->format_allocated = 1;
>
> - trace_mark(metadata, core_marker_format,
> + __trace_mark(0, metadata, core_marker_format, NULL,
> "channel %s name %s format %s",
> entry->channel, entry->name, entry->format);
> return 0;
> @@ -781,7 +781,7 @@ int marker_probe_register(const char *channel, const char *name,
> goto error_unregister_channel;
> entry->event_id = ret;
> ret = 0;
> - trace_mark(metadata, core_marker_id,
> + __trace_mark(0, metadata, core_marker_id, NULL,
> "channel %s name %s event_id %hu "
> "int #1u%zu long #1u%zu pointer #1u%zu "
> "size_t #1u%zu alignment #1u%u",
> diff --git a/libust/tracectl.c b/libust/tracectl.c
> index 96053b7..e84a35a 100644
> --- a/libust/tracectl.c
> +++ b/libust/tracectl.c
> @@ -1576,7 +1576,7 @@ static void __attribute__((destructor)) keepalive()
>
> void ust_potential_exec(void)
> {
> - trace_mark(ust, potential_exec, MARK_NOARGS);
> + trace_mark(potential_exec, MARK_NOARGS);
>
> DBG("test");
>
> diff --git a/libustinstr-malloc/mallocwrap.c b/libustinstr-malloc/mallocwrap.c
> index f5d5ce3..c473567 100644
> --- a/libustinstr-malloc/mallocwrap.c
> +++ b/libustinstr-malloc/mallocwrap.c
> @@ -75,7 +75,7 @@ void *malloc(size_t size)
>
> retval = plibc_malloc(size);
>
> - trace_mark(ust, malloc, "size %d ptr %p", (int)size, retval);
> + trace_mark(malloc, "size %d ptr %p", (int)size, retval);
>
> return retval;
> }
> @@ -92,7 +92,7 @@ void free(void *ptr)
> }
> }
>
> - trace_mark(ust, free, "ptr %p", ptr);
> + trace_mark(free, "ptr %p", ptr);
>
> plibc_free(ptr);
> }
> diff --git a/tests/basic/basic.c b/tests/basic/basic.c
> index a810877..293feba 100644
> --- a/tests/basic/basic.c
> +++ b/tests/basic/basic.c
> @@ -29,8 +29,8 @@ int main()
> printf("Basic test program\n");
>
> for(i=0; i<50; i++) {
> - trace_mark(ust, bar, "str %s", "FOOBAZ");
> - trace_mark(ust, bar2, "number1 %d number2 %d", 53, 9800);
> + trace_mark(bar, "str %s", "FOOBAZ");
> + trace_mark(bar2, "number1 %d number2 %d", 53, 9800);
> usleep(100000);
> }
>
> diff --git a/tests/basic_long/basic_long.c b/tests/basic_long/basic_long.c
> index 6cf44b6..06ee2f5 100644
> --- a/tests/basic_long/basic_long.c
> +++ b/tests/basic_long/basic_long.c
> @@ -25,8 +25,8 @@ int main()
> printf("Basic test program\n");
>
> for(;;) {
> - trace_mark(ust, bar, "str %s", "FOOBAZ");
> - trace_mark(ust, bar2, "number1 %d number2 %d", 53, 9800);
> + trace_mark(bar, "str %s", "FOOBAZ");
> + trace_mark(bar2, "number1 %d number2 %d", 53, 9800);
> usleep(1000000);
> }
>
> diff --git a/tests/benchmark/bench.c b/tests/benchmark/bench.c
> index bc6a389..4e9c355 100644
> --- a/tests/benchmark/bench.c
> +++ b/tests/benchmark/bench.c
> @@ -29,7 +29,7 @@ void do_stuff(void)
> time(NULL);
>
> #ifdef MARKER
> - trace_mark(ust, event, "event %d", v);
> + trace_mark(event, "event %d", v);
> #endif
>
> }
> diff --git a/tests/dlopen/dlopen.c b/tests/dlopen/dlopen.c
> index 28d0ee6..d367580 100644
> --- a/tests/dlopen/dlopen.c
> +++ b/tests/dlopen/dlopen.c
> @@ -28,7 +28,7 @@ int main()
> {
> int (*fptr)();
>
> - trace_mark(ust, from_main_before_lib, "%s", "Event occured in the main program before"
> + trace_mark(from_main_before_lib, "%s", "Event occured in the main program before"
> " the opening of the library\n");
> void *lib_handle = dlopen("libdummy.so", RTLD_LAZY);
>
> @@ -47,7 +47,7 @@ int main()
> (*fptr)();
> dlclose(lib_handle);
>
> - trace_mark(ust, from_main_after_lib,"%s", "Event occured in the main program after "
> + trace_mark(from_main_after_lib,"%s", "Event occured in the main program after "
> "the library has been closed\n");
>
> return 0;
> diff --git a/tests/dlopen/libdummy.c b/tests/dlopen/libdummy.c
> index 45507c0..16f7b4d 100644
> --- a/tests/dlopen/libdummy.c
> +++ b/tests/dlopen/libdummy.c
> @@ -19,5 +19,5 @@
>
> void exported_function()
> {
> - trace_mark(ust, from_library, "%s", "Event occured in library function");
> + trace_mark(from_library, "%s", "Event occured in library function");
> }
> diff --git a/tests/fork/fork.c b/tests/fork/fork.c
> index a80518d..3b84644 100644
> --- a/tests/fork/fork.c
> +++ b/tests/fork/fork.c
> @@ -32,7 +32,7 @@ int main(int argc, char **argv, char *env[])
> }
>
> printf("Fork test program, parent pid is %d\n", getpid());
> - trace_mark(ust, before_fork, MARK_NOARGS);
> + trace_mark(before_fork, MARK_NOARGS);
>
> /* Sleep here to make sure the consumer is initialized before we fork */
> sleep(1);
> @@ -47,9 +47,9 @@ int main(int argc, char **argv, char *env[])
>
> printf("Child pid is %d\n", getpid());
>
> - trace_mark(ust, after_fork_child, MARK_NOARGS);
> + trace_mark(after_fork_child, MARK_NOARGS);
>
> - trace_mark(ust, before_exec, "pid %d", getpid());
> + trace_mark(before_exec, "pid %d", getpid());
>
> result = execve(argv[1], args, env);
> if(result == -1) {
> @@ -57,10 +57,10 @@ int main(int argc, char **argv, char *env[])
> return 1;
> }
>
> - trace_mark(ust, after_exec, "pid %d", getpid());
> + trace_mark(after_exec, "pid %d", getpid());
> }
> else {
> - trace_mark(ust, after_fork_parent, MARK_NOARGS);
> + trace_mark(after_fork_parent, MARK_NOARGS);
> }
>
> return 0;
> diff --git a/tests/fork/fork2.c b/tests/fork/fork2.c
> index a290044..8a14e1a 100644
> --- a/tests/fork/fork2.c
> +++ b/tests/fork/fork2.c
> @@ -24,7 +24,7 @@ int main()
> {
> printf("IN FORK2\n");
>
> - trace_mark(ust, after_exec, MARK_NOARGS);
> + trace_mark(after_exec, MARK_NOARGS);
>
> return 0;
> }
> diff --git a/tests/hello/hello.c b/tests/hello/hello.c
> index c0b541f..6ba2e61 100644
> --- a/tests/hello/hello.c
> +++ b/tests/hello/hello.c
> @@ -71,8 +71,8 @@ int main()
>
> sleep(1);
> for(i=0; i<50; i++) {
> - trace_mark(ust, bar, "str %s", "FOOBAZ");
> - trace_mark(ust, bar2, "number1 %d number2 %d", 53, 9800);
> + trace_mark(bar, "str %s", "FOOBAZ");
> + trace_mark(bar2, "number1 %d number2 %d", 53, 9800);
> trace_hello_tptest(i);
> usleep(100000);
> }
> diff --git a/tests/hello2/hello2.c b/tests/hello2/hello2.c
> index 7dc1b97..175a140 100644
> --- a/tests/hello2/hello2.c
> +++ b/tests/hello2/hello2.c
> @@ -37,8 +37,8 @@ int main()
> printf("Hello, World!\n");
>
> for(i=0; i<500; i++) {
> - trace_mark(ust, bar, "str %d", i);
> - trace_mark(ust, bar2, "number1 %d number2 %d", (int)53, (int)9800);
> + trace_mark(bar, "str %d", i);
> + trace_mark(bar2, "number1 %d number2 %d", (int)53, (int)9800);
> }
>
> // ltt_trace_stop("auto");
> diff --git a/tests/libustctl_function_tests/libustctl_function_tests.c b/tests/libustctl_function_tests/libustctl_function_tests.c
> index 7406243..cf184e6 100644
> --- a/tests/libustctl_function_tests/libustctl_function_tests.c
> +++ b/tests/libustctl_function_tests/libustctl_function_tests.c
> @@ -179,8 +179,8 @@ int main(int argc, char **argv)
> child_pid = fork();
> if (child_pid) {
> for(i=0; i<10; i++) {
> - trace_mark(ust, bar, "str %s", "FOOBAZ");
> - trace_mark(ust, bar2, "number1 %d number2 %d", 53, 9800);
> + trace_mark(bar, "str %s", "FOOBAZ");
> + trace_mark(bar2, "number1 %d number2 %d", 53, 9800);
> usleep(100000);
> }
>
> diff --git a/tests/make_shared_lib/basic_lib.c b/tests/make_shared_lib/basic_lib.c
> index 2c1366e..97874f3 100644
> --- a/tests/make_shared_lib/basic_lib.c
> +++ b/tests/make_shared_lib/basic_lib.c
> @@ -3,7 +3,7 @@
>
> void myfunc(void)
> {
> - trace_mark(ust, in_lib, MARK_NOARGS);
> + trace_mark(in_lib, MARK_NOARGS);
> printf("testfunc\n");
> }
>
> diff --git a/tests/make_shared_lib/prog.c b/tests/make_shared_lib/prog.c
> index c1f5ac8..777f4c6 100644
> --- a/tests/make_shared_lib/prog.c
> +++ b/tests/make_shared_lib/prog.c
> @@ -5,6 +5,6 @@ extern myfunc(void);
> int main(void)
> {
> myfunc();
> - trace_mark(ust, in_prog, MARK_NOARGS);
> + trace_mark(in_prog, MARK_NOARGS);
> return 0;
> }
> diff --git a/tests/same_line_marker/same_line_marker.c b/tests/same_line_marker/same_line_marker.c
> index d9c0f22..51cf5c3 100644
> --- a/tests/same_line_marker/same_line_marker.c
> +++ b/tests/same_line_marker/same_line_marker.c
> @@ -19,6 +19,6 @@
>
> int main()
> {
> - trace_mark(ust, same_line_event, "%s","An event occured in the same line"); trace_mark(ust, same_line_event, "%s","An event occured in the same line");
> + trace_mark(same_line_event, "%s","An event occured in the same line"); trace_mark(same_line_event, "%s","An event occured in the same line");
> return 0;
> }
> diff --git a/tests/test-nevents/prog.c b/tests/test-nevents/prog.c
> index b2350cc..4e70915 100644
> --- a/tests/test-nevents/prog.c
> +++ b/tests/test-nevents/prog.c
> @@ -30,8 +30,8 @@ int main()
> int i;
>
> for(i=0; i<N_ITER; i++) {
> - trace_mark(ust, an_event, "%d", i);
> - trace_mark(ust, another_event, "%s", "Hello, World!");
> + trace_mark(an_event, "%d", i);
> + trace_mark(another_event, "%s", "Hello, World!");
> }
>
> return 0;
> diff --git a/tests/tracepoint/benchmark/tracepoint_benchmark.c b/tests/tracepoint/benchmark/tracepoint_benchmark.c
> index 8af4b84..c8ba8c1 100644
> --- a/tests/tracepoint/benchmark/tracepoint_benchmark.c
> +++ b/tests/tracepoint/benchmark/tracepoint_benchmark.c
> @@ -34,7 +34,7 @@ DEFINE_TRACE(ust_event);
>
> void tp_probe(void *data, unsigned int p1);
>
> -DEFINE_MARKER_TP(ust, event, ust_event, tp_probe, "p1 %u");
> +DEFINE_MARKER_TP(event, ust_event, tp_probe, "p1 %u");
>
> /*
> * Probe 1 --> ust_event
> @@ -43,7 +43,7 @@ void tp_probe(void *data, unsigned int p1)
> {
> struct marker *marker;
>
> - marker = &GET_MARKER(ust, event);
> + marker = &GET_MARKER(event);
> ltt_specialized_trace(marker, data, &p1, sizeof(p1), sizeof(p1));
> }
>
> diff --git a/tests/tracepoint/tracepoint_test.c b/tests/tracepoint/tracepoint_test.c
> index cd3939c..6a5f691 100644
> --- a/tests/tracepoint/tracepoint_test.c
> +++ b/tests/tracepoint/tracepoint_test.c
> @@ -47,7 +47,7 @@ void tp_probe4(void *data, unsigned int p4)
> {
> int i;
> for (i = 0; i < 100; i++) {
> - trace_mark_tp(ust, event2, ust_event2, tp_probe4, "probe4 %u", p4);
> + trace_mark_tp(event2, ust_event2, tp_probe4, "probe4 %u", p4);
> }
> }
>
> @@ -60,7 +60,7 @@ void tp_probe3(void *data, unsigned int p3)
> {
> struct message *msg;
> msg = (struct message*) data;
> - trace_mark_tp(ust, event_msg, ust_event_msg,
> + trace_mark_tp(event_msg, ust_event_msg,
> tp_probe3, "probe %s", msg->payload);
> }
>
> @@ -72,7 +72,7 @@ void tp_probe2(void *data, unsigned int p2)
> {
> int i;
> for (i = 0; i < 5; i++) {
> - trace_mark_tp(ust, event, ust_event, tp_probe2, "probe %u", 13);
> + trace_mark_tp(event, ust_event, tp_probe2, "probe %u", 13);
> }
> }
>
> @@ -84,7 +84,7 @@ void tp_probe(void *data, unsigned int p1)
> {
> int i;
> for (i = 0; i < 5; i++) {
> - trace_mark_tp(ust, event, ust_event, tp_probe, "probe %u", p1);
> + trace_mark_tp(event, ust_event, tp_probe, "probe %u", p1);
> }
> }
>
> --
> Mathieu Desnoyers
> Operating System Efficiency R&D Consultant
> EfficiOS Inc.
> http://www.efficios.com
>
More information about the lttng-dev
mailing list