Skip to contentSkip to conversation

perf tools: Fix annotation with kcore

PATCH In review Adrian Hunter 3 participants 5 messages Nov 11, 2014 – Nov 20, 2014 perf events

lore
Share & tools
lore

Adrian Hunter

author reviewer
#0 lore raw

PATCH 1/1 perf tools: Fix annotation with kcore

Patch "perf tools: Fix build-id matching on vmlinux" breaks annotation with kcore. The problem is that symbol__annotate() first gets the filename based on the build-id which was previously not set.
This patch provides a quick fix, however there should probably be only one way to determine the filename. e.g. symbol__annotate() should use the same way as dso__data_fd().

---
 tools/perf/util/annotate.c | 2 ++
 1 file changed, 2 insertions(+)
tools/perf/util/annotate.c +2 -0
index 7dabde1..873c877 100644
@@ -915,6 +915,8 @@ int symbol__annotate(struct symbol *sym, struct map *map, size_t privsize)
 			return -ENOMEM;
 		}
 		goto fallback;
+	} else if (dso__is_kcore(dso)) {
+		goto fallback;
 	} else if (readlink(symfs_filename, command, sizeof(command)) < 0 ||
 		   strstr(command, "[kernel.kallsyms]") ||
 		   access(symfs_filename, R_OK)) {
Signature · 1 line
1.9.1
#1 lore raw

Em Tue, Nov 11, 2014 at 12:04:54PM +0200, Adrian Hunter escreveu:

Quoted 1 line

Patch "perf tools: Fix build-id matching on vmlinux"

So, I will check this, but please state for which branch this is supposed to be applied, i.e. does it fixes perf/urgent stuff and thus needs to go ASAP, or is this something that affects perf/core and thus can wait till I process more urgent stuff?

- Arnaldo

Quoted 28 lines

breaks annotation with kcore. The problem is that symbol__annotate() first gets the filename based on the build-id which was previously not set.
This patch provides a quick fix, however there should probably be only one way to determine the filename. e.g. symbol__annotate() should use the same way as dso__data_fd().

---
 tools/perf/util/annotate.c | 2 ++
 1 file changed, 2 insertions(+)
tools/perf/util/annotate.c +2 -0
index 7dabde1..873c877 100644
@@ -915,6 +915,8 @@ int symbol__annotate(struct symbol *sym, struct map *map, size_t privsize)
 			return -ENOMEM;
 		}
 		goto fallback;
+	} else if (dso__is_kcore(dso)) {
+		goto fallback;
 	} else if (readlink(symfs_filename, command, sizeof(command)) < 0 ||
 		   strstr(command, "[kernel.kallsyms]") ||
 		   access(symfs_filename, R_OK)) {
Signature · 1 line
1.9.1
author reviewer
#2 lore raw
Quoted 7 lines · Arnaldo Carvalho de Melo

Em Tue, Nov 11, 2014 at 12:04:54PM +0200, Adrian Hunter escreveu:

Quoted 1 line

Patch "perf tools: Fix build-id matching on vmlinux"

So, I will check this, but please state for which branch this is supposed to be applied, i.e. does it fixes perf/urgent stuff and thus needs to go ASAP, or is this something that affects perf/core and thus can wait till I process more urgent stuff?

It is perf/core so it can wait.

Quoted 33 lines

- Arnaldo

Quoted 28 lines

breaks annotation with kcore. The problem is that symbol__annotate() first gets the filename based on the build-id which was previously not set.
This patch provides a quick fix, however there should probably be only one way to determine the filename. e.g. symbol__annotate() should use the same way as dso__data_fd().

---
 tools/perf/util/annotate.c | 2 ++
 1 file changed, 2 insertions(+)
tools/perf/util/annotate.c +2 -0
index 7dabde1..873c877 100644
@@ -915,6 +915,8 @@ int symbol__annotate(struct symbol *sym, struct map *map, size_t privsize)
 			return -ENOMEM;
 		}
 		goto fallback;
+	} else if (dso__is_kcore(dso)) {
+		goto fallback;
 	} else if (readlink(symfs_filename, command, sizeof(command)) < 0 ||
 		   strstr(command, "[kernel.kallsyms]") ||
 		   access(symfs_filename, R_OK)) {
Signature · 1 line
1.9.1
#3 lore raw
In reply to Adrian Hunter

Em Tue, Nov 11, 2014 at 12:04:54PM +0200, Adrian Hunter escreveu:

Quoted 8 lines

Patch "perf tools: Fix build-id matching on vmlinux" breaks annotation with kcore. The problem is that symbol__annotate() first gets the filename based on the build-id which was previously not set.
This patch provides a quick fix, however there should probably be only one way to determine the filename. e.g. symbol__annotate() should use the same way as dso__data_fd().

Agreed, please provide a patch if you feel like cleaning this up :-)

- Arnaldo

Quoted 20 lines
---
 tools/perf/util/annotate.c | 2 ++
 1 file changed, 2 insertions(+)
tools/perf/util/annotate.c +2 -0
index 7dabde1..873c877 100644
@@ -915,6 +915,8 @@ int symbol__annotate(struct symbol *sym, struct map *map, size_t privsize)
 			return -ENOMEM;
 		}
 		goto fallback;
+	} else if (dso__is_kcore(dso)) {
+		goto fallback;
 	} else if (readlink(symfs_filename, command, sizeof(command)) < 0 ||
 		   strstr(command, "[kernel.kallsyms]") ||
 		   access(symfs_filename, R_OK)) {
Signature · 1 line
1.9.1
#4 lore raw
In reply to Adrian Hunter
perf tools: Fix annotation with kcore
Commit-ID:  ee205503f2333d639550eaed37abb455733510a3
Gitweb:     http://git.kernel.org/tip/ee205503f2333d639550eaed37abb455733510a3
Author:     Adrian Hunter <[email protected]>
AuthorDate: Tue, 11 Nov 2014 12:04:54 +0200
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 19 Nov 2014 12:33:47 -0300

perf tools: Fix annotation with kcore

Patch "perf tools: Fix build-id matching on vmlinux" breaks annotation with kcore. The problem is that symbol__annotate() first gets the filename based on the build-id which was previously not set.

This patch provides a quick fix, however there should probably be only one way to determine the filename. e.g. symbol__annotate() should use the same way as dso__data_fd().

---
 tools/perf/util/annotate.c | 2 ++
 1 file changed, 2 insertions(+)
tools/perf/util/annotate.c +2 -0
index 7dabde1..873c877 100644
@@ -915,6 +915,8 @@ int symbol__annotate(struct symbol *sym, struct map *map, size_t privsize)
 			return -ENOMEM;
 		}
 		goto fallback;
+	} else if (dso__is_kcore(dso)) {
+		goto fallback;
 	} else if (readlink(symfs_filename, command, sizeof(command)) < 0 ||
 		   strstr(command, "[kernel.kallsyms]") ||
 		   access(symfs_filename, R_OK)) {