From 128c32ed1866e6cf3d3944e7dcdcea06bc060b0d Mon Sep 17 00:00:00 2001 From: "Nam T. Nguyen" Date: Mon, 18 May 2015 11:37:27 -0700 Subject: [PATCH 01/25] perf tools: Separate the tests and tools in installation This refactors out install-bin to install-tests and install-tools so that downstream could opt to only install the tools, and not the tests. Signed-off-by: Nam T. Nguyen Acked-by: Jiri Olsa Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Simon Que Link: http://lkml.kernel.org/r/1431974247-22275-1-git-send-email-namnguyen@chromium.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/Makefile.perf | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf index 03409cc02117..5816a3bb7e9f 100644 --- a/tools/perf/Makefile.perf +++ b/tools/perf/Makefile.perf @@ -464,7 +464,7 @@ check: $(OUTPUT)common-cmds.h install-gtk: -install-bin: all install-gtk +install-tools: all install-gtk $(call QUIET_INSTALL, binaries) \ $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'; \ $(INSTALL) $(OUTPUT)perf '$(DESTDIR_SQ)$(bindir_SQ)'; \ @@ -502,12 +502,16 @@ endif $(call QUIET_INSTALL, perf_completion-script) \ $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(sysconfdir_SQ)/bash_completion.d'; \ $(INSTALL) perf-completion.sh '$(DESTDIR_SQ)$(sysconfdir_SQ)/bash_completion.d/perf' + +install-tests: all install-gtk $(call QUIET_INSTALL, tests) \ $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/tests'; \ $(INSTALL) tests/attr.py '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/tests'; \ $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/tests/attr'; \ $(INSTALL) tests/attr/* '$(DESTDIR_SQ)$(perfexec_instdir_SQ)/tests/attr' +install-bin: install-tools install-tests + install: install-bin try-install-man install-traceevent-plugins install-python_ext: From bb78ce7d0598fb277290f8ee2443b8f4e0eb7cb2 Mon Sep 17 00:00:00 2001 From: Adrian Hunter Date: Tue, 19 May 2015 16:05:42 +0300 Subject: [PATCH 02/25] perf tools: Fix function declarations needed by parse-events.y Patch "perf tools: Add location to pmu event terms" moved declarations for parse_events_term__num() and parse_events_term__str() so that they were no longer visible in parse-events.y. That can result in segfaults as the arguments no longer need match the function prototype. Move the declarations back, changing YYLTYPE pointers to pointers-to-void because YYLTYPE is not generated until parse-events.y is processed. Signed-off-by: Adrian Hunter Cc: Jiri Olsa Cc: Namhyung Kim Link: http://lkml.kernel.org/r/1432040746-1755-2-git-send-email-adrian.hunter@intel.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/parse-events.c | 16 ++++++++-------- tools/perf/util/parse-events.h | 6 ++++++ 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index 80a50fdb6d8a..78032d887c1a 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -25,12 +25,6 @@ extern int parse_events_debug; #endif int parse_events_parse(void *data, void *scanner); -int parse_events_term__num(struct parse_events_term **term, - int type_term, char *config, u64 num, - YYLTYPE *loc_term, YYLTYPE *loc_val); -int parse_events_term__str(struct parse_events_term **term, - int type_term, char *config, char *str, - YYLTYPE *loc_term, YYLTYPE *loc_val); static struct perf_pmu_event_symbol *perf_pmu_events_list; /* @@ -1601,8 +1595,11 @@ static int new_term(struct parse_events_term **_term, int type_val, int parse_events_term__num(struct parse_events_term **term, int type_term, char *config, u64 num, - YYLTYPE *loc_term, YYLTYPE *loc_val) + void *loc_term_, void *loc_val_) { + YYLTYPE *loc_term = loc_term_; + YYLTYPE *loc_val = loc_val_; + return new_term(term, PARSE_EVENTS__TERM_TYPE_NUM, type_term, config, NULL, num, loc_term ? loc_term->first_column : 0, @@ -1611,8 +1608,11 @@ int parse_events_term__num(struct parse_events_term **term, int parse_events_term__str(struct parse_events_term **term, int type_term, char *config, char *str, - YYLTYPE *loc_term, YYLTYPE *loc_val) + void *loc_term_, void *loc_val_) { + YYLTYPE *loc_term = loc_term_; + YYLTYPE *loc_val = loc_val_; + return new_term(term, PARSE_EVENTS__TERM_TYPE_STR, type_term, config, str, 0, loc_term ? loc_term->first_column : 0, diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h index e236f1b6ac6f..131f29b2f132 100644 --- a/tools/perf/util/parse-events.h +++ b/tools/perf/util/parse-events.h @@ -98,6 +98,12 @@ struct parse_events_terms { }; int parse_events__is_hardcoded_term(struct parse_events_term *term); +int parse_events_term__num(struct parse_events_term **term, + int type_term, char *config, u64 num, + void *loc_term, void *loc_val); +int parse_events_term__str(struct parse_events_term **term, + int type_term, char *config, char *str, + void *loc_term, void *loc_val); int parse_events_term__sym_hw(struct parse_events_term **term, char *config, unsigned idx); int parse_events_term__clone(struct parse_events_term **new, From a6ced2be06c302402c52dedba97d169d22cce99c Mon Sep 17 00:00:00 2001 From: Adrian Hunter Date: Tue, 19 May 2015 16:05:44 +0300 Subject: [PATCH 03/25] perf tools: Fix parse_events_error dereferences Parse errors can be reported in struct parse_events_error but the pointer passed is optional and can be NULL. Ensure it is not NULL before dereferencing it. Signed-off-by: Adrian Hunter Cc: Jiri Olsa Cc: Namhyung Kim Link: http://lkml.kernel.org/r/1432040746-1755-4-git-send-email-adrian.hunter@intel.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/parse-events.c | 2 ++ tools/perf/util/parse-events.y | 6 ++++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index 78032d887c1a..2a4d1ec02846 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -1659,6 +1659,8 @@ void parse_events_evlist_error(struct parse_events_evlist *data, { struct parse_events_error *err = data->error; + if (!err) + return; err->idx = idx; err->str = strdup(str); WARN_ONCE(!err->str, "WARNING: failed to allocate error string"); diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y index 3d11e00243e3..591905a02b92 100644 --- a/tools/perf/util/parse-events.y +++ b/tools/perf/util/parse-events.y @@ -389,8 +389,10 @@ PE_NAME ':' PE_NAME if (parse_events_add_tracepoint(list, &data->idx, $1, $3)) { struct parse_events_error *error = data->error; - error->idx = @1.first_column; - error->str = strdup("unknown tracepoint"); + if (error) { + error->idx = @1.first_column; + error->str = strdup("unknown tracepoint"); + } return -1; } $$ = list; From 05b41775e2edd69a83f592e3534930c934d4038e Mon Sep 17 00:00:00 2001 From: Adrian Hunter Date: Tue, 19 May 2015 16:05:43 +0300 Subject: [PATCH 04/25] perf build: Fix libunwind feature detection on 32-bit x86 The libunwind feature would never detect because of the following error: $ cat tools/build/feature/test-libunwind.make.output /usr/lib/gcc/i686-linux-gnu/4.8/../../../i386-linux-gnu/libunwind-x86.so: undefined reference to `lzma_stream_buffer_decode' /usr/lib/gcc/i686-linux-gnu/4.8/../../../i386-linux-gnu/libunwind-x86.so: undefined reference to `lzma_index_uncompressed_size' /usr/lib/gcc/i686-linux-gnu/4.8/../../../i386-linux-gnu/libunwind-x86.so: undefined reference to `lzma_index_end' /usr/lib/gcc/i686-linux-gnu/4.8/../../../i386-linux-gnu/libunwind-x86.so: undefined reference to `lzma_index_buffer_decode' /usr/lib/gcc/i686-linux-gnu/4.8/../../../i386-linux-gnu/libunwind-x86.so: undefined reference to `lzma_stream_footer_decode' /usr/lib/gcc/i686-linux-gnu/4.8/../../../i386-linux-gnu/libunwind-x86.so: undefined reference to `lzma_index_size' collect2: error: ld returned 1 exit status Fix by adding -llzma and re-ordering to match the dependencies. Signed-off-by: Adrian Hunter Cc: Jiri Olsa Cc: Namhyung Kim Link: http://lkml.kernel.org/r/1432040746-1755-3-git-send-email-adrian.hunter@intel.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/config/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile index 1b957a1272d0..e3b3724e73ff 100644 --- a/tools/perf/config/Makefile +++ b/tools/perf/config/Makefile @@ -32,7 +32,7 @@ ifeq ($(ARCH),x86) LIBUNWIND_LIBS = -lunwind -lunwind-x86_64 $(call detected,CONFIG_X86_64) else - LIBUNWIND_LIBS = -lunwind -lunwind-x86 + LIBUNWIND_LIBS = -lunwind-x86 -llzma -lunwind endif NO_PERF_REGS := 0 endif From 554e92ed8fcdbcad736ef906c393847d44d52692 Mon Sep 17 00:00:00 2001 From: Adrian Hunter Date: Tue, 19 May 2015 16:05:45 +0300 Subject: [PATCH 05/25] perf session: Fix perf_session__peek_event() perf_session__peek_event() generally leverages there being a single mmap of the perf.data file, however on 32-bit platforms when there is more that 32MiB of data, then there are multiple mmaps, so perf_session__peek_event() reads from the file. In that case a couple of bugs were exposed (note how the seg. fault appears with >32M of data): $ perf record --per-thread -e intel_bts// ../rtit-tests/loopy 1000000 [ perf record: Woken up 13 times to write data ] [ perf record: Captured and wrote 24.568 MB perf.data ] $ perf script > /dev/null $ perf record --per-thread -e intel_bts// ../rtit-tests/loopy 10000000 [ perf record: Woken up 136 times to write data ] [ perf record: Captured and wrote 270.794 MB perf.data ] $ perf script > /dev/null Segmentation fault (core dumped) The wrong address was being passed to the readn() function and the buffer size was not being checked. Signed-off-by: Adrian Hunter Cc: Jiri Olsa Cc: Namhyung Kim Link: http://lkml.kernel.org/r/1432040746-1755-5-git-send-email-adrian.hunter@intel.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/session.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c index e722107f932a..39fe09d5a87e 100644 --- a/tools/perf/util/session.c +++ b/tools/perf/util/session.c @@ -1182,7 +1182,7 @@ int perf_session__peek_event(struct perf_session *session, off_t file_offset, return -1; if (lseek(fd, file_offset, SEEK_SET) == (off_t)-1 || - readn(fd, &buf, hdr_sz) != (ssize_t)hdr_sz) + readn(fd, buf, hdr_sz) != (ssize_t)hdr_sz) return -1; event = (union perf_event *)buf; @@ -1190,12 +1190,12 @@ int perf_session__peek_event(struct perf_session *session, off_t file_offset, if (session->header.needs_swap) perf_event_header__bswap(&event->header); - if (event->header.size < hdr_sz) + if (event->header.size < hdr_sz || event->header.size > buf_sz) return -1; rest = event->header.size - hdr_sz; - if (readn(fd, &buf, rest) != (ssize_t)rest) + if (readn(fd, buf, rest) != (ssize_t)rest) return -1; if (session->header.needs_swap) From 063bd9363bb8979b2939bdc0412d98a8ac062e3b Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Tue, 19 May 2015 17:04:10 +0900 Subject: [PATCH 06/25] perf hists: Reducing arguments of hist_entry_iter__add() The evsel and sample arguments are to set iter for later use. As it also receives an iter as another argument, just set them before calling the function. Signed-off-by: Namhyung Kim Cc: David Ahern Cc: Jiri Olsa Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/1432022650-18205-1-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-report.c | 9 +++++---- tools/perf/builtin-top.c | 7 ++++--- tools/perf/tests/hists_cumulate.c | 6 ++++-- tools/perf/tests/hists_filter.c | 4 +++- tools/perf/tests/hists_output.c | 6 ++++-- tools/perf/util/hist.c | 8 ++------ tools/perf/util/hist.h | 1 - 7 files changed, 22 insertions(+), 19 deletions(-) diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c index 92fca2157e5e..56025d90622f 100644 --- a/tools/perf/builtin-report.c +++ b/tools/perf/builtin-report.c @@ -139,8 +139,10 @@ static int process_sample_event(struct perf_tool *tool, struct report *rep = container_of(tool, struct report, tool); struct addr_location al; struct hist_entry_iter iter = { - .hide_unresolved = rep->hide_unresolved, - .add_entry_cb = hist_iter__report_callback, + .evsel = evsel, + .sample = sample, + .hide_unresolved = rep->hide_unresolved, + .add_entry_cb = hist_iter__report_callback, }; int ret = 0; @@ -168,8 +170,7 @@ static int process_sample_event(struct perf_tool *tool, if (al.map != NULL) al.map->dso->hit = 1; - ret = hist_entry_iter__add(&iter, &al, evsel, sample, rep->max_stack, - rep); + ret = hist_entry_iter__add(&iter, &al, rep->max_stack, rep); if (ret < 0) pr_debug("problem adding hist entry, skipping event\n"); out_put: diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c index a19351728f0f..6b987424d015 100644 --- a/tools/perf/builtin-top.c +++ b/tools/perf/builtin-top.c @@ -775,7 +775,9 @@ static void perf_event__process_sample(struct perf_tool *tool, if (al.sym == NULL || !al.sym->ignore) { struct hists *hists = evsel__hists(evsel); struct hist_entry_iter iter = { - .add_entry_cb = hist_iter__top_callback, + .evsel = evsel, + .sample = sample, + .add_entry_cb = hist_iter__top_callback, }; if (symbol_conf.cumulate_callchain) @@ -785,8 +787,7 @@ static void perf_event__process_sample(struct perf_tool *tool, pthread_mutex_lock(&hists->lock); - err = hist_entry_iter__add(&iter, &al, evsel, sample, - top->max_stack, top); + err = hist_entry_iter__add(&iter, &al, top->max_stack, top); if (err < 0) pr_err("Problem incrementing symbol period, skipping event\n"); diff --git a/tools/perf/tests/hists_cumulate.c b/tools/perf/tests/hists_cumulate.c index 620f626e5b35..7d82c8be5e36 100644 --- a/tools/perf/tests/hists_cumulate.c +++ b/tools/perf/tests/hists_cumulate.c @@ -87,6 +87,8 @@ static int add_hist_entries(struct hists *hists, struct machine *machine) }, }; struct hist_entry_iter iter = { + .evsel = evsel, + .sample = &sample, .hide_unresolved = false, }; @@ -104,8 +106,8 @@ static int add_hist_entries(struct hists *hists, struct machine *machine) &sample) < 0) goto out; - if (hist_entry_iter__add(&iter, &al, evsel, &sample, - PERF_MAX_STACK_DEPTH, NULL) < 0) { + if (hist_entry_iter__add(&iter, &al, PERF_MAX_STACK_DEPTH, + NULL) < 0) { addr_location__put(&al); goto out; } diff --git a/tools/perf/tests/hists_filter.c b/tools/perf/tests/hists_filter.c index 82e1ee52e024..ce48775e6ada 100644 --- a/tools/perf/tests/hists_filter.c +++ b/tools/perf/tests/hists_filter.c @@ -63,6 +63,8 @@ static int add_hist_entries(struct perf_evlist *evlist, }, }; struct hist_entry_iter iter = { + .evsel = evsel, + .sample = &sample, .ops = &hist_iter_normal, .hide_unresolved = false, }; @@ -81,7 +83,7 @@ static int add_hist_entries(struct perf_evlist *evlist, &sample) < 0) goto out; - if (hist_entry_iter__add(&iter, &al, evsel, &sample, + if (hist_entry_iter__add(&iter, &al, PERF_MAX_STACK_DEPTH, NULL) < 0) { addr_location__put(&al); goto out; diff --git a/tools/perf/tests/hists_output.c b/tools/perf/tests/hists_output.c index fd7ec4f9aeb4..adbebc852cc8 100644 --- a/tools/perf/tests/hists_output.c +++ b/tools/perf/tests/hists_output.c @@ -57,6 +57,8 @@ static int add_hist_entries(struct hists *hists, struct machine *machine) }, }; struct hist_entry_iter iter = { + .evsel = evsel, + .sample = &sample, .ops = &hist_iter_normal, .hide_unresolved = false, }; @@ -70,8 +72,8 @@ static int add_hist_entries(struct hists *hists, struct machine *machine) &sample) < 0) goto out; - if (hist_entry_iter__add(&iter, &al, evsel, &sample, - PERF_MAX_STACK_DEPTH, NULL) < 0) { + if (hist_entry_iter__add(&iter, &al, PERF_MAX_STACK_DEPTH, + NULL) < 0) { addr_location__put(&al); goto out; } diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c index 338770679863..2504b5b1a308 100644 --- a/tools/perf/util/hist.c +++ b/tools/perf/util/hist.c @@ -851,19 +851,15 @@ const struct hist_iter_ops hist_iter_cumulative = { }; int hist_entry_iter__add(struct hist_entry_iter *iter, struct addr_location *al, - struct perf_evsel *evsel, struct perf_sample *sample, int max_stack_depth, void *arg) { int err, err2; - err = sample__resolve_callchain(sample, &iter->parent, evsel, al, - max_stack_depth); + err = sample__resolve_callchain(iter->sample, &iter->parent, + iter->evsel, al, max_stack_depth); if (err) return err; - iter->evsel = evsel; - iter->sample = sample; - err = iter->ops->prepare_entry(iter, al); if (err) goto out; diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h index 9f31b89a527a..5ed8d9c22981 100644 --- a/tools/perf/util/hist.h +++ b/tools/perf/util/hist.h @@ -111,7 +111,6 @@ struct hist_entry *__hists__add_entry(struct hists *hists, u64 weight, u64 transaction, bool sample_self); int hist_entry_iter__add(struct hist_entry_iter *iter, struct addr_location *al, - struct perf_evsel *evsel, struct perf_sample *sample, int max_stack_depth, void *arg); int64_t hist_entry__cmp(struct hist_entry *left, struct hist_entry *right); From e7e0efcdb807a570b11f240e2608d7aed5ccdfb1 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Tue, 19 May 2015 11:31:22 -0300 Subject: [PATCH 07/25] perf hists: Rename add_hist_entry to hists__findnew_entry To match the convention used elsewhere. Cc: Adrian Hunter Cc: Borislav Petkov Cc: David Ahern Cc: Don Zickus Cc: Frederic Weisbecker Cc: Jiri Olsa Cc: Namhyung Kim Cc: Stephane Eranian Link: http://lkml.kernel.org/n/tip-66oo6yn8upssfeuprwy0il1q@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/hist.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c index 2504b5b1a308..f53d017c7c22 100644 --- a/tools/perf/util/hist.c +++ b/tools/perf/util/hist.c @@ -362,10 +362,10 @@ static u8 symbol__parent_filter(const struct symbol *parent) return 0; } -static struct hist_entry *add_hist_entry(struct hists *hists, - struct hist_entry *entry, - struct addr_location *al, - bool sample_self) +static struct hist_entry *hists__findnew_entry(struct hists *hists, + struct hist_entry *entry, + struct addr_location *al, + bool sample_self) { struct rb_node **p; struct rb_node *parent = NULL; @@ -468,7 +468,7 @@ struct hist_entry *__hists__add_entry(struct hists *hists, .transaction = transaction, }; - return add_hist_entry(hists, &entry, al, sample_self); + return hists__findnew_entry(hists, &entry, al, sample_self); } static int @@ -548,9 +548,9 @@ iter_finish_mem_entry(struct hist_entry_iter *iter, out: /* - * We don't need to free iter->priv (mem_info) here since - * the mem info was either already freed in add_hist_entry() or - * passed to a new hist entry by hist_entry__new(). + * We don't need to free iter->priv (mem_info) here since the mem info + * was either already freed in hists__findnew_entry() or passed to a + * new hist entry by hist_entry__new(). */ iter->priv = NULL; From 86c19525b7e953217e5ad2b5496029b1ac6fe26b Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Tue, 19 May 2015 19:07:42 -0300 Subject: [PATCH 08/25] perf comm: Use atomic.h for refcounting Now that we have atomic.h, we should convert all of the existing refcounts to use it. Cc: Adrian Hunter Cc: Borislav Petkov Cc: David Ahern Cc: Don Zickus Cc: Frederic Weisbecker Cc: Jiri Olsa Cc: Namhyung Kim Cc: Stephane Eranian Link: http://lkml.kernel.org/n/tip-quzeuy3jwsyod6e06o39cl6y@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/comm.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tools/perf/util/comm.c b/tools/perf/util/comm.c index b2bb59df65e1..21b7ff382c3f 100644 --- a/tools/perf/util/comm.c +++ b/tools/perf/util/comm.c @@ -2,24 +2,27 @@ #include "util.h" #include #include +#include struct comm_str { char *str; struct rb_node rb_node; - int ref; + atomic_t refcnt; }; /* Should perhaps be moved to struct machine */ static struct rb_root comm_str_root; -static void comm_str__get(struct comm_str *cs) +static struct comm_str *comm_str__get(struct comm_str *cs) { - cs->ref++; + if (cs) + atomic_inc(&cs->refcnt); + return cs; } static void comm_str__put(struct comm_str *cs) { - if (!--cs->ref) { + if (cs && atomic_dec_and_test(&cs->refcnt)) { rb_erase(&cs->rb_node, &comm_str_root); zfree(&cs->str); free(cs); @@ -40,6 +43,8 @@ static struct comm_str *comm_str__alloc(const char *str) return NULL; } + atomic_set(&cs->refcnt, 0); + return cs; } From 8e160b2e1e3efdd84ddef726f9b5136dd192a682 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Tue, 19 May 2015 20:07:14 -0300 Subject: [PATCH 09/25] perf machine: Do not call map_groups__delete(), drop refcnt instead It could be used somewhere, so just call map__groups_put() to make sure we don't delete it prematurely Cc: Adrian Hunter Cc: Borislav Petkov Cc: David Ahern Cc: Don Zickus Cc: Frederic Weisbecker Cc: Jiri Olsa Cc: Namhyung Kim Cc: Stephane Eranian Link: http://lkml.kernel.org/n/tip-dxmh8mr12i65p8h909vi88cp@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/machine.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c index daa55910ff28..7ec3188d3cb3 100644 --- a/tools/perf/util/machine.c +++ b/tools/perf/util/machine.c @@ -333,7 +333,7 @@ static void machine__update_thread_pid(struct machine *machine, if (!map_groups__empty(th->mg)) pr_err("Discarding thread maps for %d:%d\n", th->pid_, th->tid); - map_groups__delete(th->mg); + map_groups__put(th->mg); } th->mg = map_groups__get(leader->mg); From 71ff824a60a7b0d9d0746e6e237fe4735077e5b4 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Thu, 21 May 2015 01:03:39 +0900 Subject: [PATCH 10/25] perf tools: Fix dso__data_read_offset() file opening When dso__data_read_offset/addr() is called without prior dso__data_fd() (or other functions which call it internally), it failed to open dso in data_file_size() since its binary type was not identified. However calling dso__data_fd() in dso__data_read_offset() will hurt performance as it grabs a global lock everytime. So factor out the loop on the binary type in dso__data_fd(), and call it from both. Reported-by: Adrian Hunter Signed-off-by: Namhyung Kim Acked-by: Adrian Hunter Cc: David Ahern Cc: Jiri Olsa Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/1432137821-10853-1-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/dso.c | 59 +++++++++++++++++++++++-------------------- 1 file changed, 32 insertions(+), 27 deletions(-) diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c index 1b96c8d18435..516e0c25ea16 100644 --- a/tools/perf/util/dso.c +++ b/tools/perf/util/dso.c @@ -440,15 +440,7 @@ void dso__data_close(struct dso *dso) pthread_mutex_unlock(&dso__data_open_lock); } -/** - * dso__data_fd - Get dso's data file descriptor - * @dso: dso object - * @machine: machine object - * - * External interface to find dso's file, open it and - * returns file descriptor. - */ -int dso__data_fd(struct dso *dso, struct machine *machine) +static void try_to_open_dso(struct dso *dso, struct machine *machine) { enum dso_binary_type binary_type_data[] = { DSO_BINARY_TYPE__BUILD_ID_CACHE, @@ -457,13 +449,8 @@ int dso__data_fd(struct dso *dso, struct machine *machine) }; int i = 0; - if (dso->data.status == DSO_DATA_STATUS_ERROR) - return -1; - - pthread_mutex_lock(&dso__data_open_lock); - if (dso->data.fd >= 0) - goto out; + return; if (dso->binary_type != DSO_BINARY_TYPE__NOT_FOUND) { dso->data.fd = open_dso(dso, machine); @@ -483,8 +470,25 @@ out: dso->data.status = DSO_DATA_STATUS_OK; else dso->data.status = DSO_DATA_STATUS_ERROR; +} +/** + * dso__data_fd - Get dso's data file descriptor + * @dso: dso object + * @machine: machine object + * + * External interface to find dso's file, open it and + * returns file descriptor. + */ +int dso__data_fd(struct dso *dso, struct machine *machine) +{ + if (dso->data.status == DSO_DATA_STATUS_ERROR) + return -1; + + pthread_mutex_lock(&dso__data_open_lock); + try_to_open_dso(dso, machine); pthread_mutex_unlock(&dso__data_open_lock); + return dso->data.fd; } @@ -609,13 +613,12 @@ dso_cache__read(struct dso *dso, struct machine *machine, * dso->data.fd might be closed if other thread opened another * file (dso) due to open file limit (RLIMIT_NOFILE). */ + try_to_open_dso(dso, machine); + if (dso->data.fd < 0) { - dso->data.fd = open_dso(dso, machine); - if (dso->data.fd < 0) { - ret = -errno; - dso->data.status = DSO_DATA_STATUS_ERROR; - break; - } + ret = -errno; + dso->data.status = DSO_DATA_STATUS_ERROR; + break; } cache_offset = offset & DSO__DATA_CACHE_MASK; @@ -702,19 +705,21 @@ static int data_file_size(struct dso *dso, struct machine *machine) if (dso->data.file_size) return 0; + if (dso->data.status == DSO_DATA_STATUS_ERROR) + return -1; + pthread_mutex_lock(&dso__data_open_lock); /* * dso->data.fd might be closed if other thread opened another * file (dso) due to open file limit (RLIMIT_NOFILE). */ + try_to_open_dso(dso, machine); + if (dso->data.fd < 0) { - dso->data.fd = open_dso(dso, machine); - if (dso->data.fd < 0) { - ret = -errno; - dso->data.status = DSO_DATA_STATUS_ERROR; - goto out; - } + ret = -errno; + dso->data.status = DSO_DATA_STATUS_ERROR; + goto out; } if (fstat(dso->data.fd, &st) < 0) { From e840238d7c6afcde0f6402aac3a74723ee9c448f Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Thu, 21 May 2015 01:03:40 +0900 Subject: [PATCH 11/25] perf tools: Get rid of dso__data_fd() from dso__data_size() It seems that the dso__data_fd() was needed to find a binary type since open in data_file_size() alone used to fail. But as it can open the dso fine now, the dso__data_fd() can go away. Signed-off-by: Namhyung Kim Acked-by: Adrian Hunter Cc: David Ahern Cc: Jiri Olsa Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/1432137821-10853-2-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/dso.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c index 516e0c25ea16..e95e850dd832 100644 --- a/tools/perf/util/dso.c +++ b/tools/perf/util/dso.c @@ -745,12 +745,6 @@ out: */ off_t dso__data_size(struct dso *dso, struct machine *machine) { - int fd; - - fd = dso__data_fd(dso, machine); - if (fd < 0) - return fd; - if (data_file_size(dso, machine)) return -1; From 4bb11d012ab248d0e383008d725be0d26a74fac2 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Thu, 21 May 2015 01:03:41 +0900 Subject: [PATCH 12/25] perf tools: Add dso__data_get/put_fd() Using dso__data_fd() in multi-thread environment is not safe since returned fd can be closed and/or reused anytime. So convert it to the dso__data_get/put_fd() pair to protect the access with lock. The original dso__data_fd() is deprecated and kept only for testing. Signed-off-by: Namhyung Kim Acked-by: Adrian Hunter Cc: David Ahern Cc: Jiri Olsa Cc: Peter Zijlstra Link: http://lkml.kernel.org/r/1432137821-10853-3-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/tests/dso-data.c | 11 +++++++++++ tools/perf/util/dso.c | 31 +++++++++++++++++++++--------- tools/perf/util/dso.h | 13 +++++++++---- tools/perf/util/unwind-libunwind.c | 11 ++++++++--- 4 files changed, 50 insertions(+), 16 deletions(-) diff --git a/tools/perf/tests/dso-data.c b/tools/perf/tests/dso-data.c index 513e5febbe5a..3e41c61bd861 100644 --- a/tools/perf/tests/dso-data.c +++ b/tools/perf/tests/dso-data.c @@ -99,6 +99,17 @@ struct test_data_offset offsets[] = { }, }; +/* move it from util/dso.c for compatibility */ +static int dso__data_fd(struct dso *dso, struct machine *machine) +{ + int fd = dso__data_get_fd(dso, machine); + + if (fd >= 0) + dso__data_put_fd(dso); + + return fd; +} + int test__dso_data(void) { struct machine machine; diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c index e95e850dd832..7e11a700303f 100644 --- a/tools/perf/util/dso.c +++ b/tools/perf/util/dso.c @@ -473,25 +473,35 @@ out: } /** - * dso__data_fd - Get dso's data file descriptor + * dso__data_get_fd - Get dso's data file descriptor * @dso: dso object * @machine: machine object * * External interface to find dso's file, open it and - * returns file descriptor. + * returns file descriptor. It should be paired with + * dso__data_put_fd() if it returns non-negative value. */ -int dso__data_fd(struct dso *dso, struct machine *machine) +int dso__data_get_fd(struct dso *dso, struct machine *machine) { if (dso->data.status == DSO_DATA_STATUS_ERROR) return -1; - pthread_mutex_lock(&dso__data_open_lock); + if (pthread_mutex_lock(&dso__data_open_lock) < 0) + return -1; + try_to_open_dso(dso, machine); - pthread_mutex_unlock(&dso__data_open_lock); + + if (dso->data.fd < 0) + pthread_mutex_unlock(&dso__data_open_lock); return dso->data.fd; } +void dso__data_put_fd(struct dso *dso __maybe_unused) +{ + pthread_mutex_unlock(&dso__data_open_lock); +} + bool dso__data_status_seen(struct dso *dso, enum dso_data_status_seen by) { u32 flag = 1 << by; @@ -1199,12 +1209,15 @@ size_t dso__fprintf(struct dso *dso, enum map_type type, FILE *fp) enum dso_type dso__type(struct dso *dso, struct machine *machine) { int fd; + enum dso_type type = DSO__TYPE_UNKNOWN; - fd = dso__data_fd(dso, machine); - if (fd < 0) - return DSO__TYPE_UNKNOWN; + fd = dso__data_get_fd(dso, machine); + if (fd >= 0) { + type = dso__type_fd(fd); + dso__data_put_fd(dso); + } - return dso__type_fd(fd); + return type; } int dso__strerror_load(struct dso *dso, char *buf, size_t buflen) diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h index b26ec3ab1336..bcec06ad73a2 100644 --- a/tools/perf/util/dso.h +++ b/tools/perf/util/dso.h @@ -240,7 +240,8 @@ int __kmod_path__parse(struct kmod_path *m, const char *path, /* * The dso__data_* external interface provides following functions: - * dso__data_fd + * dso__data_get_fd + * dso__data_put_fd * dso__data_close * dso__data_size * dso__data_read_offset @@ -257,8 +258,11 @@ int __kmod_path__parse(struct kmod_path *m, const char *path, * The current usage of the dso__data_* interface is as follows: * * Get DSO's fd: - * int fd = dso__data_fd(dso, machine); - * USE 'fd' SOMEHOW + * int fd = dso__data_get_fd(dso, machine); + * if (fd >= 0) { + * USE 'fd' SOMEHOW + * dso__data_put_fd(dso); + * } * * Read DSO's data: * n = dso__data_read_offset(dso_0, &machine, 0, buf, BUFSIZE); @@ -277,7 +281,8 @@ int __kmod_path__parse(struct kmod_path *m, const char *path, * * TODO */ -int dso__data_fd(struct dso *dso, struct machine *machine); +int dso__data_get_fd(struct dso *dso, struct machine *machine); +void dso__data_put_fd(struct dso *dso __maybe_unused); void dso__data_close(struct dso *dso); off_t dso__data_size(struct dso *dso, struct machine *machine); diff --git a/tools/perf/util/unwind-libunwind.c b/tools/perf/util/unwind-libunwind.c index 7b09a443a280..f079b63f0b7f 100644 --- a/tools/perf/util/unwind-libunwind.c +++ b/tools/perf/util/unwind-libunwind.c @@ -269,13 +269,14 @@ static int read_unwind_spec_eh_frame(struct dso *dso, struct machine *machine, u64 offset = dso->data.eh_frame_hdr_offset; if (offset == 0) { - fd = dso__data_fd(dso, machine); + fd = dso__data_get_fd(dso, machine); if (fd < 0) return -EINVAL; /* Check the .eh_frame section for unwinding info */ offset = elf_section_offset(fd, ".eh_frame_hdr"); dso->data.eh_frame_hdr_offset = offset; + dso__data_put_fd(dso); } if (offset) @@ -294,13 +295,14 @@ static int read_unwind_spec_debug_frame(struct dso *dso, u64 ofs = dso->data.debug_frame_offset; if (ofs == 0) { - fd = dso__data_fd(dso, machine); + fd = dso__data_get_fd(dso, machine); if (fd < 0) return -EINVAL; /* Check the .debug_frame section for unwinding info */ ofs = elf_section_offset(fd, ".debug_frame"); dso->data.debug_frame_offset = ofs; + dso__data_put_fd(dso); } *offset = ofs; @@ -353,10 +355,13 @@ find_proc_info(unw_addr_space_t as, unw_word_t ip, unw_proc_info_t *pi, #ifndef NO_LIBUNWIND_DEBUG_FRAME /* Check the .debug_frame section for unwinding info */ if (!read_unwind_spec_debug_frame(map->dso, ui->machine, &segbase)) { - int fd = dso__data_fd(map->dso, ui->machine); + int fd = dso__data_get_fd(map->dso, ui->machine); int is_exec = elf_is_exec(fd, map->dso->name); unw_word_t base = is_exec ? 0 : map->start; + if (fd >= 0) + dso__data_put_fd(dso); + memset(&di, 0, sizeof(di)); if (dwarf_find_debug_frame(0, &di, ip, base, map->dso->name, map->start, map->end)) From 4d4dee9a9609819309a84cd3f2d19dcc50ece195 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Thu, 21 May 2015 17:48:33 -0300 Subject: [PATCH 13/25] perf tools: Rename maps__next It really is a 'struct map' method, and since we're introducing a new 'struct maps' class, fix it to avoid confusion. Cc: Adrian Hunter Cc: Borislav Petkov Cc: David Ahern Cc: Don Zickus Cc: Frederic Weisbecker Cc: Jiri Olsa Cc: Namhyung Kim Cc: Stephane Eranian Link: http://lkml.kernel.org/n/tip-xo9ifhk53cfl30wqcuhxpnvl@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/map.c | 2 +- tools/perf/util/map.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c index 2d20c5ff8653..09a62731e035 100644 --- a/tools/perf/util/map.c +++ b/tools/perf/util/map.c @@ -775,7 +775,7 @@ struct map *maps__first(struct rb_root *maps) return NULL; } -struct map *maps__next(struct map *map) +struct map *map__next(struct map *map) { struct rb_node *next = rb_next(&map->rb_node); diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h index 7f39217d29bf..aba9569165ea 100644 --- a/tools/perf/util/map.h +++ b/tools/perf/util/map.h @@ -166,7 +166,7 @@ void maps__insert(struct rb_root *maps, struct map *map); void maps__remove(struct rb_root *maps, struct map *map); struct map *maps__find(struct rb_root *maps, u64 addr); struct map *maps__first(struct rb_root *maps); -struct map *maps__next(struct map *map); +struct map *map__next(struct map *map); void map_groups__init(struct map_groups *mg, struct machine *machine); void map_groups__exit(struct map_groups *mg); int map_groups__clone(struct map_groups *mg, @@ -201,7 +201,7 @@ static inline struct map *map_groups__first(struct map_groups *mg, static inline struct map *map_groups__next(struct map *map) { - return maps__next(map); + return map__next(map); } struct symbol *map_groups__find_symbol(struct map_groups *mg, From fdce6a4edaada40136f0e61569b938c9a25f61d5 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Fri, 22 May 2015 17:42:37 -0300 Subject: [PATCH 14/25] perf tools: Remove redundant initialization of thread linkage members A thread moves from a rb tree to a list, but can't be on both, because those linkage members are in a union. This is leftover from when I was debugging thread refcounting and had nuked that union. It is harmless duplication, as RB_CLEAR_NODE() does again what INIT_LIST_HEAD does. Cc: Adrian Hunter Cc: Borislav Petkov Cc: David Ahern Cc: Don Zickus Cc: Frederic Weisbecker Cc: Jiri Olsa Cc: Namhyung Kim Cc: Stephane Eranian Link: http://lkml.kernel.org/n/tip-hmma9lmip6qlhzhgkhp9tzd1@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/thread.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c index 16c28a37a9e4..28c4b746baa1 100644 --- a/tools/perf/util/thread.c +++ b/tools/perf/util/thread.c @@ -54,7 +54,6 @@ struct thread *thread__new(pid_t pid, pid_t tid) list_add(&comm->list, &thread->comm_list); atomic_set(&thread->refcnt, 0); - INIT_LIST_HEAD(&thread->node); RB_CLEAR_NODE(&thread->rb_node); } @@ -70,7 +69,6 @@ void thread__delete(struct thread *thread) struct comm *comm, *tmp; BUG_ON(!RB_EMPTY_NODE(&thread->rb_node)); - BUG_ON(!list_empty(&thread->node)); thread_stack__free(thread); From f7e365eb61d4d78f2f5e66d859664048c2921df2 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Mon, 25 May 2015 18:03:44 -0300 Subject: [PATCH 15/25] perf tools: Nuke unused map_groups__flush() Since: 9fdbf671ba7e "perf tools: do not flush maps on COMM for perf report" We have no users of this function, nuke it. Cc: Adrian Hunter Cc: Borislav Petkov Cc: David Ahern Cc: Don Zickus Cc: Frederic Weisbecker Cc: Jiri Olsa Cc: Luigi Semenzato Cc: Namhyung Kim Cc: Stephane Eranian Link: http://lkml.kernel.org/n/tip-hsac1t42ehtva8gut8qe6hih@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/map.c | 22 ---------------------- tools/perf/util/map.h | 2 -- 2 files changed, 24 deletions(-) diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c index 09a62731e035..c1bfd0a12a94 100644 --- a/tools/perf/util/map.c +++ b/tools/perf/util/map.c @@ -498,28 +498,6 @@ void map_groups__put(struct map_groups *mg) map_groups__delete(mg); } -void map_groups__flush(struct map_groups *mg) -{ - int type; - - for (type = 0; type < MAP__NR_TYPES; type++) { - struct rb_root *root = &mg->maps[type]; - struct rb_node *next = rb_first(root); - - while (next) { - struct map *pos = rb_entry(next, struct map, rb_node); - next = rb_next(&pos->rb_node); - rb_erase(&pos->rb_node, root); - /* - * We may have references to this map, for - * instance in some hist_entry instances, so - * just move them to a separate list. - */ - list_add_tail(&pos->node, &mg->removed_maps[pos->type]); - } - } -} - struct symbol *map_groups__find_symbol(struct map_groups *mg, enum map_type type, u64 addr, struct map **mapp, diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h index aba9569165ea..f2b27566d986 100644 --- a/tools/perf/util/map.h +++ b/tools/perf/util/map.h @@ -233,6 +233,4 @@ int map_groups__fixup_overlappings(struct map_groups *mg, struct map *map, struct map *map_groups__find_by_name(struct map_groups *mg, enum map_type type, const char *name); -void map_groups__flush(struct map_groups *mg); - #endif /* __PERF_MAP_H */ From 9402e23f90c5a672db7170e4c0f1fc80ca8c009a Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Mon, 25 May 2015 11:49:11 -0300 Subject: [PATCH 16/25] perf tools: Import rb_erase_init from block/ in the kernel sources I was assuming rb_erase() was setting things up like list_del_init, but the fact that thread__delete() was being sucessfull is because the last thing before deleting is to remove the thread from the machine->dead_threads list, using list_del_init(), that has the same effect as using rb_erase_init()... Introduce this function so that we can use it when removing objects from rb_trees. Then we will be able to BUG_ON(still on a list) in destructors. Cc: Adrian Hunter Cc: Borislav Petkov Cc: David Ahern Cc: Don Zickus Cc: Frederic Weisbecker Cc: Jiri Olsa Cc: Namhyung Kim Cc: Stephane Eranian Link: http://lkml.kernel.org/n/tip-55b16mbtndjyd7zzg8nmnamx@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/include/linux/rbtree.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tools/perf/util/include/linux/rbtree.h b/tools/perf/util/include/linux/rbtree.h index 2a030c5af3aa..f06d89f0b867 100644 --- a/tools/perf/util/include/linux/rbtree.h +++ b/tools/perf/util/include/linux/rbtree.h @@ -1,2 +1,16 @@ +#ifndef __TOOLS_LINUX_PERF_RBTREE_H +#define __TOOLS_LINUX_PERF_RBTREE_H #include #include "../../../../include/linux/rbtree.h" + +/* + * Handy for checking that we are not deleting an entry that is + * already in a list, found in block/{blk-throttle,cfq-iosched}.c, + * probably should be moved to lib/rbtree.c... + */ +static inline void rb_erase_init(struct rb_node *n, struct rb_root *root) +{ + rb_erase(n, root); + RB_CLEAR_NODE(n); +} +#endif /* __TOOLS_LINUX_PERF_RBTREE_H */ From 0170b14f5f5462524d05ee96275b7a0a0d34ae77 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Mon, 25 May 2015 15:23:05 -0300 Subject: [PATCH 17/25] perf machine: Mark removed threads as such We use: BUG_ON(!RB_EMPTY_NODE(&thread->rb_node)); in the thread destructor as a debugging check to find out about possibly still referenced thread instances being deleted, to do that we need to make sure we use RB_CLEAR_NODE() right after rb_erase(), i.e. that we use the newly introduced rb_erase_init(), that works just like list_del_init(). Cc: Adrian Hunter Cc: Borislav Petkov Cc: David Ahern Cc: Don Zickus Cc: Frederic Weisbecker Cc: Jiri Olsa Cc: Namhyung Kim Cc: Stephane Eranian Link: http://lkml.kernel.org/n/tip-4fcqo5ypy1cjjf15ilb0hn78@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/machine.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c index 7ec3188d3cb3..6bf845758ae3 100644 --- a/tools/perf/util/machine.c +++ b/tools/perf/util/machine.c @@ -400,7 +400,7 @@ static struct thread *____machine__findnew_thread(struct machine *machine, * leader and that would screwed the rb tree. */ if (thread__init_map_groups(th, machine)) { - rb_erase(&th->rb_node, &machine->threads); + rb_erase_init(&th->rb_node, &machine->threads); RB_CLEAR_NODE(&th->rb_node); thread__delete(th); return NULL; @@ -1314,7 +1314,7 @@ static void __machine__remove_thread(struct machine *machine, struct thread *th, BUG_ON(atomic_read(&th->refcnt) == 0); if (lock) pthread_rwlock_wrlock(&machine->threads_lock); - rb_erase(&th->rb_node, &machine->threads); + rb_erase_init(&th->rb_node, &machine->threads); RB_CLEAR_NODE(&th->rb_node); /* * Move it first to the dead_threads list, then drop the reference, From 614c6b570d5157c2cf835d334bc89af071fc2e44 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Mon, 25 May 2015 16:21:53 -0300 Subject: [PATCH 18/25] perf tools: Leave DSO destruction to the map destruction As the way DSOs are created are normally via dsos__findnew, so that we don't have to load the same dso multiple times for multiple maps (think about /lib64/libc.so.6), so they may be shared and dso__delete() should be left to be done as part of the map destruction process. This will all be properly solved by reference counting struct dso, which will be done soon. Cc: Adrian Hunter Cc: Borislav Petkov Cc: David Ahern Cc: Don Zickus Cc: Frederic Weisbecker Cc: Jiri Olsa Cc: Namhyung Kim Cc: Stephane Eranian Link: http://lkml.kernel.org/n/tip-gbrohe1nvkjxw3u5a1bgj3yh@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/probe-event.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c index 1faa1e67398b..db6021834e8f 100644 --- a/tools/perf/util/probe-event.c +++ b/tools/perf/util/probe-event.c @@ -195,7 +195,6 @@ static void put_target_map(struct map *map, bool user) { if (map && user) { /* Only the user map needs to be released */ - dso__delete(map->dso); map__delete(map); } } @@ -1791,7 +1790,6 @@ static int find_perf_probe_point_from_map(struct probe_trace_point *tp, out: if (map && !is_kprobe) { - dso__delete(map->dso); map__delete(map); } @@ -2884,7 +2882,6 @@ int show_available_funcs(const char *target, struct strfilter *_filter, dso__fprintf_symbols_by_name(map->dso, map->type, stdout); end: if (user) { - dso__delete(map->dso); map__delete(map); } exit_symbol_maps(); From 4bb7123dcfa7aa1d963ad4a8f01b88d54a2bb873 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Fri, 22 May 2015 11:52:22 -0300 Subject: [PATCH 19/25] perf tools: Use maps__first()/map__next() In a few more remaining places, for consistency. Cc: Adrian Hunter Cc: Borislav Petkov Cc: David Ahern Cc: Don Zickus Cc: Frederic Weisbecker Cc: Jiri Olsa Cc: Namhyung Kim Cc: Stephane Eranian Link: http://lkml.kernel.org/n/tip-c2n7slwtto29wndfttdrhfrx@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/tests/vmlinux-kallsyms.c | 34 ++++++++++++++--------------- tools/perf/util/event.c | 7 +++--- tools/perf/util/map.c | 7 +++--- tools/perf/util/probe-event.c | 6 ++--- tools/perf/util/symbol.c | 23 +++++++++---------- 5 files changed, 37 insertions(+), 40 deletions(-) diff --git a/tools/perf/tests/vmlinux-kallsyms.c b/tools/perf/tests/vmlinux-kallsyms.c index 3d9088003a5b..94ac6924df65 100644 --- a/tools/perf/tests/vmlinux-kallsyms.c +++ b/tools/perf/tests/vmlinux-kallsyms.c @@ -23,9 +23,10 @@ int test__vmlinux_matches_kallsyms(void) int err = -1; struct rb_node *nd; struct symbol *sym; - struct map *kallsyms_map, *vmlinux_map; + struct map *kallsyms_map, *vmlinux_map, *map; struct machine kallsyms, vmlinux; enum map_type type = MAP__FUNCTION; + struct rb_root *maps = &vmlinux.kmaps.maps[type]; u64 mem_start, mem_end; /* @@ -184,8 +185,8 @@ detour: pr_info("Maps only in vmlinux:\n"); - for (nd = rb_first(&vmlinux.kmaps.maps[type]); nd; nd = rb_next(nd)) { - struct map *pos = rb_entry(nd, struct map, rb_node), *pair; + for (map = maps__first(maps); map; map = map__next(map)) { + struct map * /* * If it is the kernel, kallsyms is always "[kernel.kallsyms]", while * the kernel will have the path for the vmlinux file being used, @@ -193,22 +194,22 @@ detour: * both cases. */ pair = map_groups__find_by_name(&kallsyms.kmaps, type, - (pos->dso->kernel ? - pos->dso->short_name : - pos->dso->name)); + (map->dso->kernel ? + map->dso->short_name : + map->dso->name)); if (pair) pair->priv = 1; else - map__fprintf(pos, stderr); + map__fprintf(map, stderr); } pr_info("Maps in vmlinux with a different name in kallsyms:\n"); - for (nd = rb_first(&vmlinux.kmaps.maps[type]); nd; nd = rb_next(nd)) { - struct map *pos = rb_entry(nd, struct map, rb_node), *pair; + for (map = maps__first(maps); map; map = map__next(map)) { + struct map *pair; - mem_start = vmlinux_map->unmap_ip(vmlinux_map, pos->start); - mem_end = vmlinux_map->unmap_ip(vmlinux_map, pos->end); + mem_start = vmlinux_map->unmap_ip(vmlinux_map, map->start); + mem_end = vmlinux_map->unmap_ip(vmlinux_map, map->end); pair = map_groups__find(&kallsyms.kmaps, type, mem_start); if (pair == NULL || pair->priv) @@ -217,7 +218,7 @@ detour: if (pair->start == mem_start) { pair->priv = 1; pr_info(" %" PRIx64 "-%" PRIx64 " %" PRIx64 " %s in kallsyms as", - pos->start, pos->end, pos->pgoff, pos->dso->name); + map->start, map->end, map->pgoff, map->dso->name); if (mem_end != pair->end) pr_info(":\n*%" PRIx64 "-%" PRIx64 " %" PRIx64, pair->start, pair->end, pair->pgoff); @@ -228,12 +229,11 @@ detour: pr_info("Maps only in kallsyms:\n"); - for (nd = rb_first(&kallsyms.kmaps.maps[type]); - nd; nd = rb_next(nd)) { - struct map *pos = rb_entry(nd, struct map, rb_node); + maps = &kallsyms.kmaps.maps[type]; - if (!pos->priv) - map__fprintf(pos, stderr); + for (map = maps__first(maps); map; map = map__next(map)) { + if (!map->priv) + map__fprintf(map, stderr); } out: machine__exit(&kallsyms); diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c index a513a51f7330..9d3bba175423 100644 --- a/tools/perf/util/event.c +++ b/tools/perf/util/event.c @@ -329,8 +329,9 @@ int perf_event__synthesize_modules(struct perf_tool *tool, struct machine *machine) { int rc = 0; - struct rb_node *nd; + struct map *pos; struct map_groups *kmaps = &machine->kmaps; + struct rb_root *maps = &kmaps->maps[MAP__FUNCTION]; union perf_event *event = zalloc((sizeof(event->mmap) + machine->id_hdr_size)); if (event == NULL) { @@ -350,10 +351,8 @@ int perf_event__synthesize_modules(struct perf_tool *tool, else event->header.misc = PERF_RECORD_MISC_GUEST_KERNEL; - for (nd = rb_first(&kmaps->maps[MAP__FUNCTION]); - nd; nd = rb_next(nd)) { + for (pos = maps__first(maps); pos; pos = map__next(pos)) { size_t size; - struct map *pos = rb_entry(nd, struct map, rb_node); if (pos->dso->kernel) continue; diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c index c1bfd0a12a94..898ab92a98dd 100644 --- a/tools/perf/util/map.c +++ b/tools/perf/util/map.c @@ -688,9 +688,10 @@ move_map: int map_groups__clone(struct map_groups *mg, struct map_groups *parent, enum map_type type) { - struct rb_node *nd; - for (nd = rb_first(&parent->maps[type]); nd; nd = rb_next(nd)) { - struct map *map = rb_entry(nd, struct map, rb_node); + struct map *map; + struct rb_root *maps = &parent->maps[type]; + + for (map = maps__first(maps); map; map = map__next(map)) { struct map *new = map__clone(map); if (new == NULL) return -ENOMEM; diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c index db6021834e8f..092256516262 100644 --- a/tools/perf/util/probe-event.c +++ b/tools/perf/util/probe-event.c @@ -162,8 +162,9 @@ static u64 kernel_get_symbol_address_by_name(const char *name, bool reloc) static struct map *kernel_get_module_map(const char *module) { - struct rb_node *nd; struct map_groups *grp = &host_machine->kmaps; + struct rb_root *maps = &grp->maps[MAP__FUNCTION]; + struct map *pos; /* A file path -- this is an offline module */ if (module && strchr(module, '/')) @@ -172,8 +173,7 @@ static struct map *kernel_get_module_map(const char *module) if (!module) module = "kernel"; - for (nd = rb_first(&grp->maps[MAP__FUNCTION]); nd; nd = rb_next(nd)) { - struct map *pos = rb_entry(nd, struct map, rb_node); + for (pos = maps__first(maps); pos; pos = map__next(pos)) { if (strncmp(pos->dso->short_name + 1, module, pos->dso->short_name_len - 2) == 0) { return pos; diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index 82a31fd0fcf5..00b6b17e74a7 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -202,18 +202,16 @@ void symbols__fixup_end(struct rb_root *symbols) void __map_groups__fixup_end(struct map_groups *mg, enum map_type type) { - struct map *prev, *curr; - struct rb_node *nd, *prevnd = rb_first(&mg->maps[type]); + struct rb_root *maps = &mg->maps[type]; + struct map *next, *curr; - if (prevnd == NULL) + curr = maps__first(maps); + if (curr == NULL) return; - curr = rb_entry(prevnd, struct map, rb_node); - - for (nd = rb_next(prevnd); nd; nd = rb_next(nd)) { - prev = curr; - curr = rb_entry(nd, struct map, rb_node); - prev->end = curr->start; + for (next = map__next(curr); next; next = map__next(curr)) { + curr->end = next->start; + curr = next; } /* @@ -1522,11 +1520,10 @@ out: struct map *map_groups__find_by_name(struct map_groups *mg, enum map_type type, const char *name) { - struct rb_node *nd; - - for (nd = rb_first(&mg->maps[type]); nd; nd = rb_next(nd)) { - struct map *map = rb_entry(nd, struct map, rb_node); + struct rb_root *maps = &mg->maps[type]; + struct map *map; + for (map = maps__first(maps); map; map = map__next(map)) { if (map->dso && strcmp(map->dso->short_name, name) == 0) return map; } From 5bcaaca3e4d15ce39008a0b9c431c0ac4be784bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Li=C5=A1ka?= Date: Tue, 26 May 2015 11:41:37 -0300 Subject: [PATCH 20/25] perf tools: Assign default value for some pointers Assign default value for pointers that are identified by the compiler as non-initialized. Signed-off-by: Martin Liska Acked-by: Ingo Molnar Link: http://lkml.kernel.org/r/5564393C.1090104@suse.cz Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/arch/common.c | 2 +- tools/perf/util/symbol.c | 2 +- tools/perf/util/trace-event-parse.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c index 49776f190abf..b7bb42c44694 100644 --- a/tools/perf/arch/common.c +++ b/tools/perf/arch/common.c @@ -61,7 +61,7 @@ const char *const mips_triplets[] = { static bool lookup_path(char *name) { bool found = false; - char *path, *tmp; + char *path, *tmp = NULL; char buf[PATH_MAX]; char *env = getenv("PATH"); diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index 00b6b17e74a7..b9e3eb581884 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -398,7 +398,7 @@ static struct symbol *symbols__find_by_name(struct rb_root *symbols, const char *name) { struct rb_node *n; - struct symbol_name_rb_node *s; + struct symbol_name_rb_node *s = NULL; if (symbols == NULL) return NULL; diff --git a/tools/perf/util/trace-event-parse.c b/tools/perf/util/trace-event-parse.c index 25d6c737be3e..d4957418657e 100644 --- a/tools/perf/util/trace-event-parse.c +++ b/tools/perf/util/trace-event-parse.c @@ -173,7 +173,7 @@ void parse_ftrace_printk(struct pevent *pevent, char *line; char *next = NULL; char *addr_str; - char *fmt; + char *fmt = NULL; line = strtok_r(file, "\n", &next); while (line) { From e8b7ea4356fdd3c4de5478f3418eb84f8dce2b61 Mon Sep 17 00:00:00 2001 From: Martin Liska Date: Tue, 26 May 2015 12:23:24 -0300 Subject: [PATCH 21/25] perf tools: Improve setting of gcc debug option Correct debugging experience is given by passing -Og to compiler. Do it in a way that supports older compilers Signed-off-by: Martin Liska Acked-by: Ingo Molnar Link: http://lkml.kernel.org/r/5564393C.1090104@suse.cz Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/config/Makefile | 2 ++ tools/perf/config/utilities.mak | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile index e3b3724e73ff..317001c94660 100644 --- a/tools/perf/config/Makefile +++ b/tools/perf/config/Makefile @@ -130,6 +130,8 @@ endif ifeq ($(DEBUG),0) CFLAGS += -O6 +else + CFLAGS += $(call cc-option,-Og,-O0) endif ifdef PARSER_DEBUG diff --git a/tools/perf/config/utilities.mak b/tools/perf/config/utilities.mak index c16ce833079c..0ebef09c0842 100644 --- a/tools/perf/config/utilities.mak +++ b/tools/perf/config/utilities.mak @@ -177,3 +177,22 @@ $(if $($(1)),$(call _ge_attempt,$($(1)),$(1)),$(call _ge_attempt,$(2))) endef _ge_attempt = $(if $(get-executable),$(get-executable),$(call _gea_err,$(2))) _gea_err = $(if $(1),$(error Please set '$(1)' appropriately)) + +# try-run +# Usage: option = $(call try-run, $(CC)...-o "$$TMP",option-ok,otherwise) +# Exit code chooses option. "$$TMP" is can be used as temporary file and +# is automatically cleaned up. +try-run = $(shell set -e; \ + TMP="$(TMPOUT).$$$$.tmp"; \ + TMPO="$(TMPOUT).$$$$.o"; \ + if ($(1)) >/dev/null 2>&1; \ + then echo "$(2)"; \ + else echo "$(3)"; \ + fi; \ + rm -f "$$TMP" "$$TMPO") + +# cc-option +# Usage: cflags-y += $(call cc-option,-march=winchip-c6,-march=i586) + +cc-option = $(call try-run,\ + $(CC) $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(1) -c -x c /dev/null -o "$$TMP",$(1),$(2)) From 2f80dd4488c204a4850554746eb31f25f5a84405 Mon Sep 17 00:00:00 2001 From: Josef Bacik Date: Fri, 22 May 2015 09:18:40 -0400 Subject: [PATCH 22/25] perf sched: Add option to merge like comms to lat output Sometimes when debugging large multi-threaded applications it is helpful to collate all of the latency numbers into one bulk record to get an idea of what is going on. This patch does this by merging any entries that belong to the same comm into one entry and then spits out those totals. I've also slightly changed the output so you can see how many threads were merged in the processing. Here is the new default output format ----------------------------------------------------------------------------------------------------------- Task | Runtime ms | Switches | Average delay ms | Maximum delay ms | Maximum delay at | ----------------------------------------------------------------------------------------------------------- chrome:(23) | 740.878 ms | 2612 | avg: 0.022 ms | max: 0.845 ms | max at: 7935.254223 s pulseaudio:1523 | 94.440 ms | 597 | avg: 0.027 ms | max: 0.110 ms | max at: 7934.668372 s threaded-ml:6042 | 72.554 ms | 386 | avg: 0.035 ms | max: 1.186 ms | max at: 7935.330911 s Chrome_IOThread:3832 | 52.388 ms | 456 | avg: 0.021 ms | max: 1.365 ms | max at: 7935.330602 s Chrome_ChildIOT:(7) | 50.694 ms | 743 | avg: 0.021 ms | max: 1.448 ms | max at: 7935.256659 s Compositor:5510 | 30.012 ms | 192 | avg: 0.019 ms | max: 0.131 ms | max at: 7936.636815 s plugin_audio_th:6043 | 24.828 ms | 314 | avg: 0.018 ms | max: 0.143 ms | max at: 7936.205994 s CompositorTileW:(2) | 14.099 ms | 45 | avg: 0.022 ms | max: 0.153 ms | max at: 7937.521800 s the (#) after the task is the number of tasks merged, and then if there were no tasks merged it just shows the pid. Here is the same trace file with the -p option to print the per-pid latency numbers ----------------------------------------------------------------------------------------------------------- Task | Runtime ms | Switches | Average delay ms | Maximum delay ms | Maximum delay at | ----------------------------------------------------------------------------------------------------------- chrome:5500 | 386.872 ms | 387 | avg: 0.023 ms | max: 0.241 ms | max at: 7936.001694 s pulseaudio:1523 | 94.440 ms | 597 | avg: 0.027 ms | max: 0.110 ms | max at: 7934.668372 s threaded-ml:6042 | 72.554 ms | 386 | avg: 0.035 ms | max: 1.186 ms | max at: 7935.330911 s chrome:10226 | 69.710 ms | 251 | avg: 0.023 ms | max: 0.764 ms | max at: 7935.992305 s chrome:4267 | 64.551 ms | 418 | avg: 0.021 ms | max: 0.294 ms | max at: 7937.862427 s chrome:4827 | 62.268 ms | 54 | avg: 0.029 ms | max: 0.666 ms | max at: 7935.992813 s Chrome_IOThread:3832 | 52.388 ms | 456 | avg: 0.021 ms | max: 1.365 ms | max at: 7935.330602 s chrome:3776 | 46.150 ms | 349 | avg: 0.023 ms | max: 0.845 ms | max at: 7935.254223 s Signed-off-by: Josef Bacik Acked-by: Ingo Molnar Cc: Peter Zijlstra Cc: kernel-team@fb.com Link: http://lkml.kernel.org/r/1432300720-30478-1-git-send-email-jbacik@fb.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-sched.c | 77 +++++++++++++++++++++++++++++++++++--- 1 file changed, 72 insertions(+), 5 deletions(-) diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c index 79273ecf92eb..33962612a5e9 100644 --- a/tools/perf/builtin-sched.c +++ b/tools/perf/builtin-sched.c @@ -95,6 +95,7 @@ struct work_atoms { u64 total_lat; u64 nb_atoms; u64 total_runtime; + int num_merged; }; typedef int (*sort_fn_t)(struct work_atoms *, struct work_atoms *); @@ -168,9 +169,10 @@ struct perf_sched { u64 all_runtime; u64 all_count; u64 cpu_last_switched[MAX_CPUS]; - struct rb_root atom_root, sorted_atom_root; + struct rb_root atom_root, sorted_atom_root, merged_atom_root; struct list_head sort_list, cmp_pid; bool force; + bool skip_merge; }; static u64 get_nsecs(void) @@ -1182,7 +1184,10 @@ static void output_lat_thread(struct perf_sched *sched, struct work_atoms *work_ sched->all_runtime += work_list->total_runtime; sched->all_count += work_list->nb_atoms; - ret = printf(" %s:%d ", thread__comm_str(work_list->thread), work_list->thread->tid); + if (work_list->num_merged > 1) + ret = printf(" %s:(%d) ", thread__comm_str(work_list->thread), work_list->num_merged); + else + ret = printf(" %s:%d ", thread__comm_str(work_list->thread), work_list->thread->tid); for (i = 0; i < 24 - ret; i++) printf(" "); @@ -1302,17 +1307,22 @@ static int sort_dimension__add(const char *tok, struct list_head *list) static void perf_sched__sort_lat(struct perf_sched *sched) { struct rb_node *node; - + struct rb_root *root = &sched->atom_root; +again: for (;;) { struct work_atoms *data; - node = rb_first(&sched->atom_root); + node = rb_first(root); if (!node) break; - rb_erase(node, &sched->atom_root); + rb_erase(node, root); data = rb_entry(node, struct work_atoms, node); __thread_latency_insert(&sched->sorted_atom_root, data, &sched->sort_list); } + if (root == &sched->atom_root) { + root = &sched->merged_atom_root; + goto again; + } } static int process_sched_wakeup_event(struct perf_tool *tool, @@ -1572,6 +1582,59 @@ static void print_bad_events(struct perf_sched *sched) } } +static void __merge_work_atoms(struct rb_root *root, struct work_atoms *data) +{ + struct rb_node **new = &(root->rb_node), *parent = NULL; + struct work_atoms *this; + const char *comm = thread__comm_str(data->thread), *this_comm; + + while (*new) { + int cmp; + + this = container_of(*new, struct work_atoms, node); + parent = *new; + + this_comm = thread__comm_str(this->thread); + cmp = strcmp(comm, this_comm); + if (cmp > 0) { + new = &((*new)->rb_left); + } else if (cmp < 0) { + new = &((*new)->rb_right); + } else { + this->num_merged++; + this->total_runtime += data->total_runtime; + this->nb_atoms += data->nb_atoms; + this->total_lat += data->total_lat; + list_splice(&data->work_list, &this->work_list); + if (this->max_lat < data->max_lat) { + this->max_lat = data->max_lat; + this->max_lat_at = data->max_lat_at; + } + zfree(&data); + return; + } + } + + data->num_merged++; + rb_link_node(&data->node, parent, new); + rb_insert_color(&data->node, root); +} + +static void perf_sched__merge_lat(struct perf_sched *sched) +{ + struct work_atoms *data; + struct rb_node *node; + + if (sched->skip_merge) + return; + + while ((node = rb_first(&sched->atom_root))) { + rb_erase(node, &sched->atom_root); + data = rb_entry(node, struct work_atoms, node); + __merge_work_atoms(&sched->merged_atom_root, data); + } +} + static int perf_sched__lat(struct perf_sched *sched) { struct rb_node *next; @@ -1581,6 +1644,7 @@ static int perf_sched__lat(struct perf_sched *sched) if (perf_sched__read_events(sched)) return -1; + perf_sched__merge_lat(sched); perf_sched__sort_lat(sched); printf("\n -----------------------------------------------------------------------------------------------------------------\n"); @@ -1732,6 +1796,7 @@ int cmd_sched(int argc, const char **argv, const char *prefix __maybe_unused) .profile_cpu = -1, .next_shortname1 = 'A', .next_shortname2 = '0', + .skip_merge = 0, }; const struct option latency_options[] = { OPT_STRING('s', "sort", &sched.sort_order, "key[,key2...]", @@ -1742,6 +1807,8 @@ int cmd_sched(int argc, const char **argv, const char *prefix __maybe_unused) "CPU to profile on"), OPT_BOOLEAN('D', "dump-raw-trace", &dump_trace, "dump raw trace in ASCII"), + OPT_BOOLEAN('p', "pids", &sched.skip_merge, + "latency stats per pid instead of per comm"), OPT_END() }; const struct option replay_options[] = { From 9b5d1c29556989aa9dc1240566e78806ddefd160 Mon Sep 17 00:00:00 2001 From: Adrian Hunter Date: Fri, 22 May 2015 14:53:58 +0300 Subject: [PATCH 23/25] perf tools: Disallow PMU events intel_pt and intel_bts until there is support Disallow PMU events intel_pt and intel_bts until the tools support them. By default any PMU is selectable as an event but until the tools have intel_pt and intel_bts support using them would result in no data being recorded without any indication as to why. Before the change: $ perf record -e intel_bts// sleep 1 [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.008 MB perf.data ] $ perf report --stdio Error: The perf.data file has no samples! After the change: $ perf record -e intel_bts// sleep 1 invalid or unsupported event: 'intel_bts//' Run 'perf list' for a list of valid events Reported-by: Arnaldo Carvalho de Melo Signed-off-by: Adrian Hunter Cc: Jiri Olsa Link: http://lkml.kernel.org/r/1432295653-13989-2-git-send-email-adrian.hunter@intel.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/pmu.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c index 244c66f89891..5d3ab7c8ceaf 100644 --- a/tools/perf/util/pmu.c +++ b/tools/perf/util/pmu.c @@ -442,6 +442,10 @@ static struct perf_pmu *pmu_lookup(const char *name) LIST_HEAD(aliases); __u32 type; + /* No support for intel_bts or intel_pt so disallow them */ + if (!strcmp(name, "intel_bts") || !strcmp(name, "intel_pt")) + return NULL; + /* * The pmu data we store & need consists of the pmu * type value and format definitions. Load both right From 419e87382873b11b17cb31e2f21859570a32e0d1 Mon Sep 17 00:00:00 2001 From: Masami Hiramatsu Date: Wed, 27 May 2015 17:37:18 +0900 Subject: [PATCH 24/25] perf probe: Show the error reason comes from invalid DSO Show the reason of error when dso__load* fails. This shows when user gives wrong kernel image or wrong path. Without this, perf probe shows an obscure message: ---- $ perf probe -k ~/kbin/linux-3.x86_64/vmlinux -L vfs_read Failed to find path of kernel module. Error: Failed to show lines. ---- With this, perf shows appropriate error message: ---- $ perf probe -k ~/kbin/linux-3.x86_64/vmlinux -L vfs_read Failed to find the path for kernel: Mismatching build id Error: Failed to show lines. ---- And: ---- $ perf probe -k /non-exist/kernel/vmlinux -L vfs_read Failed to find the path for kernel: No such file or directory Error: Failed to show lines. ---- Signed-off-by: Masami Hiramatsu Tested-by: Arnaldo Carvalho de Melo Cc: David Ahern Cc: Jiri Olsa Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Richard Weinberger Link: http://lkml.kernel.org/r/20150527083718.23880.84100.stgit@localhost.localdomain Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/probe-event.c | 47 ++++++++++++++++++----------------- tools/perf/util/probe-event.h | 3 --- 2 files changed, 24 insertions(+), 26 deletions(-) diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c index 092256516262..f5be411bc69c 100644 --- a/tools/perf/util/probe-event.c +++ b/tools/perf/util/probe-event.c @@ -200,11 +200,12 @@ static void put_target_map(struct map *map, bool user) } -static struct dso *kernel_get_module_dso(const char *module) +static int kernel_get_module_dso(const char *module, struct dso **pdso) { struct dso *dso; struct map *map; const char *vmlinux_name; + int ret = 0; if (module) { list_for_each_entry(dso, &host_machine->kernel_dsos.head, @@ -214,30 +215,21 @@ static struct dso *kernel_get_module_dso(const char *module) goto found; } pr_debug("Failed to find module %s.\n", module); - return NULL; + return -ENOENT; } map = host_machine->vmlinux_maps[MAP__FUNCTION]; dso = map->dso; vmlinux_name = symbol_conf.vmlinux_name; - if (vmlinux_name) { - if (dso__load_vmlinux(dso, map, vmlinux_name, false, NULL) <= 0) - return NULL; - } else { - if (dso__load_vmlinux_path(dso, map, NULL) <= 0) { - pr_debug("Failed to load kernel map.\n"); - return NULL; - } - } + dso->load_errno = 0; + if (vmlinux_name) + ret = dso__load_vmlinux(dso, map, vmlinux_name, false, NULL); + else + ret = dso__load_vmlinux_path(dso, map, NULL); found: - return dso; -} - -const char *kernel_get_module_path(const char *module) -{ - struct dso *dso = kernel_get_module_dso(module); - return (dso) ? dso->long_name : NULL; + *pdso = dso; + return ret; } static int convert_exec_to_group(const char *exec, char **result) @@ -389,16 +381,25 @@ static int get_alternative_line_range(struct debuginfo *dinfo, static struct debuginfo *open_debuginfo(const char *module, bool silent) { const char *path = module; - struct debuginfo *ret; + char reason[STRERR_BUFSIZE]; + struct debuginfo *ret = NULL; + struct dso *dso = NULL; + int err; if (!module || !strchr(module, '/')) { - path = kernel_get_module_path(module); - if (!path) { + err = kernel_get_module_dso(module, &dso); + if (err < 0) { + if (!dso || dso->load_errno == 0) { + if (!strerror_r(-err, reason, STRERR_BUFSIZE)) + strcpy(reason, "(unknown)"); + } else + dso__strerror_load(dso, reason, STRERR_BUFSIZE); if (!silent) - pr_err("Failed to find path of %s module.\n", - module ?: "kernel"); + pr_err("Failed to find the path for %s: %s\n", + module ?: "kernel", reason); return NULL; } + path = dso->long_name; } ret = debuginfo__new(path); if (!ret && !silent) { diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h index 537eb329c2cf..31db6ee7db54 100644 --- a/tools/perf/util/probe-event.h +++ b/tools/perf/util/probe-event.h @@ -131,9 +131,6 @@ extern void line_range__clear(struct line_range *lr); /* Initialize line range */ extern int line_range__init(struct line_range *lr); -/* Internal use: Return kernel/module path */ -extern const char *kernel_get_module_path(const char *module); - extern int add_perf_probe_events(struct perf_probe_event *pevs, int npevs); extern int del_perf_probe_events(struct strfilter *filter); extern int show_perf_probe_events(struct strfilter *filter); From dddc7ee32fa13efc66afa71ebd83bce545c8392a Mon Sep 17 00:00:00 2001 From: Masami Hiramatsu Date: Wed, 27 May 2015 17:37:25 +0900 Subject: [PATCH 25/25] perf probe: Fix an error when deleting probes successfully Fix a bug in del_perf_probe_events() which returns an error (-ENOENT) even if the probes are successfully deleted. This happens only if the probes are on user-apps and not on kernel, simply because it doesn't clear the previous error. So, without this fix, we get an error even though events are being successfully removed. ------ # ./perf probe -x ./perf del_perf_probe_events Added new event: probe_perf:del_perf_probe_events (on del_perf_probe_events in ... You can now use it in all perf tools, such as: perf record -e probe_perf:del_perf_probe_events -aR sleep 1 # ./perf probe -d \*:\* Removed event: probe_perf:del_perf_probe_events Error: Failed to delete events. ------ This fixes the above error. ------ # ./perf probe -d \*:\* Removed event: probe_perf:del_perf_probe_events ------ Reported-by: Arnaldo Carvalho de Melo Signed-off-by: Masami Hiramatsu Tested-by: Arnaldo Carvalho de Melo Cc: David Ahern Cc: Jiri Olsa Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Richard Weinberger Link: http://lkml.kernel.org/r/20150527083725.23880.45209.stgit@localhost.localdomain Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/probe-event.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c index f5be411bc69c..97da98481d89 100644 --- a/tools/perf/util/probe-event.c +++ b/tools/perf/util/probe-event.c @@ -2811,13 +2811,14 @@ int del_perf_probe_events(struct strfilter *filter) goto error; ret2 = del_trace_probe_events(ufd, filter, unamelist); - if (ret2 < 0 && ret2 != -ENOENT) + if (ret2 < 0 && ret2 != -ENOENT) { ret = ret2; - else if (ret == -ENOENT && ret2 == -ENOENT) { + goto error; + } + if (ret == -ENOENT && ret2 == -ENOENT) pr_debug("\"%s\" does not hit any event.\n", str); /* Note that this is silently ignored */ - ret = 0; - } + ret = 0; error: if (kfd >= 0) {