* Nils Carlson ([email protected]) 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 <[email protected]> >> --- >> 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 _______________________________________________ ltt-dev mailing list [email protected] http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
