Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1659930 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCHSET v3 0/6] perf tools: Fix for Compressed kernel modules Namhyung Kim <namhyung@kernel.org> - 2017-06-07 17:40 +0200
    [PATCH v3 3/6] perf tools: Decompress kernel module when reading DSO data Namhyung Kim <namhyung@kernel.org> - 2017-06-07 17:40 +0200
      Re: [PATCH v3 3/6] perf tools: Decompress kernel module when reading  DSO data Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-06-08 00:30 +0200
        Re: [PATCH v3 3/6] perf tools: Decompress kernel module when reading  DSO data Namhyung Kim <namhyung@kernel.org> - 2017-06-08 02:00 +0200
    [PATCH v3 2/6] perf annotate: Use dso__decompress_kmodule_path() Namhyung Kim <namhyung@kernel.org> - 2017-06-07 17:40 +0200
    [PATCH v3 6/6] perf symbols: Kill dso__build_id_is_kmod() Namhyung Kim <namhyung@kernel.org> - 2017-06-07 17:50 +0200
    [PATCH v3 4/6] perf test: Decompress kernel module before objdump Namhyung Kim <namhyung@kernel.org> - 2017-06-07 17:50 +0200
      Re: [PATCH v3 4/6] perf test: Decompress kernel module before objdump Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-06-08 00:30 +0200
        Re: [PATCH v3 4/6] perf test: Decompress kernel module before objdump Namhyung Kim <namhyung@kernel.org> - 2017-06-08 02:00 +0200
        Re: [PATCH v3 4/6] perf test: Decompress kernel module before objdump Namhyung Kim <namhyung@kernel.org> - 2017-06-08 02:40 +0200
    [PATCH v3 5/6] perf symbols: Keep DSO->symtab_type after decompress Namhyung Kim <namhyung@kernel.org> - 2017-06-07 17:50 +0200
    [PATCH v3 1/6] perf tools: Introduce dso__decompress_kmodule_{fd,path} Namhyung Kim <namhyung@kernel.org> - 2017-06-07 17:50 +0200

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

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

[toc] | [next] | [standalone]

#1659931 — [PATCH v3 3/6] 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.

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

#1659939 — [PATCH v3 6/6] 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
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

[toc] | [prev] | [next] | [standalone]

#1659942 — [PATCH v3 4/6] 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.

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [next] | [standalone]

#1659947 — [PATCH v3 5/6] 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.

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

[toc] | [prev] | [next] | [standalone]

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

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web