[PATCHSET v3 0/6] perf tools: Fix for Compressed kernel modules

[PATCHSET v3 0/6] perf tools: Fix for Compressed kernel modules

[Namhyung Kim]

Hello,

This is v3 of my compressed kernel module work.  This version
addresses Adrian's comment and also fixes perf annotate to handle
compressed kernel modules in the build-id cache.

The code is available at 'perf/kmod-decomp-v3' branch in my tree:

  git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git

Thanks,
Namhyung


Namhyung Kim (6):
  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 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      | 27 ++---------------
 tools/perf/util/build-id.c      | 45 ---------------------------
 tools/perf/util/build-id.h      |  1 -
 tools/perf/util/dso.c           | 67 ++++++++++++++++++++++++++++++++++++++++-
 tools/perf/util/dso.h           |  6 ++++
 tools/perf/util/symbol-elf.c    | 38 ++---------------------
 tools/perf/util/symbol.c        |  4 ---
 8 files changed, 97 insertions(+), 111 deletions(-)

-- 
2.13.0

[PATCH v3 1/6] perf tools: Introduce dso__decompress_kmodule_{fd,path}

[Namhyung Kim]

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

[PATCH v3 2/6] perf annotate: Use dso__decompress_kmodule_path()

[Namhyung Kim]

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

[PATCH v3 3/6] perf tools: Decompress kernel module when reading DSO data

[Namhyung Kim]

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

[PATCH v3 4/6] perf test: Decompress kernel module before objdump

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

[PATCH v3 5/6] perf symbols: Keep DSO->symtab_type after decompress

[Namhyung Kim]

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

[PATCH v3 6/6] perf symbols: Kill dso__build_id_is_kmod()

[Namhyung Kim]

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

Re: [PATCH v3 3/6] perf tools: Decompress kernel module when reading DSO data

[Arnaldo Carvalho de Melo]

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

Re: [PATCH v3 4/6] perf test: Decompress kernel module before objdump

[Arnaldo Carvalho de Melo]

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

Re: [PATCH v3 3/6] perf tools: Decompress kernel module when reading DSO data

[Namhyung Kim]

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

Re: [PATCH v3 4/6] perf test: Decompress kernel module before objdump

[Namhyung Kim]

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

Re: [PATCH v3 4/6] perf test: Decompress kernel module before objdump

[Namhyung Kim]

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