tools lib traceevent: Fixing the API to be less policy driven
cover tools lib traceevent: Fixing the API to be less policy driven
Hi Arnaldo and Jiri,
We are still working on getting libtraceevent ready to be a stand alone library. Part of that is to audit all the interfaces. We noticed that the most the tep_print_*() interfaces define policy and limit the way an application can display data. Instead of fixing this later and being stuck with a limiting API that we must maintain for backward compatibility, we removed and replaced most of it. perf was only affected by a single function that was removed. These functions are replaced by a more flexible one that allows the user to place what they want where they want it (timestamps, event info, latency format, COMM, PID, etc).
The other noticeable perf change, is that we changed the location to where the plugins are loaded from:
${HOME}/.traceevent/plugins
to
${HOME}/.local/lib/traceevent/plugins
As Patrick McLean (Gentoo package maintainer) informed us of the XGD layout.
Should we have something the warns people if they have plugins in the old directory. Should we move them on install? Currently, we just ignore them.
Anyway, please add these patches to tip.
Thanks!
-- Steve
Tzvetomir Stoyanov (3):
tools/lib/traceevent, tools/perf: Changes in tep_print_event_* APIs tools/lib/traceevent: Remove tep_register_trace_clock() tools/lib/traceevent: Change user's plugin directory
----
tools/lib/traceevent/Makefile | 6 +- tools/lib/traceevent/event-parse-api.c | 40 ---- tools/lib/traceevent/event-parse-local.h | 6 - tools/lib/traceevent/event-parse.c | 333 +++++++++++++++++-------------- tools/lib/traceevent/event-parse.h | 30 +-- tools/lib/traceevent/event-plugin.c | 2 +- tools/perf/builtin-kmem.c | 3 +- tools/perf/util/sort.c | 3 +- tools/perf/util/trace-event-parse.c | 2 +- 9 files changed, 207 insertions(+), 218 deletions(-)
PATCH 1/3 tools/lib/traceevent, tools/perf: Changes in tep_print_event_* APIs
From: Tzvetomir Stoyanov <[email protected]>
Libtraceevent APIs for printing various trace events information are complicated, there are complex extra parameters. To control the way event information is printed, the user should call a set of functions in a specific sequence.
These APIs are reimplemented to provide a more simple interface for printing event information.
Removed APIs:
tep_print_event_task()
tep_print_event_time() tep_print_event_data() tep_event_info() tep_is_latency_format() tep_set_latency_format() tep_data_latency_format() tep_set_print_raw()
A new API for printing event information is introduced:
void tep_print_event(struct tep_handle *tep, struct trace_seq *s,
struct tep_record *record, const char *fmt, ...);
where "fmt" is a printf-like format string, followed by the event fields to be printed. Supported fields:
TEP_PRINT_PID, "%d" - event PID
TEP_PRINT_CPU, "%d" - event CPU
TEP_PRINT_COMM, "%s" - event command string TEP_PRINT_NAME, "%s" - event name
TEP_PRINT_LATENCY, "%s" - event latency
TEP_PRINT_TIME, %d - event time stamp. A divisor and precision
can be specified as part of this format string:
"%precision.divisord". Example:
"%3.1000d" - divide the time by 1000 and print the first 3 digits before the dot. Thus, the time stamp "123456000" will be printed as "123.456"
TEP_PRINT_INFO, "%s" - event information. TEP_PRINT_INFO_RAW, "%s" - event information, in raw format.
Example:
tep_print_event(tep, s, record, "%16s-%-5d [%03d] %s %6.1000d %s %s",
TEP_PRINT_COMM, TEP_PRINT_PID, TEP_PRINT_CPU, TEP_PRINT_LATENCY, TEP_PRINT_TIME, TEP_PRINT_NAME, TEP_PRINT_INFO); Output: ls-11314 [005] d.h. 185207.366383 function __wake_up
--- tools/lib/traceevent/event-parse-api.c | 40 --- tools/lib/traceevent/event-parse-local.h | 4 - tools/lib/traceevent/event-parse.c | 322 +++++++++++++---------- tools/lib/traceevent/event-parse.h | 29 +- tools/perf/builtin-kmem.c | 3 +- tools/perf/util/sort.c | 3 +- tools/perf/util/trace-event-parse.c | 2 +- 7 files changed, 203 insertions(+), 200 deletions(-)
tools/lib/traceevent/event-parse-api.c +0 -40
@@ -302,33 +302,6 @@ void tep_set_local_bigendian(struct tep_handle *tep, enum tep_endian endian) tep->host_bigendian = endian; } -/** -* tep_is_latency_format - get if the latency output format is configured -* @tep: a handle to the tep_handle -* -* This returns true if the latency output format is configured -* If @tep is NULL, false is returned. -*/ -bool tep_is_latency_format (struct tep_handle *tep) -{ -if (tep) -return (tep->latency_format); -return false; -} - -/** -* tep_set_latency_format - set the latency output format -* @tep: a handle to the tep_handle -* @lat: non zero for latency output format -* -* This sets the latency output format -*/ -void tep_set_latency_format (struct tep_handle *tep,int lat) -{ -if (tep) - tep->latency_format = lat; -} -/** * tep_is_old_format - get if an old kernel is used * @tep: a handle to the tep_handle
@@ -344,19 +317,6 @@ bool tep_is_old_format(struct tep_handle *tep)return false; } -/** -* tep_set_print_raw - set a flag to force print in raw format -* @tep: a handle to the tep_handle -* @print_raw: the new value of the print_raw flag -* -* This sets a flag to force print in raw format -*/ -void tep_set_print_raw (struct tep_handle *tep,int print_raw) -{ -if (tep) - tep->print_raw = print_raw; -} -/** * tep_set_test_filters - set a flag to test a filter string * @tep: a handle to the tep_handle
tools/lib/traceevent/event-parse-local.h +0 -4
@@ -28,8 +28,6 @@ struct tep_handle {enum tep_endian file_bigendian;enum tep_endian host_bigendian; -int latency_format; -int old_format;int cpus;
@@ -70,8 +68,6 @@ struct tep_handle {int ld_offset;int ld_size; -int print_raw; -int test_filters;int flags;
tools/lib/traceevent/event-parse.c +187 -135
418 lines not on this page · raw message
tools/lib/traceevent/event-parse.h +11 -18
@@ -442,18 +442,18 @@ int tep_register_print_string(struct tep_handle *tep, const char *fmt, unsigned long long addr);bool tep_is_pid_registered (struct tep_handle *tep,int pid); -void tep_print_event_task (struct tep_handle *tep,struct trace_seq *s, -struct tep_event *event, -struct tep_record *record); -void tep_print_event_time (struct tep_handle *tep,struct trace_seq *s, -struct tep_event *event, -struct tep_record *record, -bool use_trace_clock); -void tep_print_event_data (struct tep_handle *tep,struct trace_seq *s, -struct tep_event *event, -struct tep_record *record); +#define TEP_PRINT_INFO "INFO" +#define TEP_PRINT_INFO_RAW "INFO_RAW" +#define TEP_PRINT_COMM "COMM" +#define TEP_PRINT_LATENCY "LATENCY" +#define TEP_PRINT_NAME "NAME" +#define TEP_PRINT_PID 1U +#define TEP_PRINT_TIME 2U +#define TEP_PRINT_CPU 3U +void tep_print_event (struct tep_handle *tep,struct trace_seq *s, -struct tep_record *record,bool use_trace_clock); +struct tep_record *record,const char *fmt, ...) + __attribute__ ((format (printf,4 ,5 )));int tep_parse_header_page (struct tep_handle *tep,char *buf,unsigned long size,int long_size);
@@ -525,8 +525,6 @@ tep_find_event_by_name(struct tep_handle *tep, const char *sys, const char *namestruct tep_event *tep_find_event_by_record (struct tep_handle *tep,struct tep_record *record); -void tep_data_latency_format (struct tep_handle *tep, -struct trace_seq *s,struct tep_record *record);int tep_data_type (struct tep_handle *tep,struct tep_record *rec);int tep_data_pid (struct tep_handle *tep,struct tep_record *rec);int tep_data_preempt_count (struct tep_handle *tep,struct tep_record *rec);
@@ -541,8 +539,6 @@ void tep_print_field(struct trace_seq *s, void *data, struct tep_format_field *field);void tep_print_fields (struct trace_seq *s,void *data,int size__maybe_unused ,struct tep_event *event); -void tep_event_info (struct trace_seq *s,struct tep_event *event, -struct tep_record *record);int tep_strerror (struct tep_handle *tep,enum tep_errno errnum,char *buf,size_t buflen);
@@ -566,12 +562,9 @@ bool tep_is_file_bigendian(struct tep_handle *tep);void tep_set_file_bigendian (struct tep_handle *tep,enum tep_endian endian);bool tep_is_local_bigendian (struct tep_handle *tep);void tep_set_local_bigendian (struct tep_handle *tep,enum tep_endian endian); -bool tep_is_latency_format (struct tep_handle *tep); -void tep_set_latency_format (struct tep_handle *tep,int lat);int tep_get_header_page_size (struct tep_handle *tep);int tep_get_header_timestamp_size (struct tep_handle *tep);bool tep_is_old_format (struct tep_handle *tep); -void tep_set_print_raw (struct tep_handle *tep,int print_raw);void tep_set_test_filters (struct tep_handle *tep,int test_filters);struct tep_handle *tep_alloc (void );
tools/perf/builtin-kmem.c +2 -1
@@ -749,7 +749,8 @@ static int parse_gfp_flags(struct evsel *evsel, struct perf_sample *sample, }trace_seq_init (&seq); -tep_event_info (&seq, evsel->tp_format, &record); +tep_print_event (evsel->tp_format->tep, + &seq, &record,"%s" ,TEP_PRINT_INFO ); str =strtok_r (seq.buffer," " , &pos);while (str) {
tools/perf/util/sort.c +2 -1
@@ -711,7 +711,8 @@ static char *get_trace_output(struct hist_entry *he)tep_print_fields (&seq, he->raw_data, he->raw_size, evsel->tp_format); }else { -tep_event_info (&seq, evsel->tp_format, &rec); +tep_print_event (evsel->tp_format->tep, + &seq, &rec,"%s" ,TEP_PRINT_INFO ); }/* * Trim the buffer, it starts at 4KB and we're not going to
tools/perf/util/trace-event-parse.c +1 -1
@@ -110,7 +110,7 @@ void event_format__fprintf(struct tep_event *event, record.data = data;trace_seq_init (&s); -tep_event_info (&s, event, &record); +tep_print_event (event->tep, &s, &record,"%s" ,TEP_PRINT_INFO );trace_seq_do_fprintf (&s, fp);trace_seq_destroy (&s); }
Signature · 1 line
2.20.1
PATCH 2/3 tools/lib/traceevent: Remove tep_register_trace_clock()
In reply to Steven RostedtFrom: Tzvetomir Stoyanov <[email protected]>
The tep_register_trace_clock() API is used to instruct the traceevent library how to print the event time stamps. As event print interface if redesigned, this API is not needed any more. The new event print API is flexible and the user can specify how the time stamps are printed.
--- tools/lib/traceevent/event-parse-local.h | 2 -- tools/lib/traceevent/event-parse.c | 11 ----------- tools/lib/traceevent/event-parse.h | 1 - 3 files changed, 14 deletions(-)
tools/lib/traceevent/event-parse-local.h +0 -2
@@ -81,8 +81,6 @@ struct tep_handle {/* cache */ struct tep_event *last_event; - -char *trace_clock; };void tep_free_event (struct tep_event *event);
tools/lib/traceevent/event-parse.c +0 -11
@@ -351,16 +351,6 @@ int tep_override_comm(struct tep_handle *tep, const char *comm, int pid)return _tep_register_comm (tep, comm, pid, true); } -int tep_register_trace_clock (struct tep_handle *tep,const char *trace_clock) -{ - tep->trace_clock =strdup (trace_clock); -if (!tep->trace_clock) { - errno =ENOMEM ; -return -1 ; - } -return 0 ; -} -struct func_map {unsigned long long addr;char *func;
@@ -7015,7 +7005,6 @@ void tep_free(struct tep_handle *tep)free_handler (handle); } -free (tep->trace_clock);free (tep->events);free (tep->sort_events);free (tep->func_resolver);
tools/lib/traceevent/event-parse.h +0 -1
@@ -435,7 +435,6 @@ int tep_set_function_resolver(struct tep_handle *tep,void tep_reset_function_resolver (struct tep_handle *tep);int tep_register_comm (struct tep_handle *tep,const char *comm,int pid);int tep_override_comm (struct tep_handle *tep,const char *comm,int pid); -int tep_register_trace_clock (struct tep_handle *tep,const char *trace_clock);int tep_register_function (struct tep_handle *tep,char *name,unsigned long long addr,char *mod); int tep_register_print_string(struct tep_handle *tep, const char *fmt,
Signature · 1 line
2.20.1
PATCH 3/3 tools/lib/traceevent: Change users plugin directory
In reply to Steven RostedtFrom: Tzvetomir Stoyanov <[email protected]>
To be compliant with XDG user directory layout, the user's plugin directory is changed from ~/.traceevent/plugins to ~/.local/lib/traceevent/plugins/
--- tools/lib/traceevent/Makefile | 6 +++--- tools/lib/traceevent/event-plugin.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-)
tools/lib/traceevent/Makefile +3 -3
@@ -62,15 +62,15 @@ set_plugin_dir := 1 # Set plugin_dir to preffered global plugin location # If we install under $HOME directory we go under -# $(HOME)/.traceevent/plugins +# $(HOME)/.local/lib/traceevent/plugins # # We dont set PLUGIN_DIR in case we install under $HOME # directory, because by default the code looks under: -# $(HOME)/.traceevent/plugins by default. +# $(HOME)/.local/lib/traceevent/plugins by default. # ifeq ($(plugin_dir),) ifeq ($(prefix),$(HOME)) -override plugin_dir = $(HOME)/.traceevent/plugins +override plugin_dir = $(HOME)/.local/lib/traceevent/plugins set_plugin_dir := 0 else override plugin_dir = $(libdir)/traceevent/plugins
tools/lib/traceevent/event-plugin.c +1 -1
@@ -18,7 +18,7 @@#include "event-utils.h" #include "trace-seq.h" -#define LOCAL_PLUGIN_DIR ".traceevent/plugins" +#define LOCAL_PLUGIN_DIR ".local/lib/traceevent/plugins/" static struct registered_plugin_options { struct registered_plugin_options *next;
Signature · 1 line
2.20.1
Quoted 1 line · On Mon, 05 Aug 2019 16:43:12 -0400 Steven Rostedt <[email protected]> wrote:
Hi Arnaldo and Jiri,
Hi Arnaldo,
I think these fell through the cracks.
-- Steve
Quoted 50 lines
We are still working on getting libtraceevent ready to be a stand alone library. Part of that is to audit all the interfaces. We noticed that the most the tep_print_*() interfaces define policy and limit the way an application can display data. Instead of fixing this later and being stuck with a limiting API that we must maintain for backward compatibility, we removed and replaced most of it. perf was only affected by a single function that was removed. These functions are replaced by a more flexible one that allows the user to place what they want where they want it (timestamps, event info, latency format, COMM, PID, etc).
The other noticeable perf change, is that we changed the location to where the plugins are loaded from:
${HOME}/.traceevent/plugins
to
${HOME}/.local/lib/traceevent/plugins
As Patrick McLean (Gentoo package maintainer) informed us of the XGD layout.
Should we have something the warns people if they have plugins in the old directory. Should we move them on install? Currently, we just ignore them.
Anyway, please add these patches to tip.
Thanks!
-- Steve
Tzvetomir Stoyanov (3):
tools/lib/traceevent,tools/perf: Changes in tep_print_event_* APIstools/lib/traceevent: Removetep_register_trace_clock()tools/lib/traceevent: Change user's plugin directory----
tools/lib/traceevent/Makefile | 6 +- tools/lib/traceevent/event-parse-api.c | 40 ---- tools/lib/traceevent/event-parse-local.h | 6 - tools/lib/traceevent/event-parse.c | 333 +++++++++++++++++-------------- tools/lib/traceevent/event-parse.h | 30 +-- tools/lib/traceevent/event-plugin.c | 2 +- tools/perf/builtin-kmem.c | 3 +- tools/perf/util/sort.c | 3 +- tools/perf/util/trace-event-parse.c | 2 +- 9 files changed, 207 insertions(+), 218 deletions(-)
Em Thu, Aug 29, 2019 at 02:00:10PM -0400, Steven Rostedt escreveu:
Quoted 8 lines
Quoted 1 line · On Mon, 05 Aug 2019 16:43:12 -0400 Steven Rostedt <[email protected]> wrote:
Hi Arnaldo and Jiri,
Hi Arnaldo,
I think these fell through the cracks.
Got it now, pushed to my perf/core branch, soon to be pushed upstream.
Thanks,
- Arnaldo
Quoted 52 lines
-- Steve
Quoted 50 lines
We are still working on getting libtraceevent ready to be a stand alone library. Part of that is to audit all the interfaces. We noticed that the most the tep_print_*() interfaces define policy and limit the way an application can display data. Instead of fixing this later and being stuck with a limiting API that we must maintain for backward compatibility, we removed and replaced most of it. perf was only affected by a single function that was removed. These functions are replaced by a more flexible one that allows the user to place what they want where they want it (timestamps, event info, latency format, COMM, PID, etc).
The other noticeable perf change, is that we changed the location to where the plugins are loaded from:
${HOME}/.traceevent/plugins
to
${HOME}/.local/lib/traceevent/plugins
As Patrick McLean (Gentoo package maintainer) informed us of the XGD layout.
Should we have something the warns people if they have plugins in the old directory. Should we move them on install? Currently, we just ignore them.
Anyway, please add these patches to tip.
Thanks!
-- Steve
Tzvetomir Stoyanov (3):
tools/lib/traceevent,tools/perf: Changes in tep_print_event_* APIstools/lib/traceevent: Removetep_register_trace_clock()tools/lib/traceevent: Change user's plugin directory----
tools/lib/traceevent/Makefile | 6 +- tools/lib/traceevent/event-parse-api.c | 40 ---- tools/lib/traceevent/event-parse-local.h | 6 - tools/lib/traceevent/event-parse.c | 333 +++++++++++++++++-------------- tools/lib/traceevent/event-parse.h | 30 +-- tools/lib/traceevent/event-plugin.c | 2 +- tools/perf/builtin-kmem.c | 3 +- tools/perf/util/sort.c | 3 +- tools/perf/util/trace-event-parse.c | 2 +- 9 files changed, 207 insertions(+), 218 deletions(-)
Signature · 1 line
- Arnaldo