Re: [PATCH] perf, tools: Add better errors to annotate

From: Arnaldo Carvalho de Melo
Date: Fri Nov 06 2015 - 08:13:32 EST


Em Thu, Nov 05, 2015 at 07:06:07PM -0800, Andi Kleen escreveu:
> From: Andi Kleen <ak@xxxxxxxxxxxxxxx>
>
> 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


> Signed-off-by: Andi Kleen <ak@xxxxxxxxxxxxxxx>
> ---
> tools/perf/util/annotate.c | 20 ++++++++++++++++++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index d1eece7..07f9544 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -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.
> --
> 2.4.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/