perf tools: Decompress kernel module when reading DSO data
Namhyung Kim
maintainerPATCH 1/2 perf tools: Decompress kernel module when reading DSO data
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.
--- 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(-)
tools/perf/util/dso.c +56 -0
@@ -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; }
tools/perf/util/dso.h +2 -0
@@ -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;
tools/perf/util/symbol-elf.c +5 -35
@@ -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 ) {
Signature · 1 line
2.13.0
Namhyung Kim
maintainerPATCH 2/2 perf test: Decompress kernel module before objdump
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.
--- tools/perf/tests/code-reading.c | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-)
tools/perf/tests/code-reading.c +28 -1
@@ -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
Signature · 1 line
2.13.0
On Thu, Jun 01, 2017 at 06:06:04PM +0900, Namhyung Kim wrote:
SNIP
Quoted 9 lines
tools/perf/util/dso.c +1
-0@@ -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 ;
tools/perf/util/dso.c static int __open_dso(struct dso *dso, struct machine *machine) · line 462
Quoted 8 lines
tools/perf/util/dso.c +5
-0@@ -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)
tools/perf/util/dso.c static int __open_dso(struct dso *dso, struct machine *machine) · line 466
Quoted 4 lines
tools/perf/util/dso.c +4
-0+return -1 ; + +close (fd); +strcpy (name, buf);
also why not use name directly instead of buf?
jirka
Namhyung Kim
maintainerHi Jiri,
Quoted 16 lines · Jiri Olsa
On Thu, Jun 01, 2017 at 06:06:04PM +0900, Namhyung Kim wrote:
SNIP
Quoted 12 lines
tools/perf/util/dso.c +1
-0@@ -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 ;
Quoted 7 lines
tools/perf/util/dso.c static int __open_dso(struct dso *dso, struct machine *machine) · line 462
Quoted 5 lines
tools/perf/util/dso.c +5
-0+if (dso__needs_decompress (dso)) { +char buf[32 ]; + + fd =dso__decompress_kmodule (dso, name, buf,sizeof (buf)); +if (fd <0 )free(name)
ok.
Quoted 7 lines
tools/perf/util/dso.c static int __open_dso(struct dso *dso, struct machine *machine) · line 466
Quoted 5 lines
tools/perf/util/dso.c +4
-0+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
On Thu, Jun 01, 2017 at 06:06:04PM +0900, Namhyung Kim wrote:
SNIP
Quoted 14 lines
tools/perf/util/symbol-elf.c +0
-9- } - -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;
tools/perf/util/symbol-elf.c int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name, · line 677
Quoted 10 lines
tools/perf/util/symbol-elf.c +5
-1if (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
Namhyung Kim
maintainerQuoted 31 lines · Jiri Olsa
On Thu, Jun 01, 2017 at 06:06:04PM +0900, Namhyung Kim wrote:
SNIP
Quoted 14 lines
tools/perf/util/symbol-elf.c +0
-9- } - -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;tools/perf/util/symbol-elf.c int symsrc__init(struct symsrc *ss, struct dso *dso, const char *name, · line 677
Quoted 10 lines
tools/perf/util/symbol-elf.c +5
-1if (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:
Quoted 7 lines
dso__decompress_kmodule_fd
dso__decompress_kmodule_path
_fdversion would remove the file and_pathversion would close the descriptorin any case, could you please split the change so the changes to
decompress_kmoduleare apparent?
Will do.
Thanks,
Namhyung