All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] perf symbols: Fix comparision of build_ids
@ 2009-11-18 22:20 Arnaldo Carvalho de Melo
  2009-11-18 22:20 ` [PATCH 2/4] perf symbols: Kill struct build_id_list and die() another day Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2009-11-18 22:20 UTC (permalink / raw
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo,
	Frédéric Weisbecker, Mike Galbraith, Peter Zijlstra,
	Paul Mackerras

From: Arnaldo Carvalho de Melo <acme@redhat.com>

When we read the build_id from the DSO name to then index into
/usr/lib/debug/.buildid/DSO_BUILD_ID[0:2]/DSO_BUILD_ID[2:], we were
jumping directly to the comparision with the buildid we already have in
dso->build_id (that came from the perf.data build_id section, collected
at perf record time) unconditionally, even if we didn't had recorded it,
and furthermore, comparing a formatted buildid with a rawbuildid, yikes.

Fix it by deleting the dso__read_build_id function, that was really
misdesigned anyway, and do the necessary checks and correct comparision
of raw buildids.

Cc: Frédéric Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol.c |   52 ++++++++++++++-------------------------------
 1 files changed, 16 insertions(+), 36 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 5cc96c8..594f36a 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -962,25 +962,6 @@ out:
 	return err;
 }
 
-static char *dso__read_build_id(struct dso *self)
-{
-	int len;
-	char *build_id = NULL;
-	unsigned char rawbf[BUILD_ID_SIZE];
-
-	len = filename__read_build_id(self->long_name, rawbf, sizeof(rawbf));
-	if (len < 0)
-		goto out;
-
-	build_id = malloc(len * 2 + 1);
-	if (build_id == NULL)
-		goto out;
-
-	build_id__sprintf(rawbf, len, build_id);
-out:
-	return build_id;
-}
-
 char dso__symtab_origin(const struct dso *self)
 {
 	static const char origin[] = {
@@ -1001,7 +982,8 @@ char dso__symtab_origin(const struct dso *self)
 int dso__load(struct dso *self, struct map *map, symbol_filter_t filter)
 {
 	int size = PATH_MAX;
-	char *name = malloc(size), *build_id = NULL;
+	char *name = malloc(size);
+	u8 build_id[BUILD_ID_SIZE];
 	int ret = -1;
 	int fd;
 
@@ -1023,8 +1005,6 @@ int dso__load(struct dso *self, struct map *map, symbol_filter_t filter)
 
 more:
 	do {
-		int berr = 0;
-
 		self->origin++;
 		switch (self->origin) {
 		case DSO__ORIG_FEDORA:
@@ -1036,12 +1016,18 @@ more:
 				 self->long_name);
 			break;
 		case DSO__ORIG_BUILDID:
-			build_id = dso__read_build_id(self);
-			if (build_id != NULL) {
+			if (filename__read_build_id(self->long_name, build_id,
+						    sizeof(build_id))) {
+				char build_id_hex[BUILD_ID_SIZE * 2 + 1];
+
+				build_id__sprintf(build_id, sizeof(build_id),
+						  build_id_hex);
 				snprintf(name, size,
 					 "/usr/lib/debug/.build-id/%.2s/%s.debug",
-					build_id, build_id + 2);
-				goto compare_build_id;
+					build_id_hex, build_id_hex + 2);
+				if (self->has_build_id)
+					goto compare_build_id;
+				break;
 			}
 			self->origin++;
 			/* Fall thru */
@@ -1054,18 +1040,12 @@ more:
 		}
 
 		if (self->has_build_id) {
-			bool match;
-			build_id = malloc(BUILD_ID_SIZE);
-			if (build_id == NULL)
+			if (filename__read_build_id(name, build_id,
+						    sizeof(build_id)) < 0)
 				goto more;
-			berr = filename__read_build_id(name, build_id,
-						       BUILD_ID_SIZE);
 compare_build_id:
-			match = berr > 0 && memcmp(build_id, self->build_id,
-						   sizeof(self->build_id)) == 0;
-			free(build_id);
-			build_id = NULL;
-			if (!match)
+			if (memcmp(build_id, self->build_id,
+				   sizeof(self->build_id)) != 0)
 				goto more;
 		}
 
-- 
1.6.2.5


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/4] perf symbols: Kill struct build_id_list and die() another day
  2009-11-18 22:20 [PATCH 1/4] perf symbols: Fix comparision of build_ids Arnaldo Carvalho de Melo
@ 2009-11-18 22:20 ` Arnaldo Carvalho de Melo
  2009-11-18 22:20   ` [PATCH 3/4] perf symbols: Record the build_ids of kernel modules too Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2009-11-18 22:20 UTC (permalink / raw
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo,
	Frédéric Weisbecker, Mike Galbraith, Peter Zijlstra,
	Paul Mackerras

From: Arnaldo Carvalho de Melo <acme@redhat.com>

No need for this struct and its allocations, we can just use the
->build_id member we already have in struct dso, then ask for it to be
read, and later traverse the dsos list, writing the buildid table to the
perf.data file.

As a bonus, one more die() function got killed.

Cc: Frédéric Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/event.h  |    7 -------
 tools/perf/util/header.c |   31 ++++++++++++++++++-------------
 tools/perf/util/symbol.c |   37 +++++++++----------------------------
 tools/perf/util/symbol.h |    2 +-
 4 files changed, 28 insertions(+), 49 deletions(-)

diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index 1f771ce..34c6fcb 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -69,13 +69,6 @@ struct build_id_event {
 	char			 filename[];
 };
 
-struct build_id_list {
-	struct build_id_event	event;
-	struct list_head	list;
-	const char		*dso_name;
-	int			len;
-};
-
 typedef union event_union {
 	struct perf_event_header	header;
 	struct ip_event			ip;
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index b01a953..31731f1 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -176,18 +176,24 @@ static int do_write(int fd, const void *buf, size_t size)
 	return 0;
 }
 
-static int write_buildid_table(int fd, struct list_head *id_head)
+static int dsos__write_buildid_table(int fd)
 {
-	struct build_id_list *iter, *next;
-
-	list_for_each_entry_safe(iter, next, id_head, list) {
-		struct build_id_event *b = &iter->event;
-
-		if (do_write(fd, b, sizeof(*b)) < 0 ||
-		    do_write(fd, iter->dso_name, iter->len) < 0)
+	struct dso *pos;
+
+	list_for_each_entry(pos, &dsos, node) {
+		struct build_id_event b;
+		size_t len;
+
+		if (!pos->has_build_id)
+			continue;
+		len = pos->long_name_len + 1;
+		len = ALIGN(len, 64);
+		memset(&b, 0, sizeof(b));
+		memcpy(&b.build_id, pos->build_id, sizeof(pos->build_id));
+		b.header.size = sizeof(b) + len;
+		if (do_write(fd, &b, sizeof(b)) < 0 ||
+		    do_write(fd, pos->long_name, len) < 0)
 			return -1;
-		list_del(&iter->list);
-		free(iter);
 	}
 
 	return 0;
@@ -196,14 +202,13 @@ static int write_buildid_table(int fd, struct list_head *id_head)
 static void
 perf_header__adds_write(struct perf_header *self, int fd)
 {
-	LIST_HEAD(id_list);
 	int nr_sections;
 	struct perf_file_section *feat_sec;
 	int sec_size;
 	u64 sec_start;
 	int idx = 0;
 
-	if (fetch_build_id_table(&id_list))
+	if (dsos__read_build_ids())
 		perf_header__set_feat(self, HEADER_BUILD_ID);
 
 	nr_sections = bitmap_weight(self->adds_features, HEADER_FEAT_BITS);
@@ -238,7 +243,7 @@ perf_header__adds_write(struct perf_header *self, int fd)
 
 		/* Write build-ids */
 		buildid_sec->offset = lseek(fd, 0, SEEK_CUR);
-		if (write_buildid_table(fd, &id_list) < 0)
+		if (dsos__write_buildid_table(fd) < 0)
 			die("failed to write buildid table");
 		buildid_sec->size = lseek(fd, 0, SEEK_CUR) - buildid_sec->offset;
 	}
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 594f36a..946ec31 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -883,38 +883,19 @@ out_close:
 	return err;
 }
 
-bool fetch_build_id_table(struct list_head *head)
+bool dsos__read_build_ids(void)
 {
-	bool have_buildid = false;
+	bool have_build_id = false;
 	struct dso *pos;
 
-	list_for_each_entry(pos, &dsos, node) {
-		struct build_id_list *new;
-		struct build_id_event b;
-		size_t len;
-
-		if (filename__read_build_id(pos->long_name,
-					    &b.build_id,
-					    sizeof(b.build_id)) < 0)
-			continue;
-		have_buildid = true;
-		memset(&b.header, 0, sizeof(b.header));
-		len = pos->long_name_len + 1;
-		len = ALIGN(len, 64);
-		b.header.size = sizeof(b) + len;
-
-		new = malloc(sizeof(*new));
-		if (!new)
-			die("No memory\n");
-
-		memcpy(&new->event, &b, sizeof(b));
-		new->dso_name = pos->long_name;
-		new->len = len;
-
-		list_add_tail(&new->list, head);
-	}
+	list_for_each_entry(pos, &dsos, node)
+		if (filename__read_build_id(pos->long_name, pos->build_id,
+					    sizeof(pos->build_id)) > 0) {
+			have_build_id	  = true;
+			pos->has_build_id = true;
+		}
 
-	return have_buildid;
+	return have_build_id;
 }
 
 int filename__read_build_id(const char *filename, void *bf, size_t size)
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 5ad1019..546eb76 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -89,7 +89,7 @@ char dso__symtab_origin(const struct dso *self);
 void dso__set_build_id(struct dso *self, void *build_id);
 
 int filename__read_build_id(const char *filename, void *bf, size_t size);
-bool fetch_build_id_table(struct list_head *head);
+bool dsos__read_build_ids(void);
 int build_id__sprintf(u8 *self, int len, char *bf);
 
 int load_kernel(symbol_filter_t filter);
-- 
1.6.2.5


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/4] perf symbols: Record the build_ids of kernel modules too
  2009-11-18 22:20 ` [PATCH 2/4] perf symbols: Kill struct build_id_list and die() another day Arnaldo Carvalho de Melo
@ 2009-11-18 22:20   ` Arnaldo Carvalho de Melo
  2009-11-18 22:20     ` [PATCH 4/4] perf symbols: Capture the running kernel buildid too Arnaldo Carvalho de Melo
  2009-11-18 22:29     ` [PATCH 3/4] perf symbols: Record the build_ids of kernel modules too Roland McGrath
  0 siblings, 2 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2009-11-18 22:20 UTC (permalink / raw
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Roland McGrath,
	Frédéric Weisbecker, Mike Galbraith, Peter Zijlstra,
	Paul Mackerras

From: Arnaldo Carvalho de Melo <acme@redhat.com>

[root@doppio linux-2.6-tip]# perf record -a sleep 2s;perf buildid-list|tail
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.162 MB perf.data (~7078 samples) ]
881588fa57b3c1696bc91e5e804a11304f093535 [cfg80211]
4d47ce1da9d16bad00c962c072451b7c681e82df [snd_page_alloc]
5146377e89a7caac617f9782f1a02e46263d3a31 [rfkill]
2153b937bff0d345fea83b63a2e1d3138569f83d [i915]
4e6fb1bb97362e3ee4d306988b9ad6912d5fb9ae [drm_kms_helper]
f56ef2bf853e3a798f0d8d51f797622e5dc4420e [drm]
b0d157a3b5c4e017329ffc07c64623cd6ad65e95 [i2c_algo_bit]
8125374b905ef9fa8b65d98e166b008ad952f198 [i2c_core]
fc875c6e5a90e7b915e9d445d0efc859e1b2678c [video]
4b43c5006589f977e9762fdfc7ac1a92b72fca52 [output]
[root@doppio linux-2.6-tip]#

elfutils libdwfl/linux-kernel-modules.c was used as reference, as
suggested by Roland McGrath.

Cc: Roland McGrath <roland@redhat.com>
Cc: Frédéric Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c |    5 ++++
 tools/perf/util/symbol.c |   53 +++++++++++++++++++++++++++++++++++++++++++++-
 tools/perf/util/symbol.h |    2 +
 3 files changed, 59 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 31731f1..d3d656f 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -241,6 +241,11 @@ perf_header__adds_write(struct perf_header *self, int fd)
 
 		buildid_sec = &feat_sec[idx++];
 
+		/*
+		 * Read the list of loaded modules with its build_ids
+		 */
+		dsos__load_modules();
+
 		/* Write build-ids */
 		buildid_sec->offset = lseek(fd, 0, SEEK_CUR);
 		if (dsos__write_buildid_table(fd) < 0)
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 946ec31..7b4cede 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -9,6 +9,7 @@
 #include <libelf.h>
 #include <gelf.h>
 #include <elf.h>
+#include <limits.h>
 #include <sys/utsname.h>
 
 enum dso_origin {
@@ -943,6 +944,50 @@ out:
 	return err;
 }
 
+int sysfs__read_build_id(const char *filename, void *build_id, size_t size)
+{
+	int fd, err = -1;
+
+	if (size < BUILD_ID_SIZE)
+		goto out;
+
+	fd = open(filename, O_RDONLY);
+	if (fd < 0)
+		goto out;
+
+	while (1) {
+		char bf[BUFSIZ];
+		GElf_Nhdr nhdr;
+		int namesz, descsz;
+
+		if (read(fd, &nhdr, sizeof(nhdr)) != sizeof(nhdr))
+			break;
+
+		namesz = (nhdr.n_namesz + 3) & -4U;
+		descsz = (nhdr.n_descsz + 3) & -4U;
+		if (nhdr.n_type == NT_GNU_BUILD_ID &&
+		    nhdr.n_namesz == sizeof("GNU")) {
+			if (read(fd, bf, namesz) != namesz)
+				break;
+			if (memcmp(bf, "GNU", sizeof("GNU")) == 0) {
+				if (read(fd, build_id,
+				    BUILD_ID_SIZE) == BUILD_ID_SIZE) {
+					err = 0;
+					break;
+				}
+			} else if (read(fd, bf, descsz) != descsz)
+				break;
+		} else {
+			int n = namesz + descsz;
+			if (read(fd, bf, n) != n)
+				break;
+		}
+	}
+	close(fd);
+out:
+	return err;
+}
+
 char dso__symtab_origin(const struct dso *self)
 {
 	static const char origin[] = {
@@ -1218,7 +1263,7 @@ static struct map *map__new2(u64 start, struct dso *dso)
 	return self;
 }
 
-static int dsos__load_modules(void)
+int dsos__load_modules(void)
 {
 	char *line = NULL;
 	size_t n;
@@ -1268,6 +1313,12 @@ static int dsos__load_modules(void)
 			goto out_delete_line;
 		}
 
+		snprintf(name, sizeof(name),
+			 "/sys/module/%s/notes/.note.gnu.build-id", line);
+		if (sysfs__read_build_id(name, dso->build_id,
+					 sizeof(dso->build_id)) == 0)
+			dso->has_build_id = true;
+
 		dso->origin = DSO__ORIG_KMODULE;
 		kernel_maps__insert(map);
 		dsos__add(dso);
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 546eb76..da7ec1a 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -78,6 +78,7 @@ void dso__delete(struct dso *self);
 struct symbol *dso__find_symbol(struct dso *self, u64 ip);
 
 int dsos__load_kernel(const char *vmlinux, symbol_filter_t filter, int modules);
+int dsos__load_modules(void);
 struct dso *dsos__findnew(const char *name);
 int dso__load(struct dso *self, struct map *map, symbol_filter_t filter);
 void dsos__fprintf(FILE *fp);
@@ -89,6 +90,7 @@ char dso__symtab_origin(const struct dso *self);
 void dso__set_build_id(struct dso *self, void *build_id);
 
 int filename__read_build_id(const char *filename, void *bf, size_t size);
+int sysfs__read_build_id(const char *filename, void *bf, size_t size);
 bool dsos__read_build_ids(void);
 int build_id__sprintf(u8 *self, int len, char *bf);
 
-- 
1.6.2.5


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 4/4] perf symbols: Capture the running kernel buildid too
  2009-11-18 22:20   ` [PATCH 3/4] perf symbols: Record the build_ids of kernel modules too Arnaldo Carvalho de Melo
@ 2009-11-18 22:20     ` Arnaldo Carvalho de Melo
  2009-11-18 22:29       ` Arnaldo Carvalho de Melo
  2009-11-18 22:29     ` [PATCH 3/4] perf symbols: Record the build_ids of kernel modules too Roland McGrath
  1 sibling, 1 reply; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2009-11-18 22:20 UTC (permalink / raw
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Roland McGrath,
	Frédéric Weisbecker, Mike Galbraith, Peter Zijlstra,
	Paul Mackerras

From: Arnaldo Carvalho de Melo <acme@redhat.com>

[root@doppio linux-2.6-tip]# perf record -a -f sleep 3s ; perf
buildid-list | grep vmlinux
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.171 MB perf.data (~7489 samples) ]
18e7cc53db62a7d35e9d6f6c9ddc23017d38ee9a vmlinux
[root@doppio linux-2.6-tip]#

Several refactorings were needed so that we can have simmetry between
dsos__load_modules and dsos__load_kernel, i.e. those functions will
respectively create and add to the dsos list the loaded modules and
kernel, with its buildids, but not load its symbols. That is something
the subcomands that need will have to call dso__load_kernel_sym(), just
like we do with modules with dsos__load_module_sym()/dso__load_module_sym().

Nexts csets will actually use this info to stop producing bogus results
using mismatched vmlinux and .ko files.

Yeah, we need to close some races here and there, but aside from evil
corner cases (that you should know and avoid, of course), this is good
enough till we do the right thing :-)

Cc: Roland McGrath <roland@redhat.com>
Cc: Frédéric Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-top.c |    7 ++++-
 tools/perf/util/header.c |    1 +
 tools/perf/util/symbol.c |   65 ++++++++++++++++++++++++++-------------------
 tools/perf/util/symbol.h |    3 +-
 4 files changed, 46 insertions(+), 30 deletions(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 07b92c3..6d770ac 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -948,7 +948,12 @@ static int symbol_filter(struct map *map, struct symbol *sym)
 
 static int parse_symbols(void)
 {
-	if (dsos__load_kernel(vmlinux_name, symbol_filter, 1) <= 0)
+	struct dso *kernel = dsos__load_kernel();
+
+	if (kernel == NULL)
+		return -1;
+
+	if (dso__load_kernel_sym(kernel, symbol_filter, 1) <= 0)
 		return -1;
 
 	if (dump_symtab)
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index d3d656f..425a29b 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -241,6 +241,7 @@ perf_header__adds_write(struct perf_header *self, int fd)
 
 		buildid_sec = &feat_sec[idx++];
 
+		dsos__load_kernel();
 		/*
 		 * Read the list of loaded modules with its build_ids
 		 */
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 7b4cede..4d75e74 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1352,17 +1352,11 @@ static int dso__load_vmlinux(struct dso *self, struct map *map,
 	return err;
 }
 
-int dsos__load_kernel(const char *vmlinux, symbol_filter_t filter,
-		      int use_modules)
+int dso__load_kernel_sym(struct dso *self, symbol_filter_t filter, int use_modules)
 {
 	int err = -1;
-	struct dso *dso = dso__new(vmlinux);
 
-	if (dso == NULL)
-		return -1;
-
-	dso->short_name = "[kernel]";
-	kernel_map = map__new2(0, dso);
+	kernel_map = map__new2(0, self);
 	if (kernel_map == NULL)
 		goto out_delete_dso;
 
@@ -1374,39 +1368,36 @@ int dsos__load_kernel(const char *vmlinux, symbol_filter_t filter,
 		use_modules = 0;
 	}
 
-	if (vmlinux) {
-		err = dso__load_vmlinux(dso, kernel_map, vmlinux, filter);
-		if (err > 0 && use_modules) {
-			int syms = dsos__load_modules_sym(filter);
+	err = dso__load_vmlinux(self, kernel_map, self->name, filter);
+	if (err > 0 && use_modules) {
+		int syms = dsos__load_modules_sym(filter);
 
-			if (syms < 0)
-				pr_warning("Failed to read module symbols!"
-					   " Continuing...\n");
-			else
-				err += syms;
-		}
+		if (syms < 0)
+			pr_warning("Failed to read module symbols!"
+				   " Continuing...\n");
+		else
+			err += syms;
 	}
 
 	if (err <= 0)
 		err = kernel_maps__load_kallsyms(filter, use_modules);
 
 	if (err > 0) {
-		struct rb_node *node = rb_first(&dso->syms);
+		struct rb_node *node = rb_first(&self->syms);
 		struct symbol *sym = rb_entry(node, struct symbol, rb_node);
 
 		kernel_map->start = sym->start;
-		node = rb_last(&dso->syms);
+		node = rb_last(&self->syms);
 		sym = rb_entry(node, struct symbol, rb_node);
 		kernel_map->end = sym->end;
 
-		dso->origin = DSO__ORIG_KERNEL;
+		self->origin = DSO__ORIG_KERNEL;
 		kernel_maps__insert(kernel_map);
 		/*
 		 * Now that we have all sorted out, just set the ->end of all
 		 * maps:
 		 */
 		kernel_maps__fixup_end();
-		dsos__add(dso);
 
 		if (verbose)
 			kernel_maps__fprintf(stderr);
@@ -1415,7 +1406,7 @@ int dsos__load_kernel(const char *vmlinux, symbol_filter_t filter,
 	return err;
 
 out_delete_dso:
-	dso__delete(dso);
+	dso__delete(self);
 	return -1;
 }
 
@@ -1475,18 +1466,36 @@ size_t dsos__fprintf_buildid(FILE *fp)
 	return ret;
 }
 
-int load_kernel(symbol_filter_t filter)
+struct dso *dsos__load_kernel(void)
 {
-	if (dsos__load_kernel(vmlinux_name, filter, modules) <= 0)
-		return -1;
+	struct dso *kernel = dso__new(vmlinux_name);
 
+	if (kernel == NULL)
+		return NULL;
+
+	kernel->short_name = "[kernel]";
 	vdso = dso__new("[vdso]");
 	if (!vdso)
-		return -1;
+		return NULL;
+
+	if (sysfs__read_build_id("/sys/kernel/notes", kernel->build_id,
+				 sizeof(kernel->build_id)) == 0)
+		kernel->has_build_id = true;
 
+	dsos__add(kernel);
 	dsos__add(vdso);
 
-	return 0;
+	return kernel;
+}
+
+int load_kernel(symbol_filter_t filter)
+{
+	struct dso *kernel = dsos__load_kernel();
+
+	if (kernel == NULL)
+		return -1;
+
+	return dso__load_kernel_sym(kernel, filter, modules);
 }
 
 void symbol__init(unsigned int priv_size)
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index da7ec1a..f0593a6 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -77,10 +77,10 @@ void dso__delete(struct dso *self);
 
 struct symbol *dso__find_symbol(struct dso *self, u64 ip);
 
-int dsos__load_kernel(const char *vmlinux, symbol_filter_t filter, int modules);
 int dsos__load_modules(void);
 struct dso *dsos__findnew(const char *name);
 int dso__load(struct dso *self, struct map *map, symbol_filter_t filter);
+int dso__load_kernel_sym(struct dso *self, symbol_filter_t filter, int modules);
 void dsos__fprintf(FILE *fp);
 size_t dsos__fprintf_buildid(FILE *fp);
 
@@ -94,6 +94,7 @@ int sysfs__read_build_id(const char *filename, void *bf, size_t size);
 bool dsos__read_build_ids(void);
 int build_id__sprintf(u8 *self, int len, char *bf);
 
+struct dso *dsos__load_kernel(void);
 int load_kernel(symbol_filter_t filter);
 
 void symbol__init(unsigned int priv_size);
-- 
1.6.2.5


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/4] perf symbols: Record the build_ids of kernel modules too
  2009-11-18 22:20   ` [PATCH 3/4] perf symbols: Record the build_ids of kernel modules too Arnaldo Carvalho de Melo
  2009-11-18 22:20     ` [PATCH 4/4] perf symbols: Capture the running kernel buildid too Arnaldo Carvalho de Melo
@ 2009-11-18 22:29     ` Roland McGrath
  2009-11-18 22:33       ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 7+ messages in thread
From: Roland McGrath @ 2009-11-18 22:29 UTC (permalink / raw
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, linux-kernel, Arnaldo Carvalho de Melo,
	Frédéric Weisbecker, Mike Galbraith, Peter Zijlstra,
	Paul Mackerras

> +		snprintf(name, sizeof(name),
> +			 "/sys/module/%s/notes/.note.gnu.build-id", line);

The arch linker script for .ko's is free to fold notes into a single
section with a different name or suchlike, and maybe future ones will.  So
it's not pedantically kosher to assume the name of the section rather than
just looking at all the /sys/module/%s/notes/* files present until you find
NT_GNU_BUILD_ID.  (In practice there is probably only ever one file there
to look at, but you never know.)


Thanks,
Roland

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 4/4] perf symbols: Capture the running kernel buildid too
  2009-11-18 22:20     ` [PATCH 4/4] perf symbols: Capture the running kernel buildid too Arnaldo Carvalho de Melo
@ 2009-11-18 22:29       ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2009-11-18 22:29 UTC (permalink / raw
  To: Ingo Molnar
  Cc: linux-kernel, Roland McGrath, Frédéric Weisbecker,
	Mike Galbraith, Peter Zijlstra, Paul Mackerras

Em Wed, Nov 18, 2009 at 08:20:53PM -0200, Arnaldo Carvalho de Melo escreveu:
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> [root@doppio linux-2.6-tip]# perf record -a -f sleep 3s ; perf
> buildid-list | grep vmlinux
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.171 MB perf.data (~7489 samples) ]
> 18e7cc53db62a7d35e9d6f6c9ddc23017d38ee9a vmlinux
> [root@doppio linux-2.6-tip]#
> 
> Several refactorings were needed so that we can have simmetry between
> dsos__load_modules and dsos__load_kernel, i.e. those functions will
> respectively create and add to the dsos list the loaded modules and
> kernel, with its buildids, but not load its symbols. That is something
> the subcomands that need will have to call dso__load_kernel_sym(), just
> like we do with modules with dsos__load_module_sym()/dso__load_module_sym().

And that can even be deferred to when we get a kernel sample, thinking
about 'perf top --hide_kernel_symbols', this makes a lot of sense 8-)

Tomorrow tho.

- Arnaldo

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/4] perf symbols: Record the build_ids of kernel modules too
  2009-11-18 22:29     ` [PATCH 3/4] perf symbols: Record the build_ids of kernel modules too Roland McGrath
@ 2009-11-18 22:33       ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2009-11-18 22:33 UTC (permalink / raw
  To: Roland McGrath
  Cc: Ingo Molnar, linux-kernel, Frédéric Weisbecker,
	Mike Galbraith, Peter Zijlstra, Paul Mackerras

Em Wed, Nov 18, 2009 at 02:29:10PM -0800, Roland McGrath escreveu:
> > +		snprintf(name, sizeof(name),
> > +			 "/sys/module/%s/notes/.note.gnu.build-id", line);
> 
> The arch linker script for .ko's is free to fold notes into a single
> section with a different name or suchlike, and maybe future ones will.  So
> it's not pedantically kosher to assume the name of the section rather than
> just looking at all the /sys/module/%s/notes/* files present until you find
> NT_GNU_BUILD_ID.  (In practice there is probably only ever one file there
> to look at, but you never know.)

Ahah! That seems to explain why the kernel one is "notes" while the
modules's are ".note.gnu.build-id"! I'll cook a super-dupa-careful
cooker follow on patch to get that right, thanks again!

- Arnaldo

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2009-11-18 22:34 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-11-18 22:20 [PATCH 1/4] perf symbols: Fix comparision of build_ids Arnaldo Carvalho de Melo
2009-11-18 22:20 ` [PATCH 2/4] perf symbols: Kill struct build_id_list and die() another day Arnaldo Carvalho de Melo
2009-11-18 22:20   ` [PATCH 3/4] perf symbols: Record the build_ids of kernel modules too Arnaldo Carvalho de Melo
2009-11-18 22:20     ` [PATCH 4/4] perf symbols: Capture the running kernel buildid too Arnaldo Carvalho de Melo
2009-11-18 22:29       ` Arnaldo Carvalho de Melo
2009-11-18 22:29     ` [PATCH 3/4] perf symbols: Record the build_ids of kernel modules too Roland McGrath
2009-11-18 22:33       ` 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.