perf, tools: Add better errors to annotate

PATCH In review Andi Kleen 3 participants 3 messages Nov 6, 2015 – Nov 8, 2015 perf events

lore

Share & tools

lore

Andi Kleen

author

2015-11-06 03:06 UTC #0 lore raw

PATCH 1/1 perf, tools: Add better errors to annotate

From: Andi Kleen <[email protected]>

When the browser fails to annotate it is difficult for users to find out what went wrong.

Add some errors for objdump failures that are displayed in the UI.

Note it would be event better to handle these errors smarter, like falling back to the binary when the debug info is somehow corrupted. But for now just giving a better error is an improvement.

Signed-off-by Andi Kleen <[email protected]>

---
 tools/perf/util/annotate.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

tools/perf/util/annotate.c +18 -2

index d1eece7..07f9544 100644
@@ -1081,6 +1081,7 @@ int symbol__annotate(struct symbol *sym, struct map *map, size_t privsize)
 	struct kcore_extract kce;
 	bool delete_extract = false;
 	int lineno = 0;
+	int nline;
 
 	if (filename)
 		symbol__join_symfs(symfs_filename, filename);

@@ -1176,6 +1177,9 @@ fallback:
 
 		ret = decompress_to_file(m.ext, symfs_filename, fd);
 
+		if (ret)
+			pr_err("Cannot decompress %s %s\n", m.ext, symfs_filename);
+
 		free(m.ext);
 		close(fd);

@@ -1201,13 +1205,25 @@ fallback:
 	pr_debug("Executing: %s\n", command);
 
 	file = popen(command, "r");
-	if (!file)
+	if (!file) {
+		pr_err("Failure running %s\n", command);
+		/*
+		 * If we were using debug info should retry with
+		 * original binary.
+		 */
 		goto out_remove_tmp;
+	}
 
-	while (!feof(file))
+	nline = 0;
+	while (!feof(file)) {
 		if (symbol__parse_objdump_line(sym, map, file, privsize,
 			    &lineno) < 0)
 			break;
+		nline++;
+	}
+
+	if (nline == 0)
+		pr_err("No output from %s\n", command);
 
 	/*
 	 * kallsyms does not have symbol sizes so there may a nop at the end.

Signature · 1 line

2.4.3

Arnaldo Carvalho de Melo

maintainer

2015-11-06 13:13 UTC #1 lore raw

Em Thu, Nov 05, 2015 at 07:06:07PM -0800, Andi Kleen escreveu:

Quoted 10 lines

From: Andi Kleen <[email protected]>
When the browser fails to annotate it is difficult for users to find out what went wrong.
Add some errors for objdump failures that are displayed in the UI.
Note it would be event better to handle these errors smarter, like falling back to the binary when the debug info is somehow corrupted. But for now just giving a better error is an improvement.

So this is an improvement for 'perf ann otate --stdio', where, after I added a (1 ||) to one of the paths you added a pr_err(), I got:

[root@zoo ~]# perf annotate --stdio intel_idle
no symbols found in /usr/bin/procmail, maybe install a debug package?
Failed to open /tmp/perf-8683.map, continuing without symbols
Failure running objdump --start-address=0xffffffff81418290 --stop-address=0xffffffff814183ae -l -d --no-show-ra
Percent | Source code & Disassembly of vmlinux for cycles:pp
------------------------------------------------------------------

But doesn't work at all for --tui (and probably --gtk), where it will print the warning on the last line in the screen, that gets immediatelly rewritten :-\

I am applying it, as it improves the --stdio case, and will fix this in a way that works for all UIs, with some strerror() interface to convert whatever error happened to a string that then gets used in whatever UI is being used.

Thanks,

- Arnaldo

Quoted 57 lines

Signed-off-by Andi Kleen <[email protected]>

---
 tools/perf/util/annotate.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

tools/perf/util/annotate.c +18 -2

index d1eece7..07f9544 100644
@@ -1081,6 +1081,7 @@ int symbol__annotate(struct symbol *sym, struct map *map, size_t privsize)
 	struct kcore_extract kce;
 	bool delete_extract = false;
 	int lineno = 0;
+	int nline;
 
 	if (filename)
 		symbol__join_symfs(symfs_filename, filename);

@@ -1176,6 +1177,9 @@ fallback:
 
 		ret = decompress_to_file(m.ext, symfs_filename, fd);
 
+		if (ret)
+			pr_err("Cannot decompress %s %s\n", m.ext, symfs_filename);
+
 		free(m.ext);
 		close(fd);

@@ -1201,13 +1205,25 @@ fallback:
 	pr_debug("Executing: %s\n", command);
 
 	file = popen(command, "r");
-	if (!file)
+	if (!file) {
+		pr_err("Failure running %s\n", command);
+		/*
+		 * If we were using debug info should retry with
+		 * original binary.
+		 */
 		goto out_remove_tmp;
+	}
 
-	while (!feof(file))
+	nline = 0;
+	while (!feof(file)) {
 		if (symbol__parse_objdump_line(sym, map, file, privsize,
 			    &lineno) < 0)
 			break;
+		nline++;
+	}
+
+	if (nline == 0)
+		pr_err("No output from %s\n", command);
 
 	/*
 	 * kallsyms does not have symbol sizes so there may a nop at the end.

Signature · 1 line

2.4.3

tip-bot for Andi Kleen

merge bot

2015-11-08 07:32 UTC #2 lore raw

[tip:perf/urgent] perf annotate: Inform the user about objdump failures in --stdio

In reply to Andi Kleen

perf annotate: Inform the user about objdump failures in --stdio

Commit-ID:  62ec9b3f02a9bccaf699bd4691db98f779c3075f
Gitweb:     http://git.kernel.org/tip/62ec9b3f02a9bccaf699bd4691db98f779c3075f
Author:     Andi Kleen <[email protected]>
AuthorDate: Thu, 5 Nov 2015 19:06:07 -0800
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Fri, 6 Nov 2015 10:20:48 -0300

perf annotate: Inform the user about objdump failures in --stdio

When the browser fails to annotate it is difficult for users to find out what went wrong.

Add some errors for objdump failures that are displayed in the UI.

Note it would be even better to handle these errors smarter, like falling back to the binary when the debug info is somehow corrupted. But for now just giving a better error is an improvement.

Committer note:

This works for --stdio, where errors just scroll by the screen:

  # perf annotate --stdio intel_idle
  Failure running objdump  --start-address=0xffffffff81418290 --stop-address=0xffffffff814183ae -l -d --no-show-raw -S -C /root/.debug/.build-id/28/2777c262e6b3c0451375163c9a81c893218ab1 2>/dev/null|grep -v /root/.debug/.build-id/28/2777c262e6b3c0451375163c9a81c893218ab1|expand
   Percent |      Source code & Disassembly of vmlinux for cycles:pp
  ------------------------------------------------------------------

And with that one can use that command line to try to find out more about what happened instead of getting a blank screen, an improvement.

We need tho to improve this further to get it to work with other UIs, like --tui and --gtk, where it continues showing a blank screen, no messages, as the pr_err() used is enough just for --stdio.

Signed-off-by Andi Kleen <[email protected]>

Tested-by Arnaldo Carvalho de Melo <[email protected]>

Cc Jiri Olsa <[email protected]>

Cc Namhyung Kim <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/util/annotate.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

tools/perf/util/annotate.c +18 -2

index 0fc8d7a..f2974da 100644
@@ -1084,6 +1084,7 @@ int symbol__annotate(struct symbol *sym, struct map *map, size_t privsize)
 	struct kcore_extract kce;
 	bool delete_extract = false;
 	int lineno = 0;
+	int nline;
 
 	if (filename)
 		symbol__join_symfs(symfs_filename, filename);

@@ -1179,6 +1180,9 @@ fallback:
 
 		ret = decompress_to_file(m.ext, symfs_filename, fd);
 
+		if (ret)
+			pr_err("Cannot decompress %s %s\n", m.ext, symfs_filename);
+
 		free(m.ext);
 		close(fd);

@@ -1204,13 +1208,25 @@ fallback:
 	pr_debug("Executing: %s\n", command);
 
 	file = popen(command, "r");
-	if (!file)
+	if (!file) {
+		pr_err("Failure running %s\n", command);
+		/*
+		 * If we were using debug info should retry with
+		 * original binary.
+		 */
 		goto out_remove_tmp;
+	}
 
-	while (!feof(file))
+	nline = 0;
+	while (!feof(file)) {
 		if (symbol__parse_objdump_line(sym, map, file, privsize,
 			    &lineno) < 0)
 			break;
+		nline++;
+	}
+
+	if (nline == 0)
+		pr_err("No output from %s\n", command);
 
 	/*
 	 * kallsyms does not have symbol sizes so there may a nop at the end.