* [PATCH 1/2] perf tools: Decompress kernel module when reading DSO data
@ 2017-06-01 9:06 Namhyung Kim
2017-06-01 9:06 ` [PATCH 2/2] perf test: Decompress kernel module before objdump Namhyung Kim
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Namhyung Kim @ 2017-06-01 9:06 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team,
David Ahern, Adrian Hunter
Currently perf decompresses kernel modules when loading symbol table but
it missed to do it when reading raw data. Move decompress_kmodule() to
util/dso.c and do not unlink it immediately.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/dso.c | 56 ++++++++++++++++++++++++++++++++++++++++++++
tools/perf/util/dso.h | 2 ++
tools/perf/util/symbol-elf.c | 40 ++++---------------------------
3 files changed, 63 insertions(+), 35 deletions(-)
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index b27d127cdf68..8dc1f147547c 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -350,6 +350,45 @@ void dso__set_module_info(struct dso *dso, struct kmod_path *m,
dso__set_short_name(dso, strdup(m->name), true);
}
+int dso__decompress_kmodule(struct dso *dso, const char *filename,
+ char *buf, size_t sz)
+{
+ struct kmod_path m = {};
+ int ret = -1;
+ int fd = -1;
+
+ if (!dso__needs_decompress(dso))
+ return -1;
+
+ if (kmod_path__parse_ext(&m, filename) < 0)
+ goto out;
+
+ if (!is_supported_compression(m.ext))
+ goto out;
+
+ snprintf(buf, sz, "/tmp/perf-kmod-XXXXXX");
+
+ fd = mkstemp(buf);
+ if (fd < 0) {
+ dso->load_errno = errno;
+ goto out;
+ }
+
+ pr_debug("decompressing to %s\n", buf);
+ if (!decompress_to_file(m.ext, filename, fd)) {
+ dso->load_errno = DSO_LOAD_ERRNO__DECOMPRESSION_FAILURE;
+ unlink(buf);
+ close(fd);
+ goto out;
+ }
+ ret = fd;
+
+out:
+ free(m.ext);
+
+ return ret;
+}
+
/*
* Global list of open DSOs and the counter.
*/
@@ -399,6 +438,7 @@ static int __open_dso(struct dso *dso, struct machine *machine)
int fd;
char *root_dir = (char *)"";
char *name = malloc(PATH_MAX);
+ bool needs_unlink = false;
if (!name)
return -ENOMEM;
@@ -415,7 +455,23 @@ static int __open_dso(struct dso *dso, struct machine *machine)
if (!is_regular_file(name))
return -EINVAL;
+ if (dso__needs_decompress(dso)) {
+ char buf[32];
+
+ fd = dso__decompress_kmodule(dso, name, buf, sizeof(buf));
+ if (fd < 0)
+ return -1;
+
+ close(fd);
+ strcpy(name, buf);
+ needs_unlink = true;
+ }
+
fd = do_open(name);
+
+ if (needs_unlink)
+ unlink(name);
+
free(name);
return fd;
}
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index 5fe2ab5877bd..c1e4b94d41aa 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -244,6 +244,8 @@ 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(struct dso *dso, const char *filename,
+ char *buf, size_t sz);
struct kmod_path {
char *name;
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 1fb2efae4f02..b335f91f0cb3 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,9 +668,13 @@ 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);
+ char buf[32];
+
+ fd = dso__decompress_kmodule(dso, name, buf, sizeof(buf));
if (fd < 0)
return -1;
+
+ unlink(buf);
} else {
fd = open(name, O_RDONLY);
if (fd < 0) {
--
2.13.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] perf test: Decompress kernel module before objdump
2017-06-01 9:06 [PATCH 1/2] perf tools: Decompress kernel module when reading DSO data Namhyung Kim
@ 2017-06-01 9:06 ` Namhyung Kim
2017-06-01 11:29 ` [PATCH 1/2] perf tools: Decompress kernel module when reading DSO data Jiri Olsa
2017-06-01 11:29 ` Jiri Olsa
2 siblings, 0 replies; 6+ messages in thread
From: Namhyung Kim @ 2017-06-01 9:06 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team,
David Ahern, Adrian Hunter
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 | 29 ++++++++++++++++++++++++++++-
1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
index 1f14e7612cbb..5031acdf56dd 100644
--- a/tools/perf/tests/code-reading.c
+++ b/tools/perf/tests/code-reading.c
@@ -229,6 +229,7 @@ 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;
int ret;
pr_debug("Reading object code for memory address: %#"PRIx64"\n", addr);
@@ -289,9 +290,35 @@ 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)) {
+ char buf[32]; /* for "/tmp/perf-kmod-XXXXXX" */
+ int fd;
+
+ fd = dso__decompress_kmodule(al.map->dso, objdump_name,
+ buf, sizeof(buf));
+ if (fd < 0) {
+ pr_debug("decompression failed\n");
+ return -1;
+ }
+ close(fd);
+
+ objdump_name = strdup(buf);
+ if (objdump_name == NULL) {
+ pr_debug("memory allocation failed\n");
+ return -1;
+ }
+ }
+
/* 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 (objdump_name != al.map->dso->long_name) {
+ unlink(objdump_name);
+ free((void *)objdump_name);
+ }
+
if (ret > 0) {
/*
* The kernel maps are inaccurate - assume objdump is right in
--
2.13.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] perf tools: Decompress kernel module when reading DSO data
2017-06-01 9:06 [PATCH 1/2] perf tools: Decompress kernel module when reading DSO data Namhyung Kim
2017-06-01 9:06 ` [PATCH 2/2] perf test: Decompress kernel module before objdump Namhyung Kim
@ 2017-06-01 11:29 ` Jiri Olsa
2017-06-03 1:41 ` Namhyung Kim
2017-06-01 11:29 ` Jiri Olsa
2 siblings, 1 reply; 6+ messages in thread
From: Jiri Olsa @ 2017-06-01 11:29 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra, Jiri Olsa,
LKML, kernel-team, David Ahern, Adrian Hunter
On Thu, Jun 01, 2017 at 06:06:04PM +0900, Namhyung Kim wrote:
SNIP
> @@ -399,6 +438,7 @@ static int __open_dso(struct dso *dso, struct machine *machine)
> int fd;
> char *root_dir = (char *)"";
> char *name = malloc(PATH_MAX);
> + bool needs_unlink = false;
>
> if (!name)
> return -ENOMEM;
> @@ -415,7 +455,23 @@ static int __open_dso(struct dso *dso, struct machine *machine)
> if (!is_regular_file(name))
> return -EINVAL;
>
> + if (dso__needs_decompress(dso)) {
> + char buf[32];
> +
> + fd = dso__decompress_kmodule(dso, name, buf, sizeof(buf));
> + if (fd < 0)
free(name)
> + return -1;
> +
> + close(fd);
> + strcpy(name, buf);
also why not use name directly instead of buf?
jirka
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] perf tools: Decompress kernel module when reading DSO data
2017-06-01 9:06 [PATCH 1/2] perf tools: Decompress kernel module when reading DSO data Namhyung Kim
2017-06-01 9:06 ` [PATCH 2/2] perf test: Decompress kernel module before objdump Namhyung Kim
2017-06-01 11:29 ` [PATCH 1/2] perf tools: Decompress kernel module when reading DSO data Jiri Olsa
@ 2017-06-01 11:29 ` Jiri Olsa
2017-06-03 1:43 ` Namhyung Kim
2 siblings, 1 reply; 6+ messages in thread
From: Jiri Olsa @ 2017-06-01 11:29 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra, Jiri Olsa,
LKML, kernel-team, David Ahern, Adrian Hunter
On Thu, Jun 01, 2017 at 06:06:04PM +0900, Namhyung Kim wrote:
SNIP
> - }
> -
> - unlink(tmpbuf);
> -
> -out:
> - free(m.ext);
> - return fd;
> -}
> -
> bool symsrc__possibly_runtime(struct symsrc *ss)
> {
> return ss->dynsym || ss->opdsec;
> @@ -702,9 +668,13 @@ 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);
> + char buf[32];
> +
> + fd = dso__decompress_kmodule(dso, name, buf, sizeof(buf));
> if (fd < 0)
> return -1;
> +
> + unlink(buf);
I wonder we could make this more clear and add:
dso__decompress_kmodule_fd
dso__decompress_kmodule_path
_fd version would remove the file and _path version would close the descriptor
in any case, could you please split the change so the
changes to decompress_kmodule are apparent?
thanks,
jirka
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] perf tools: Decompress kernel module when reading DSO data
2017-06-01 11:29 ` [PATCH 1/2] perf tools: Decompress kernel module when reading DSO data Jiri Olsa
@ 2017-06-03 1:41 ` Namhyung Kim
0 siblings, 0 replies; 6+ messages in thread
From: Namhyung Kim @ 2017-06-03 1:41 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra, Jiri Olsa,
LKML, kernel-team, David Ahern, Adrian Hunter
Hi Jiri,
On Thu, Jun 1, 2017 at 8:29 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> On Thu, Jun 01, 2017 at 06:06:04PM +0900, Namhyung Kim wrote:
>
> SNIP
>
>> @@ -399,6 +438,7 @@ static int __open_dso(struct dso *dso, struct machine *machine)
>> int fd;
>> char *root_dir = (char *)"";
>> char *name = malloc(PATH_MAX);
>> + bool needs_unlink = false;
>>
>> if (!name)
>> return -ENOMEM;
>> @@ -415,7 +455,23 @@ static int __open_dso(struct dso *dso, struct machine *machine)
>> if (!is_regular_file(name))
>> return -EINVAL;
>>
>> + if (dso__needs_decompress(dso)) {
>> + char buf[32];
>> +
>> + fd = dso__decompress_kmodule(dso, name, buf, sizeof(buf));
>> + if (fd < 0)
>
> free(name)
ok.
>
>> + return -1;
>> +
>> + close(fd);
>> + strcpy(name, buf);
>
> also why not use name directly instead of buf?
The name should have original compressed filename and buf will be set
to a new decompressed name.
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] perf tools: Decompress kernel module when reading DSO data
2017-06-01 11:29 ` Jiri Olsa
@ 2017-06-03 1:43 ` Namhyung Kim
0 siblings, 0 replies; 6+ messages in thread
From: Namhyung Kim @ 2017-06-03 1:43 UTC (permalink / raw)
To: Jiri Olsa
Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra, Jiri Olsa,
LKML, kernel-team, David Ahern, Adrian Hunter
On Thu, Jun 1, 2017 at 8:29 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> On Thu, Jun 01, 2017 at 06:06:04PM +0900, Namhyung Kim wrote:
>
> SNIP
>
>> - }
>> -
>> - unlink(tmpbuf);
>> -
>> -out:
>> - free(m.ext);
>> - return fd;
>> -}
>> -
>> bool symsrc__possibly_runtime(struct symsrc *ss)
>> {
>> return ss->dynsym || ss->opdsec;
>> @@ -702,9 +668,13 @@ 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);
>> + char buf[32];
>> +
>> + fd = dso__decompress_kmodule(dso, name, buf, sizeof(buf));
>> if (fd < 0)
>> return -1;
>> +
>> + unlink(buf);
>
> I wonder we could make this more clear and add:
>
> dso__decompress_kmodule_fd
> dso__decompress_kmodule_path
>
> _fd version would remove the file and _path version would close the descriptor
>
> in any case, could you please split the change so the
> changes to decompress_kmodule are apparent?
Will do.
Thanks,
Namhyung
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-06-03 1:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-01 9:06 [PATCH 1/2] perf tools: Decompress kernel module when reading DSO data Namhyung Kim
2017-06-01 9:06 ` [PATCH 2/2] perf test: Decompress kernel module before objdump Namhyung Kim
2017-06-01 11:29 ` [PATCH 1/2] perf tools: Decompress kernel module when reading DSO data Jiri Olsa
2017-06-03 1:41 ` Namhyung Kim
2017-06-01 11:29 ` Jiri Olsa
2017-06-03 1:43 ` Namhyung Kim
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome