* [PATCH v3 1/6] perf tools: Introduce dso__decompress_kmodule_{fd,path}
2017-06-07 15:38 [PATCHSET v3 0/6] perf tools: Fix for Compressed kernel modules Namhyung Kim
@ 2017-06-07 15:38 ` Namhyung Kim
2017-06-07 15:38 ` [PATCH v3 2/6] perf annotate: Use dso__decompress_kmodule_path() Namhyung Kim
` (4 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Namhyung Kim @ 2017-06-07 15:38 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team,
David Ahern, Adrian Hunter, Wang Nan
Move decompress_kmodule() to util/dso.c and split it to two functions
returning fd and (decompressed) file path. Existing user only wants the
fd version but the path version will be used soon.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/dso.c | 51 ++++++++++++++++++++++++++++++++++++++++++++
tools/perf/util/dso.h | 6 ++++++
tools/perf/util/symbol-elf.c | 36 +------------------------------
3 files changed, 58 insertions(+), 35 deletions(-)
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index b27d127cdf68..59257ce4580e 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -248,6 +248,57 @@ bool dso__needs_decompress(struct dso *dso)
dso->symtab_type == DSO_BINARY_TYPE__GUEST_KMODULE_COMP;
}
+static int decompress_kmodule(struct dso *dso, const char *name, char *tmpbuf)
+{
+ int fd = -1;
+ struct kmod_path m;
+
+ if (!dso__needs_decompress(dso))
+ return -1;
+
+ if (kmod_path__parse_ext(&m, dso->long_name) || !m.comp)
+ return -1;
+
+ fd = mkstemp(tmpbuf);
+ 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;
+ }
+
+out:
+ free(m.ext);
+ return fd;
+}
+
+int dso__decompress_kmodule_fd(struct dso *dso, const char *name)
+{
+ char tmpbuf[] = KMOD_DECOMP_NAME;
+ int fd;
+
+ fd = decompress_kmodule(dso, name, tmpbuf);
+ unlink(tmpbuf);
+ return fd;
+}
+
+int dso__decompress_kmodule_path(struct dso *dso, const char *name,
+ char *pathname, size_t len)
+{
+ char tmpbuf[] = KMOD_DECOMP_NAME;
+ int fd;
+
+ fd = decompress_kmodule(dso, name, tmpbuf);
+ close(fd);
+
+ strncpy(pathname, tmpbuf, len);
+ return fd >= 0 ? 0 : fd;
+}
+
/*
* Parses kernel module specified in @path and updates
* @m argument like:
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index 5fe2ab5877bd..bd061ba7b47c 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -244,6 +244,12 @@ bool is_supported_compression(const char *ext);
bool is_kernel_module(const char *pathname, int cpumode);
bool decompress_to_file(const char *ext, const char *filename, int output_fd);
bool dso__needs_decompress(struct dso *dso);
+int dso__decompress_kmodule_fd(struct dso *dso, const char *name);
+int dso__decompress_kmodule_path(struct dso *dso, const char *name,
+ char *pathname, size_t len);
+
+#define KMOD_DECOMP_NAME "/tmp/perf-kmod-XXXXXX"
+#define KMOD_DECOMP_LEN sizeof(KMOD_DECOMP_NAME)
struct kmod_path {
char *name;
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 1fb2efae4f02..d342e771dbad 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -637,40 +637,6 @@ static int dso__swap_init(struct dso *dso, unsigned char eidata)
return 0;
}
-static int decompress_kmodule(struct dso *dso, const char *name,
- enum dso_binary_type type)
-{
- int fd = -1;
- char tmpbuf[] = "/tmp/perf-kmod-XXXXXX";
- struct kmod_path m;
-
- if (type != DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE_COMP &&
- type != DSO_BINARY_TYPE__GUEST_KMODULE_COMP &&
- type != DSO_BINARY_TYPE__BUILD_ID_CACHE)
- return -1;
-
- if (kmod_path__parse_ext(&m, dso->long_name) || !m.comp)
- return -1;
-
- fd = mkstemp(tmpbuf);
- 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;
- }
-
- unlink(tmpbuf);
-
-out:
- free(m.ext);
- return fd;
-}
-
bool symsrc__possibly_runtime(struct symsrc *ss)
{
return ss->dynsym || ss->opdsec;
@@ -702,7 +668,7 @@ int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
int fd;
if (dso__needs_decompress(dso)) {
- fd = decompress_kmodule(dso, name, type);
+ fd = dso__decompress_kmodule_fd(dso, name);
if (fd < 0)
return -1;
} else {
--
2.13.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v3 2/6] perf annotate: Use dso__decompress_kmodule_path()
2017-06-07 15:38 [PATCHSET v3 0/6] perf tools: Fix for Compressed kernel modules Namhyung Kim
2017-06-07 15:38 ` [PATCH v3 1/6] perf tools: Introduce dso__decompress_kmodule_{fd,path} Namhyung Kim
@ 2017-06-07 15:38 ` Namhyung Kim
2017-06-07 15:39 ` [PATCH v3 3/6] perf tools: Decompress kernel module when reading DSO data Namhyung Kim
` (3 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Namhyung Kim @ 2017-06-07 15:38 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team,
David Ahern, Adrian Hunter, Wang Nan
Convert open-coded decompress routine to use the function.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/annotate.c | 27 +++------------------------
1 file changed, 3 insertions(+), 24 deletions(-)
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 1367d7e35242..bc4f81b5cf73 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1423,31 +1423,10 @@ int symbol__disassemble(struct symbol *sym, struct map *map, const char *arch_na
sizeof(symfs_filename));
}
} else if (dso__needs_decompress(dso)) {
- char tmp[PATH_MAX];
- struct kmod_path m;
- int fd;
- bool ret;
+ char tmp[KMOD_DECOMP_LEN];
- if (kmod_path__parse_ext(&m, symfs_filename))
- goto out;
-
- snprintf(tmp, PATH_MAX, "/tmp/perf-kmod-XXXXXX");
-
- fd = mkstemp(tmp);
- if (fd < 0) {
- free(m.ext);
- goto out;
- }
-
- 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);
-
- if (!ret)
+ if (dso__decompress_kmodule_path(dso, symfs_filename,
+ tmp, sizeof(tmp)) < 0)
goto out;
strcpy(symfs_filename, tmp);
--
2.13.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v3 3/6] perf tools: Decompress kernel module when reading DSO data
2017-06-07 15:38 [PATCHSET v3 0/6] perf tools: Fix for Compressed kernel modules Namhyung Kim
2017-06-07 15:38 ` [PATCH v3 1/6] perf tools: Introduce dso__decompress_kmodule_{fd,path} Namhyung Kim
2017-06-07 15:38 ` [PATCH v3 2/6] perf annotate: Use dso__decompress_kmodule_path() Namhyung Kim
@ 2017-06-07 15:39 ` Namhyung Kim
2017-06-07 22:23 ` Arnaldo Carvalho de Melo
2017-06-07 15:39 ` [PATCH v3 4/6] perf test: Decompress kernel module before objdump Namhyung Kim
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Namhyung Kim @ 2017-06-07 15:39 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team,
David Ahern, Adrian Hunter, Wang Nan
Currently perf decompresses kernel modules when loading symbol table but
it missed to do it when reading raw data.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/dso.c | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 59257ce4580e..2a0c689227f8 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -463,8 +463,22 @@ static int __open_dso(struct dso *dso, struct machine *machine)
return -EINVAL;
}
- if (!is_regular_file(name))
+ if (!is_regular_file(name)) {
+ free(name);
return -EINVAL;
+ }
+
+ if (dso__needs_decompress(dso)) {
+ char newpath[KMOD_DECOMP_LEN];
+ size_t len = sizeof(newpath);
+
+ if (dso__decompress_kmodule_path(dso, name, newpath, len) < 0) {
+ free(name);
+ return -1;
+ }
+
+ strcpy(name, newpath);
+ }
fd = do_open(name);
free(name);
--
2.13.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v3 3/6] perf tools: Decompress kernel module when reading DSO data
2017-06-07 15:39 ` [PATCH v3 3/6] perf tools: Decompress kernel module when reading DSO data Namhyung Kim
@ 2017-06-07 22:23 ` Arnaldo Carvalho de Melo
2017-06-07 23:53 ` Namhyung Kim
0 siblings, 1 reply; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-06-07 22:23 UTC (permalink / raw)
To: Namhyung Kim
Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team,
David Ahern, Adrian Hunter, Wang Nan
Em Thu, Jun 08, 2017 at 12:39:00AM +0900, Namhyung Kim escreveu:
> Currently perf decompresses kernel modules when loading symbol table but
> it missed to do it when reading raw data.
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/util/dso.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index 59257ce4580e..2a0c689227f8 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -463,8 +463,22 @@ static int __open_dso(struct dso *dso, struct machine *machine)
> return -EINVAL;
> }
>
> - if (!is_regular_file(name))
> + if (!is_regular_file(name)) {
> + free(name);
> return -EINVAL;
> + }
Humm, this looks like something for a separate patch? Its a pre-existing
leak, separate issue.
That error path in __open_dso() could be consolidated, but the above
hunk is the minimal fix for perf/urgent, where I think your series
should go from what I've read so far, agreed?
- Arnaldo
> +
> + if (dso__needs_decompress(dso)) {
> + char newpath[KMOD_DECOMP_LEN];
> + size_t len = sizeof(newpath);
> +
> + if (dso__decompress_kmodule_path(dso, name, newpath, len) < 0) {
> + free(name);
> + return -1;
> + }
> +
> + strcpy(name, newpath);
> + }
>
> fd = do_open(name);
> free(name);
> --
> 2.13.0
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v3 3/6] perf tools: Decompress kernel module when reading DSO data
2017-06-07 22:23 ` Arnaldo Carvalho de Melo
@ 2017-06-07 23:53 ` Namhyung Kim
0 siblings, 0 replies; 12+ messages in thread
From: Namhyung Kim @ 2017-06-07 23:53 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team,
David Ahern, Adrian Hunter, Wang Nan
Hi Arnaldo,
On Wed, Jun 07, 2017 at 07:23:05PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Jun 08, 2017 at 12:39:00AM +0900, Namhyung Kim escreveu:
> > Currently perf decompresses kernel modules when loading symbol table but
> > it missed to do it when reading raw data.
> >
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> > tools/perf/util/dso.c | 16 +++++++++++++++-
> > 1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> > index 59257ce4580e..2a0c689227f8 100644
> > --- a/tools/perf/util/dso.c
> > +++ b/tools/perf/util/dso.c
> > @@ -463,8 +463,22 @@ static int __open_dso(struct dso *dso, struct machine *machine)
> > return -EINVAL;
> > }
> >
> > - if (!is_regular_file(name))
> > + if (!is_regular_file(name)) {
> > + free(name);
> > return -EINVAL;
> > + }
>
> Humm, this looks like something for a separate patch? Its a pre-existing
> leak, separate issue.
OK, will separate.
>
> That error path in __open_dso() could be consolidated, but the above
> hunk is the minimal fix for perf/urgent, where I think your series
> should go from what I've read so far, agreed?
Agreed.
Thanks,
Namhyung
>
> > +
> > + if (dso__needs_decompress(dso)) {
> > + char newpath[KMOD_DECOMP_LEN];
> > + size_t len = sizeof(newpath);
> > +
> > + if (dso__decompress_kmodule_path(dso, name, newpath, len) < 0) {
> > + free(name);
> > + return -1;
> > + }
> > +
> > + strcpy(name, newpath);
> > + }
> >
> > fd = do_open(name);
> > free(name);
> > --
> > 2.13.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 4/6] perf test: Decompress kernel module before objdump
2017-06-07 15:38 [PATCHSET v3 0/6] perf tools: Fix for Compressed kernel modules Namhyung Kim
` (2 preceding siblings ...)
2017-06-07 15:39 ` [PATCH v3 3/6] perf tools: Decompress kernel module when reading DSO data Namhyung Kim
@ 2017-06-07 15:39 ` Namhyung Kim
2017-06-07 22:25 ` Arnaldo Carvalho de Melo
2017-06-07 15:39 ` [PATCH v3 5/6] perf symbols: Keep DSO->symtab_type after decompress Namhyung Kim
2017-06-07 15:39 ` [PATCH v3 6/6] perf symbols: Kill dso__build_id_is_kmod() Namhyung Kim
5 siblings, 1 reply; 12+ messages in thread
From: Namhyung Kim @ 2017-06-07 15:39 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team,
David Ahern, Adrian Hunter, Wang Nan
If a kernel modules is compressed, it should be decompressed before
running objdump to parse binary data correctly. This fixes a failure of
object code reading test for me.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/tests/code-reading.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
index 1f14e7612cbb..94b7c7b02bde 100644
--- a/tools/perf/tests/code-reading.c
+++ b/tools/perf/tests/code-reading.c
@@ -229,6 +229,8 @@ static int read_object_code(u64 addr, size_t len, u8 cpumode,
unsigned char buf2[BUFSZ];
size_t ret_len;
u64 objdump_addr;
+ const char *objdump_name;
+ char decomp_name[KMOD_DECOMP_LEN];
int ret;
pr_debug("Reading object code for memory address: %#"PRIx64"\n", addr);
@@ -289,9 +291,25 @@ static int read_object_code(u64 addr, size_t len, u8 cpumode,
state->done[state->done_cnt++] = al.map->start;
}
+ objdump_name = al.map->dso->long_name;
+ if (dso__needs_decompress(al.map->dso)) {
+ if (dso__decompress_kmodule_path(al.map->dso, objdump_name,
+ decomp_name,
+ sizeof(decomp_name)) < 0) {
+ pr_debug("decompression failed\n");
+ return -1;
+ }
+
+ objdump_name = decomp_name;
+ }
+
/* Read the object code using objdump */
objdump_addr = map__rip_2objdump(al.map, al.addr);
- ret = read_via_objdump(al.map->dso->long_name, objdump_addr, buf2, len);
+ ret = read_via_objdump(objdump_name, objdump_addr, buf2, len);
+
+ if (dso__needs_decompress(al.map->dso))
+ unlink(objdump_name);
+
if (ret > 0) {
/*
* The kernel maps are inaccurate - assume objdump is right in
--
2.13.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v3 4/6] perf test: Decompress kernel module before objdump
2017-06-07 15:39 ` [PATCH v3 4/6] perf test: Decompress kernel module before objdump Namhyung Kim
@ 2017-06-07 22:25 ` Arnaldo Carvalho de Melo
2017-06-07 23:57 ` Namhyung Kim
2017-06-08 0:37 ` Namhyung Kim
0 siblings, 2 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-06-07 22:25 UTC (permalink / raw)
To: Namhyung Kim
Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team,
David Ahern, Adrian Hunter, Wang Nan
Em Thu, Jun 08, 2017 at 12:39:01AM +0900, Namhyung Kim escreveu:
> If a kernel modules is compressed, it should be decompressed before
> running objdump to parse binary data correctly. This fixes a failure of
> object code reading test for me.
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/tests/code-reading.c | 20 +++++++++++++++++++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
> index 1f14e7612cbb..94b7c7b02bde 100644
> --- a/tools/perf/tests/code-reading.c
> +++ b/tools/perf/tests/code-reading.c
> @@ -229,6 +229,8 @@ static int read_object_code(u64 addr, size_t len, u8 cpumode,
> unsigned char buf2[BUFSZ];
> size_t ret_len;
> u64 objdump_addr;
> + const char *objdump_name;
> + char decomp_name[KMOD_DECOMP_LEN];
> int ret;
>
> pr_debug("Reading object code for memory address: %#"PRIx64"\n", addr);
> @@ -289,9 +291,25 @@ static int read_object_code(u64 addr, size_t len, u8 cpumode,
> state->done[state->done_cnt++] = al.map->start;
> }
>
> + objdump_name = al.map->dso->long_name;
> + if (dso__needs_decompress(al.map->dso)) {
> + if (dso__decompress_kmodule_path(al.map->dso, objdump_name,
> + decomp_name,
> + sizeof(decomp_name)) < 0) {
> + pr_debug("decompression failed\n");
This is a too vague message, to help with debugging I suggest printing
more detauls such as the file being decompressed and perhaps the
decomp_name as well.
- Arnaldo
> + return -1;
> + }
> +
> + objdump_name = decomp_name;
> + }
> +
> /* Read the object code using objdump */
> objdump_addr = map__rip_2objdump(al.map, al.addr);
> - ret = read_via_objdump(al.map->dso->long_name, objdump_addr, buf2, len);
> + ret = read_via_objdump(objdump_name, objdump_addr, buf2, len);
> +
> + if (dso__needs_decompress(al.map->dso))
> + unlink(objdump_name);
> +
> if (ret > 0) {
> /*
> * The kernel maps are inaccurate - assume objdump is right in
> --
> 2.13.0
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v3 4/6] perf test: Decompress kernel module before objdump
2017-06-07 22:25 ` Arnaldo Carvalho de Melo
@ 2017-06-07 23:57 ` Namhyung Kim
2017-06-08 0:37 ` Namhyung Kim
1 sibling, 0 replies; 12+ messages in thread
From: Namhyung Kim @ 2017-06-07 23:57 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team,
David Ahern, Adrian Hunter, Wang Nan
On Wed, Jun 07, 2017 at 07:25:20PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Jun 08, 2017 at 12:39:01AM +0900, Namhyung Kim escreveu:
> > If a kernel modules is compressed, it should be decompressed before
> > running objdump to parse binary data correctly. This fixes a failure of
> > object code reading test for me.
> >
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> > tools/perf/tests/code-reading.c | 20 +++++++++++++++++++-
> > 1 file changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
> > index 1f14e7612cbb..94b7c7b02bde 100644
> > --- a/tools/perf/tests/code-reading.c
> > +++ b/tools/perf/tests/code-reading.c
> > @@ -229,6 +229,8 @@ static int read_object_code(u64 addr, size_t len, u8 cpumode,
> > unsigned char buf2[BUFSZ];
> > size_t ret_len;
> > u64 objdump_addr;
> > + const char *objdump_name;
> > + char decomp_name[KMOD_DECOMP_LEN];
> > int ret;
> >
> > pr_debug("Reading object code for memory address: %#"PRIx64"\n", addr);
> > @@ -289,9 +291,25 @@ static int read_object_code(u64 addr, size_t len, u8 cpumode,
> > state->done[state->done_cnt++] = al.map->start;
> > }
> >
> > + objdump_name = al.map->dso->long_name;
> > + if (dso__needs_decompress(al.map->dso)) {
> > + if (dso__decompress_kmodule_path(al.map->dso, objdump_name,
> > + decomp_name,
> > + sizeof(decomp_name)) < 0) {
> > + pr_debug("decompression failed\n");
>
> This is a too vague message, to help with debugging I suggest printing
> more detauls such as the file being decompressed and perhaps the
> decomp_name as well.
Will add file name, but decomp_name might not be set on the failure path.
Thanks,
Namhyung
>
> > + return -1;
> > + }
> > +
> > + objdump_name = decomp_name;
> > + }
> > +
> > /* Read the object code using objdump */
> > objdump_addr = map__rip_2objdump(al.map, al.addr);
> > - ret = read_via_objdump(al.map->dso->long_name, objdump_addr, buf2, len);
> > + ret = read_via_objdump(objdump_name, objdump_addr, buf2, len);
> > +
> > + if (dso__needs_decompress(al.map->dso))
> > + unlink(objdump_name);
> > +
> > if (ret > 0) {
> > /*
> > * The kernel maps are inaccurate - assume objdump is right in
> > --
> > 2.13.0
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v3 4/6] perf test: Decompress kernel module before objdump
2017-06-07 22:25 ` Arnaldo Carvalho de Melo
2017-06-07 23:57 ` Namhyung Kim
@ 2017-06-08 0:37 ` Namhyung Kim
1 sibling, 0 replies; 12+ messages in thread
From: Namhyung Kim @ 2017-06-08 0:37 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team,
David Ahern, Adrian Hunter, Wang Nan
On Wed, Jun 07, 2017 at 07:25:20PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Jun 08, 2017 at 12:39:01AM +0900, Namhyung Kim escreveu:
> > If a kernel modules is compressed, it should be decompressed before
> > running objdump to parse binary data correctly. This fixes a failure of
> > object code reading test for me.
> >
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> > tools/perf/tests/code-reading.c | 20 +++++++++++++++++++-
> > 1 file changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
> > index 1f14e7612cbb..94b7c7b02bde 100644
> > --- a/tools/perf/tests/code-reading.c
> > +++ b/tools/perf/tests/code-reading.c
> > @@ -229,6 +229,8 @@ static int read_object_code(u64 addr, size_t len, u8 cpumode,
> > unsigned char buf2[BUFSZ];
> > size_t ret_len;
> > u64 objdump_addr;
> > + const char *objdump_name;
> > + char decomp_name[KMOD_DECOMP_LEN];
> > int ret;
> >
> > pr_debug("Reading object code for memory address: %#"PRIx64"\n", addr);
> > @@ -289,9 +291,25 @@ static int read_object_code(u64 addr, size_t len, u8 cpumode,
> > state->done[state->done_cnt++] = al.map->start;
> > }
> >
> > + objdump_name = al.map->dso->long_name;
> > + if (dso__needs_decompress(al.map->dso)) {
> > + if (dso__decompress_kmodule_path(al.map->dso, objdump_name,
> > + decomp_name,
> > + sizeof(decomp_name)) < 0) {
> > + pr_debug("decompression failed\n");
>
> This is a too vague message, to help with debugging I suggest printing
> more detauls such as the file being decompressed and perhaps the
> decomp_name as well.
Looking at the code, it already printed the filename so current
message seems ok.
$ sudo perf test -v 'code reading'
22: Object code reading :
--- start ---
...
Reading object code for memory address: 0xffffffffa0305c56
File is: /lib/modules/4.11.3-1-ARCH/kernel/fs/btrfs/btrfs.ko.gz
On file address is: 0x61cc6
decompression failed
test child finished with -1
---- end ----
Object code reading: FAILED!
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3 5/6] perf symbols: Keep DSO->symtab_type after decompress
2017-06-07 15:38 [PATCHSET v3 0/6] perf tools: Fix for Compressed kernel modules Namhyung Kim
` (3 preceding siblings ...)
2017-06-07 15:39 ` [PATCH v3 4/6] perf test: Decompress kernel module before objdump Namhyung Kim
@ 2017-06-07 15:39 ` Namhyung Kim
2017-06-07 15:39 ` [PATCH v3 6/6] perf symbols: Kill dso__build_id_is_kmod() Namhyung Kim
5 siblings, 0 replies; 12+ messages in thread
From: Namhyung Kim @ 2017-06-07 15:39 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team,
David Ahern, Adrian Hunter, Wang Nan
The symsrc__init() overwrites dso->symtab_type as symsrc->type in
dso__load_sym(). But for compressed kernel modules in the build-id
cache, it should have original symtab type to be decompressed as needed.
This fixes perf annotate to show disassembly of the function properly.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/symbol-elf.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index d342e771dbad..502505cf236a 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -671,6 +671,8 @@ int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name,
fd = dso__decompress_kmodule_fd(dso, name);
if (fd < 0)
return -1;
+
+ type = dso->symtab_type;
} else {
fd = open(name, O_RDONLY);
if (fd < 0) {
--
2.13.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v3 6/6] perf symbols: Kill dso__build_id_is_kmod()
2017-06-07 15:38 [PATCHSET v3 0/6] perf tools: Fix for Compressed kernel modules Namhyung Kim
` (4 preceding siblings ...)
2017-06-07 15:39 ` [PATCH v3 5/6] perf symbols: Keep DSO->symtab_type after decompress Namhyung Kim
@ 2017-06-07 15:39 ` Namhyung Kim
5 siblings, 0 replies; 12+ messages in thread
From: Namhyung Kim @ 2017-06-07 15:39 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team,
David Ahern, Adrian Hunter, Wang Nan
The commit e7ee40475760 ("perf symbols: Fix symbols searching for module
in buildid-cache") added the function to check kernel modules reside in
the build-id cache. This was because there's no way to identify a DSO
which is actually a kernel module. So it searched linkname of the file
and find ".ko" suffix.
But this does not work for compressed kernel modules and now such DSOs
have correct symtab_type now. So no need to check it anymore. This
patch essentially reverts the commit.
Cc: Wang Nan <wangnan0@huawei.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/build-id.c | 45 ---------------------------------------------
tools/perf/util/build-id.h | 1 -
tools/perf/util/symbol.c | 4 ----
3 files changed, 50 deletions(-)
diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index 168cc49654e7..e0148b081bdf 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -278,51 +278,6 @@ char *dso__build_id_filename(const struct dso *dso, char *bf, size_t size)
return bf;
}
-bool dso__build_id_is_kmod(const struct dso *dso, char *bf, size_t size)
-{
- char *id_name = NULL, *ch;
- struct stat sb;
- char sbuild_id[SBUILD_ID_SIZE];
-
- if (!dso->has_build_id)
- goto err;
-
- build_id__sprintf(dso->build_id, sizeof(dso->build_id), sbuild_id);
- id_name = build_id_cache__linkname(sbuild_id, NULL, 0);
- if (!id_name)
- goto err;
- if (access(id_name, F_OK))
- goto err;
- if (lstat(id_name, &sb) == -1)
- goto err;
- if ((size_t)sb.st_size > size - 1)
- goto err;
- if (readlink(id_name, bf, size - 1) < 0)
- goto err;
-
- bf[sb.st_size] = '\0';
-
- /*
- * link should be:
- * ../../lib/modules/4.4.0-rc4/kernel/net/ipv4/netfilter/nf_nat_ipv4.ko/a09fe3eb3147dafa4e3b31dbd6257e4d696bdc92
- */
- ch = strrchr(bf, '/');
- if (!ch)
- goto err;
- if (ch - 3 < bf)
- goto err;
-
- free(id_name);
- return strncmp(".ko", ch - 3, 3) == 0;
-err:
- pr_err("Invalid build id: %s\n", id_name ? :
- dso->long_name ? :
- dso->short_name ? :
- "[unknown]");
- free(id_name);
- return false;
-}
-
#define dsos__for_each_with_build_id(pos, head) \
list_for_each_entry(pos, head, node) \
if (!pos->has_build_id) \
diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h
index 8a89b195c1fc..96690a55c62c 100644
--- a/tools/perf/util/build-id.h
+++ b/tools/perf/util/build-id.h
@@ -17,7 +17,6 @@ char *build_id_cache__kallsyms_path(const char *sbuild_id, char *bf,
size_t size);
char *dso__build_id_filename(const struct dso *dso, char *bf, size_t size);
-bool dso__build_id_is_kmod(const struct dso *dso, char *bf, size_t size);
int build_id__mark_dso_hit(struct perf_tool *tool, union perf_event *event,
struct perf_sample *sample, struct perf_evsel *evsel,
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 8f2b068ff756..e7a98dbd2aed 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -1562,10 +1562,6 @@ int dso__load(struct dso *dso, struct map *map)
if (!runtime_ss && syms_ss)
runtime_ss = syms_ss;
- if (syms_ss && syms_ss->type == DSO_BINARY_TYPE__BUILD_ID_CACHE)
- if (dso__build_id_is_kmod(dso, name, PATH_MAX))
- kmod = true;
-
if (syms_ss)
ret = dso__load_sym(dso, map, syms_ss, runtime_ss, kmod);
else
--
2.13.0
^ permalink raw reply related [flat|nested] 12+ messages in thread