perf tools: Fix for compressed kernel modules (v4)
Namhyung Kim
maintainercover perf tools: Fix for compressed kernel modules (v4)
Hello,
This is v4 of my compressed kernel module work. Please take a look.
* changes in v4)
- fix build-id cache symbolic link handling in annotate
- separate fix for a memory leak (Arnaldo)
- consolidate error path in __open_dso (Arnaldo)
- remove decompressed file after use
The code is avaiable at 'perf/kmod-decomp-v4' branch in my tree:
git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
Thanks,
Namhyung
Namhyung Kim (9):
perf annotate: Fix symbolic link of build-id cache perf tools: Fix a memory leak in __open_dso() perf tools: Introduce dso__decompress_kmodule_{fd,path} perf annotate: Use dso__decompress_kmodule_path() perf tools: Decompress kernel module when reading DSO data perf tools: Consolidate error path in __open_dso() perf test: Decompress kernel module before objdump perf symbols: Keep DSO->symtab_type after decompress perf symbols: Kill dso__build_id_is_kmod()
tools/perf/tests/code-reading.c | 20 +++++++++- tools/perf/util/annotate.c | 37 ++++++------------ tools/perf/util/build-id.c | 45 ---------------------- tools/perf/util/build-id.h | 1 - tools/perf/util/dso.c | 85 ++++++++++++++++++++++++++++++++++++++--- tools/perf/util/dso.h | 6 +++ tools/perf/util/symbol-elf.c | 38 ++---------------- tools/perf/util/symbol.c | 4 -- 8 files changed, 119 insertions(+), 117 deletions(-)
Signature · 1 line
2.13.1
Namhyung Kim
maintainerPATCH v4 1/9 perf annotate: Fix symbolic link of build-id cache
The commit 6ebd2547dd24 ("perf annotate: Fix a bug following symbolic link of a build-id file") changed to use dirname to follow the symlink. But it only considers new-style build-id cache names so old names fail on readlink() and force to use system path which might not available.
--- tools/perf/util/annotate.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
tools/perf/util/annotate.c +9 -1
@@ -1321,6 +1321,7 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t filchar linkname[PATH_MAX ];char *build_id_filename;char *build_id_path =NULL ; +char *pos;if (dso->symtab_type ==DSO_BINARY_TYPE__KALLSYMS && !dso__is_kcore (dso))
@@ -1340,7 +1341,14 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t filif (!build_id_path)return -1 ; -dirname (build_id_path); +/* +* old style build-id cache has name of XX/XXXXXXX.. while +* new style has XX/XXXXXXX../{elf,kallsyms,vdso}. +* extract the build-id part of dirname in the new style only. +*/ + pos =strrchr (build_id_path,'/' ); +if (pos &&strlen (pos) <SBUILD_ID_SIZE -2 ) +dirname (build_id_path); if (dso__is_kcore(dso) || readlink(build_id_path, linkname, sizeof(linkname)) < 0 ||
Signature · 1 line
2.13.1
tip-bot for Namhyung Kim
merge bot[tip:perf/urgent] perf annotate: Fix symbolic link of build-id cache
perf annotate: Fix symbolic link of build-id cache
Commit-ID: 3619ef76b37d4803bc9daee9d03d82c8526db378 Gitweb: http://git.kernel.org/tip/3619ef76b37d4803bc9daee9d03d82c8526db378 Author: Namhyung Kim <[email protected]> AuthorDate: Thu, 8 Jun 2017 16:31:01 +0900 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Thu, 8 Jun 2017 15:38:41 -0300
perf annotate: Fix symbolic link of build-id cache
The commit 6ebd2547dd24 ("perf annotate: Fix a bug following symbolic link of a build-id file") changed to use dirname to follow the symlink. But it only considers new-style build-id cache names so old names fail on readlink() and force to use system path which might not available.
--- tools/perf/util/annotate.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
tools/perf/util/annotate.c +9 -1
@@ -1321,6 +1321,7 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t filchar linkname[PATH_MAX ];char *build_id_filename;char *build_id_path =NULL ; +char *pos;if (dso->symtab_type ==DSO_BINARY_TYPE__KALLSYMS && !dso__is_kcore (dso))
@@ -1340,7 +1341,14 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t filif (!build_id_path)return -1 ; -dirname (build_id_path); +/* +* old style build-id cache has name of XX/XXXXXXX.. while +* new style has XX/XXXXXXX../{elf,kallsyms,vdso}. +* extract the build-id part of dirname in the new style only. +*/ + pos =strrchr (build_id_path,'/' ); +if (pos &&strlen (pos) <SBUILD_ID_SIZE -2 ) +dirname (build_id_path); if (dso__is_kcore(dso) || readlink(build_id_path, linkname, sizeof(linkname)) < 0 ||
Namhyung Kim
maintainerPATCH v4 2/9 perf tools: Fix a memory leak in __open_dso()
In reply to Namhyung KimThe name should be freed on error path.
--- tools/perf/util/dso.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
tools/perf/util/dso.c +3 -1
@@ -412,8 +412,10 @@ 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 ; + } fd =do_open (name);free (name);
Signature · 1 line
2.13.1
tip-bot for Namhyung Kim
merge bot[tip:perf/urgent] perf tools: Fix a memory leak in __open_dso()
perf tools: Fix a memory leak in __open_dso()
Commit-ID: 44ad6b8852529eb39066edbedc027a6901da6803 Gitweb: http://git.kernel.org/tip/44ad6b8852529eb39066edbedc027a6901da6803 Author: Namhyung Kim <[email protected]> AuthorDate: Thu, 8 Jun 2017 16:31:02 +0900 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Thu, 8 Jun 2017 15:38:47 -0300
perf tools: Fix a memory leak in __open_dso()
The 'name' variable should be freed on the error path.
--- tools/perf/util/dso.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
tools/perf/util/dso.c +3 -1
@@ -412,8 +412,10 @@ 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 ; + } fd =do_open (name);free (name);
Namhyung Kim
maintainerPATCH v4 3/9 perf tools: Introduce dso__decompress_kmodule_{fd,path}
In reply to Namhyung KimMove 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.
--- tools/perf/util/dso.c | 58 ++++++++++++++++++++++++++++++++++++++++++++ tools/perf/util/dso.h | 6 +++++ tools/perf/util/symbol-elf.c | 36 +-------------------------- 3 files changed, 65 insertions(+), 35 deletions(-)
tools/perf/util/dso.c +58 -0
@@ -248,6 +248,64 @@ 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)) +return -1 ; + +if (!m.comp) + goto out; + + 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); +if (fd <0 ) { +unlink (tmpbuf); +return -1 ; + } + +strncpy (pathname, tmpbuf, len); +close (fd); +return 0 ; +} +/* * Parses kernel module specified in @path and updates * @m argument like:
tools/perf/util/dso.h +6 -0
@@ -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;
tools/perf/util/symbol-elf.c +1 -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,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 {
Signature · 1 line
2.13.1
tip-bot for Namhyung Kim
merge bot[tip:perf/urgent] perf tools: Introduce dso__decompress_kmodule_{fd,path}
perf tools: Introduce dso__decompress_kmodule_{fd,path}
Commit-ID: 42b3fa670825983fc8bd0ac7b80cc84ae3abb75b Gitweb: http://git.kernel.org/tip/42b3fa670825983fc8bd0ac7b80cc84ae3abb75b Author: Namhyung Kim <[email protected]> AuthorDate: Thu, 8 Jun 2017 16:31:03 +0900 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Thu, 8 Jun 2017 15:38:55 -0300
perf tools: Introduce dso__decompress_kmodule_{fd,path}
Move decompress_kmodule() to util/dso.c and split it into two functions returning fd and (decompressed) file path. The existing user only wants the fd version but the path version will be used soon.
--- tools/perf/util/dso.c | 58 ++++++++++++++++++++++++++++++++++++++++++++ tools/perf/util/dso.h | 6 +++++ tools/perf/util/symbol-elf.c | 36 +-------------------------- 3 files changed, 65 insertions(+), 35 deletions(-)
tools/perf/util/dso.c +58 -0
@@ -248,6 +248,64 @@ 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)) +return -1 ; + +if (!m.comp) + goto out; + + 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); +if (fd <0 ) { +unlink (tmpbuf); +return -1 ; + } + +strncpy (pathname, tmpbuf, len); +close (fd); +return 0 ; +} +/* * Parses kernel module specified in @path and updates * @m argument like:
tools/perf/util/dso.h +6 -0
@@ -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;
tools/perf/util/symbol-elf.c +1 -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,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 {
Namhyung Kim
maintainerPATCH v4 4/9 perf annotate: Use dso__decompress_kmodule_path()
In reply to Namhyung KimConvert open-coded decompress routine to use the function.
--- tools/perf/util/annotate.c | 27 +++------------------------ 1 file changed, 3 insertions(+), 24 deletions(-)
tools/perf/util/annotate.c +3 -24
@@ -1431,31 +1431,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);
Signature · 1 line
2.13.1
tip-bot for Namhyung Kim
merge bot[tip:perf/urgent] perf annotate: Use dso__decompress_kmodule_path()
perf annotate: Use dso__decompress_kmodule_path()
Commit-ID: 3c84fd53044f98017271101b59a21ddb20fb312b Gitweb: http://git.kernel.org/tip/3c84fd53044f98017271101b59a21ddb20fb312b Author: Namhyung Kim <[email protected]> AuthorDate: Thu, 8 Jun 2017 16:31:04 +0900 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Thu, 8 Jun 2017 15:39:02 -0300
perf annotate: Use dso__decompress_kmodule_path()
Convert open-coded decompress routine to use the function.
--- tools/perf/util/annotate.c | 27 +++------------------------ 1 file changed, 3 insertions(+), 24 deletions(-)
tools/perf/util/annotate.c +3 -24
@@ -1431,31 +1431,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);
Namhyung Kim
maintainerPATCH v4 5/9 perf tools: Decompress kernel module when reading DSO data
In reply to Namhyung KimCurrently perf decompresses kernel modules when loading symbol table but it missed to do it when reading raw data.
--- tools/perf/util/dso.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
tools/perf/util/dso.c +16 -0
@@ -475,7 +475,23 @@ static int __open_dso(struct dso *dso, struct machine *machine)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 -dso->load_errno; + } + +strcpy (name, newpath); + } + fd =do_open (name); + +if (dso__needs_decompress (dso)) +unlink (name); +free (name);return fd; }
Signature · 1 line
2.13.1
tip-bot for Namhyung Kim
merge bot[tip:perf/urgent] perf tools: Decompress kernel module when reading DSO data
perf tools: Decompress kernel module when reading DSO data
Commit-ID: 1d6b3c9ba756a5134fd7ad1959acac776d17404b Gitweb: http://git.kernel.org/tip/1d6b3c9ba756a5134fd7ad1959acac776d17404b Author: Namhyung Kim <[email protected]> AuthorDate: Thu, 8 Jun 2017 16:31:05 +0900 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Thu, 8 Jun 2017 15:39:07 -0300
perf tools: Decompress kernel module when reading DSO data
Currently perf decompresses kernel modules when loading the symbol table but it missed to do it when reading raw data.
--- tools/perf/util/dso.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
tools/perf/util/dso.c +16 -0
@@ -475,7 +475,23 @@ static int __open_dso(struct dso *dso, struct machine *machine)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 -dso->load_errno; + } + +strcpy (name, newpath); + } + fd =do_open (name); + +if (dso__needs_decompress (dso)) +unlink (name); +free (name);return fd; }
Namhyung Kim
maintainerPATCH v4 6/9 perf tools: Consolidate error path in __open_dso()
In reply to Namhyung KimOn failure, it should free the 'name', so clean up the error path using goto.
--- tools/perf/util/dso.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-)
tools/perf/util/dso.c +8 -11
@@ -454,7 +454,7 @@ static int do_open(char *name)static int __open_dso (struct dso *dso,struct machine *machine) { -int fd; +int fd = -EINVAL ;char *root_dir = (char *)"" ;char *name =malloc (PATH_MAX );
@@ -465,23 +465,19 @@ static int __open_dso(struct dso *dso, struct machine *machine) root_dir = machine->root_dir;if (dso__read_binary_type_filename (dso, dso->binary_type, - root_dir, name,PATH_MAX )) { -free (name); -return -EINVAL ; - } + root_dir, name,PATH_MAX )) + goto out; -if (!is_regular_file (name)) { -free (name); -return -EINVAL ; - } +if (!is_regular_file (name)) + goto out;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 -dso->load_errno; + fd = -dso->load_errno; + goto out; }strcpy (name, newpath);
@@ -492,6 +488,7 @@ static int __open_dso(struct dso *dso, struct machine *machine)if (dso__needs_decompress (dso))unlink (name); +out:free (name);return fd; }
Signature · 1 line
2.13.1
tip-bot for Namhyung Kim
merge bot[tip:perf/urgent] perf tools: Consolidate error path in __open_dso()
perf tools: Consolidate error path in __open_dso()
Commit-ID: 8ba29adf9a7cdff3c7283d5fc0dcf5e777d3b40f Gitweb: http://git.kernel.org/tip/8ba29adf9a7cdff3c7283d5fc0dcf5e777d3b40f Author: Namhyung Kim <[email protected]> AuthorDate: Thu, 8 Jun 2017 16:31:06 +0900 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Thu, 8 Jun 2017 15:39:13 -0300
perf tools: Consolidate error path in __open_dso()
On failure, it should free the 'name', so clean up the error path using goto.
--- tools/perf/util/dso.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-)
tools/perf/util/dso.c +8 -11
@@ -454,7 +454,7 @@ static int do_open(char *name)static int __open_dso (struct dso *dso,struct machine *machine) { -int fd; +int fd = -EINVAL ;char *root_dir = (char *)"" ;char *name =malloc (PATH_MAX );
@@ -465,23 +465,19 @@ static int __open_dso(struct dso *dso, struct machine *machine) root_dir = machine->root_dir;if (dso__read_binary_type_filename (dso, dso->binary_type, - root_dir, name,PATH_MAX )) { -free (name); -return -EINVAL ; - } + root_dir, name,PATH_MAX )) + goto out; -if (!is_regular_file (name)) { -free (name); -return -EINVAL ; - } +if (!is_regular_file (name)) + goto out;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 -dso->load_errno; + fd = -dso->load_errno; + goto out; }strcpy (name, newpath);
@@ -492,6 +488,7 @@ static int __open_dso(struct dso *dso, struct machine *machine)if (dso__needs_decompress (dso))unlink (name); +out:free (name);return fd; }
Namhyung Kim
maintainerPATCH v4 7/9 perf test: Decompress kernel module before objdump
In reply to Namhyung KimIf 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 | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)
tools/perf/tests/code-reading.c +19 -1
@@ -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
Signature · 1 line
2.13.1
Re: [PATCH v4 7/9] perf test: Decompress kernel module before objdump
Quoted 5 lines · Namhyung Kim
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 <[email protected]>
Quoted 45 lines
--- tools/perf/tests/code-reading.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)tools/perf/tests/code-reading.c +19
-1@@ -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
tip-bot for Namhyung Kim
merge bot[tip:perf/urgent] perf tests: Decompress kernel module before objdump
In reply to Namhyung Kimperf tests: Decompress kernel module before objdump
Commit-ID: 94df1040b1e6aacd8dec0ba3c61d7e77cd695f26 Gitweb: http://git.kernel.org/tip/94df1040b1e6aacd8dec0ba3c61d7e77cd695f26 Author: Namhyung Kim <[email protected]> AuthorDate: Thu, 8 Jun 2017 16:31:07 +0900 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Thu, 8 Jun 2017 15:39:19 -0300
perf tests: 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 | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)
tools/perf/tests/code-reading.c +19 -1
@@ -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
Namhyung Kim
maintainerPATCH v4 8/9 perf symbols: Keep DSO->symtab_type after decompress
In reply to Namhyung KimThe 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.
--- tools/perf/util/symbol-elf.c | 2 ++ 1 file changed, 2 insertions(+)
tools/perf/util/symbol-elf.c +2 -0
@@ -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 ) {
Signature · 1 line
2.13.1
tip-bot for Namhyung Kim
merge bot[tip:perf/urgent] perf symbols: Keep DSO->symtab_type after decompress
perf symbols: Keep DSO->symtab_type after decompress
Commit-ID: c25ec42f846f702f8f532fbc890171e3a1f6ec85 Gitweb: http://git.kernel.org/tip/c25ec42f846f702f8f532fbc890171e3a1f6ec85 Author: Namhyung Kim <[email protected]> AuthorDate: Thu, 8 Jun 2017 16:31:08 +0900 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Thu, 8 Jun 2017 15:39:26 -0300
perf symbols: Keep DSO->symtab_type after decompress
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.
--- tools/perf/util/symbol-elf.c | 2 ++ 1 file changed, 2 insertions(+)
tools/perf/util/symbol-elf.c +2 -0
@@ -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 ) {
Namhyung Kim
maintainerPATCH v4 9/9 perf symbols: Kill dso__build_id_is_kmod()
In reply to Namhyung KimThe 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.
--- 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(-)
tools/perf/util/build-id.c +0 -45
@@ -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) \
tools/perf/util/build-id.h +0 -1
@@ -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,
tools/perf/util/symbol.c +0 -4
@@ -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
Signature · 1 line
2.13.1
tip-bot for Namhyung Kim
merge bot[tip:perf/urgent] perf symbols: Kill dso__build_id_is_kmod()
perf symbols: Kill dso__build_id_is_kmod()
Commit-ID: b89fe63fbafe307fb72546f7a2320380bf41bdd4 Gitweb: http://git.kernel.org/tip/b89fe63fbafe307fb72546f7a2320380bf41bdd4 Author: Namhyung Kim <[email protected]> AuthorDate: Thu, 8 Jun 2017 16:31:09 +0900 Committer: Arnaldo Carvalho de Melo <[email protected]> CommitDate: Thu, 8 Jun 2017 15:39:34 -0300
perf symbols: Kill dso__build_id_is_kmod()
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 hCcave correct symtab_type now. So no need to check it anymore. This patch essentially reverts the commit.
--- 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(-)
tools/perf/util/build-id.c +0 -45
@@ -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) \
tools/perf/util/build-id.h +0 -1
@@ -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,
tools/perf/util/symbol.c +0 -4
@@ -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
Quoted 18 lines · Namhyung Kim
Hello,
This is v4 of my compressed kernel module work. Please take a look.
* changes in v4)
- fix build-id cache symbolic link handling in annotate
- separate fix for a memory leak (Arnaldo)
- consolidate error path in__open_dso(Arnaldo)
- remove decompressed file after useThe code is avaiable at 'perf/kmod-decomp-v4' branch in my tree:
git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.gitThanks,
Namhyung
Quoted 10 lines
Namhyung Kim (9):
perf annotate: Fix symbolic link of build-id cache perf tools: Fix a memory leak in__open_dso()perf tools: Introduce dso__decompress_kmodule_{fd,path} perf annotate: Usedso__decompress_kmodule_path()perf tools: Decompress kernel module when reading DSO data perf tools: Consolidate error path in__open_dso()perf test: Decompress kernel module before objdump perf symbols: KeepDSO->symtab_typeafter decompress perf symbols: Killdso__build_id_is_kmod()
thanks,
jirka