Re: [PATCH v4 2/7] perf/sdt: Directly record SDT events with 'perf record'
Hi Ravi, On Mon, 6 Mar 2017 18:42:59 +0530 Ravi Bangoriawrote: > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c > index bc84a37..e87b19b 100644 > --- a/tools/perf/builtin-record.c > +++ b/tools/perf/builtin-record.c > @@ -39,6 +39,7 @@ > #include "util/trigger.h" > #include "util/perf-hooks.h" > #include "asm/bug.h" > +#include "util/probe-file.h" > > #include > #include > @@ -73,6 +74,7 @@ struct record { > booltimestamp_filename; > struct switch_outputswitch_output; > unsigned long long samples; > + struct list_headsdt_event_list; > }; > > static volatile int auxtrace_record__snapshot_started; > @@ -1503,6 +1505,26 @@ static struct record record = { > }, > }; > > +void sdt_event_list__add(struct list_head *sdt_event_list) > +{ > + if (list_empty(sdt_event_list)) > + return; > + list_splice(sdt_event_list, _event_list); > +} > + > +bool is_cmd_record(void) > +{ > + return (record.evlist != NULL); > +} Hmm, exporting this from builtin-record.c makes inverted reference from builtin-record.o to libperf. I think it is not good way to do. [...] > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c > index 54355d3..1fcc9d13 100644 > --- a/tools/perf/util/parse-events.c > +++ b/tools/perf/util/parse-events.c > @@ -1727,12 +1727,66 @@ static void parse_events_print_error(struct > parse_events_error *err, > > #undef MAX_WIDTH > > +/* SDT event needs LIBELF support for creating a probe point */ > +#ifdef HAVE_LIBELF_SUPPORT > +static int parse_sdt_event(struct perf_evlist *evlist, const char *str, > +struct parse_events_error *err) > +{ > + char *ptr = NULL; > + int ret; > + struct list_head *sdt_evlist; > + struct sdt_event_list *sdt_event; > + > + if (str[0] == '%') > + str++; > + > + ptr = strdup(str); > + if (ptr == NULL) > + return -ENOMEM; > + > + sdt_evlist = zalloc(sizeof(*sdt_evlist)); > + if (!sdt_evlist) { > + free(ptr); > + pr_debug("Error in sdt_evlist memory allocation\n"); > + return -ENOMEM; > + } > + INIT_LIST_HEAD(sdt_evlist); > + > + /* > + * If there is an error in this call, no need to free > + * up sdt_evlist, its already free'ed up in the previous > + * call. Free up 'ptr' though. > + */ > + ret = add_sdt_event(ptr, sdt_evlist); > + if (!ret) { > + list_for_each_entry(sdt_event, sdt_evlist, list) { > + ret = parse_events(evlist, sdt_event->name, err); > + if (ret < 0) > + goto ret; > + } > + /* Add it to the record struct */ > + sdt_event_list__add(sdt_evlist); > + } > + > +ret: > + free(ptr); > + return ret; > +} > +#endif /* HAVE_LIBELF_SUPPORT */ > + > int parse_events_option(const struct option *opt, const char *str, > int unset __maybe_unused) > { > struct perf_evlist *evlist = *(struct perf_evlist **)opt->value; > struct parse_events_error err = { .idx = 0, }; > - int ret = parse_events(evlist, str, ); > + int ret = 0; > + > +#ifdef HAVE_LIBELF_SUPPORT > + if (is_sdt_event((char *)str) && is_cmd_record()) > + ret = parse_sdt_event(evlist, str, ); > + else > +#endif To avoid these ifdefs, could you add a dummy (error return) function of add_sdt_event() for !HAVE_LIBELF_SUPPORT case in probe-event.h? > + ret = parse_events(evlist, str, ); > > if (ret) > parse_events_print_error(, str); > diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h > index c6172cd..0887269 100644 > --- a/tools/perf/util/parse-events.h > +++ b/tools/perf/util/parse-events.h > @@ -208,4 +208,6 @@ static inline bool is_sdt_event(char *str) > (!strncmp(str, "sdt_", 4) && >!!strchr(str, ':') && !strchr(str, '='))); > } > + > +void sdt_event_list__add(struct list_head *sdt_event_list); > #endif /* __PERF_PARSE_EVENTS_H */ > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c > index 2b1409f..b879076 100644 > --- a/tools/perf/util/probe-event.c > +++ b/tools/perf/util/probe-event.c > @@ -1293,7 +1293,7 @@ int parse_line_range_desc(const char *arg, struct > line_range *lr) > return err; > } > > -static int parse_perf_probe_event_name(char **arg, struct perf_probe_event > *pev) > +int parse_perf_probe_event_name(char **arg, struct perf_probe_event *pev) > { > char *ptr; > > @@ -3125,8 +3125,8 @@ static int find_cached_events(struct perf_probe_event > *pev, > } > > /* Try to find probe_trace_event from all probe caches */ > -static int find_cached_events_all(struct perf_probe_event *pev, > -struct
Re: [PATCH v4 2/7] perf/sdt: Directly record SDT events with 'perf record'
Hi Ravi, On Mon, 6 Mar 2017 18:42:59 +0530 Ravi Bangoria wrote: > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c > index bc84a37..e87b19b 100644 > --- a/tools/perf/builtin-record.c > +++ b/tools/perf/builtin-record.c > @@ -39,6 +39,7 @@ > #include "util/trigger.h" > #include "util/perf-hooks.h" > #include "asm/bug.h" > +#include "util/probe-file.h" > > #include > #include > @@ -73,6 +74,7 @@ struct record { > booltimestamp_filename; > struct switch_outputswitch_output; > unsigned long long samples; > + struct list_headsdt_event_list; > }; > > static volatile int auxtrace_record__snapshot_started; > @@ -1503,6 +1505,26 @@ static struct record record = { > }, > }; > > +void sdt_event_list__add(struct list_head *sdt_event_list) > +{ > + if (list_empty(sdt_event_list)) > + return; > + list_splice(sdt_event_list, _event_list); > +} > + > +bool is_cmd_record(void) > +{ > + return (record.evlist != NULL); > +} Hmm, exporting this from builtin-record.c makes inverted reference from builtin-record.o to libperf. I think it is not good way to do. [...] > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c > index 54355d3..1fcc9d13 100644 > --- a/tools/perf/util/parse-events.c > +++ b/tools/perf/util/parse-events.c > @@ -1727,12 +1727,66 @@ static void parse_events_print_error(struct > parse_events_error *err, > > #undef MAX_WIDTH > > +/* SDT event needs LIBELF support for creating a probe point */ > +#ifdef HAVE_LIBELF_SUPPORT > +static int parse_sdt_event(struct perf_evlist *evlist, const char *str, > +struct parse_events_error *err) > +{ > + char *ptr = NULL; > + int ret; > + struct list_head *sdt_evlist; > + struct sdt_event_list *sdt_event; > + > + if (str[0] == '%') > + str++; > + > + ptr = strdup(str); > + if (ptr == NULL) > + return -ENOMEM; > + > + sdt_evlist = zalloc(sizeof(*sdt_evlist)); > + if (!sdt_evlist) { > + free(ptr); > + pr_debug("Error in sdt_evlist memory allocation\n"); > + return -ENOMEM; > + } > + INIT_LIST_HEAD(sdt_evlist); > + > + /* > + * If there is an error in this call, no need to free > + * up sdt_evlist, its already free'ed up in the previous > + * call. Free up 'ptr' though. > + */ > + ret = add_sdt_event(ptr, sdt_evlist); > + if (!ret) { > + list_for_each_entry(sdt_event, sdt_evlist, list) { > + ret = parse_events(evlist, sdt_event->name, err); > + if (ret < 0) > + goto ret; > + } > + /* Add it to the record struct */ > + sdt_event_list__add(sdt_evlist); > + } > + > +ret: > + free(ptr); > + return ret; > +} > +#endif /* HAVE_LIBELF_SUPPORT */ > + > int parse_events_option(const struct option *opt, const char *str, > int unset __maybe_unused) > { > struct perf_evlist *evlist = *(struct perf_evlist **)opt->value; > struct parse_events_error err = { .idx = 0, }; > - int ret = parse_events(evlist, str, ); > + int ret = 0; > + > +#ifdef HAVE_LIBELF_SUPPORT > + if (is_sdt_event((char *)str) && is_cmd_record()) > + ret = parse_sdt_event(evlist, str, ); > + else > +#endif To avoid these ifdefs, could you add a dummy (error return) function of add_sdt_event() for !HAVE_LIBELF_SUPPORT case in probe-event.h? > + ret = parse_events(evlist, str, ); > > if (ret) > parse_events_print_error(, str); > diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h > index c6172cd..0887269 100644 > --- a/tools/perf/util/parse-events.h > +++ b/tools/perf/util/parse-events.h > @@ -208,4 +208,6 @@ static inline bool is_sdt_event(char *str) > (!strncmp(str, "sdt_", 4) && >!!strchr(str, ':') && !strchr(str, '='))); > } > + > +void sdt_event_list__add(struct list_head *sdt_event_list); > #endif /* __PERF_PARSE_EVENTS_H */ > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c > index 2b1409f..b879076 100644 > --- a/tools/perf/util/probe-event.c > +++ b/tools/perf/util/probe-event.c > @@ -1293,7 +1293,7 @@ int parse_line_range_desc(const char *arg, struct > line_range *lr) > return err; > } > > -static int parse_perf_probe_event_name(char **arg, struct perf_probe_event > *pev) > +int parse_perf_probe_event_name(char **arg, struct perf_probe_event *pev) > { > char *ptr; > > @@ -3125,8 +3125,8 @@ static int find_cached_events(struct perf_probe_event > *pev, > } > > /* Try to find probe_trace_event from all probe caches */ > -static int find_cached_events_all(struct perf_probe_event *pev, > -struct probe_trace_event **tevs) > +int
[PATCH v4 2/7] perf/sdt: Directly record SDT events with 'perf record'
From: Hemant KumarAdd basic support for directly recording SDT events which are present in the probe cache. Without this patch, we could probe into SDT events using 'perf probe' and 'perf record'. With this patch, we can probe the SDT events directly using 'perf record'. For example : $ sudo ./perf list sdt sdt_libpthread:mutex_entry [SDT event] sdt_libc:setjmp[SDT event] $ sudo ./perf record -a -e sdt_libc:setjmp $ sudo ./perf script bash 793 [002] 260.382957: sdt_libc:setjmp: (7ff85b6596a1) reset 1296 [000] 260.511983: sdt_libc:setjmp: (7f26862e06a1) Recording on SDT events with same provider and marker names is also supported: $ readelf -n /usr/lib64/libpthread-2.24.so | grep -A2 Provider Provider: libpthread Name: mutex_entry Location: 0x9ddb, Base: 0x000139cc, ... -- Provider: libpthread Name: mutex_entry Location: 0xbcbb, Base: 0x000139cc, ... $ sudo ./perf record -a -e sdt_libpthread:mutex_entry Warning : Recording on 2 occurences of sdt_libpthread:mutex_entry $ sudo ./perf evlist sdt_libpthread:mutex_entry_1 sdt_libpthread:mutex_entry After invoking 'perf record', behind the scenes, it checks whether the event specified is an SDT event using the string 'sdt_' or flag '%'. After that, it does a lookup of the probe cache to find out the SDT event. If its not present, it throws an error. Otherwise, it goes on and writes the event into the uprobe_events file and starts recording. It also maintains a list of the event names that were written to uprobe_events file. At the end of the record session, it removes the events from the uprobe_events file using the maintained name list. As mentioned, it always tries to look for sdt event in probe cache and ignores entries of uprobe_events. Hence, it creates new probe points for event even if it already exists. $ sudo ./perf probe sdt_libpthread:mutex_entry Added new events: sdt_libpthread:mutex_entry (on %mutex_entry in /usr/lib64/libpthread-2.24.so) sdt_libpthread:mutex_entry_1 (on %mutex_entry in /usr/lib64/libpthread-2.24.so) $ sudo ./perf record -a -e sdt_libpthread:mutex_entry Warning : Recording on 2 occurences of sdt_libpthread:mutex_entry $ sudo ./perf evlist sdt_libpthread:mutex_entry_3 sdt_libpthread:mutex_entry_2 As it does not look at uprobe_events file, it can't record those events whose probe points are created with different name. For ex, $ sudo ./perf record -a -e sdt_libpthread:mutex_entry_1 Error: sdt_libpthread:mutex_entry_1 not found in the cache invalid or unsupported event: 'sdt_libpthread:mutex_entry_1' Signed-off-by: Hemant Kumar Signed-off-by: Ravi Bangoria --- tools/perf/builtin-record.c| 24 tools/perf/perf.h | 1 + tools/perf/util/parse-events.c | 56 +- tools/perf/util/parse-events.h | 2 + tools/perf/util/probe-event.c | 35 ++- tools/perf/util/probe-event.h | 4 ++ tools/perf/util/probe-file.c | 131 + tools/perf/util/probe-file.h | 8 +++ 8 files changed, 257 insertions(+), 4 deletions(-) diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c index bc84a37..e87b19b 100644 --- a/tools/perf/builtin-record.c +++ b/tools/perf/builtin-record.c @@ -39,6 +39,7 @@ #include "util/trigger.h" #include "util/perf-hooks.h" #include "asm/bug.h" +#include "util/probe-file.h" #include #include @@ -73,6 +74,7 @@ struct record { booltimestamp_filename; struct switch_outputswitch_output; unsigned long long samples; + struct list_headsdt_event_list; }; static volatile int auxtrace_record__snapshot_started; @@ -1503,6 +1505,26 @@ static struct record record = { }, }; +void sdt_event_list__add(struct list_head *sdt_event_list) +{ + if (list_empty(sdt_event_list)) + return; + list_splice(sdt_event_list, _event_list); +} + +bool is_cmd_record(void) +{ + return (record.evlist != NULL); +} + +static void +sdt_event_list__remove(struct list_head *sdt_event_list __maybe_unused) +{ +#ifdef HAVE_LIBELF_SUPPORT + return remove_sdt_event_list(sdt_event_list); +#endif +} + const char record_callchain_help[] = CALLCHAIN_RECORD_HELP "\n\t\t\t\tDefault: fp"; @@ -1671,6 +1693,7 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused) if (rec->evlist == NULL) return -ENOMEM; + INIT_LIST_HEAD(>sdt_event_list); err = perf_config(perf_record_config, rec); if (err) return err; @@ -1841,6 +1864,7 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused)
[PATCH v4 2/7] perf/sdt: Directly record SDT events with 'perf record'
From: Hemant Kumar Add basic support for directly recording SDT events which are present in the probe cache. Without this patch, we could probe into SDT events using 'perf probe' and 'perf record'. With this patch, we can probe the SDT events directly using 'perf record'. For example : $ sudo ./perf list sdt sdt_libpthread:mutex_entry [SDT event] sdt_libc:setjmp[SDT event] $ sudo ./perf record -a -e sdt_libc:setjmp $ sudo ./perf script bash 793 [002] 260.382957: sdt_libc:setjmp: (7ff85b6596a1) reset 1296 [000] 260.511983: sdt_libc:setjmp: (7f26862e06a1) Recording on SDT events with same provider and marker names is also supported: $ readelf -n /usr/lib64/libpthread-2.24.so | grep -A2 Provider Provider: libpthread Name: mutex_entry Location: 0x9ddb, Base: 0x000139cc, ... -- Provider: libpthread Name: mutex_entry Location: 0xbcbb, Base: 0x000139cc, ... $ sudo ./perf record -a -e sdt_libpthread:mutex_entry Warning : Recording on 2 occurences of sdt_libpthread:mutex_entry $ sudo ./perf evlist sdt_libpthread:mutex_entry_1 sdt_libpthread:mutex_entry After invoking 'perf record', behind the scenes, it checks whether the event specified is an SDT event using the string 'sdt_' or flag '%'. After that, it does a lookup of the probe cache to find out the SDT event. If its not present, it throws an error. Otherwise, it goes on and writes the event into the uprobe_events file and starts recording. It also maintains a list of the event names that were written to uprobe_events file. At the end of the record session, it removes the events from the uprobe_events file using the maintained name list. As mentioned, it always tries to look for sdt event in probe cache and ignores entries of uprobe_events. Hence, it creates new probe points for event even if it already exists. $ sudo ./perf probe sdt_libpthread:mutex_entry Added new events: sdt_libpthread:mutex_entry (on %mutex_entry in /usr/lib64/libpthread-2.24.so) sdt_libpthread:mutex_entry_1 (on %mutex_entry in /usr/lib64/libpthread-2.24.so) $ sudo ./perf record -a -e sdt_libpthread:mutex_entry Warning : Recording on 2 occurences of sdt_libpthread:mutex_entry $ sudo ./perf evlist sdt_libpthread:mutex_entry_3 sdt_libpthread:mutex_entry_2 As it does not look at uprobe_events file, it can't record those events whose probe points are created with different name. For ex, $ sudo ./perf record -a -e sdt_libpthread:mutex_entry_1 Error: sdt_libpthread:mutex_entry_1 not found in the cache invalid or unsupported event: 'sdt_libpthread:mutex_entry_1' Signed-off-by: Hemant Kumar Signed-off-by: Ravi Bangoria --- tools/perf/builtin-record.c| 24 tools/perf/perf.h | 1 + tools/perf/util/parse-events.c | 56 +- tools/perf/util/parse-events.h | 2 + tools/perf/util/probe-event.c | 35 ++- tools/perf/util/probe-event.h | 4 ++ tools/perf/util/probe-file.c | 131 + tools/perf/util/probe-file.h | 8 +++ 8 files changed, 257 insertions(+), 4 deletions(-) diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c index bc84a37..e87b19b 100644 --- a/tools/perf/builtin-record.c +++ b/tools/perf/builtin-record.c @@ -39,6 +39,7 @@ #include "util/trigger.h" #include "util/perf-hooks.h" #include "asm/bug.h" +#include "util/probe-file.h" #include #include @@ -73,6 +74,7 @@ struct record { booltimestamp_filename; struct switch_outputswitch_output; unsigned long long samples; + struct list_headsdt_event_list; }; static volatile int auxtrace_record__snapshot_started; @@ -1503,6 +1505,26 @@ static struct record record = { }, }; +void sdt_event_list__add(struct list_head *sdt_event_list) +{ + if (list_empty(sdt_event_list)) + return; + list_splice(sdt_event_list, _event_list); +} + +bool is_cmd_record(void) +{ + return (record.evlist != NULL); +} + +static void +sdt_event_list__remove(struct list_head *sdt_event_list __maybe_unused) +{ +#ifdef HAVE_LIBELF_SUPPORT + return remove_sdt_event_list(sdt_event_list); +#endif +} + const char record_callchain_help[] = CALLCHAIN_RECORD_HELP "\n\t\t\t\tDefault: fp"; @@ -1671,6 +1693,7 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused) if (rec->evlist == NULL) return -ENOMEM; + INIT_LIST_HEAD(>sdt_event_list); err = perf_config(perf_record_config, rec); if (err) return err; @@ -1841,6 +1864,7 @@ int cmd_record(int argc, const char **argv, const char *prefix __maybe_unused) perf_evlist__delete(rec->evlist); symbol__exit();