* [PATCH 0/3] perf tools: 3 fixes
@ 2015-11-13 9:48 Adrian Hunter
2015-11-13 9:48 ` [PATCH 1/3] perf tools: Fix dso lookup by long name and missing buildids Adrian Hunter
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Adrian Hunter @ 2015-11-13 9:48 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo; +Cc: Jiri Olsa, linux-kernel
Hi
Here are a 3 fixes. The patches apply cleanly to 4.3, although
the problems go back further than that.
Adrian Hunter (3):
perf tools: Fix dso lookup by long name and missing buildids
perf buildid-list: Requires ordered events
perf inject: Also re-pipe lost_samples event
tools/perf/builtin-inject.c | 1 +
tools/perf/util/build-id.c | 1 +
tools/perf/util/dso.c | 17 +++++++++++++++++
tools/perf/util/dso.h | 1 +
tools/perf/util/machine.c | 1 +
5 files changed, 21 insertions(+)
Regards
Adrian
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 1/3] perf tools: Fix dso lookup by long name and missing buildids 2015-11-13 9:48 [PATCH 0/3] perf tools: 3 fixes Adrian Hunter @ 2015-11-13 9:48 ` Adrian Hunter 2015-11-18 6:18 ` [tip:perf/urgent] perf symbols: " tip-bot for Adrian Hunter 2015-11-13 9:48 ` [PATCH 2/3] perf buildid-list: Requires ordered events Adrian Hunter ` (2 subsequent siblings) 3 siblings, 1 reply; 8+ messages in thread From: Adrian Hunter @ 2015-11-13 9:48 UTC (permalink / raw) To: Arnaldo Carvalho de Melo; +Cc: Jiri Olsa, linux-kernel Commit 4598a0a6d22f ("perf symbols: Improve DSO long names lookup speed with rbtree") Added a tree to lookup dsos by long name. That tree gets corrupted whenever a dso long name is changed because the tree is not updated. One effect of that is buildid-list does not work with the 'with-hits' option because dso lookup fails and results in two structs for the same dso. The first has the buildid but no hits, the second has hits but no buildid. e.g. Before: $ tools/perf/perf record ls arch certs CREDITS Documentation firmware include ipc Kconfig lib Makefile net REPORTING-BUGS scripts sound usr block COPYING crypto drivers fs init Kbuild kernel MAINTAINERS mm README samples security tools virt [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.012 MB perf.data (11 samples) ] $ tools/perf/perf buildid-list 574da826c66538a8d9060d393a8866289bd06005 [kernel.kallsyms] 30c94dc66a1fe95180c3d68d2b89e576d5ae213c /lib/x86_64-linux-gnu/libc-2.19.so $ tools/perf/perf buildid-list -H 574da826c66538a8d9060d393a8866289bd06005 [kernel.kallsyms] 0000000000000000000000000000000000000000 /lib/x86_64-linux-gnu/libc-2.19.so After: $ tools/perf/perf buildid-list -H 574da826c66538a8d9060d393a8866289bd06005 [kernel.kallsyms] 30c94dc66a1fe95180c3d68d2b89e576d5ae213c /lib/x86_64-linux-gnu/libc-2.19.so The fix is to record the root of the tree on the dso so that dso__set_long_name() can update the tree when the long name changes. Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> --- tools/perf/util/dso.c | 17 +++++++++++++++++ tools/perf/util/dso.h | 1 + tools/perf/util/machine.c | 1 + 3 files changed, 19 insertions(+) diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c index 7c0c08386a1d..425df5c86c9c 100644 --- a/tools/perf/util/dso.c +++ b/tools/perf/util/dso.c @@ -933,6 +933,7 @@ static struct dso *__dso__findlink_by_longname(struct rb_root *root, /* Add new node and rebalance tree */ rb_link_node(&dso->rb_node, parent, p); rb_insert_color(&dso->rb_node, root); + dso->root = root; } return NULL; } @@ -945,15 +946,30 @@ static inline struct dso *__dso__find_by_longname(struct rb_root *root, void dso__set_long_name(struct dso *dso, const char *name, bool name_allocated) { + struct rb_root *root = dso->root; + if (name == NULL) return; if (dso->long_name_allocated) free((char *)dso->long_name); + if (root) { + rb_erase(&dso->rb_node, root); + /* + * __dso__findlink_by_longname() isn't guaranteed to add it + * back, so a clean removal is required here. + */ + RB_CLEAR_NODE(&dso->rb_node); + dso->root = NULL; + } + dso->long_name = name; dso->long_name_len = strlen(name); dso->long_name_allocated = name_allocated; + + if (root) + __dso__findlink_by_longname(root, dso, NULL); } void dso__set_short_name(struct dso *dso, const char *name, bool name_allocated) @@ -1046,6 +1062,7 @@ struct dso *dso__new(const char *name) dso->kernel = DSO_TYPE_USER; dso->needs_swap = DSO_SWAP__UNSET; RB_CLEAR_NODE(&dso->rb_node); + dso->root = NULL; INIT_LIST_HEAD(&dso->node); INIT_LIST_HEAD(&dso->data.open_entry); pthread_mutex_init(&dso->lock, NULL); diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h index fc8db9c764ac..45ec4d0a50ed 100644 --- a/tools/perf/util/dso.h +++ b/tools/perf/util/dso.h @@ -135,6 +135,7 @@ struct dso { pthread_mutex_t lock; struct list_head node; struct rb_node rb_node; /* rbtree node sorted by long name */ + struct rb_root *root; /* root of rbtree that rb_node is in */ struct rb_root symbols[MAP__NR_TYPES]; struct rb_root symbol_names[MAP__NR_TYPES]; struct { diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c index 5ef90be2a249..8b303ff20289 100644 --- a/tools/perf/util/machine.c +++ b/tools/perf/util/machine.c @@ -91,6 +91,7 @@ static void dsos__purge(struct dsos *dsos) list_for_each_entry_safe(pos, n, &dsos->head, node) { RB_CLEAR_NODE(&pos->rb_node); + pos->root = NULL; list_del_init(&pos->node); dso__put(pos); } -- 1.9.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [tip:perf/urgent] perf symbols: Fix dso lookup by long name and missing buildids 2015-11-13 9:48 ` [PATCH 1/3] perf tools: Fix dso lookup by long name and missing buildids Adrian Hunter @ 2015-11-18 6:18 ` tip-bot for Adrian Hunter 0 siblings, 0 replies; 8+ messages in thread From: tip-bot for Adrian Hunter @ 2015-11-18 6:18 UTC (permalink / raw) To: linux-tip-commits Cc: scott.norton, mingo, namhyung, adrian.hunter, hpa, tglx, dzickus, doug.hatch, peterz, Waiman.Long, linux-kernel, acme, jolsa Commit-ID: e266a753bf51b2c3b46d0d230349662c35ac5629 Gitweb: http://git.kernel.org/tip/e266a753bf51b2c3b46d0d230349662c35ac5629 Author: Adrian Hunter <adrian.hunter@intel.com> AuthorDate: Fri, 13 Nov 2015 11:48:30 +0200 Committer: Arnaldo Carvalho de Melo <acme@redhat.com> CommitDate: Fri, 13 Nov 2015 11:14:36 -0300 perf symbols: Fix dso lookup by long name and missing buildids Commit 4598a0a6d22f ("perf symbols: Improve DSO long names lookup speed with rbtree") Added a tree to lookup dsos by long name. That tree gets corrupted whenever a dso long name is changed because the tree is not updated. One effect of that is buildid-list does not work with the 'with-hits' option because dso lookup fails and results in two structs for the same dso. The first has the buildid but no hits, the second has hits but no buildid. e.g. Before: $ tools/perf/perf record ls arch certs CREDITS Documentation firmware include ipc Kconfig lib Makefile net REPORTING-BUGS scripts sound usr block COPYING crypto drivers fs init Kbuild kernel MAINTAINERS mm README samples security tools virt [ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.012 MB perf.data (11 samples) ] $ tools/perf/perf buildid-list 574da826c66538a8d9060d393a8866289bd06005 [kernel.kallsyms] 30c94dc66a1fe95180c3d68d2b89e576d5ae213c /lib/x86_64-linux-gnu/libc-2.19.so $ tools/perf/perf buildid-list -H 574da826c66538a8d9060d393a8866289bd06005 [kernel.kallsyms] 0000000000000000000000000000000000000000 /lib/x86_64-linux-gnu/libc-2.19.so After: $ tools/perf/perf buildid-list -H 574da826c66538a8d9060d393a8866289bd06005 [kernel.kallsyms] 30c94dc66a1fe95180c3d68d2b89e576d5ae213c /lib/x86_64-linux-gnu/libc-2.19.so The fix is to record the root of the tree on the dso so that dso__set_long_name() can update the tree when the long name changes. Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com> Cc: Adrian Hunter <adrian.hunter@intel.com> Cc: Don Zickus <dzickus@redhat.com> Cc: Douglas Hatch <doug.hatch@hp.com> Cc: Jiri Olsa <jolsa@kernel.org> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Scott J Norton <scott.norton@hp.com> Cc: Waiman Long <Waiman.Long@hp.com> Fixes: 4598a0a6d22f ("perf symbols: Improve DSO long names lookup speed with rbtree") Link: http://lkml.kernel.org/r/1447408112-1920-2-git-send-email-adrian.hunter@intel.com Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/util/dso.c | 17 +++++++++++++++++ tools/perf/util/dso.h | 1 + tools/perf/util/machine.c | 1 + 3 files changed, 19 insertions(+) diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c index 7c0c083..425df5c 100644 --- a/tools/perf/util/dso.c +++ b/tools/perf/util/dso.c @@ -933,6 +933,7 @@ static struct dso *__dso__findlink_by_longname(struct rb_root *root, /* Add new node and rebalance tree */ rb_link_node(&dso->rb_node, parent, p); rb_insert_color(&dso->rb_node, root); + dso->root = root; } return NULL; } @@ -945,15 +946,30 @@ static inline struct dso *__dso__find_by_longname(struct rb_root *root, void dso__set_long_name(struct dso *dso, const char *name, bool name_allocated) { + struct rb_root *root = dso->root; + if (name == NULL) return; if (dso->long_name_allocated) free((char *)dso->long_name); + if (root) { + rb_erase(&dso->rb_node, root); + /* + * __dso__findlink_by_longname() isn't guaranteed to add it + * back, so a clean removal is required here. + */ + RB_CLEAR_NODE(&dso->rb_node); + dso->root = NULL; + } + dso->long_name = name; dso->long_name_len = strlen(name); dso->long_name_allocated = name_allocated; + + if (root) + __dso__findlink_by_longname(root, dso, NULL); } void dso__set_short_name(struct dso *dso, const char *name, bool name_allocated) @@ -1046,6 +1062,7 @@ struct dso *dso__new(const char *name) dso->kernel = DSO_TYPE_USER; dso->needs_swap = DSO_SWAP__UNSET; RB_CLEAR_NODE(&dso->rb_node); + dso->root = NULL; INIT_LIST_HEAD(&dso->node); INIT_LIST_HEAD(&dso->data.open_entry); pthread_mutex_init(&dso->lock, NULL); diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h index fc8db9c..45ec4d0 100644 --- a/tools/perf/util/dso.h +++ b/tools/perf/util/dso.h @@ -135,6 +135,7 @@ struct dso { pthread_mutex_t lock; struct list_head node; struct rb_node rb_node; /* rbtree node sorted by long name */ + struct rb_root *root; /* root of rbtree that rb_node is in */ struct rb_root symbols[MAP__NR_TYPES]; struct rb_root symbol_names[MAP__NR_TYPES]; struct { diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c index 5ef90be..8b303ff 100644 --- a/tools/perf/util/machine.c +++ b/tools/perf/util/machine.c @@ -91,6 +91,7 @@ static void dsos__purge(struct dsos *dsos) list_for_each_entry_safe(pos, n, &dsos->head, node) { RB_CLEAR_NODE(&pos->rb_node); + pos->root = NULL; list_del_init(&pos->node); dso__put(pos); } ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] perf buildid-list: Requires ordered events 2015-11-13 9:48 [PATCH 0/3] perf tools: 3 fixes Adrian Hunter 2015-11-13 9:48 ` [PATCH 1/3] perf tools: Fix dso lookup by long name and missing buildids Adrian Hunter @ 2015-11-13 9:48 ` Adrian Hunter 2015-11-18 6:19 ` [tip:perf/urgent] " tip-bot for Adrian Hunter 2015-11-13 9:48 ` [PATCH 3/3] perf inject: Also re-pipe lost_samples event Adrian Hunter 2015-11-13 15:24 ` [PATCH 0/3] perf tools: 3 fixes Arnaldo Carvalho de Melo 3 siblings, 1 reply; 8+ messages in thread From: Adrian Hunter @ 2015-11-13 9:48 UTC (permalink / raw) To: Arnaldo Carvalho de Melo; +Cc: Jiri Olsa, linux-kernel perf buildid-list processes events to determine hits (i.e. with-hits option). That may not work if events are not sorted in order. i.e. MMAP events must be processed before the samples that depend on them so that sample processing can 'hit' the DSO to which the MMAP refers. Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> --- tools/perf/util/build-id.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c index d909459fb54c..217b5a60e2ab 100644 --- a/tools/perf/util/build-id.c +++ b/tools/perf/util/build-id.c @@ -76,6 +76,7 @@ struct perf_tool build_id__mark_dso_hit_ops = { .exit = perf_event__exit_del_thread, .attr = perf_event__process_attr, .build_id = perf_event__process_build_id, + .ordered_events = true, }; int build_id__sprintf(const u8 *build_id, int len, char *bf) -- 1.9.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [tip:perf/urgent] perf buildid-list: Requires ordered events 2015-11-13 9:48 ` [PATCH 2/3] perf buildid-list: Requires ordered events Adrian Hunter @ 2015-11-18 6:19 ` tip-bot for Adrian Hunter 0 siblings, 0 replies; 8+ messages in thread From: tip-bot for Adrian Hunter @ 2015-11-18 6:19 UTC (permalink / raw) To: linux-tip-commits Cc: tglx, acme, hpa, linux-kernel, adrian.hunter, jolsa, mingo Commit-ID: 1216b65c502e0f130cc9984dfd5f9e1968c1eb46 Gitweb: http://git.kernel.org/tip/1216b65c502e0f130cc9984dfd5f9e1968c1eb46 Author: Adrian Hunter <adrian.hunter@intel.com> AuthorDate: Fri, 13 Nov 2015 11:48:31 +0200 Committer: Arnaldo Carvalho de Melo <acme@redhat.com> CommitDate: Fri, 13 Nov 2015 12:22:04 -0300 perf buildid-list: Requires ordered events 'perf buildid-list' processes events to determine hits (i.e. with-hits option). That may not work if events are not sorted in order. i.e. MMAP events must be processed before the samples that depend on them so that sample processing can 'hit' the DSO to which the MMAP refers. Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> Cc: Jiri Olsa <jolsa@redhat.com> Link: http://lkml.kernel.org/r/1447408112-1920-3-git-send-email-adrian.hunter@intel.com Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/util/build-id.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c index d909459..217b5a6 100644 --- a/tools/perf/util/build-id.c +++ b/tools/perf/util/build-id.c @@ -76,6 +76,7 @@ struct perf_tool build_id__mark_dso_hit_ops = { .exit = perf_event__exit_del_thread, .attr = perf_event__process_attr, .build_id = perf_event__process_build_id, + .ordered_events = true, }; int build_id__sprintf(const u8 *build_id, int len, char *bf) ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] perf inject: Also re-pipe lost_samples event 2015-11-13 9:48 [PATCH 0/3] perf tools: 3 fixes Adrian Hunter 2015-11-13 9:48 ` [PATCH 1/3] perf tools: Fix dso lookup by long name and missing buildids Adrian Hunter 2015-11-13 9:48 ` [PATCH 2/3] perf buildid-list: Requires ordered events Adrian Hunter @ 2015-11-13 9:48 ` Adrian Hunter 2015-11-18 6:19 ` [tip:perf/urgent] " tip-bot for Adrian Hunter 2015-11-13 15:24 ` [PATCH 0/3] perf tools: 3 fixes Arnaldo Carvalho de Melo 3 siblings, 1 reply; 8+ messages in thread From: Adrian Hunter @ 2015-11-13 9:48 UTC (permalink / raw) To: Arnaldo Carvalho de Melo; +Cc: Jiri Olsa, linux-kernel perf inject must re-pipe all events otherwise they get dropped from the output file. Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> --- tools/perf/builtin-inject.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c index 0a945d2e8ca5..99d127fe9c35 100644 --- a/tools/perf/builtin-inject.c +++ b/tools/perf/builtin-inject.c @@ -675,6 +675,7 @@ int cmd_inject(int argc, const char **argv, const char *prefix __maybe_unused) .fork = perf_event__repipe, .exit = perf_event__repipe, .lost = perf_event__repipe, + .lost_samples = perf_event__repipe, .aux = perf_event__repipe, .itrace_start = perf_event__repipe, .context_switch = perf_event__repipe, -- 1.9.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [tip:perf/urgent] perf inject: Also re-pipe lost_samples event 2015-11-13 9:48 ` [PATCH 3/3] perf inject: Also re-pipe lost_samples event Adrian Hunter @ 2015-11-18 6:19 ` tip-bot for Adrian Hunter 0 siblings, 0 replies; 8+ messages in thread From: tip-bot for Adrian Hunter @ 2015-11-18 6:19 UTC (permalink / raw) To: linux-tip-commits Cc: linux-kernel, tglx, hpa, jolsa, adrian.hunter, acme, mingo Commit-ID: d8145b3e30a24280c396d88c8703c50a1ea0aa3a Gitweb: http://git.kernel.org/tip/d8145b3e30a24280c396d88c8703c50a1ea0aa3a Author: Adrian Hunter <adrian.hunter@intel.com> AuthorDate: Fri, 13 Nov 2015 11:48:32 +0200 Committer: Arnaldo Carvalho de Melo <acme@redhat.com> CommitDate: Fri, 13 Nov 2015 12:23:12 -0300 perf inject: Also re-pipe lost_samples event perf inject must re-pipe all events otherwise they get dropped from the output file. Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> Cc: Jiri Olsa <jolsa@redhat.com> Link: http://lkml.kernel.org/r/1447408112-1920-4-git-send-email-adrian.hunter@intel.com Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/builtin-inject.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c index 0a945d2..99d127f 100644 --- a/tools/perf/builtin-inject.c +++ b/tools/perf/builtin-inject.c @@ -675,6 +675,7 @@ int cmd_inject(int argc, const char **argv, const char *prefix __maybe_unused) .fork = perf_event__repipe, .exit = perf_event__repipe, .lost = perf_event__repipe, + .lost_samples = perf_event__repipe, .aux = perf_event__repipe, .itrace_start = perf_event__repipe, .context_switch = perf_event__repipe, ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 0/3] perf tools: 3 fixes 2015-11-13 9:48 [PATCH 0/3] perf tools: 3 fixes Adrian Hunter ` (2 preceding siblings ...) 2015-11-13 9:48 ` [PATCH 3/3] perf inject: Also re-pipe lost_samples event Adrian Hunter @ 2015-11-13 15:24 ` Arnaldo Carvalho de Melo 3 siblings, 0 replies; 8+ messages in thread From: Arnaldo Carvalho de Melo @ 2015-11-13 15:24 UTC (permalink / raw) To: Adrian Hunter; +Cc: Jiri Olsa, linux-kernel Em Fri, Nov 13, 2015 at 11:48:29AM +0200, Adrian Hunter escreveu: > Hi > > Here are a 3 fixes. The patches apply cleanly to 4.3, although > the problems go back further than that. Thanks for the patches and for the before/after for the rbtree problem, I reproduced it here, fixes the problem! - Arnaldo > > Adrian Hunter (3): > perf tools: Fix dso lookup by long name and missing buildids > perf buildid-list: Requires ordered events > perf inject: Also re-pipe lost_samples event > > tools/perf/builtin-inject.c | 1 + > tools/perf/util/build-id.c | 1 + > tools/perf/util/dso.c | 17 +++++++++++++++++ > tools/perf/util/dso.h | 1 + > tools/perf/util/machine.c | 1 + > 5 files changed, 21 insertions(+) > > > Regards > Adrian ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-11-18 6:21 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-11-13 9:48 [PATCH 0/3] perf tools: 3 fixes Adrian Hunter 2015-11-13 9:48 ` [PATCH 1/3] perf tools: Fix dso lookup by long name and missing buildids Adrian Hunter 2015-11-18 6:18 ` [tip:perf/urgent] perf symbols: " tip-bot for Adrian Hunter 2015-11-13 9:48 ` [PATCH 2/3] perf buildid-list: Requires ordered events Adrian Hunter 2015-11-18 6:19 ` [tip:perf/urgent] " tip-bot for Adrian Hunter 2015-11-13 9:48 ` [PATCH 3/3] perf inject: Also re-pipe lost_samples event Adrian Hunter 2015-11-18 6:19 ` [tip:perf/urgent] " tip-bot for Adrian Hunter 2015-11-13 15:24 ` [PATCH 0/3] perf tools: 3 fixes Arnaldo Carvalho de Melo
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.