[ltt-dev] [UST PATCH] Markers: remove channel name from trace_mark()
Mathieu Desnoyers
mathieu.desnoyers at efficios.com
Wed Apr 13 13:30:19 EDT 2011
* Nils Carlson (nils.carlson at ericsson.com) wrote:
> 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_ ?
OK. Will do the trace_ -> ust_ change in a separate commit. Thanks,
Mathieu
>
> /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
>>
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
More information about the lttng-dev
mailing list