| Message ID | 20181005122225.522155df@gandalf.local.home (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | tools/lib/traceevent, tools/perf: Move struct tep_handler definition in a local header file | expand |
On Fri, 5 Oct 2018 12:22:25 -0400 Steven Rostedt <rostedt@goodmis.org> wrote: > +/** > + * tep_get_first_event - returns the first event in the events array > + * @tep: a handle to the tep_handle > + * > + * This returns pointer to the first element of the events array > + * If @tep is NULL, NULL is returned. > + */ > +struct tep_event_format *tep_get_first_event(struct tep_handle *tep) > +{ > + if(tep && tep->events) > + return tep->events[0]; > + > + return NULL; > +} > + Hold off on pulling this in. This patch has some whitespace issues that need to be cleaned up. Thanks! -- Steve
Em Fri, Oct 05, 2018 at 12:22:25PM -0400, Steven Rostedt escreveu: > > From: Tzvetomir Stoyanov <tstoyanov@vmware.com> > > As traceevent is going to be transferred into a proper library, > its local data should be protected from the library users. > This patch encapsulates struct tep_handler into a local header, > not visible outside of the library. It implements also a bunch > of new APIs, which library users can use to access tep_handler members. > > Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com> > Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org> Thanks, applied to perf/core. - Arnaldo
Em Fri, Oct 05, 2018 at 12:28:46PM -0400, Steven Rostedt escreveu: > On Fri, 5 Oct 2018 12:22:25 -0400 > Steven Rostedt <rostedt@goodmis.org> wrote: > > > +/** > > + * tep_get_first_event - returns the first event in the events array > > + * @tep: a handle to the tep_handle > > + * > > + * This returns pointer to the first element of the events array > > + * If @tep is NULL, NULL is returned. > > + */ > > +struct tep_event_format *tep_get_first_event(struct tep_handle *tep) > > +{ > > + if(tep && tep->events) > > + return tep->events[0]; > > + > > + return NULL; > > +} > > + > > Hold off on pulling this in. This patch has some whitespace issues that > need to be cleaned up. Ah, that if( thing? ?I can fix it here, holler if there is more than that.
On Mon, 8 Oct 2018 14:32:24 -0300 Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > Em Fri, Oct 05, 2018 at 12:28:46PM -0400, Steven Rostedt escreveu: > > On Fri, 5 Oct 2018 12:22:25 -0400 > > Steven Rostedt <rostedt@goodmis.org> wrote: > > > > > +/** > > > + * tep_get_first_event - returns the first event in the events array > > > + * @tep: a handle to the tep_handle > > > + * > > > + * This returns pointer to the first element of the events array > > > + * If @tep is NULL, NULL is returned. > > > + */ > > > +struct tep_event_format *tep_get_first_event(struct tep_handle *tep) > > > +{ > > > + if(tep && tep->events) > > > + return tep->events[0]; > > > + > > > + return NULL; > > > +} > > > + > > > > Hold off on pulling this in. This patch has some whitespace issues that > > need to be cleaned up. > > Ah, that if( thing? ?I can fix it here, holler if there is more than > that. Well, here's a fixed up version anyway... -- Steve >From 96dd7937a31d3a715c450c9a67f62a0022cd6a7c Mon Sep 17 00:00:00 2001 From: Tzvetomir Stoyanov <tstoyanov@vmware.com> Date: Mon, 8 Oct 2018 15:55:00 +0300 Subject: [PATCH] tools/lib/traceevent, tools/perf: Move struct tep_handler definition in a local header file As traceevent is going to be transferred into a proper library, its local data should be protected from the library users. This patch capsulates struct tep_handler into a local header, not visible outside of the library. It implements also a bunch of new APIs, which library users can use to access tep_handler members. Version 3 of the patch - fixed whitespace issues. Signed-off-by: Tzvetomir Stoyanov <tstoyanov@vmware.com> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org> --- tools/lib/traceevent/Build | 1 + tools/lib/traceevent/event-parse-api.c | 275 +++++++++++++++++++++++++++++++ tools/lib/traceevent/event-parse-local.h | 92 +++++++++++ tools/lib/traceevent/event-parse.c | 2 + tools/lib/traceevent/event-parse.h | 230 ++++---------------------- tools/lib/traceevent/event-plugin.c | 1 + tools/lib/traceevent/parse-filter.c | 1 + tools/perf/util/trace-event-parse.c | 25 +-- tools/perf/util/trace-event-read.c | 2 +- 9 files changed, 417 insertions(+), 212 deletions(-) create mode 100644 tools/lib/traceevent/event-parse-api.c create mode 100644 tools/lib/traceevent/event-parse-local.h diff --git a/tools/lib/traceevent/Build b/tools/lib/traceevent/Build index 0050c145d806..ba54bfce0b0b 100644 --- a/tools/lib/traceevent/Build +++ b/tools/lib/traceevent/Build @@ -5,6 +5,7 @@ libtraceevent-y += parse-filter.o libtraceevent-y += parse-utils.o libtraceevent-y += kbuffer-parse.o libtraceevent-y += tep_strerror.o +libtraceevent-y += event-parse-api.o plugin_jbd2-y += plugin_jbd2.o plugin_hrtimer-y += plugin_hrtimer.o diff --git a/tools/lib/traceevent/event-parse-api.c b/tools/lib/traceevent/event-parse-api.c new file mode 100644 index 000000000000..3b9371fb41cf --- /dev/null +++ b/tools/lib/traceevent/event-parse-api.c @@ -0,0 +1,275 @@ +// SPDX-License-Identifier: LGPL-2.1 +/* + * Copyright (C) 2009, 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com> + * + */ + +#include "event-parse.h" +#include "event-parse-local.h" +#include "event-utils.h" + +/** + * tep_get_first_event - returns the first event in the events array + * @tep: a handle to the tep_handle + * + * This returns pointer to the first element of the events array + * If @tep is NULL, NULL is returned. + */ +struct tep_event_format *tep_get_first_event(struct tep_handle *tep) +{ + if (tep && tep->events) + return tep->events[0]; + + return NULL; +} + +/** + * tep_get_events_count - get the number of defined events + * @tep: a handle to the tep_handle + * + * This returns number of elements in event array + * If @tep is NULL, 0 is returned. + */ +int tep_get_events_count(struct tep_handle *tep) +{ + if (tep) + return tep->nr_events; + return 0; +} + +/** + * tep_set_flag - set event parser flag + * @tep: a handle to the tep_handle + * @flag: flag, or combination of flags to be set + * can be any combination from enum tep_flag + * + * This sets a flag or mbination of flags from enum tep_flag + */ +void tep_set_flag(struct tep_handle *tep, int flag) +{ + if (tep) + tep->flags |= flag; +} + +unsigned short __tep_data2host2(struct tep_handle *pevent, unsigned short data) +{ + unsigned short swap; + + if (!pevent || pevent->host_bigendian == pevent->file_bigendian) + return data; + + swap = ((data & 0xffULL) << 8) | + ((data & (0xffULL << 8)) >> 8); + + return swap; +} + +unsigned int __tep_data2host4(struct tep_handle *pevent, unsigned int data) +{ + unsigned int swap; + + if (!pevent || pevent->host_bigendian == pevent->file_bigendian) + return data; + + swap = ((data & 0xffULL) << 24) | + ((data & (0xffULL << 8)) << 8) | + ((data & (0xffULL << 16)) >> 8) | + ((data & (0xffULL << 24)) >> 24); + + return swap; +} + +unsigned long long +__tep_data2host8(struct tep_handle *pevent, unsigned long long data) +{ + unsigned long long swap; + + if (!pevent || pevent->host_bigendian == pevent->file_bigendian) + return data; + + swap = ((data & 0xffULL) << 56) | + ((data & (0xffULL << 8)) << 40) | + ((data & (0xffULL << 16)) << 24) | + ((data & (0xffULL << 24)) << 8) | + ((data & (0xffULL << 32)) >> 8) | + ((data & (0xffULL << 40)) >> 24) | + ((data & (0xffULL << 48)) >> 40) | + ((data & (0xffULL << 56)) >> 56); + + return swap; +} + +/** + * tep_get_header_page_size - get size of the header page + * @pevent: a handle to the tep_handle + * + * This returns size of the header page + * If @pevent is NULL, 0 is returned. + */ +int tep_get_header_page_size(struct tep_handle *pevent) +{ + if (pevent) + return pevent->header_page_size_size; + return 0; +} + +/** + * tep_get_cpus - get the number of CPUs + * @pevent: a handle to the tep_handle + * + * This returns the number of CPUs + * If @pevent is NULL, 0 is returned. + */ +int tep_get_cpus(struct tep_handle *pevent) +{ + if (pevent) + return pevent->cpus; + return 0; +} + +/** + * tep_set_cpus - set the number of CPUs + * @pevent: a handle to the tep_handle + * + * This sets the number of CPUs + */ +void tep_set_cpus(struct tep_handle *pevent, int cpus) +{ + if (pevent) + pevent->cpus = cpus; +} + +/** + * tep_get_long_size - get the size of a long integer on the current machine + * @pevent: a handle to the tep_handle + * + * This returns the size of a long integer on the current machine + * If @pevent is NULL, 0 is returned. + */ +int tep_get_long_size(struct tep_handle *pevent) +{ + if (pevent) + return pevent->long_size; + return 0; +} + +/** + * tep_set_long_size - set the size of a long integer on the current machine + * @pevent: a handle to the tep_handle + * @size: size, in bytes, of a long integer + * + * This sets the size of a long integer on the current machine + */ +void tep_set_long_size(struct tep_handle *pevent, int long_size) +{ + if (pevent) + pevent->long_size = long_size; +} + +/** + * tep_get_page_size - get the size of a memory page on the current machine + * @pevent: a handle to the tep_handle + * + * This returns the size of a memory page on the current machine + * If @pevent is NULL, 0 is returned. + */ +int tep_get_page_size(struct tep_handle *pevent) +{ + if (pevent) + return pevent->page_size; + return 0; +} + +/** + * tep_set_page_size - set the size of a memory page on the current machine + * @pevent: a handle to the tep_handle + * @_page_size: size of a memory page, in bytes + * + * This sets the size of a memory page on the current machine + */ +void tep_set_page_size(struct tep_handle *pevent, int _page_size) +{ + if (pevent) + pevent->page_size = _page_size; +} + +/** + * tep_is_file_bigendian - get if the file is in big endian order + * @pevent: a handle to the tep_handle + * + * This returns if the file is in big endian order + * If @pevent is NULL, 0 is returned. + */ +int tep_is_file_bigendian(struct tep_handle *pevent) +{ + if (pevent) + return pevent->file_bigendian; + return 0; +} + +/** + * tep_set_file_bigendian - set if the file is in big endian order + * @pevent: a handle to the tep_handle + * @endian: non zero, if the file is in big endian order + * + * This sets if the file is in big endian order + */ +void tep_set_file_bigendian(struct tep_handle *pevent, enum tep_endian endian) +{ + if (pevent) + pevent->file_bigendian = endian; +} + +/** + * tep_is_host_bigendian - get if the order of the current host is big endian + * @pevent: a handle to the tep_handle + * + * This gets if the order of the current host is big endian + * If @pevent is NULL, 0 is returned. + */ +int tep_is_host_bigendian(struct tep_handle *pevent) +{ + if (pevent) + return pevent->host_bigendian; + return 0; +} + +/** + * tep_set_host_bigendian - set the order of the local host + * @pevent: a handle to the tep_handle + * @endian: non zero, if the local host has big endian order + * + * This sets the order of the local host + */ +void tep_set_host_bigendian(struct tep_handle *pevent, enum tep_endian endian) +{ + if (pevent) + pevent->host_bigendian = endian; +} + +/** + * tep_is_latency_format - get if the latency output format is configured + * @pevent: a handle to the tep_handle + * + * This gets if the latency output format is configured + * If @pevent is NULL, 0 is returned. + */ +int tep_is_latency_format(struct tep_handle *pevent) +{ + if (pevent) + return pevent->latency_format; + return 0; +} + +/** + * tep_set_latency_format - set the latency output format + * @pevent: 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 *pevent, int lat) +{ + if (pevent) + pevent->latency_format = lat; +} diff --git a/tools/lib/traceevent/event-parse-local.h b/tools/lib/traceevent/event-parse-local.h new file mode 100644 index 000000000000..b9bddde577f8 --- /dev/null +++ b/tools/lib/traceevent/event-parse-local.h @@ -0,0 +1,92 @@ +// SPDX-License-Identifier: LGPL-2.1 +/* + * Copyright (C) 2009, 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com> + * + */ + +#ifndef _PARSE_EVENTS_INT_H +#define _PARSE_EVENTS_INT_H + +struct cmdline; +struct cmdline_list; +struct func_map; +struct func_list; +struct event_handler; +struct func_resolver; + +struct tep_handle { + int ref_count; + + int header_page_ts_offset; + int header_page_ts_size; + int header_page_size_offset; + int header_page_size_size; + int header_page_data_offset; + int header_page_data_size; + int header_page_overwrite; + + enum tep_endian file_bigendian; + enum tep_endian host_bigendian; + + int latency_format; + + int old_format; + + int cpus; + int long_size; + int page_size; + + struct cmdline *cmdlines; + struct cmdline_list *cmdlist; + int cmdline_count; + + struct func_map *func_map; + struct func_resolver *func_resolver; + struct func_list *funclist; + unsigned int func_count; + + struct printk_map *printk_map; + struct printk_list *printklist; + unsigned int printk_count; + + + struct tep_event_format **events; + int nr_events; + struct tep_event_format **sort_events; + enum tep_event_sort_type last_type; + + int type_offset; + int type_size; + + int pid_offset; + int pid_size; + + int pc_offset; + int pc_size; + + int flags_offset; + int flags_size; + + int ld_offset; + int ld_size; + + int print_raw; + + int test_filters; + + int flags; + + struct tep_format_field *bprint_ip_field; + struct tep_format_field *bprint_fmt_field; + struct tep_format_field *bprint_buf_field; + + struct event_handler *handlers; + struct tep_function_handler *func_handlers; + + /* cache */ + struct tep_event_format *last_event; + + char *trace_clock; +}; + +#endif /* _PARSE_EVENTS_INT_H */ diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c index 233179a712d6..3692f29fee46 100644 --- a/tools/lib/traceevent/event-parse.c +++ b/tools/lib/traceevent/event-parse.c @@ -22,6 +22,8 @@ #include <netinet/in.h> #include "event-parse.h" + +#include "event-parse-local.h" #include "event-utils.h" #include "trace-seq.h" diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h index 9c29a5f7aa39..856334677c59 100644 --- a/tools/lib/traceevent/event-parse.h +++ b/tools/lib/traceevent/event-parse.h @@ -405,154 +405,23 @@ void tep_print_plugins(struct trace_seq *s, const char *prefix, const char *suffix, const struct tep_plugin_list *list); -struct cmdline; -struct cmdline_list; -struct func_map; -struct func_list; -struct event_handler; -struct func_resolver; - +/* tep_handle */ typedef char *(tep_func_resolver_t)(void *priv, unsigned long long *addrp, char **modp); +void tep_set_flag(struct tep_handle *tep, int flag); +unsigned short __tep_data2host2(struct tep_handle *pevent, unsigned short data); +unsigned int __tep_data2host4(struct tep_handle *pevent, unsigned int data); +unsigned long long +__tep_data2host8(struct tep_handle *pevent, unsigned long long data); -struct tep_handle { - int ref_count; - - int header_page_ts_offset; - int header_page_ts_size; - int header_page_size_offset; - int header_page_size_size; - int header_page_data_offset; - int header_page_data_size; - int header_page_overwrite; - - int file_bigendian; - int host_bigendian; - - int latency_format; - - int old_format; - - int cpus; - int long_size; - int page_size; - - struct cmdline *cmdlines; - struct cmdline_list *cmdlist; - int cmdline_count; - - struct func_map *func_map; - struct func_resolver *func_resolver; - struct func_list *funclist; - unsigned int func_count; - - struct printk_map *printk_map; - struct printk_list *printklist; - unsigned int printk_count; - - - struct tep_event_format **events; - int nr_events; - struct tep_event_format **sort_events; - enum tep_event_sort_type last_type; - - int type_offset; - int type_size; - - int pid_offset; - int pid_size; - - int pc_offset; - int pc_size; - - int flags_offset; - int flags_size; - - int ld_offset; - int ld_size; - - int print_raw; - - int test_filters; - - int flags; - - struct tep_format_field *bprint_ip_field; - struct tep_format_field *bprint_fmt_field; - struct tep_format_field *bprint_buf_field; - - struct event_handler *handlers; - struct tep_function_handler *func_handlers; - - /* cache */ - struct tep_event_format *last_event; - - char *trace_clock; -}; - -static inline void tep_set_flag(struct tep_handle *pevent, int flag) -{ - pevent->flags |= flag; -} - -static inline unsigned short -__tep_data2host2(struct tep_handle *pevent, unsigned short data) -{ - unsigned short swap; - - if (pevent->host_bigendian == pevent->file_bigendian) - return data; - - swap = ((data & 0xffULL) << 8) | - ((data & (0xffULL << 8)) >> 8); - - return swap; -} - -static inline unsigned int -__tep_data2host4(struct tep_handle *pevent, unsigned int data) -{ - unsigned int swap; - - if (pevent->host_bigendian == pevent->file_bigendian) - return data; - - swap = ((data & 0xffULL) << 24) | - ((data & (0xffULL << 8)) << 8) | - ((data & (0xffULL << 16)) >> 8) | - ((data & (0xffULL << 24)) >> 24); - - return swap; -} - -static inline unsigned long long -__tep_data2host8(struct tep_handle *pevent, unsigned long long data) -{ - unsigned long long swap; - - if (pevent->host_bigendian == pevent->file_bigendian) - return data; - - swap = ((data & 0xffULL) << 56) | - ((data & (0xffULL << 8)) << 40) | - ((data & (0xffULL << 16)) << 24) | - ((data & (0xffULL << 24)) << 8) | - ((data & (0xffULL << 32)) >> 8) | - ((data & (0xffULL << 40)) >> 24) | - ((data & (0xffULL << 48)) >> 40) | - ((data & (0xffULL << 56)) >> 56); - - return swap; -} - -#define tep_data2host2(pevent, ptr) __tep_data2host2(pevent, *(unsigned short *)(ptr)) -#define tep_data2host4(pevent, ptr) __tep_data2host4(pevent, *(unsigned int *)(ptr)) -#define tep_data2host8(pevent, ptr) \ +#define tep_data2host2(pevent, ptr) __tep_data2host2(pevent, *(unsigned short *)(ptr)) +#define tep_data2host4(pevent, ptr) __tep_data2host4(pevent, *(unsigned int *)(ptr)) +#define tep_data2host8(pevent, ptr) \ ({ \ unsigned long long __val; \ \ memcpy(&__val, (ptr), sizeof(unsigned long long)); \ - __tep_data2host8(pevent, __val); \ + __tep_data2host8(pevent, __val); \ }) static inline int tep_host_bigendian(void) @@ -655,11 +524,12 @@ unsigned long long tep_read_number(struct tep_handle *pevent, const void *ptr, i int tep_read_number_field(struct tep_format_field *field, const void *data, unsigned long long *value); +struct tep_event_format *tep_get_first_event(struct tep_handle *tep); +int tep_get_events_count(struct tep_handle *tep); struct tep_event_format *tep_find_event(struct tep_handle *pevent, int id); struct tep_event_format * tep_find_event_by_name(struct tep_handle *pevent, const char *sys, const char *name); - struct tep_event_format * tep_find_event_by_record(struct tep_handle *pevent, struct tep_record *record); @@ -689,65 +559,23 @@ struct tep_event_format **tep_list_events(struct tep_handle *pevent, enum tep_ev struct tep_format_field **tep_event_common_fields(struct tep_event_format *event); struct tep_format_field **tep_event_fields(struct tep_event_format *event); -static inline int tep_get_cpus(struct tep_handle *pevent) -{ - return pevent->cpus; -} - -static inline void tep_set_cpus(struct tep_handle *pevent, int cpus) -{ - pevent->cpus = cpus; -} - -static inline int tep_get_long_size(struct tep_handle *pevent) -{ - return pevent->long_size; -} - -static inline void tep_set_long_size(struct tep_handle *pevent, int long_size) -{ - pevent->long_size = long_size; -} - -static inline int tep_get_page_size(struct tep_handle *pevent) -{ - return pevent->page_size; -} - -static inline void tep_set_page_size(struct tep_handle *pevent, int _page_size) -{ - pevent->page_size = _page_size; -} - -static inline int tep_is_file_bigendian(struct tep_handle *pevent) -{ - return pevent->file_bigendian; -} - -static inline void tep_set_file_bigendian(struct tep_handle *pevent, int endian) -{ - pevent->file_bigendian = endian; -} - -static inline int tep_is_host_bigendian(struct tep_handle *pevent) -{ - return pevent->host_bigendian; -} - -static inline void tep_set_host_bigendian(struct tep_handle *pevent, int endian) -{ - pevent->host_bigendian = endian; -} - -static inline int tep_is_latency_format(struct tep_handle *pevent) -{ - return pevent->latency_format; -} - -static inline void tep_set_latency_format(struct tep_handle *pevent, int lat) -{ - pevent->latency_format = lat; -} +enum tep_endian { + TEP_LITTLE_ENDIAN = 0, + TEP_BIG_ENDIAN +}; +int tep_get_cpus(struct tep_handle *pevent); +void tep_set_cpus(struct tep_handle *pevent, int cpus); +int tep_get_long_size(struct tep_handle *pevent); +void tep_set_long_size(struct tep_handle *pevent, int long_size); +int tep_get_page_size(struct tep_handle *pevent); +void tep_set_page_size(struct tep_handle *pevent, int _page_size); +int tep_is_file_bigendian(struct tep_handle *pevent); +void tep_set_file_bigendian(struct tep_handle *pevent, enum tep_endian endian); +int tep_is_host_bigendian(struct tep_handle *pevent); +void tep_set_host_bigendian(struct tep_handle *pevent, enum tep_endian endian); +int tep_is_latency_format(struct tep_handle *pevent); +void tep_set_latency_format(struct tep_handle *pevent, int lat); +int tep_get_header_page_size(struct tep_handle *pevent); struct tep_handle *tep_alloc(void); void tep_free(struct tep_handle *pevent); diff --git a/tools/lib/traceevent/event-plugin.c b/tools/lib/traceevent/event-plugin.c index 46eb64eb0c2e..e74f16c88398 100644 --- a/tools/lib/traceevent/event-plugin.c +++ b/tools/lib/traceevent/event-plugin.c @@ -14,6 +14,7 @@ #include <unistd.h> #include <dirent.h> #include "event-parse.h" +#include "event-parse-local.h" #include "event-utils.h" #include "trace-seq.h" diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c index d64b6128fa7d..ed87cb56713d 100644 --- a/tools/lib/traceevent/parse-filter.c +++ b/tools/lib/traceevent/parse-filter.c @@ -11,6 +11,7 @@ #include <sys/types.h> #include "event-parse.h" +#include "event-parse-local.h" #include "event-utils.h" #define COMM "COMM" diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c index a4d7de1c96d1..9b78160e23a2 100644 --- a/tools/perf/util/trace-event-parse.c +++ b/tools/perf/util/trace-event-parse.c @@ -37,10 +37,11 @@ static int get_common_field(struct scripting_context *context, struct tep_format_field *field; if (!*size) { - if (!pevent->events) + + event = tep_get_first_event(pevent); + if (!event) return 0; - event = pevent->events[0]; field = tep_find_common_field(event, type); if (!field) return 0; @@ -192,25 +193,29 @@ struct tep_event_format *trace_find_next_event(struct tep_handle *pevent, struct tep_event_format *event) { static int idx; + int events_count; + struct tep_event_format *all_events; - if (!pevent || !pevent->events) + all_events = tep_get_first_event(pevent); + events_count = tep_get_events_count(pevent); + if (!pevent || !all_events || events_count < 1) return NULL; if (!event) { idx = 0; - return pevent->events[0]; + return all_events; } - if (idx < pevent->nr_events && event == pevent->events[idx]) { + if (idx < events_count && event == (all_events + idx)) { idx++; - if (idx == pevent->nr_events) + if (idx == events_count) return NULL; - return pevent->events[idx]; + return (all_events + idx); } - for (idx = 1; idx < pevent->nr_events; idx++) { - if (event == pevent->events[idx - 1]) - return pevent->events[idx]; + for (idx = 1; idx < events_count; idx++) { + if (event == (all_events + (idx - 1))) + return (all_events + idx); } return NULL; } diff --git a/tools/perf/util/trace-event-read.c b/tools/perf/util/trace-event-read.c index b98ee2a2eb44..cdf6de82a507 100644 --- a/tools/perf/util/trace-event-read.c +++ b/tools/perf/util/trace-event-read.c @@ -241,7 +241,7 @@ static int read_header_files(struct tep_handle *pevent) * The commit field in the page is of type long, * use that instead, since it represents the kernel. */ - tep_set_long_size(pevent, pevent->header_page_size_size); + tep_set_long_size(pevent, tep_get_header_page_size(pevent)); } free(header_page);
Hi, On 2018-10-05 12:22:25 -0400, Steven Rostedt wrote: > From: Tzvetomir Stoyanov <tstoyanov@vmware.com> > > As traceevent is going to be transferred into a proper library, > its local data should be protected from the library users. > This patch encapsulates struct tep_handler into a local header, > not visible outside of the library. It implements also a bunch > of new APIs, which library users can use to access tep_handler members. This commit appears to have broken perf script --gen-script / trace_find_next_event(). As far as I can tell: @ -192,25 +193,29 @@ struct tep_event_format *trace_find_next_event(struct tep_handle *pevent, > struct tep_event_format *event) > { > static int idx; > + int events_count; > + struct tep_event_format *all_events; > > - if (!pevent || !pevent->events) > + all_events = tep_get_first_event(pevent); > + events_count = tep_get_events_count(pevent); > + if (!pevent || !all_events || events_count < 1) > return NULL; > > if (!event) { > idx = 0; > - return pevent->events[0]; > + return all_events; > } > > - if (idx < pevent->nr_events && event == pevent->events[idx]) { > + if (idx < events_count && event == (all_events + idx)) { > idx++; > - if (idx == pevent->nr_events) > + if (idx == events_count) > return NULL; > - return pevent->events[idx]; > + return (all_events + idx); > } > > - for (idx = 1; idx < pevent->nr_events; idx++) { > - if (event == pevent->events[idx - 1]) > - return pevent->events[idx]; > + for (idx = 1; idx < events_count; idx++) { > + if (event == (all_events + (idx - 1))) > + return (all_events + idx); > } > return NULL; > } Is just plain wrong, as: > - return pevent->events[idx]; > + return (all_events + idx); that's not a valid conversion. ->events isn't an array of tep_handle, it's an array of tep_handle* (and even if it were, the previous notation would have needed to dereference the value to make it comparable). What this does is look idx behind the individually allocated event. Which is incorrect. To reproduce the crash, just generating a trace file with at least two events suffices: perf record -e raw_syscalls:sys_enter,raw_syscalls:sys_exit sleep 1 gdb --args perf script -g py Program received signal SIGSEGV, Segmentation fault. __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:96 96 ../sysdeps/x86_64/multiarch/strlen-avx2.S: No such file or directory. (gdb) bt #0 __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:96 #1 0x00007ffff726f9ef in _IO_vfprintf_internal (s=s@entry=0x555555c23970, format=format@entry=0x5555558fed76 "def %s__%s(", ap=ap@entry=0x7fffffffb900) at vfprintf.c:1638 #2 0x00007ffff7326536 in ___fprintf_chk (fp=fp@entry=0x555555c23970, flag=flag@entry=1, format=format@entry=0x5555558fed76 "def %s__%s(") at fprintf_chk.c:35 #3 0x000055555587065c in fprintf (__fmt=0x5555558fed76 "def %s__%s(", __stream=0x555555c23970) at /usr/include/x86_64-linux-gnu/bits/stdio2.h:100 #4 python_generate_script (pevent=0x555555c2e620, outfile=<optimized out>) at util/scripting-engines/trace-event-python.c:1651 #5 0x0000555555745762 in cmd_script (argc=<optimized out>, argv=0x7fffffffe4d0) at builtin-script.c:3743 #6 0x000055555579821d in run_builtin (p=0x555555abeb38 <commands+408>, argc=3, argv=0x7fffffffe4d0) at perf.c:303 #7 0x0000555555712b7a in handle_internal_command (argv=0x7fffffffe4d0, argc=3) at perf.c:355 #8 run_argv (argcp=<synthetic pointer>, argv=<synthetic pointer>) at perf.c:399 #9 main (argc=3, argv=0x7fffffffe4d0) at perf.c:521 The fix (in recent kernel versions) appears to just bee to use tep_get_event(), instead of the old pevent->events[...]. But it appears to me that commit 80c5526c8544ae76cba31fb9702ab8accac1f0f3 Author: Tzvetomir Stoyanov <tstoyanov@vmware.com> Date: 2019-04-01 12:43:12 -0400 tools lib traceevent: Implement new traceevent APIs for accessing struct tep_handler fields ommitted adding it to event-parse.h. It appears to be intended as public API however, given that it got documented in commit 747e942c3925bb85e2865371664499a98fca83b0 Author: Tzvetomir Stoyanov <tstoyanov@vmware.com> Date: 2019-05-10 15:56:23 -0400 tools lib traceevent: Man pages for libtraceevent event get APIs The following patch fixes this for me. I can polish it up, but I'm wondering if I'm missing something here? diff --git i/tools/lib/traceevent/event-parse.h w/tools/lib/traceevent/event-parse.h index 642f68ab5fb2..7ebc9b5308d4 100644 --- i/tools/lib/traceevent/event-parse.h +++ w/tools/lib/traceevent/event-parse.h @@ -517,6 +517,7 @@ int tep_read_number_field(struct tep_format_field *field, const void *data, unsigned long long *value); struct tep_event *tep_get_first_event(struct tep_handle *tep); +struct tep_event *tep_get_event(struct tep_handle *tep, int index); int tep_get_events_count(struct tep_handle *tep); struct tep_event *tep_find_event(struct tep_handle *tep, int id); diff --git i/tools/perf/util/trace-event-parse.c w/tools/perf/util/trace-event-parse.c index 62bc61155dd1..6a035ffd58ac 100644 --- i/tools/perf/util/trace-event-parse.c +++ w/tools/perf/util/trace-event-parse.c @@ -179,28 +179,26 @@ struct tep_event *trace_find_next_event(struct tep_handle *pevent, { static int idx; int events_count; - struct tep_event *all_events; - all_events = tep_get_first_event(pevent); events_count = tep_get_events_count(pevent); - if (!pevent || !all_events || events_count < 1) + if (!pevent || events_count < 1) return NULL; if (!event) { idx = 0; - return all_events; + return tep_get_event(pevent, 0); } - if (idx < events_count && event == (all_events + idx)) { + if (idx < events_count && event == tep_get_event(pevent, idx)) { idx++; if (idx == events_count) return NULL; - return (all_events + idx); + return tep_get_event(pevent, idx); } for (idx = 1; idx < events_count; idx++) { - if (event == (all_events + (idx - 1))) - return (all_events + idx); + if (event == tep_get_event(pevent, idx - 1)) + return tep_get_event(pevent, idx); } return NULL; } Not related to this crash, but it also seems that the whole trace_find_next_event() API ought to be removed. Back when it was -struct event *trace_find_next_event(struct event *event) -{ - if (!event) - return event_list; - - return event->next; -} it made some sense, but the changes in commit aaf045f72335653b24784d6042be8e4aee114403 Author: Steven Rostedt <srostedt@redhat.com> Date: 2012-04-06 00:47:56 +0200 perf: Have perf use the new libtraceevent.a library seem to make the current API somewhat absurd, as evidenced by the complexity in trace_find_next_event(). I mean even just removing that function and changing the two callsites to simple for loops with tep_get_events_count() & tep_get_event() ought to be a lot better. Greetings, Andres Freund
Hi Andres, On Fri, Jul 26, 2019 at 6:58 AM Andres Freund <andres@anarazel.de> wrote: > > Hi, > ... > > Is just plain wrong, as: > > > - return pevent->events[idx]; > > + return (all_events + idx); > > that's not a valid conversion. ->events isn't an array of tep_handle, > it's an array of tep_handle* (and even if it were, the previous notation > would have needed to dereference the value to make it comparable). What > this does is look idx behind the individually allocated event. Which is > incorrect. > You are right, it is a bug. > ... > > The following patch fixes this for me. I can polish it up, but I'm > wondering if I'm missing something here? > > diff --git i/tools/lib/traceevent/event-parse.h w/tools/lib/traceevent/event-parse.h > index 642f68ab5fb2..7ebc9b5308d4 100644 > --- i/tools/lib/traceevent/event-parse.h > +++ w/tools/lib/traceevent/event-parse.h > @@ -517,6 +517,7 @@ int tep_read_number_field(struct tep_format_field *field, const void *data, > unsigned long long *value); > > struct tep_event *tep_get_first_event(struct tep_handle *tep); > +struct tep_event *tep_get_event(struct tep_handle *tep, int index); > int tep_get_events_count(struct tep_handle *tep); > struct tep_event *tep_find_event(struct tep_handle *tep, int id); > > diff --git i/tools/perf/util/trace-event-parse.c w/tools/perf/util/trace-event-parse.c > index 62bc61155dd1..6a035ffd58ac 100644 > --- i/tools/perf/util/trace-event-parse.c > +++ w/tools/perf/util/trace-event-parse.c > @@ -179,28 +179,26 @@ struct tep_event *trace_find_next_event(struct tep_handle *pevent, > { > static int idx; > int events_count; > - struct tep_event *all_events; > > - all_events = tep_get_first_event(pevent); > events_count = tep_get_events_count(pevent); > - if (!pevent || !all_events || events_count < 1) > + if (!pevent || events_count < 1) > return NULL; > > if (!event) { > idx = 0; > - return all_events; > + return tep_get_event(pevent, 0); > } > > - if (idx < events_count && event == (all_events + idx)) { > + if (idx < events_count && event == tep_get_event(pevent, idx)) { > idx++; > if (idx == events_count) > return NULL; > - return (all_events + idx); > + return tep_get_event(pevent, idx); > } > > for (idx = 1; idx < events_count; idx++) { > - if (event == (all_events + (idx - 1))) > - return (all_events + idx); > + if (event == tep_get_event(pevent, idx - 1)) > + return tep_get_event(pevent, idx); > } > return NULL; > } > > I'm OK with the first part of the patch, the changes in tools/lib/traceevent/event-parse.h. tep_get_event() is an API and is omitted in the header by mistake, it should be there. As for the fix for the crash itself, I think it would be better to implement your suggestion - removing trace_find_next_event() at all and replacing it with tep_get_events_count() and tep_get_event() loop. > > > Not related to this crash, but it also seems that the whole > trace_find_next_event() API ought to be removed. Back when it was > > -struct event *trace_find_next_event(struct event *event) > -{ > - if (!event) > - return event_list; > - > - return event->next; > -} > > it made some sense, but the changes in > > commit aaf045f72335653b24784d6042be8e4aee114403 > Author: Steven Rostedt <srostedt@redhat.com> > Date: 2012-04-06 00:47:56 +0200 > > perf: Have perf use the new libtraceevent.a library > > seem to make the current API somewhat absurd, as evidenced by the > complexity in trace_find_next_event(). I mean even just removing that > function and changing the two callsites to simple for loops with > tep_get_events_count() & tep_get_event() ought to be a lot better. > > Greetings, > > Andres Freund Thanks!
On Thu, 25 Jul 2019 20:58:29 -0700 Andres Freund <andres@anarazel.de> wrote: > Hi, > Hi Andres, > On 2018-10-05 12:22:25 -0400, Steven Rostedt wrote: > > From: Tzvetomir Stoyanov <tstoyanov@vmware.com> > > > > As traceevent is going to be transferred into a proper library, > > its local data should be protected from the library users. > > This patch encapsulates struct tep_handler into a local header, > > not visible outside of the library. It implements also a bunch > > of new APIs, which library users can use to access tep_handler members. > > This commit appears to have broken perf script --gen-script / > trace_find_next_event(). As far as I can tell: > > @ -192,25 +193,29 @@ struct tep_event_format *trace_find_next_event(struct tep_handle *pevent, > > struct tep_event_format *event) > > { > > static int idx; > > + int events_count; > > + struct tep_event_format *all_events; > > > > - if (!pevent || !pevent->events) > > + all_events = tep_get_first_event(pevent); > > + events_count = tep_get_events_count(pevent); > > Is just plain wrong, as: > > > - return pevent->events[idx]; > > + return (all_events + idx); > > that's not a valid conversion. ->events isn't an array of tep_handle, > it's an array of tep_handle* (and even if it were, the previous notation You're right, it is wrong, but it's not tep_handle* but tep_event_format*. The tep_get_first_event() returns a pointer to the first event, but that's not an array. We can't use that to index to other events. > would have needed to dereference the value to make it comparable). What > this does is look idx behind the individually allocated event. Which is > incorrect. > > > > The fix (in recent kernel versions) appears to just bee to use > tep_get_event(), instead of the old pevent->events[...]. But it appears > to me that > commit 80c5526c8544ae76cba31fb9702ab8accac1f0f3 > Author: Tzvetomir Stoyanov <tstoyanov@vmware.com> > Date: 2019-04-01 12:43:12 -0400 > > tools lib traceevent: Implement new traceevent APIs for accessing struct tep_handler fields > > ommitted adding it to event-parse.h. It appears to be intended as public > API however, given that it got documented in And it appears that we missed to add it ;-) > > commit 747e942c3925bb85e2865371664499a98fca83b0 > Author: Tzvetomir Stoyanov <tstoyanov@vmware.com> > Date: 2019-05-10 15:56:23 -0400 > > tools lib traceevent: Man pages for libtraceevent event get APIs > > > The following patch fixes this for me. I can polish it up, but I'm > wondering if I'm missing something here? > > diff --git i/tools/lib/traceevent/event-parse.h w/tools/lib/traceevent/event-parse.h > index 642f68ab5fb2..7ebc9b5308d4 100644 > --- i/tools/lib/traceevent/event-parse.h > +++ w/tools/lib/traceevent/event-parse.h > @@ -517,6 +517,7 @@ int tep_read_number_field(struct tep_format_field *field, const void *data, > unsigned long long *value); > > struct tep_event *tep_get_first_event(struct tep_handle *tep); > +struct tep_event *tep_get_event(struct tep_handle *tep, int index); I was looking at the tep_get_event() code, and we should switch that to "unsigned int index" otherwise passing in a negative number will return an address outside the array. > int tep_get_events_count(struct tep_handle *tep); > struct tep_event *tep_find_event(struct tep_handle *tep, int id); > > diff --git i/tools/perf/util/trace-event-parse.c w/tools/perf/util/trace-event-parse.c > index 62bc61155dd1..6a035ffd58ac 100644 > --- i/tools/perf/util/trace-event-parse.c > +++ w/tools/perf/util/trace-event-parse.c > @@ -179,28 +179,26 @@ struct tep_event *trace_find_next_event(struct tep_handle *pevent, > { > static int idx; > int events_count; > - struct tep_event *all_events; > > - all_events = tep_get_first_event(pevent); > events_count = tep_get_events_count(pevent); I think we can get rid of the events_count and all its checks, as the same check is done within tep_get_event(). > - if (!pevent || !all_events || events_count < 1) > + if (!pevent || events_count < 1) if (!pevent) > return NULL; > > if (!event) { > idx = 0; > - return all_events; > + return tep_get_event(pevent, 0); > } > > - if (idx < events_count && event == (all_events + idx)) { > + if (idx < events_count && event == tep_get_event(pevent, idx)) { if (event == tep_get_event(pevent, idx)) return tep_get_event(pevent, ++idx); > idx++; > if (idx == events_count) > return NULL; > - return (all_events + idx); > + return tep_get_event(pevent, idx); > } > struct tep_event_format *next_event; for (idx = 0; next_event = tep_get_event(pevent, idx); idx++) if (event == next_event) return tep_get_event(pevent, ++idx); Also, I think setting the idx to 1 in the loop is wrong. Why? think of this: first_event = trace_find_next_event(pevent, NULL); next_event = trace_find_next_event(pevent, first_event); for (i = 0; i < 5; i++) next_event = trace_find_next_event(pevent, next_event); second_event = trace_find_next_event(pevent, first_event); second_event would become NULL. Care to send a formal patch? Thanks! -- Steve > for (idx = 1; idx < events_count; idx++) { > - if (event == (all_events + (idx - 1))) > - return (all_events + idx); > + if (event == tep_get_event(pevent, idx - 1)) > + return tep_get_event(pevent, idx); > } > return NULL; > } > > > > > Not related to this crash, but it also seems that the whole > trace_find_next_event() API ought to be removed. Back when it was > > -struct event *trace_find_next_event(struct event *event) > -{ > - if (!event) > - return event_list; > - > - return event->next; > -} > > it made some sense, but the changes in > > commit aaf045f72335653b24784d6042be8e4aee114403 > Author: Steven Rostedt <srostedt@redhat.com> > Date: 2012-04-06 00:47:56 +0200 > > perf: Have perf use the new libtraceevent.a library > > seem to make the current API somewhat absurd, as evidenced by the > complexity in trace_find_next_event(). I mean even just removing that > function and changing the two callsites to simple for loops with > tep_get_events_count() & tep_get_event() ought to be a lot better. > > Greetings, > > Andres Freund
Hi, On 2019-07-26 09:12:00 -0400, Steven Rostedt wrote: > On Thu, 25 Jul 2019 20:58:29 -0700 > Andres Freund <andres@anarazel.de> wrote: > > > > Is just plain wrong, as: > > > > > - return pevent->events[idx]; > > > + return (all_events + idx); > > > > that's not a valid conversion. ->events isn't an array of tep_handle, > > it's an array of tep_handle* (and even if it were, the previous notation > > You're right, it is wrong, but it's not tep_handle* but > tep_event_format*. Err, yea. Typo. > > diff --git i/tools/lib/traceevent/event-parse.h w/tools/lib/traceevent/event-parse.h > > index 642f68ab5fb2..7ebc9b5308d4 100644 > > --- i/tools/lib/traceevent/event-parse.h > > +++ w/tools/lib/traceevent/event-parse.h > > @@ -517,6 +517,7 @@ int tep_read_number_field(struct tep_format_field *field, const void *data, > > unsigned long long *value); > > > > struct tep_event *tep_get_first_event(struct tep_handle *tep); > > +struct tep_event *tep_get_event(struct tep_handle *tep, int index); > > I was looking at the tep_get_event() code, and we should switch that to > "unsigned int index" otherwise passing in a negative number will return > an address outside the array. Makes sense. > > int tep_get_events_count(struct tep_handle *tep); > > struct tep_event *tep_find_event(struct tep_handle *tep, int id); > > > > diff --git i/tools/perf/util/trace-event-parse.c w/tools/perf/util/trace-event-parse.c > > index 62bc61155dd1..6a035ffd58ac 100644 > > --- i/tools/perf/util/trace-event-parse.c > > +++ w/tools/perf/util/trace-event-parse.c > > @@ -179,28 +179,26 @@ struct tep_event *trace_find_next_event(struct tep_handle *pevent, > > { > > static int idx; > > int events_count; > > - struct tep_event *all_events; > > > > - all_events = tep_get_first_event(pevent); > > events_count = tep_get_events_count(pevent); > > I think we can get rid of the events_count and all its checks, as the > same check is done within tep_get_event(). > > - if (!pevent || !all_events || events_count < 1) > > + if (!pevent || events_count < 1) > > if (!pevent) > > > return NULL; > > > > if (!event) { > > idx = 0; > > - return all_events; > > + return tep_get_event(pevent, 0); > > } > > > > - if (idx < events_count && event == (all_events + idx)) { > > + if (idx < events_count && event == tep_get_event(pevent, idx)) { > > if (event == tep_get_event(pevent, idx)) > return tep_get_event(pevent, ++idx); > > > idx++; > > if (idx == events_count) > > return NULL; > > - return (all_events + idx); > > + return tep_get_event(pevent, idx); > > } > > > > struct tep_event_format *next_event; > > for (idx = 0; next_event = tep_get_event(pevent, idx); idx++) > if (event == next_event) > return tep_get_event(pevent, ++idx); > > Also, I think setting the idx to 1 in the loop is wrong. Why? think of > this: > > first_event = trace_find_next_event(pevent, NULL); > > next_event = trace_find_next_event(pevent, first_event); > for (i = 0; i < 5; i++) > next_event = trace_find_next_event(pevent, next_event); > > second_event = trace_find_next_event(pevent, first_event); > > second_event would become NULL. How about my proposal to instead change the loops in trace-event-{python,perl}.c, the only callers of trace_find_next_event, to be something akin to [idx_type_for_tep_get_event] event_count = tep_get_events_count(pevent); for ([idx_type_for_tep_get_event] idx = 0; idx < event_count; idx++) { struct tep_event *event = tep_get_events(...); } and just removing trace_find_next_event()? It's not a nice API imo, and seems unnecessary given that the events aren't a linked list anymore. > Care to send a formal patch? Will do. Greetings, Andres Freund
On Fri, 26 Jul 2019 13:55:44 -0700 Andres Freund <andres@anarazel.de> wrote: > How about my proposal to instead change the loops in > trace-event-{python,perl}.c, the only callers of trace_find_next_event, > to be something akin to > > [idx_type_for_tep_get_event] event_count = tep_get_events_count(pevent); > for ([idx_type_for_tep_get_event] idx = 0; idx < event_count; idx++) > { > struct tep_event *event = tep_get_events(...); > > } > > and just removing trace_find_next_event()? It's not a nice API imo, and > seems unnecessary given that the events aren't a linked list anymore. Yep, go for it :-) > > > > Care to send a formal patch? > > Will do. Thanks! -- Steve
On Fri, 26 Jul 2019 13:55:44 -0700 Andres Freund <andres@anarazel.de> wrote: > > Care to send a formal patch? > > Will do. Hi Andres, Have you had a chance to send a patch? Thanks! -- Steve
diff --git a/tools/lib/traceevent/Build b/tools/lib/traceevent/Build index 0050c145d806..ba54bfce0b0b 100644 --- a/tools/lib/traceevent/Build +++ b/tools/lib/traceevent/Build @@ -5,6 +5,7 @@ libtraceevent-y += parse-filter.o libtraceevent-y += parse-utils.o libtraceevent-y += kbuffer-parse.o libtraceevent-y += tep_strerror.o +libtraceevent-y += event-parse-api.o plugin_jbd2-y += plugin_jbd2.o plugin_hrtimer-y += plugin_hrtimer.o diff --git a/tools/lib/traceevent/event-parse-api.c b/tools/lib/traceevent/event-parse-api.c new file mode 100644 index 000000000000..f4f9dc332ab2 --- /dev/null +++ b/tools/lib/traceevent/event-parse-api.c @@ -0,0 +1,275 @@ +// SPDX-License-Identifier: LGPL-2.1 +/* + * Copyright (C) 2009, 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com> + * + */ + +#include "event-parse.h" +#include "event-parse-local.h" +#include "event-utils.h" + +/** + * tep_get_first_event - returns the first event in the events array + * @tep: a handle to the tep_handle + * + * This returns pointer to the first element of the events array + * If @tep is NULL, NULL is returned. + */ +struct tep_event_format *tep_get_first_event(struct tep_handle *tep) +{ + if(tep && tep->events) + return tep->events[0]; + + return NULL; +} + +/** + * tep_get_events_count - get the number of defined events + * @tep: a handle to the tep_handle + * + * This returns number of elements in event array + * If @tep is NULL, 0 is returned. + */ +int tep_get_events_count(struct tep_handle *tep) +{ + if(tep) + return tep->nr_events; + return 0; +} + +/** + * tep_set_flag - set event parser flag + * @tep: a handle to the tep_handle + * @flag: flag, or combination of flags to be set + * can be any combination from enum tep_flag + * + * This sets a flag or mbination of flags from enum tep_flag + */ +void tep_set_flag(struct tep_handle *tep, int flag) +{ + if(tep) + tep->flags |= flag; +} + +unsigned short __tep_data2host2(struct tep_handle *pevent, unsigned short data) +{ + unsigned short swap; + + if (!pevent || pevent->host_bigendian == pevent->file_bigendian) + return data; + + swap = ((data & 0xffULL) << 8) | + ((data & (0xffULL << 8)) >> 8); + + return swap; +} + +unsigned int __tep_data2host4(struct tep_handle *pevent, unsigned int data) +{ + unsigned int swap; + + if (!pevent || pevent->host_bigendian == pevent->file_bigendian) + return data; + + swap = ((data & 0xffULL) << 24) | + ((data & (0xffULL << 8)) << 8) | + ((data & (0xffULL << 16)) >> 8) | + ((data & (0xffULL << 24)) >> 24); + + return swap; +} + +unsigned long long +__tep_data2host8(struct tep_handle *pevent, unsigned long long data) +{ + unsigned long long swap; + + if (!pevent || pevent->host_bigendian == pevent->file_bigendian) + return data; + + swap = ((data & 0xffULL) << 56) | + ((data & (0xffULL << 8)) << 40) | + ((data & (0xffULL << 16)) << 24) | + ((data & (0xffULL << 24)) << 8) | + ((data & (0xffULL << 32)) >> 8) | + ((data & (0xffULL << 40)) >> 24) | + ((data & (0xffULL << 48)) >> 40) | + ((data & (0xffULL << 56)) >> 56); + + return swap; +} + +/** + * tep_get_header_page_size - get size of the header page + * @pevent: a handle to the tep_handle + * + * This returns size of the header page + * If @pevent is NULL, 0 is returned. + */ +int tep_get_header_page_size(struct tep_handle *pevent) +{ + if(pevent) + return pevent->header_page_size_size; + return 0; +} + +/** + * tep_get_cpus - get the number of CPUs + * @pevent: a handle to the tep_handle + * + * This returns the number of CPUs + * If @pevent is NULL, 0 is returned. + */ +int tep_get_cpus(struct tep_handle *pevent) +{ + if(pevent) + return pevent->cpus; + return 0; +} + +/** + * tep_set_cpus - set the number of CPUs + * @pevent: a handle to the tep_handle + * + * This sets the number of CPUs + */ +void tep_set_cpus(struct tep_handle *pevent, int cpus) +{ + if(pevent) + pevent->cpus = cpus; +} + +/** + * tep_get_long_size - get the size of a long integer on the current machine + * @pevent: a handle to the tep_handle + * + * This returns the size of a long integer on the current machine + * If @pevent is NULL, 0 is returned. + */ +int tep_get_long_size(struct tep_handle *pevent) +{ + if(pevent) + return pevent->long_size; + return 0; +} + +/** + * tep_set_long_size - set the size of a long integer on the current machine + * @pevent: a handle to the tep_handle + * @size: size, in bytes, of a long integer + * + * This sets the size of a long integer on the current machine + */ +void tep_set_long_size(struct tep_handle *pevent, int long_size) +{ + if(pevent) + pevent->long_size = long_size; +} + +/** + * tep_get_page_size - get the size of a memory page on the current machine + * @pevent: a handle to the tep_handle + * + * This returns the size of a memory page on the current machine + * If @pevent is NULL, 0 is returned. + */ +int tep_get_page_size(struct tep_handle *pevent) +{ + if(pevent) + return pevent->page_size; + return 0; +} + +/** + * tep_set_page_size - set the size of a memory page on the current machine + * @pevent: a handle to the tep_handle + * @_page_size: size of a memory page, in bytes + * + * This sets the size of a memory page on the current machine + */ +void tep_set_page_size(struct tep_handle *pevent, int _page_size) +{ + if(pevent) + pevent->page_size = _page_size; +} + +/** + * tep_is_file_bigendian - get if the file is in big endian order + * @pevent: a handle to the tep_handle + * + * This returns if the file is in big endian order + * If @pevent is NULL, 0 is returned. + */ +int tep_is_file_bigendian(struct tep_handle *pevent) +{ + if(pevent) + return pevent->file_bigendian; + return 0; +} + +/** + * tep_set_file_bigendian - set if the file is in big endian order + * @pevent: a handle to the tep_handle + * @endian: non zero, if the file is in big endian order + * + * This sets if the file is in big endian order + */ +void tep_set_file_bigendian(struct tep_handle *pevent, enum tep_endian endian) +{ + if(pevent) + pevent->file_bigendian = endian; +} + +/** + * tep_is_host_bigendian - get if the order of the current host is big endian + * @pevent: a handle to the tep_handle + * + * This gets if the order of the current host is big endian + * If @pevent is NULL, 0 is returned. + */ +int tep_is_host_bigendian(struct tep_handle *pevent) +{ + if(pevent) + return pevent->host_bigendian; + return 0; +} + +/** + * tep_set_host_bigendian - set the order of the local host + * @pevent: a handle to the tep_handle + * @endian: non zero, if the local host has big endian order + * + * This sets the order of the local host + */ +void tep_set_host_bigendian(struct tep_handle *pevent, enum tep_endian endian) +{ + if(pevent) + pevent->host_bigendian = endian; +} + +/** + * tep_is_latency_format - get if the latency output format is configured + * @pevent: a handle to the tep_handle + * + * This gets if the latency output format is configured + * If @pevent is NULL, 0 is returned. + */ +int tep_is_latency_format(struct tep_handle *pevent) +{ + if(pevent) + return pevent->latency_format; + return 0; +} + +/** + * tep_set_latency_format - set the latency output format + * @pevent: 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 *pevent, int lat) +{ + if(pevent) + pevent->latency_format = lat; +} diff --git a/tools/lib/traceevent/event-parse-local.h b/tools/lib/traceevent/event-parse-local.h new file mode 100644 index 000000000000..b9bddde577f8 --- /dev/null +++ b/tools/lib/traceevent/event-parse-local.h @@ -0,0 +1,92 @@ +// SPDX-License-Identifier: LGPL-2.1 +/* + * Copyright (C) 2009, 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com> + * + */ + +#ifndef _PARSE_EVENTS_INT_H +#define _PARSE_EVENTS_INT_H + +struct cmdline; +struct cmdline_list; +struct func_map; +struct func_list; +struct event_handler; +struct func_resolver; + +struct tep_handle { + int ref_count; + + int header_page_ts_offset; + int header_page_ts_size; + int header_page_size_offset; + int header_page_size_size; + int header_page_data_offset; + int header_page_data_size; + int header_page_overwrite; + + enum tep_endian file_bigendian; + enum tep_endian host_bigendian; + + int latency_format; + + int old_format; + + int cpus; + int long_size; + int page_size; + + struct cmdline *cmdlines; + struct cmdline_list *cmdlist; + int cmdline_count; + + struct func_map *func_map; + struct func_resolver *func_resolver; + struct func_list *funclist; + unsigned int func_count; + + struct printk_map *printk_map; + struct printk_list *printklist; + unsigned int printk_count; + + + struct tep_event_format **events; + int nr_events; + struct tep_event_format **sort_events; + enum tep_event_sort_type last_type; + + int type_offset; + int type_size; + + int pid_offset; + int pid_size; + + int pc_offset; + int pc_size; + + int flags_offset; + int flags_size; + + int ld_offset; + int ld_size; + + int print_raw; + + int test_filters; + + int flags; + + struct tep_format_field *bprint_ip_field; + struct tep_format_field *bprint_fmt_field; + struct tep_format_field *bprint_buf_field; + + struct event_handler *handlers; + struct tep_function_handler *func_handlers; + + /* cache */ + struct tep_event_format *last_event; + + char *trace_clock; +}; + +#endif /* _PARSE_EVENTS_INT_H */ diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c index 233179a712d6..3692f29fee46 100644 --- a/tools/lib/traceevent/event-parse.c +++ b/tools/lib/traceevent/event-parse.c @@ -22,6 +22,8 @@ #include <netinet/in.h> #include "event-parse.h" + +#include "event-parse-local.h" #include "event-utils.h" #include "trace-seq.h" diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h index 9c29a5f7aa39..16bf4c890b6f 100644 --- a/tools/lib/traceevent/event-parse.h +++ b/tools/lib/traceevent/event-parse.h @@ -405,149 +405,18 @@ void tep_print_plugins(struct trace_seq *s, const char *prefix, const char *suffix, const struct tep_plugin_list *list); -struct cmdline; -struct cmdline_list; -struct func_map; -struct func_list; -struct event_handler; -struct func_resolver; - +/* tep_handle */ typedef char *(tep_func_resolver_t)(void *priv, unsigned long long *addrp, char **modp); +void tep_set_flag(struct tep_handle *tep, int flag); +unsigned short __tep_data2host2(struct tep_handle *pevent, unsigned short data); +unsigned int __tep_data2host4(struct tep_handle *pevent, unsigned int data); +unsigned long long +__tep_data2host8(struct tep_handle *pevent, unsigned long long data); -struct tep_handle { - int ref_count; - - int header_page_ts_offset; - int header_page_ts_size; - int header_page_size_offset; - int header_page_size_size; - int header_page_data_offset; - int header_page_data_size; - int header_page_overwrite; - - int file_bigendian; - int host_bigendian; - - int latency_format; - - int old_format; - - int cpus; - int long_size; - int page_size; - - struct cmdline *cmdlines; - struct cmdline_list *cmdlist; - int cmdline_count; - - struct func_map *func_map; - struct func_resolver *func_resolver; - struct func_list *funclist; - unsigned int func_count; - - struct printk_map *printk_map; - struct printk_list *printklist; - unsigned int printk_count; - - - struct tep_event_format **events; - int nr_events; - struct tep_event_format **sort_events; - enum tep_event_sort_type last_type; - - int type_offset; - int type_size; - - int pid_offset; - int pid_size; - - int pc_offset; - int pc_size; - - int flags_offset; - int flags_size; - - int ld_offset; - int ld_size; - - int print_raw; - - int test_filters; - - int flags; - - struct tep_format_field *bprint_ip_field; - struct tep_format_field *bprint_fmt_field; - struct tep_format_field *bprint_buf_field; - - struct event_handler *handlers; - struct tep_function_handler *func_handlers; - - /* cache */ - struct tep_event_format *last_event; - - char *trace_clock; -}; - -static inline void tep_set_flag(struct tep_handle *pevent, int flag) -{ - pevent->flags |= flag; -} - -static inline unsigned short -__tep_data2host2(struct tep_handle *pevent, unsigned short data) -{ - unsigned short swap; - - if (pevent->host_bigendian == pevent->file_bigendian) - return data; - - swap = ((data & 0xffULL) << 8) | - ((data & (0xffULL << 8)) >> 8); - - return swap; -} - -static inline unsigned int -__tep_data2host4(struct tep_handle *pevent, unsigned int data) -{ - unsigned int swap; - - if (pevent->host_bigendian == pevent->file_bigendian) - return data; - - swap = ((data & 0xffULL) << 24) | - ((data & (0xffULL << 8)) << 8) | - ((data & (0xffULL << 16)) >> 8) | - ((data & (0xffULL << 24)) >> 24); - - return swap; -} - -static inline unsigned long long -__tep_data2host8(struct tep_handle *pevent, unsigned long long data) -{ - unsigned long long swap; - - if (pevent->host_bigendian == pevent->file_bigendian) - return data; - - swap = ((data & 0xffULL) << 56) | - ((data & (0xffULL << 8)) << 40) | - ((data & (0xffULL << 16)) << 24) | - ((data & (0xffULL << 24)) << 8) | - ((data & (0xffULL << 32)) >> 8) | - ((data & (0xffULL << 40)) >> 24) | - ((data & (0xffULL << 48)) >> 40) | - ((data & (0xffULL << 56)) >> 56); - - return swap; -} - -#define tep_data2host2(pevent, ptr) __tep_data2host2(pevent, *(unsigned short *)(ptr)) -#define tep_data2host4(pevent, ptr) __tep_data2host4(pevent, *(unsigned int *)(ptr)) -#define tep_data2host8(pevent, ptr) \ +#define tep_data2host2(pevent, ptr) __tep_data2host2(pevent, *(unsigned short *)(ptr)) +#define tep_data2host4(pevent, ptr) __tep_data2host4(pevent, *(unsigned int *)(ptr)) +#define tep_data2host8(pevent, ptr) \ ({ \ unsigned long long __val; \ \ @@ -655,11 +524,12 @@ unsigned long long tep_read_number(struct tep_handle *pevent, const void *ptr, i int tep_read_number_field(struct tep_format_field *field, const void *data, unsigned long long *value); +struct tep_event_format *tep_get_first_event(struct tep_handle *tep); +int tep_get_events_count(struct tep_handle *tep); struct tep_event_format *tep_find_event(struct tep_handle *pevent, int id); struct tep_event_format * tep_find_event_by_name(struct tep_handle *pevent, const char *sys, const char *name); - struct tep_event_format * tep_find_event_by_record(struct tep_handle *pevent, struct tep_record *record); @@ -689,65 +559,23 @@ struct tep_event_format **tep_list_events(struct tep_handle *pevent, enum tep_ev struct tep_format_field **tep_event_common_fields(struct tep_event_format *event); struct tep_format_field **tep_event_fields(struct tep_event_format *event); -static inline int tep_get_cpus(struct tep_handle *pevent) -{ - return pevent->cpus; -} - -static inline void tep_set_cpus(struct tep_handle *pevent, int cpus) -{ - pevent->cpus = cpus; -} - -static inline int tep_get_long_size(struct tep_handle *pevent) -{ - return pevent->long_size; -} - -static inline void tep_set_long_size(struct tep_handle *pevent, int long_size) -{ - pevent->long_size = long_size; -} - -static inline int tep_get_page_size(struct tep_handle *pevent) -{ - return pevent->page_size; -} - -static inline void tep_set_page_size(struct tep_handle *pevent, int _page_size) -{ - pevent->page_size = _page_size; -} - -static inline int tep_is_file_bigendian(struct tep_handle *pevent) -{ - return pevent->file_bigendian; -} - -static inline void tep_set_file_bigendian(struct tep_handle *pevent, int endian) -{ - pevent->file_bigendian = endian; -} - -static inline int tep_is_host_bigendian(struct tep_handle *pevent) -{ - return pevent->host_bigendian; -} - -static inline void tep_set_host_bigendian(struct tep_handle *pevent, int endian) -{ - pevent->host_bigendian = endian; -} - -static inline int tep_is_latency_format(struct tep_handle *pevent) -{ - return pevent->latency_format; -} - -static inline void tep_set_latency_format(struct tep_handle *pevent, int lat) -{ - pevent->latency_format = lat; -} +enum tep_endian { + TEP_LITTLE_ENDIAN = 0, + TEP_BIG_ENDIAN +}; +int tep_get_cpus(struct tep_handle *pevent); +void tep_set_cpus(struct tep_handle *pevent, int cpus); +int tep_get_long_size(struct tep_handle *pevent); +void tep_set_long_size(struct tep_handle *pevent, int long_size); +int tep_get_page_size(struct tep_handle *pevent); +void tep_set_page_size(struct tep_handle *pevent, int _page_size); +int tep_is_file_bigendian(struct tep_handle *pevent); +void tep_set_file_bigendian(struct tep_handle *pevent, enum tep_endian endian); +int tep_is_host_bigendian(struct tep_handle *pevent); +void tep_set_host_bigendian(struct tep_handle *pevent, enum tep_endian endian); +int tep_is_latency_format(struct tep_handle *pevent); +void tep_set_latency_format(struct tep_handle *pevent, int lat); +int tep_get_header_page_size(struct tep_handle *pevent); struct tep_handle *tep_alloc(void); void tep_free(struct tep_handle *pevent); diff --git a/tools/lib/traceevent/event-plugin.c b/tools/lib/traceevent/event-plugin.c index 46eb64eb0c2e..e74f16c88398 100644 --- a/tools/lib/traceevent/event-plugin.c +++ b/tools/lib/traceevent/event-plugin.c @@ -14,6 +14,7 @@ #include <unistd.h> #include <dirent.h> #include "event-parse.h" +#include "event-parse-local.h" #include "event-utils.h" #include "trace-seq.h" diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c index d64b6128fa7d..ed87cb56713d 100644 --- a/tools/lib/traceevent/parse-filter.c +++ b/tools/lib/traceevent/parse-filter.c @@ -11,6 +11,7 @@ #include <sys/types.h> #include "event-parse.h" +#include "event-parse-local.h" #include "event-utils.h" #define COMM "COMM" diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c index a4d7de1c96d1..9b78160e23a2 100644 --- a/tools/perf/util/trace-event-parse.c +++ b/tools/perf/util/trace-event-parse.c @@ -37,10 +37,11 @@ static int get_common_field(struct scripting_context *context, struct tep_format_field *field; if (!*size) { - if (!pevent->events) + + event = tep_get_first_event(pevent); + if (!event) return 0; - event = pevent->events[0]; field = tep_find_common_field(event, type); if (!field) return 0; @@ -192,25 +193,29 @@ struct tep_event_format *trace_find_next_event(struct tep_handle *pevent, struct tep_event_format *event) { static int idx; + int events_count; + struct tep_event_format *all_events; - if (!pevent || !pevent->events) + all_events = tep_get_first_event(pevent); + events_count = tep_get_events_count(pevent); + if (!pevent || !all_events || events_count < 1) return NULL; if (!event) { idx = 0; - return pevent->events[0]; + return all_events; } - if (idx < pevent->nr_events && event == pevent->events[idx]) { + if (idx < events_count && event == (all_events + idx)) { idx++; - if (idx == pevent->nr_events) + if (idx == events_count) return NULL; - return pevent->events[idx]; + return (all_events + idx); } - for (idx = 1; idx < pevent->nr_events; idx++) { - if (event == pevent->events[idx - 1]) - return pevent->events[idx]; + for (idx = 1; idx < events_count; idx++) { + if (event == (all_events + (idx - 1))) + return (all_events + idx); } return NULL; } diff --git a/tools/perf/util/trace-event-read.c b/tools/perf/util/trace-event-read.c index b98ee2a2eb44..cdf6de82a507 100644 --- a/tools/perf/util/trace-event-read.c +++ b/tools/perf/util/trace-event-read.c @@ -241,7 +241,7 @@ static int read_header_files(struct tep_handle *pevent) * The commit field in the page is of type long, * use that instead, since it represents the kernel. */ - tep_set_long_size(pevent, pevent->header_page_size_size); + tep_set_long_size(pevent, tep_get_header_page_size(pevent)); } free(header_page);