[RFC] propagating symtab load errors. was: Re: [GIT PULL 00/20] perf/core improvements and fixes

From: Arnaldo Carvalho de Melo
Date: Mon Mar 23 2015 - 18:18:44 EST

Next message: Sukadev Bhattiprolu: "Re: [2/9] powerpc/hv24x7: Remove unnecessary parameter"
Previous message: "Stéphane Viau": "Re: [PATCH v3 3/4] drm/msm/mdp5: Add START signal to kick off certain pipelines"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Em Tue, Mar 10, 2015 at 11:03:31AM +0100, Ingo Molnar escreveu:
> So I got this error today:
>
> ââWarning:ââââââââââââââââââââââââââââ
> âThe vmlinux file can't be used. â
> âKernel samples will not be resolved.â
> â â
> â â
> âPress any key... â
> ââââââââââââââââââââââââââââââââââââââ
> ... and sadly perf is being passive-aggressive again: being negative
> but refusing to say why! :-)
>
> Is there a way to figure out why it did not like the vmlinux?

So, we need some infrastructure for that, i.e. a strerror like routine
like we have for other classes, because there are lots of things that
can go wrong while loading a file: ELF error, decompression stuff, you
name it, so, with the patch below it ends up as:

# perf top --vmlinux /dev/null

ââWarning:ââââââââââââââââââââââââââââââââââââââââââââ
âThe /tmp/passwd file can't be used: Invalid ELF fileâ
âKernel samples will not be resolved. â
â â
â â
âPress any key... â
ââââââââââââââââââââââââââââââââââââââââââââââââââââââ

Basically save the errno, that may be an elf_errno (in this case I just came up
with a DSO_LOAD_ERRNO__INVALID_ELF), an errno or some other error that needs
propagating.

Just a heads up, I'll polish it a bit more before submitting, sounds OK?

Jiri, Namhyung, this would go down to the decompression code, etc.

Right now some of this can be obtained via the pr_debug calls, but that is too
Ã passive aggressive, and we don't want to do the error report syncronously,
this is up for the tools/UI used, so saving it and providing a strerror routine
looks sane, right?

- Arnaldo

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 5fb8723c7128..1cb3436276d1 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -757,8 +757,10 @@ static void perf_event__process_sample(struct perf_tool *tool,
al.map == machine->vmlinux_maps[MAP__FUNCTION] &&
RB_EMPTY_ROOT(&al.map->dso->symbols[MAP__FUNCTION])) {
if (symbol_conf.vmlinux_name) {
- ui__warning("The %s file can't be used.\n%s",
- symbol_conf.vmlinux_name, msg);
+ char serr[256];
+ dso__strerror_load(al.map->dso, serr, sizeof(serr));
+ ui__warning("The %s file can't be used: %s\n%s",
+ symbol_conf.vmlinux_name, serr, msg);
} else {
ui__warning("A vmlinux file was not found.\n%s",
msg);
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 0d3667f92023..4e33be8f92ba 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -1137,3 +1137,34 @@ enum dso_type dso__type(struct dso *dso, struct machine *machine)

return dso__type_fd(fd);
}
+
+int dso__strerror_load(struct dso *dso, char *buf, size_t buflen)
+{
+ int idx, errnum = dso->load_errno;
+ /*
+ * This must have a same ordering as the enum dso_load_errno.
+ */
+ static const char *dso_load__error_str[] = {
+ "Internal tools/perf/ library error",
+ "Invalid ELF file",
+ "Decompression failure",
+ };
+
+ BUG_ON(buflen == 0);
+
+ if (errnum >= 0) {
+ const char *err = strerror_r(errnum, buf, buflen);
+
+ if (err != buf)
+ scnprintf(buf, buflen, "%s", err);
+
+ return 0;
+ }
+
+ if (errnum < __DSO_LOAD_ERRNO__START || errnum >= __DSO_LOAD_ERRNO__END)
+ return -1;
+
+ idx = errnum - __DSO_LOAD_ERRNO__START;
+ scnprintf(buf, buflen, "%s", dso_load__error_str[idx]);
+ return 0;
+}
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index 88f345cc5be2..25f78bb15819 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -60,6 +60,29 @@ enum dso_type {
DSO__TYPE_X32BIT,
};

+enum dso_load_errno {
+ DSO_LOAD_ERRNO__SUCCESS = 0,
+
+ /*
+ * Choose an arbitrary negative big number not to clash with standard
+ * errno since SUS requires the errno has distinct positive values.
+ * See 'Issue 6' in the link below.
+ *
+ * http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/errno.h.html
+ */
+ __DSO_LOAD_ERRNO__START = -10000,
+
+ DSO_LOAD_ERRNO__INTERNAL_ERROR = __DSO_LOAD_ERRNO__START,
+
+ /* for symsrc__init() */
+ DSO_LOAD_ERRNO__INVALID_ELF,
+
+ /* for decompress_kmodule */
+ DSO_LOAD_ERRNO__DECOMPRESSION_FAILURE,
+
+ __DSO_LOAD_ERRNO__END,
+};
+
#define DSO__SWAP(dso, type, val) \
({ \
type ____r = val; \
@@ -113,6 +136,7 @@ struct dso {
enum dso_swap_type needs_swap;
enum dso_binary_type symtab_type;
enum dso_binary_type binary_type;
+ enum dso_load_errno load_errno;
u8 adjust_symbols:1;
u8 has_build_id:1;
u8 has_srcline:1;
@@ -294,4 +318,6 @@ void dso__free_a2l(struct dso *dso);

enum dso_type dso__type(struct dso *dso, struct machine *machine);

+int dso__strerror_load(struct dso *dso, char *buf, size_t buflen);
+
#endif /* __PERF_DSO */
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 78ffde9df9bf..2a464273a43b 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -595,10 +595,13 @@ static int decompress_kmodule(struct dso *dso, const char *name,
return -1;

fd = mkstemp(tmpbuf);
- if (fd < 0)
+ if (fd < 0) {
+ dso->load_errno = errno;
goto out;
+ }

if (!decompress_to_file(m.ext, name, fd)) {
+ dso->load_errno = DSO_LOAD_ERRNO__DECOMPRESSION_FAILURE;
close(fd);
fd = -1;
}
@@ -635,27 +638,35 @@ int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
Elf *elf;
int fd;

- if (dso__needs_decompress(dso))
+ if (dso__needs_decompress(dso)) {
fd = decompress_kmodule(dso, name, type);
- else
+ if (fd < 0)
+ return -1;
+ } else {
fd = open(name, O_RDONLY);
-
- if (fd < 0)
- return -1;
+ if (fd < 0) {
+ dso->load_errno = errno;
+ return -1;
+ }
+ }

elf = elf_begin(fd, PERF_ELF_C_READ_MMAP, NULL);
if (elf == NULL) {
pr_debug("%s: cannot read %s ELF file.\n", __func__, name);
+ dso->load_errno = DSO_LOAD_ERRNO__INVALID_ELF;
goto out_close;
}

if (gelf_getehdr(elf, &ehdr) == NULL) {
+ dso->load_errno = DSO_LOAD_ERRNO__INVALID_ELF;
pr_debug("%s: cannot get elf header.\n", __func__);
goto out_elf_end;
}

- if (dso__swap_init(dso, ehdr.e_ident[EI_DATA]))
+ if (dso__swap_init(dso, ehdr.e_ident[EI_DATA])) {
+ dso->load_errno = DSO_LOAD_ERRNO__INTERNAL_ERROR;
goto out_elf_end;
+ }

/* Always reject images with a mismatched build-id: */
if (dso->has_build_id) {
@@ -701,8 +712,10 @@ int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
}

ss->name = strdup(name);
- if (!ss->name)
+ if (!ss->name) {
+ dso->load_errno = errno;
goto out_elf_end;
+ }

ss->elf = elf;
ss->fd = fd;

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

Next message: Sukadev Bhattiprolu: "Re: [2/9] powerpc/hv24x7: Remove unnecessary parameter"
Previous message: "Stéphane Viau": "Re: [PATCH v3 3/4] drm/msm/mdp5: Add START signal to kick off certain pipelines"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]