From a43aac299c3abc09eff856039f5b72166b780d35 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Thu, 10 Sep 2015 11:27:04 +0900 Subject: [PATCH 1/8] perf probe: Free perf_probe_event in cleanup_perf_probe_events() The cleanup_perf_probe_events() frees all resources related to a perf probe event. However it only freed resources in trace probe events, not perf probe events. So call clear_perf_probe_event() too. Reported-by: Wang Nan Signed-off-by: Namhyung Kim Acked-by: Masami Hiramatsu Cc: Jiri Olsa Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/1441852026-28974-1-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/probe-event.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c index 5964eccbe94d..3d7d60cc6f16 100644 --- a/tools/perf/util/probe-event.c +++ b/tools/perf/util/probe-event.c @@ -2792,6 +2792,7 @@ void cleanup_perf_probe_events(struct perf_probe_event *pevs, int npevs) clear_probe_trace_event(&pevs[i].tevs[j]); zfree(&pevs[i].tevs); pevs[i].ntevs = 0; + clear_perf_probe_event(&pevs[i]); } exit_symbol_maps(); From 9bae1e8c3fe5359ce17309b894f54667fd563e98 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Thu, 10 Sep 2015 11:27:05 +0900 Subject: [PATCH 2/8] perf probe: Export init/exit_probe_symbol_maps() The init/exit_symbols_maps() functions are to setup and cleanup necessary info for probe events. But they need to be called from out of the probe code now, so this patch exports them. However the names are too generic, so change them to have 'probe'. :) Signed-off-by: Namhyung Kim Acked-by: Masami Hiramatsu Cc: Jiri Olsa Cc: Peter Zijlstra Cc: Wang Nan Link: http://lkml.kernel.org/r/1441852026-28974-2-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-probe.c | 5 +++++ tools/perf/util/probe-event.c | 31 +++++++++++++++---------------- tools/perf/util/probe-event.h | 2 ++ 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c index 2bec9c1ef2a3..94385ee89dc8 100644 --- a/tools/perf/builtin-probe.c +++ b/tools/perf/builtin-probe.c @@ -317,6 +317,10 @@ static int perf_add_probe_events(struct perf_probe_event *pevs, int npevs) int i, k; const char *event = NULL, *group = NULL; + ret = init_probe_symbol_maps(pevs->uprobes); + if (ret < 0) + return ret; + ret = convert_perf_probe_events(pevs, npevs); if (ret < 0) goto out_cleanup; @@ -354,6 +358,7 @@ static int perf_add_probe_events(struct perf_probe_event *pevs, int npevs) out_cleanup: cleanup_perf_probe_events(pevs, npevs); + exit_probe_symbol_maps(); return ret; } diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c index 3d7d60cc6f16..2b78e8f19b45 100644 --- a/tools/perf/util/probe-event.c +++ b/tools/perf/util/probe-event.c @@ -71,7 +71,7 @@ static char *synthesize_perf_probe_point(struct perf_probe_point *pp); static struct machine *host_machine; /* Initialize symbol maps and path of vmlinux/modules */ -static int init_symbol_maps(bool user_only) +int init_probe_symbol_maps(bool user_only) { int ret; @@ -101,7 +101,7 @@ out: return ret; } -static void exit_symbol_maps(void) +void exit_probe_symbol_maps(void) { if (host_machine) { machine__delete(host_machine); @@ -859,11 +859,11 @@ int show_line_range(struct line_range *lr, const char *module, bool user) { int ret; - ret = init_symbol_maps(user); + ret = init_probe_symbol_maps(user); if (ret < 0) return ret; ret = __show_line_range(lr, module, user); - exit_symbol_maps(); + exit_probe_symbol_maps(); return ret; } @@ -941,7 +941,7 @@ int show_available_vars(struct perf_probe_event *pevs, int npevs, int i, ret = 0; struct debuginfo *dinfo; - ret = init_symbol_maps(pevs->uprobes); + ret = init_probe_symbol_maps(pevs->uprobes); if (ret < 0) return ret; @@ -958,7 +958,7 @@ int show_available_vars(struct perf_probe_event *pevs, int npevs, debuginfo__delete(dinfo); out: - exit_symbol_maps(); + exit_probe_symbol_maps(); return ret; } @@ -2262,7 +2262,7 @@ int show_perf_probe_events(struct strfilter *filter) setup_pager(); - ret = init_symbol_maps(false); + ret = init_probe_symbol_maps(false); if (ret < 0) return ret; @@ -2278,7 +2278,7 @@ int show_perf_probe_events(struct strfilter *filter) close(kp_fd); if (up_fd > 0) close(up_fd); - exit_symbol_maps(); + exit_probe_symbol_maps(); return ret; } @@ -2746,10 +2746,6 @@ int convert_perf_probe_events(struct perf_probe_event *pevs, int npevs) { int i, ret; - ret = init_symbol_maps(pevs->uprobes); - if (ret < 0) - return ret; - /* Loop 1: convert all events */ for (i = 0; i < npevs; i++) { /* Init kprobe blacklist if needed */ @@ -2794,20 +2790,23 @@ void cleanup_perf_probe_events(struct perf_probe_event *pevs, int npevs) pevs[i].ntevs = 0; clear_perf_probe_event(&pevs[i]); } - - exit_symbol_maps(); } int add_perf_probe_events(struct perf_probe_event *pevs, int npevs) { int ret; + ret = init_probe_symbol_maps(pevs->uprobes); + if (ret < 0) + return ret; + ret = convert_perf_probe_events(pevs, npevs); if (ret == 0) ret = apply_perf_probe_events(pevs, npevs); cleanup_perf_probe_events(pevs, npevs); + exit_probe_symbol_maps(); return ret; } @@ -2867,7 +2866,7 @@ int show_available_funcs(const char *target, struct strfilter *_filter, struct map *map; int ret; - ret = init_symbol_maps(user); + ret = init_probe_symbol_maps(user); if (ret < 0) return ret; @@ -2897,7 +2896,7 @@ end: if (user) { map__put(map); } - exit_symbol_maps(); + exit_probe_symbol_maps(); return ret; } diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h index 9bcea36359f2..ba926c30f8cd 100644 --- a/tools/perf/util/probe-event.h +++ b/tools/perf/util/probe-event.h @@ -110,6 +110,8 @@ struct variable_list { }; struct map; +int init_probe_symbol_maps(bool user_only); +void exit_probe_symbol_maps(void); /* Command string to events */ extern int parse_perf_probe_command(const char *cmd, From 01ca9fd41d6f2ad796a6b109b5253e06b6ae6dc7 Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Mon, 7 Sep 2015 10:38:03 +0200 Subject: [PATCH 3/8] tools: Add err.h with ERR_PTR PTR_ERR interface Adding part of the kernel's interface: inline void * __must_check ERR_PTR(long error); inline long __must_check PTR_ERR(__force const void *ptr); inline bool __must_check IS_ERR(__force const void *ptr); It will be used to propagate error through pointers in following patches. Signed-off-by: Jiri Olsa Reviewed-by: Raphael Beamonte Cc: David Ahern Cc: Matt Fleming Cc: Namhyung Kim Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/1441615087-13886-2-git-send-email-jolsa@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/include/linux/err.h | 49 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 tools/include/linux/err.h diff --git a/tools/include/linux/err.h b/tools/include/linux/err.h new file mode 100644 index 000000000000..c9ada48f5156 --- /dev/null +++ b/tools/include/linux/err.h @@ -0,0 +1,49 @@ +#ifndef __TOOLS_LINUX_ERR_H +#define __TOOLS_LINUX_ERR_H + +#include +#include + +#include + +/* + * Original kernel header comment: + * + * Kernel pointers have redundant information, so we can use a + * scheme where we can return either an error code or a normal + * pointer with the same return value. + * + * This should be a per-architecture thing, to allow different + * error and pointer decisions. + * + * Userspace note: + * The same principle works for userspace, because 'error' pointers + * fall down to the unused hole far from user space, as described + * in Documentation/x86/x86_64/mm.txt for x86_64 arch: + * + * 0000000000000000 - 00007fffffffffff (=47 bits) user space, different per mm hole caused by [48:63] sign extension + * ffffffffffe00000 - ffffffffffffffff (=2 MB) unused hole + * + * It should be the same case for other architectures, because + * this code is used in generic kernel code. + */ +#define MAX_ERRNO 4095 + +#define IS_ERR_VALUE(x) unlikely((x) >= (unsigned long)-MAX_ERRNO) + +static inline void * __must_check ERR_PTR(long error) +{ + return (void *) error; +} + +static inline long __must_check PTR_ERR(__force const void *ptr) +{ + return (long) ptr; +} + +static inline bool __must_check IS_ERR(__force const void *ptr) +{ + return IS_ERR_VALUE((unsigned long)ptr); +} + +#endif /* _LINUX_ERR_H */ From e2f9f8ea6a54e252e3a94a5c2321f673b5b97360 Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Mon, 7 Sep 2015 10:38:05 +0200 Subject: [PATCH 4/8] perf tools: Propagate error info for the tracepoint parsing Pass 'struct parse_events_error *error' to the parse-event.c tracepoint adding path. It will be filled with error data in following patches. Signed-off-by: Jiri Olsa Reviewed-by: Raphael Beamonte Reviewed-by: Matt Fleming Cc: David Ahern Cc: Namhyung Kim Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/1441615087-13886-4-git-send-email-jolsa@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/parse-events.c | 27 ++++++++++++++++----------- tools/perf/util/parse-events.h | 3 ++- tools/perf/util/parse-events.y | 4 ++-- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index 3840176642f8..1b284b8ad243 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -387,7 +387,8 @@ int parse_events_add_cache(struct list_head *list, int *idx, } static int add_tracepoint(struct list_head *list, int *idx, - char *sys_name, char *evt_name) + char *sys_name, char *evt_name, + struct parse_events_error *error __maybe_unused) { struct perf_evsel *evsel; @@ -401,7 +402,8 @@ static int add_tracepoint(struct list_head *list, int *idx, } static int add_tracepoint_multi_event(struct list_head *list, int *idx, - char *sys_name, char *evt_name) + char *sys_name, char *evt_name, + struct parse_events_error *error) { char evt_path[MAXPATHLEN]; struct dirent *evt_ent; @@ -425,7 +427,7 @@ static int add_tracepoint_multi_event(struct list_head *list, int *idx, if (!strglobmatch(evt_ent->d_name, evt_name)) continue; - ret = add_tracepoint(list, idx, sys_name, evt_ent->d_name); + ret = add_tracepoint(list, idx, sys_name, evt_ent->d_name, error); } closedir(evt_dir); @@ -433,15 +435,17 @@ static int add_tracepoint_multi_event(struct list_head *list, int *idx, } static int add_tracepoint_event(struct list_head *list, int *idx, - char *sys_name, char *evt_name) + char *sys_name, char *evt_name, + struct parse_events_error *error) { return strpbrk(evt_name, "*?") ? - add_tracepoint_multi_event(list, idx, sys_name, evt_name) : - add_tracepoint(list, idx, sys_name, evt_name); + add_tracepoint_multi_event(list, idx, sys_name, evt_name, error) : + add_tracepoint(list, idx, sys_name, evt_name, error); } static int add_tracepoint_multi_sys(struct list_head *list, int *idx, - char *sys_name, char *evt_name) + char *sys_name, char *evt_name, + struct parse_events_error *error) { struct dirent *events_ent; DIR *events_dir; @@ -465,7 +469,7 @@ static int add_tracepoint_multi_sys(struct list_head *list, int *idx, continue; ret = add_tracepoint_event(list, idx, events_ent->d_name, - evt_name); + evt_name, error); } closedir(events_dir); @@ -473,12 +477,13 @@ static int add_tracepoint_multi_sys(struct list_head *list, int *idx, } int parse_events_add_tracepoint(struct list_head *list, int *idx, - char *sys, char *event) + char *sys, char *event, + struct parse_events_error *error) { if (strpbrk(sys, "*?")) - return add_tracepoint_multi_sys(list, idx, sys, event); + return add_tracepoint_multi_sys(list, idx, sys, event, error); else - return add_tracepoint_event(list, idx, sys, event); + return add_tracepoint_event(list, idx, sys, event, error); } static int diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h index a09b0e210997..ffee7ece75a6 100644 --- a/tools/perf/util/parse-events.h +++ b/tools/perf/util/parse-events.h @@ -118,7 +118,8 @@ int parse_events__modifier_event(struct list_head *list, char *str, bool add); int parse_events__modifier_group(struct list_head *list, char *event_mod); int parse_events_name(struct list_head *list, char *name); int parse_events_add_tracepoint(struct list_head *list, int *idx, - char *sys, char *event); + char *sys, char *event, + struct parse_events_error *error); int parse_events_add_numeric(struct parse_events_evlist *data, struct list_head *list, u32 type, u64 config, diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y index 9cd70819c795..54a3004a8192 100644 --- a/tools/perf/util/parse-events.y +++ b/tools/perf/util/parse-events.y @@ -376,7 +376,7 @@ PE_NAME '-' PE_NAME ':' PE_NAME snprintf(&sys_name, 128, "%s-%s", $1, $3); ALLOC_LIST(list); - ABORT_ON(parse_events_add_tracepoint(list, &data->idx, &sys_name, $5)); + ABORT_ON(parse_events_add_tracepoint(list, &data->idx, &sys_name, $5, data->error)); $$ = list; } | @@ -386,7 +386,7 @@ PE_NAME ':' PE_NAME struct list_head *list; ALLOC_LIST(list); - if (parse_events_add_tracepoint(list, &data->idx, $1, $3)) { + if (parse_events_add_tracepoint(list, &data->idx, $1, $3, data->error)) { struct parse_events_error *error = data->error; if (error) { From 8dd2a1317eba2c207632dcb19adb7cb746861652 Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Mon, 7 Sep 2015 10:38:06 +0200 Subject: [PATCH 5/8] perf evsel: Propagate error info from tp_format Propagate error info from tp_format via ERR_PTR to get it all the way down to the parse-event.c tracepoint adding routines. Following functions now return pointer with encoded error: - tp_format - trace_event__tp_format - perf_evsel__newtp_idx - perf_evsel__newtp This affects several other places in perf, that cannot use pointer check anymore, but must utilize the err.h interface, when getting error information from above functions list. Signed-off-by: Jiri Olsa Cc: David Ahern Cc: Matt Fleming Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Raphael Beamonte Link: http://lkml.kernel.org/r/1441615087-13886-5-git-send-email-jolsa@kernel.org [ Add two missing ERR_PTR() and one IS_ERR() ] Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-trace.c | 19 +++++++++++-------- tools/perf/tests/evsel-tp-sched.c | 10 ++++++++-- tools/perf/tests/mmap-basic.c | 3 ++- tools/perf/tests/openat-syscall-all-cpus.c | 3 ++- tools/perf/tests/openat-syscall-tp-fields.c | 3 ++- tools/perf/tests/openat-syscall.c | 3 ++- tools/perf/util/evlist.c | 3 ++- tools/perf/util/evsel.c | 16 +++++++++++++--- tools/perf/util/evsel.h | 3 +++ tools/perf/util/parse-events.c | 6 +++--- tools/perf/util/trace-event.c | 15 ++++++++++++--- 11 files changed, 60 insertions(+), 24 deletions(-) diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c index 215653274102..93b80f12f35e 100644 --- a/tools/perf/builtin-trace.c +++ b/tools/perf/builtin-trace.c @@ -38,6 +38,7 @@ #include #include #include +#include /* For older distros: */ #ifndef MAP_STACK @@ -245,13 +246,14 @@ static struct perf_evsel *perf_evsel__syscall_newtp(const char *direction, void struct perf_evsel *evsel = perf_evsel__newtp("raw_syscalls", direction); /* older kernel (e.g., RHEL6) use syscalls:{enter,exit} */ - if (evsel == NULL) + if (IS_ERR(evsel)) evsel = perf_evsel__newtp("syscalls", direction); - if (evsel) { - if (perf_evsel__init_syscall_tp(evsel, handler)) - goto out_delete; - } + if (IS_ERR(evsel)) + return NULL; + + if (perf_evsel__init_syscall_tp(evsel, handler)) + goto out_delete; return evsel; @@ -1705,12 +1707,12 @@ static int trace__read_syscall_info(struct trace *trace, int id) snprintf(tp_name, sizeof(tp_name), "sys_enter_%s", sc->name); sc->tp_format = trace_event__tp_format("syscalls", tp_name); - if (sc->tp_format == NULL && sc->fmt && sc->fmt->alias) { + if (IS_ERR(sc->tp_format) && sc->fmt && sc->fmt->alias) { snprintf(tp_name, sizeof(tp_name), "sys_enter_%s", sc->fmt->alias); sc->tp_format = trace_event__tp_format("syscalls", tp_name); } - if (sc->tp_format == NULL) + if (IS_ERR(sc->tp_format)) return -1; sc->args = sc->tp_format->format.fields; @@ -2390,7 +2392,8 @@ static size_t trace__fprintf_thread_summary(struct trace *trace, FILE *fp); static bool perf_evlist__add_vfs_getname(struct perf_evlist *evlist) { struct perf_evsel *evsel = perf_evsel__newtp("probe", "vfs_getname"); - if (evsel == NULL) + + if (IS_ERR(evsel)) return false; if (perf_evsel__field(evsel, "pathname") == NULL) { diff --git a/tools/perf/tests/evsel-tp-sched.c b/tools/perf/tests/evsel-tp-sched.c index 52162425c969..790e413d9a1f 100644 --- a/tools/perf/tests/evsel-tp-sched.c +++ b/tools/perf/tests/evsel-tp-sched.c @@ -1,3 +1,4 @@ +#include #include #include "evsel.h" #include "tests.h" @@ -36,8 +37,8 @@ int test__perf_evsel__tp_sched_test(void) struct perf_evsel *evsel = perf_evsel__newtp("sched", "sched_switch"); int ret = 0; - if (evsel == NULL) { - pr_debug("perf_evsel__new\n"); + if (IS_ERR(evsel)) { + pr_debug("perf_evsel__newtp failed with %ld\n", PTR_ERR(evsel)); return -1; } @@ -66,6 +67,11 @@ int test__perf_evsel__tp_sched_test(void) evsel = perf_evsel__newtp("sched", "sched_wakeup"); + if (IS_ERR(evsel)) { + pr_debug("perf_evsel__newtp failed with %ld\n", PTR_ERR(evsel)); + return -1; + } + if (perf_evsel__test_field(evsel, "comm", 16, true)) ret = -1; diff --git a/tools/perf/tests/mmap-basic.c b/tools/perf/tests/mmap-basic.c index 666b67a4df9d..4495493c9431 100644 --- a/tools/perf/tests/mmap-basic.c +++ b/tools/perf/tests/mmap-basic.c @@ -3,6 +3,7 @@ #include "thread_map.h" #include "cpumap.h" #include "tests.h" +#include /* * This test will generate random numbers of calls to some getpid syscalls, @@ -65,7 +66,7 @@ int test__basic_mmap(void) snprintf(name, sizeof(name), "sys_enter_%s", syscall_names[i]); evsels[i] = perf_evsel__newtp("syscalls", name); - if (evsels[i] == NULL) { + if (IS_ERR(evsels[i])) { pr_debug("perf_evsel__new\n"); goto out_delete_evlist; } diff --git a/tools/perf/tests/openat-syscall-all-cpus.c b/tools/perf/tests/openat-syscall-all-cpus.c index 495d8126b722..9e104a2e973d 100644 --- a/tools/perf/tests/openat-syscall-all-cpus.c +++ b/tools/perf/tests/openat-syscall-all-cpus.c @@ -1,4 +1,5 @@ #include +#include #include "evsel.h" #include "tests.h" #include "thread_map.h" @@ -31,7 +32,7 @@ int test__openat_syscall_event_on_all_cpus(void) CPU_ZERO(&cpu_set); evsel = perf_evsel__newtp("syscalls", "sys_enter_openat"); - if (evsel == NULL) { + if (IS_ERR(evsel)) { tracing_path__strerror_open_tp(errno, errbuf, sizeof(errbuf), "syscalls", "sys_enter_openat"); pr_err("%s\n", errbuf); goto out_thread_map_delete; diff --git a/tools/perf/tests/openat-syscall-tp-fields.c b/tools/perf/tests/openat-syscall-tp-fields.c index 01a19626c846..473d3869727e 100644 --- a/tools/perf/tests/openat-syscall-tp-fields.c +++ b/tools/perf/tests/openat-syscall-tp-fields.c @@ -1,3 +1,4 @@ +#include #include "perf.h" #include "evlist.h" #include "evsel.h" @@ -30,7 +31,7 @@ int test__syscall_openat_tp_fields(void) } evsel = perf_evsel__newtp("syscalls", "sys_enter_openat"); - if (evsel == NULL) { + if (IS_ERR(evsel)) { pr_debug("%s: perf_evsel__newtp\n", __func__); goto out_delete_evlist; } diff --git a/tools/perf/tests/openat-syscall.c b/tools/perf/tests/openat-syscall.c index 08ac9d94a050..7b1db8306098 100644 --- a/tools/perf/tests/openat-syscall.c +++ b/tools/perf/tests/openat-syscall.c @@ -1,4 +1,5 @@ #include +#include #include "thread_map.h" #include "evsel.h" #include "debug.h" @@ -19,7 +20,7 @@ int test__openat_syscall_event(void) } evsel = perf_evsel__newtp("syscalls", "sys_enter_openat"); - if (evsel == NULL) { + if (IS_ERR(evsel)) { tracing_path__strerror_open_tp(errno, errbuf, sizeof(errbuf), "syscalls", "sys_enter_openat"); pr_err("%s\n", errbuf); goto out_thread_map_delete; diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index d51a5200c8af..3cb2bf9bd4bd 100644 --- a/tools/perf/util/evlist.c +++ b/tools/perf/util/evlist.c @@ -25,6 +25,7 @@ #include #include #include +#include static void perf_evlist__mmap_put(struct perf_evlist *evlist, int idx); static void __perf_evlist__munmap(struct perf_evlist *evlist, int idx); @@ -265,7 +266,7 @@ int perf_evlist__add_newtp(struct perf_evlist *evlist, { struct perf_evsel *evsel = perf_evsel__newtp(sys, name); - if (evsel == NULL) + if (IS_ERR(evsel)) return -1; evsel->handler = handler; diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c index 771ade4d5966..6b5d1b509148 100644 --- a/tools/perf/util/evsel.c +++ b/tools/perf/util/evsel.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include "asm/bug.h" #include "callchain.h" @@ -225,11 +226,17 @@ struct perf_evsel *perf_evsel__new_idx(struct perf_event_attr *attr, int idx) return evsel; } +/* + * Returns pointer with encoded error via interface. + */ struct perf_evsel *perf_evsel__newtp_idx(const char *sys, const char *name, int idx) { struct perf_evsel *evsel = zalloc(perf_evsel__object.size); + int err = -ENOMEM; - if (evsel != NULL) { + if (evsel == NULL) { + goto out_err; + } else { struct perf_event_attr attr = { .type = PERF_TYPE_TRACEPOINT, .sample_type = (PERF_SAMPLE_RAW | PERF_SAMPLE_TIME | @@ -240,8 +247,10 @@ struct perf_evsel *perf_evsel__newtp_idx(const char *sys, const char *name, int goto out_free; evsel->tp_format = trace_event__tp_format(sys, name); - if (evsel->tp_format == NULL) + if (IS_ERR(evsel->tp_format)) { + err = PTR_ERR(evsel->tp_format); goto out_free; + } event_attr_init(&attr); attr.config = evsel->tp_format->id; @@ -254,7 +263,8 @@ struct perf_evsel *perf_evsel__newtp_idx(const char *sys, const char *name, int out_free: zfree(&evsel->name); free(evsel); - return NULL; +out_err: + return ERR_PTR(err); } const char *perf_evsel__hw_names[PERF_COUNT_HW_MAX] = { diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h index f164a149da82..62ab307b7306 100644 --- a/tools/perf/util/evsel.h +++ b/tools/perf/util/evsel.h @@ -160,6 +160,9 @@ static inline struct perf_evsel *perf_evsel__new(struct perf_event_attr *attr) struct perf_evsel *perf_evsel__newtp_idx(const char *sys, const char *name, int idx); +/* + * Returns pointer with encoded error via interface. + */ static inline struct perf_evsel *perf_evsel__newtp(const char *sys, const char *name) { return perf_evsel__newtp_idx(sys, name, 0); diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index 1b284b8ad243..c47831c47220 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -1,4 +1,5 @@ #include +#include #include "util.h" #include "../perf.h" #include "evlist.h" @@ -393,11 +394,10 @@ static int add_tracepoint(struct list_head *list, int *idx, struct perf_evsel *evsel; evsel = perf_evsel__newtp_idx(sys_name, evt_name, (*idx)++); - if (!evsel) - return -ENOMEM; + if (IS_ERR(evsel)) + return PTR_ERR(evsel); list_add_tail(&evsel->node, list); - return 0; } diff --git a/tools/perf/util/trace-event.c b/tools/perf/util/trace-event.c index 2f4996ab313d..802bb868d446 100644 --- a/tools/perf/util/trace-event.c +++ b/tools/perf/util/trace-event.c @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include "trace-event.h" @@ -66,6 +67,9 @@ void trace_event__cleanup(struct trace_event *t) pevent_free(t->pevent); } +/* + * Returns pointer with encoded error via interface. + */ static struct event_format* tp_format(const char *sys, const char *name) { @@ -74,12 +78,14 @@ tp_format(const char *sys, const char *name) char path[PATH_MAX]; size_t size; char *data; + int err; scnprintf(path, PATH_MAX, "%s/%s/%s/format", tracing_events_path, sys, name); - if (filename__read_str(path, &data, &size)) - return NULL; + err = filename__read_str(path, &data, &size); + if (err) + return ERR_PTR(err); pevent_parse_format(pevent, &event, data, size, sys); @@ -87,11 +93,14 @@ tp_format(const char *sys, const char *name) return event; } +/* + * Returns pointer with encoded error via interface. + */ struct event_format* trace_event__tp_format(const char *sys, const char *name) { if (!tevent_initialized && trace_event__init2()) - return NULL; + return ERR_PTR(-ENOMEM); return tp_format(sys, name); } From 196581717d85f59365dc9303685cd5b1cdf106a3 Mon Sep 17 00:00:00 2001 From: Jiri Olsa Date: Mon, 7 Sep 2015 10:38:07 +0200 Subject: [PATCH 6/8] perf tools: Enhance parsing events tracepoint error output Enhancing parsing events tracepoint error output. Adding more verbose output when the tracepoint is not found or the tracing event path cannot be access. $ sudo perf record -e sched:sched_krava ls event syntax error: 'sched:sched_krava' \___ unknown tracepoint Error: File /sys/kernel/debug/tracing//tracing/events/sched/sched_krava not found. Hint: Perhaps this kernel misses some CONFIG_ setting to enable this feature?. Run 'perf list' for a list of valid events ... $ perf record -e sched:sched_krava ls event syntax error: 'sched:sched_krava' \___ can't access trace events Error: No permissions to read /sys/kernel/debug/tracing//tracing/events/sched/sched_krava Hint: Try 'sudo mount -o remount,mode=755 /sys/kernel/debug' Run 'perf list' for a list of valid events ... Signed-off-by: Jiri Olsa Tested-by: Arnaldo Carvalho de Melo Cc: Raphael Beamonte Cc: David Ahern Cc: Matt Fleming Cc: Namhyung Kim Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/1441615087-13886-6-git-send-email-jolsa@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/parse-events.c | 35 +++++++++++++++++++++++++++++++--- tools/perf/util/parse-events.y | 16 +++++++++------- 2 files changed, 41 insertions(+), 10 deletions(-) diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index c47831c47220..d3fb90be6216 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -387,6 +387,33 @@ int parse_events_add_cache(struct list_head *list, int *idx, return add_event(list, idx, &attr, name, NULL); } +static void tracepoint_error(struct parse_events_error *error, int err, + char *sys, char *name) +{ + char help[BUFSIZ]; + + /* + * We get error directly from syscall errno ( > 0), + * or from encoded pointer's error ( < 0). + */ + err = abs(err); + + switch (err) { + case EACCES: + error->str = strdup("can't access trace events"); + break; + case ENOENT: + error->str = strdup("unknown tracepoint"); + break; + default: + error->str = strdup("failed to add tracepoint"); + break; + } + + tracing_path__strerror_open_tp(err, help, sizeof(help), sys, name); + error->help = strdup(help); +} + static int add_tracepoint(struct list_head *list, int *idx, char *sys_name, char *evt_name, struct parse_events_error *error __maybe_unused) @@ -394,8 +421,10 @@ static int add_tracepoint(struct list_head *list, int *idx, struct perf_evsel *evsel; evsel = perf_evsel__newtp_idx(sys_name, evt_name, (*idx)++); - if (IS_ERR(evsel)) + if (IS_ERR(evsel)) { + tracepoint_error(error, PTR_ERR(evsel), sys_name, evt_name); return PTR_ERR(evsel); + } list_add_tail(&evsel->node, list); return 0; @@ -413,7 +442,7 @@ static int add_tracepoint_multi_event(struct list_head *list, int *idx, snprintf(evt_path, MAXPATHLEN, "%s/%s", tracing_events_path, sys_name); evt_dir = opendir(evt_path); if (!evt_dir) { - perror("Can't open event dir"); + tracepoint_error(error, errno, sys_name, evt_name); return -1; } @@ -453,7 +482,7 @@ static int add_tracepoint_multi_sys(struct list_head *list, int *idx, events_dir = opendir(tracing_events_path); if (!events_dir) { - perror("Can't open event dir"); + tracepoint_error(error, errno, sys_name, evt_name); return -1; } diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y index 54a3004a8192..8bcc45868457 100644 --- a/tools/perf/util/parse-events.y +++ b/tools/perf/util/parse-events.y @@ -371,28 +371,30 @@ event_legacy_tracepoint: PE_NAME '-' PE_NAME ':' PE_NAME { struct parse_events_evlist *data = _data; + struct parse_events_error *error = data->error; struct list_head *list; char sys_name[128]; snprintf(&sys_name, 128, "%s-%s", $1, $3); ALLOC_LIST(list); - ABORT_ON(parse_events_add_tracepoint(list, &data->idx, &sys_name, $5, data->error)); + if (parse_events_add_tracepoint(list, &data->idx, &sys_name, $5, error)) { + if (error) + error->idx = @1.first_column; + return -1; + } $$ = list; } | PE_NAME ':' PE_NAME { struct parse_events_evlist *data = _data; + struct parse_events_error *error = data->error; struct list_head *list; ALLOC_LIST(list); - if (parse_events_add_tracepoint(list, &data->idx, $1, $3, data->error)) { - struct parse_events_error *error = data->error; - - if (error) { + if (parse_events_add_tracepoint(list, &data->idx, $1, $3, error)) { + if (error) error->idx = @1.first_column; - error->str = strdup("unknown tracepoint"); - } return -1; } $$ = list; From 63ab024a5b6f295ca17a293ad81b7c728f49a89a Mon Sep 17 00:00:00 2001 From: Wang Nan Date: Mon, 14 Sep 2015 23:02:49 -0300 Subject: [PATCH 7/8] perf tools: regs_query_register_offset() infrastructure regs_query_register_offset() is a helper function which converts register name like "%rax" to offset of a register in 'struct pt_regs', which is required by BPF prologue generator. PERF_HAVE_ARCH_REGS_QUERY_REGISTER_OFFSET indicates an architecture supports converting name of a register to its offset in 'struct pt_regs'. HAVE_ARCH_REGS_QUERY_REGISTER_OFFSET is introduced as the corresponding CFLAGS of PERF_HAVE_ARCH_REGS_QUERY_REGISTER_OFFSET. Signed-off-by: Wang Nan Acked-by: Masami Hiramatsu Cc: Alexei Starovoitov Cc: Brendan Gregg Cc: Daniel Borkmann Cc: David Ahern Cc: Jiri Olsa Cc: Kaixu Xia Cc: Namhyung Kim Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Zefan Li Cc: pi3orama@163.com Link: http://lkml.kernel.org/r/1441523623-152703-19-git-send-email-wangnan0@huawei.com Signed-off-by: He Kuang [ Extracted from eBPF patches ] Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/config/Makefile | 4 ++++ tools/perf/util/include/dwarf-regs.h | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile index 827557fc7511..0435ac41cea9 100644 --- a/tools/perf/config/Makefile +++ b/tools/perf/config/Makefile @@ -109,6 +109,10 @@ endif # include ARCH specific config -include $(src-perf)/arch/$(ARCH)/Makefile +ifdef PERF_HAVE_ARCH_REGS_QUERY_REGISTER_OFFSET + CFLAGS += -DHAVE_ARCH_REGS_QUERY_REGISTER_OFFSET +endif + include $(src-perf)/config/utilities.mak ifeq ($(call get-executable,$(FLEX)),) diff --git a/tools/perf/util/include/dwarf-regs.h b/tools/perf/util/include/dwarf-regs.h index 8f149655f497..07c644ed64c4 100644 --- a/tools/perf/util/include/dwarf-regs.h +++ b/tools/perf/util/include/dwarf-regs.h @@ -5,4 +5,12 @@ const char *get_arch_regstr(unsigned int n); #endif +#ifdef HAVE_ARCH_REGS_QUERY_REGISTER_OFFSET +/* + * Arch should support fetching the offset of a register in pt_regs + * by its name. See kernel's regs_query_register_offset in + * arch/xxx/kernel/ptrace.c. + */ +int regs_query_register_offset(const char *name); +#endif #endif From bbbe6bf6037d77816c4a19aaf35f4cecf662b49a Mon Sep 17 00:00:00 2001 From: Wang Nan Date: Sun, 6 Sep 2015 07:13:35 +0000 Subject: [PATCH 8/8] perf tools: Introduce regs_query_register_offset() for x86 regs_query_register_offset() is a helper function which converts register name like "%rax" to offset of a register in 'struct pt_regs', which is required by BPF prologue generator. Since the function is identical, try to reuse the code in arch/x86/kernel/ptrace.c. Comment inside dwarf-regs.c list the differences between this implementation and kernel code. get_arch_regstr() switches to regoffset_table and the old string table is dropped. Signed-off-by: He Kuang Acked-by: Masami Hiramatsu Cc: Alexei Starovoitov Cc: Brendan Gregg Cc: Daniel Borkmann Cc: David Ahern Cc: He Kuang Cc: Jiri Olsa Cc: Kaixu Xia Cc: Namhyung Kim Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Zefan Li Cc: pi3orama@163.com Link: http://lkml.kernel.org/r/1441523623-152703-20-git-send-email-wangnan0@huawei.com Signed-off-by: Wang Nan Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/arch/x86/Makefile | 1 + tools/perf/arch/x86/util/dwarf-regs.c | 122 +++++++++++++++++++------- 2 files changed, 89 insertions(+), 34 deletions(-) diff --git a/tools/perf/arch/x86/Makefile b/tools/perf/arch/x86/Makefile index 21322e0385b8..09ba923debe8 100644 --- a/tools/perf/arch/x86/Makefile +++ b/tools/perf/arch/x86/Makefile @@ -2,3 +2,4 @@ ifndef NO_DWARF PERF_HAVE_DWARF_REGS := 1 endif HAVE_KVM_STAT_SUPPORT := 1 +PERF_HAVE_ARCH_REGS_QUERY_REGISTER_OFFSET := 1 diff --git a/tools/perf/arch/x86/util/dwarf-regs.c b/tools/perf/arch/x86/util/dwarf-regs.c index a08de0a35b83..9223c164e545 100644 --- a/tools/perf/arch/x86/util/dwarf-regs.c +++ b/tools/perf/arch/x86/util/dwarf-regs.c @@ -21,55 +21,109 @@ */ #include +#include /* for EINVAL */ +#include /* for strcmp */ +#include /* for struct pt_regs */ +#include /* for offsetof */ #include /* - * Generic dwarf analysis helpers + * See arch/x86/kernel/ptrace.c. + * Different from it: + * + * - Since struct pt_regs is defined differently for user and kernel, + * but we want to use 'ax, bx' instead of 'rax, rbx' (which is struct + * field name of user's pt_regs), we make REG_OFFSET_NAME to accept + * both string name and reg field name. + * + * - Since accessing x86_32's pt_regs from x86_64 building is difficult + * and vise versa, we simply fill offset with -1, so + * get_arch_regstr() still works but regs_query_register_offset() + * returns error. + * The only inconvenience caused by it now is that we are not allowed + * to generate BPF prologue for a x86_64 kernel if perf is built for + * x86_32. This is really a rare usecase. + * + * - Order is different from kernel's ptrace.c for get_arch_regstr(). Use + * the order defined by dwarf. */ -#define X86_32_MAX_REGS 8 -const char *x86_32_regs_table[X86_32_MAX_REGS] = { - "%ax", - "%cx", - "%dx", - "%bx", - "$stack", /* Stack address instead of %sp */ - "%bp", - "%si", - "%di", +struct pt_regs_offset { + const char *name; + int offset; }; -#define X86_64_MAX_REGS 16 -const char *x86_64_regs_table[X86_64_MAX_REGS] = { - "%ax", - "%dx", - "%cx", - "%bx", - "%si", - "%di", - "%bp", - "%sp", - "%r8", - "%r9", - "%r10", - "%r11", - "%r12", - "%r13", - "%r14", - "%r15", +#define REG_OFFSET_END {.name = NULL, .offset = 0} + +#ifdef __x86_64__ +# define REG_OFFSET_NAME_64(n, r) {.name = n, .offset = offsetof(struct pt_regs, r)} +# define REG_OFFSET_NAME_32(n, r) {.name = n, .offset = -1} +#else +# define REG_OFFSET_NAME_64(n, r) {.name = n, .offset = -1} +# define REG_OFFSET_NAME_32(n, r) {.name = n, .offset = offsetof(struct pt_regs, r)} +#endif + +static const struct pt_regs_offset x86_32_regoffset_table[] = { + REG_OFFSET_NAME_32("%ax", eax), + REG_OFFSET_NAME_32("%cx", ecx), + REG_OFFSET_NAME_32("%dx", edx), + REG_OFFSET_NAME_32("%bx", ebx), + REG_OFFSET_NAME_32("$stack", esp), /* Stack address instead of %sp */ + REG_OFFSET_NAME_32("%bp", ebp), + REG_OFFSET_NAME_32("%si", esi), + REG_OFFSET_NAME_32("%di", edi), + REG_OFFSET_END, +}; + +static const struct pt_regs_offset x86_64_regoffset_table[] = { + REG_OFFSET_NAME_64("%ax", rax), + REG_OFFSET_NAME_64("%dx", rdx), + REG_OFFSET_NAME_64("%cx", rcx), + REG_OFFSET_NAME_64("%bx", rbx), + REG_OFFSET_NAME_64("%si", rsi), + REG_OFFSET_NAME_64("%di", rdi), + REG_OFFSET_NAME_64("%bp", rbp), + REG_OFFSET_NAME_64("%sp", rsp), + REG_OFFSET_NAME_64("%r8", r8), + REG_OFFSET_NAME_64("%r9", r9), + REG_OFFSET_NAME_64("%r10", r10), + REG_OFFSET_NAME_64("%r11", r11), + REG_OFFSET_NAME_64("%r12", r12), + REG_OFFSET_NAME_64("%r13", r13), + REG_OFFSET_NAME_64("%r14", r14), + REG_OFFSET_NAME_64("%r15", r15), + REG_OFFSET_END, }; /* TODO: switching by dwarf address size */ #ifdef __x86_64__ -#define ARCH_MAX_REGS X86_64_MAX_REGS -#define arch_regs_table x86_64_regs_table +#define regoffset_table x86_64_regoffset_table #else -#define ARCH_MAX_REGS X86_32_MAX_REGS -#define arch_regs_table x86_32_regs_table +#define regoffset_table x86_32_regoffset_table #endif +/* Minus 1 for the ending REG_OFFSET_END */ +#define ARCH_MAX_REGS ((sizeof(regoffset_table) / sizeof(regoffset_table[0])) - 1) + /* Return architecture dependent register string (for kprobe-tracer) */ const char *get_arch_regstr(unsigned int n) { - return (n < ARCH_MAX_REGS) ? arch_regs_table[n] : NULL; + return (n < ARCH_MAX_REGS) ? regoffset_table[n].name : NULL; +} + +/* Reuse code from arch/x86/kernel/ptrace.c */ +/** + * regs_query_register_offset() - query register offset from its name + * @name: the name of a register + * + * regs_query_register_offset() returns the offset of a register in struct + * pt_regs from its name. If the name is invalid, this returns -EINVAL; + */ +int regs_query_register_offset(const char *name) +{ + const struct pt_regs_offset *roff; + for (roff = regoffset_table; roff->name != NULL; roff++) + if (!strcmp(roff->name, name)) + return roff->offset; + return -EINVAL; }