[PATCH 1/4] perf symbols: Fix comparision of build_ids

[PATCH 1/4] perf symbols: Fix comparision of build_ids

[Arnaldo Carvalho de Melo]

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

[PATCH 2/4] perf symbols: Kill struct build_id_list and die() another day

[Arnaldo Carvalho de Melo]

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

[PATCH 3/4] perf symbols: Record the build_ids of kernel modules too

[Arnaldo Carvalho de Melo]

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

[PATCH 4/4] perf symbols: Capture the running kernel buildid too

[Arnaldo Carvalho de Melo]

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

Re: [PATCH 4/4] perf symbols: Capture the running kernel buildid too

[Arnaldo Carvalho de Melo]

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

Re: [PATCH 3/4] perf symbols: Record the build_ids of kernel modules too

[Roland McGrath]

> +		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

Re: [PATCH 3/4] perf symbols: Record the build_ids of kernel modules too

[Arnaldo Carvalho de Melo]

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