public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Adrian Hunter <adrian.hunter@intel.com>
To: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>, linux-kernel@vger.kernel.org
Subject: [PATCH 1/3] perf tools: Fix dso lookup by long name and missing buildids
Date: Fri, 13 Nov 2015 11:48:30 +0200	[thread overview]
Message-ID: <1447408112-1920-2-git-send-email-adrian.hunter@intel.com> (raw)
In-Reply-To: <1447408112-1920-1-git-send-email-adrian.hunter@intel.com>

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


  reply	other threads:[~2015-11-13  9:53 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-13  9:48 [PATCH 0/3] perf tools: 3 fixes Adrian Hunter
2015-11-13  9:48 ` Adrian Hunter [this message]
2015-11-18  6:18   ` [tip:perf/urgent] perf symbols: Fix dso lookup by long name and missing buildids 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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1447408112-1920-2-git-send-email-adrian.hunter@intel.com \
    --to=adrian.hunter@intel.com \
    --cc=acme@kernel.org \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox