Skip to contentSkip to conversation

perf tools: Adding xz decompression support

PATCH In review Jiri Olsa 5 participants 25 messages Mar 3, 2015 – Mar 14, 2015 perf events

lore
Share & tools
lore

Jiri Olsa

author reviewer
#0 lore raw

cover perf tools: Adding xz decompression support

hi,
adding support to decompress kernel modules on Fedora 21 (lzma compression) vaguely continuing in previous RFC:
http://marc.info/?l=linux-kernel&m=142261340327588&w=2

I'm basically extending he decompression framework Namhyung introduced earlier plus other fixes for kernel modules dso objects handling.

Last patch adds decompression support to annotate command, so it now displays annotation for compressed kernel modules.

Current work available at:
git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
perf/lzma

thanks,
jirka

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>

---
Jiri Olsa (14):
perf tools: Remove superfluous thread->comm_set setting perf tools: Add feature check for lzma library
perf tools: Add lzma decompression support for kernel module perf tools: Add kmod_path__parse function
perf tools: Add dsos__new function
perf tools: Add machine__module_dso function
perf tools: Use kmod_path__parse for machine__new_dso perf tools: Use kmod_path__parse in map_groups__set_modules_path_dir perf tools: Use kmod_path__parse in decompress_kmodule perf tools: Use kmod_path__parse in is_kernel_module perf tools: Remove compressed argument from is_kernel_module perf tools: Remove is_kmodule_extension function
perf tools: Try to lookup kernel module map before creating one perf annotate: Allow annotation for decompressed kernel modules

 tools/perf/Makefile.perf                     |   2 ++
 tools/perf/config/Makefile                   |  16 ++++++++--
 tools/perf/config/feature-checks/Makefile    |   6 +++-
 tools/perf/config/feature-checks/test-lzma.c |  10 ++++++
 tools/perf/tests/Build                       |   1 +
 tools/perf/tests/builtin-test.c              |   4 +++
 tools/perf/tests/kmod-path.c                 |  73 ++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/tests/tests.h                     |   1 +
 tools/perf/util/Build                        |   1 +
 tools/perf/util/annotate.c                   |  32 +++++++++++++++++++-
 tools/perf/util/dso.c                        |  52 ++++++++++++--------------------
 tools/perf/util/dso.h                        |   4 +--
 tools/perf/util/header.c                     |   2 +-
 tools/perf/util/lzma.c                       |  95 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/machine.c                    | 142 +++++++++++++++++++++++++++++++++++++++++++++-----------------------------------------
 tools/perf/util/symbol-elf.c                 |  25 ++++++++++-----
 tools/perf/util/thread.c                     |   1 -
 tools/perf/util/util.c                       |  66 ++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/util.h                       |  18 +++++++++++
 19 files changed, 436 insertions(+), 115 deletions(-)
 create mode 100644 tools/perf/config/feature-checks/test-lzma.c
 create mode 100644 tools/perf/tests/kmod-path.c
 create mode 100644 tools/perf/util/lzma.c
author reviewer
#1 lore raw

PATCH 1/14 perf tools: Remove superfluous thread->comm_set setting

It is set by calling thread__set_comm right before the removed line.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/util/thread.c | 1 -
 1 file changed, 1 deletion(-)
tools/perf/util/thread.c +0 -1
index a5dbba95107f..1c8fbc9588c5 100644
@@ -206,7 +206,6 @@ int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp)
 		err = thread__set_comm(thread, comm, timestamp);
 		if (err)
 			return err;
-		thread->comm_set = true;
 	}
 
 	thread->ppid = parent->tid;
Signature · 1 line
1.9.3
#2 lore raw

[tip:perf/core] perf tools: Remove superfluous thread-> comm_set setting

perf tools: Remove superfluous thread->comm_set setting
Commit-ID:  01fbc1fee926888f7c256ada95fa5fa3b06cba94
Gitweb:     http://git.kernel.org/tip/01fbc1fee926888f7c256ada95fa5fa3b06cba94
Author:     Jiri Olsa <[email protected]>
AuthorDate: Tue, 3 Mar 2015 16:29:28 +0100
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Thu, 12 Mar 2015 12:39:50 -0300

perf tools: Remove superfluous thread->comm_set setting

It is set by calling thread__set_comm right before the removed line.

Cc Adrian Hunter <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/util/thread.c | 1 -
 1 file changed, 1 deletion(-)
tools/perf/util/thread.c +0 -1
index a5dbba9..1c8fbc9 100644
@@ -206,7 +206,6 @@ int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp)
 		err = thread__set_comm(thread, comm, timestamp);
 		if (err)
 			return err;
-		thread->comm_set = true;
 	}
 
 	thread->ppid = parent->tid;
author reviewer
#3 lore raw

PATCH 2/14 perf tools: Add feature check for lzma library

In reply to Jiri Olsa

Will be used to decompress 'xz' objects. The check detects the liblzma.so devel library normally delivered by xz package.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/config/Makefile                   |  6 ++++--
 tools/perf/config/feature-checks/Makefile    |  6 +++++-
 tools/perf/config/feature-checks/test-lzma.c | 10 ++++++++++
 3 files changed, 19 insertions(+), 3 deletions(-)
 create mode 100644 tools/perf/config/feature-checks/test-lzma.c
tools/perf/config/Makefile +4 -2
index d44c64d64465..d77bcea691dd 100644
@@ -228,7 +228,8 @@ CORE_FEATURE_TESTS =			\
 	timerfd				\
 	libdw-dwarf-unwind		\
 	libbabeltrace			\
-	zlib
+	zlib				\
+	lzma
 
 LIB_FEATURE_TESTS =			\
 	dwarf				\
@@ -244,7 +245,8 @@ LIB_FEATURE_TESTS =			\
 	libunwind			\
 	libdw-dwarf-unwind		\
 	libbabeltrace			\
-	zlib
+	zlib				\
+	lzma
 
 VF_FEATURE_TESTS =			\
 	backtrace			\
tools/perf/config/feature-checks/Makefile +5 -1
index 8fe067864957..f7b1b7934782 100644
@@ -32,7 +32,8 @@ FILES=					\
 	test-libbabeltrace.bin		\
 	test-compile-32.bin		\
 	test-compile-x32.bin		\
-	test-zlib.bin
+	test-zlib.bin			\
+	test-lzma.bin
 
 CC := $(CROSS_COMPILE)gcc -MD
 PKG_CONFIG := $(CROSS_COMPILE)pkg-config
@@ -151,6 +152,9 @@ test-compile-x32.bin:
 test-zlib.bin:
 	$(BUILD) -lz
 
+test-lzma.bin:
+	$(BUILD) -llzma
+
 -include *.d
 
 ###############################
tools/perf/config/feature-checks/test-lzma.c new file +10 -0
new file mode 100644
index 000000000000..95adc8ced3dd
@@ -0,0 +1,10 @@
+#include <lzma.h>
+
+int main(void)
+{
+	lzma_stream strm = LZMA_STREAM_INIT;
+	int ret;
+
+	ret = lzma_stream_decoder(&strm, UINT64_MAX, LZMA_CONCATENATED);
+	return ret ? -1 : 0;
+}
Signature · 1 line
1.9.3
#4 lore raw

Re: [PATCH 02/14] perf tools: Add feature check for lzma library

Em Tue, Mar 03, 2015 at 04:29:29PM +0100, Jiri Olsa escreveu:

Quoted 2 lines

Will be used to decompress 'xz' objects. The check detects the liblzma.so devel library normally delivered by xz package.

Hey, don't we want to emit some warning like the one below for CTF/libunwind?

- Arnaldo

[perf/core 1b23d3ab8304] perf tools: Add feature check for lzma library
Author: Jiri Olsa <[email protected]>

 3 files changed, 19 insertions(+), 3 deletions(-)
 create mode 100644 tools/perf/config/feature-checks/test-lzma.c

[acme@ssdandy linux]$ fg
-bash: fg: current: no such job
[acme@ssdandy linux]$ time make O=/tmp/build/perf -C tools/perf install-bin
make: Entering directory `/home/acme/git/linux/tools/perf'
BUILD: Doing 'make -j8' parallel build
config/Makefile:427: No libunwind found. Please install libunwind-dev[el] >= 1.1 and/or set LIBUNWIND_DIR config/Makefile:711: No libbabeltrace found, disables 'perf data' CTF format support, please install libbabeltrace-dev[el]/libbabeltrace-ctf-dev

Auto-detecting system features:

...                         dwarf: [ on  ]
...                         glibc: [ on  ]
...                          gtk2: [ on  ]
...                      libaudit: [ on  ]
...                        libbfd: [ on  ]
...                        libelf: [ on  ]
...                       libnuma: [ on  ]
...                       libperl: [ on  ]
...                     libpython: [ on  ]
...                      libslang: [ on  ]

... libunwind: [ OFF ]
... libdw-dwarf-unwind: [ on ]
... libbabeltrace: [ OFF ]
... zlib: [ on ]
... lzma: [ OFF ]
... DWARF post unwind library: libdw

  INSTALL  GTK UI
  INSTALL  binaries
  INSTALL  libexec
  INSTALL  perf-archive
  INSTALL  perf-with-kcore
  INSTALL  perl-scripts
  INSTALL  python-scripts
  INSTALL  perf_completion-script
  INSTALL  tests

make: Leaving directory `/home/acme/git/linux/tools/perf'

real 0m3.818s
user 0m3.087s
sys 0m0.970s
[acme@ssdandy linux]$

#5 lore raw

Re: [PATCH 02/14] perf tools: Add feature check for lzma library

Quoted 5 lines · Arnaldo Carvalho de Melo

Em Tue, Mar 03, 2015 at 04:29:29PM +0100, Jiri Olsa escreveu:

Quoted 2 lines

Will be used to decompress 'xz' objects. The check detects the liblzma.so devel library normally delivered by xz package.

Hey, don't we want to emit some warning like the one below for CTF/libunwind?

hum.. we probably want, right? ;-) will add in v2

thanks,
jirka

author reviewer
#6 lore raw

PATCH 3/14 perf tools: Add lzma decompression support for kernel module

In reply to Jiri Olsa

In short, Fedora compresses kernel modules now (since version 21) with lzma compression.

Adding lzma decompress support into the dso.c:compressions array introduced by Nmahyung earlier.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/Makefile.perf   |  2 +
 tools/perf/config/Makefile | 10 +++++
 tools/perf/util/Build      |  1 +
 tools/perf/util/dso.c      |  3 ++
 tools/perf/util/lzma.c     | 95 ++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/util.h     |  4 ++
 6 files changed, 115 insertions(+)
 create mode 100644 tools/perf/util/lzma.c
tools/perf/Makefile.perf +2 -0
index ec4c063ed9f3..9b981c9e3340 100644
@@ -71,6 +71,8 @@ include config/utilities.mak
 #
 # Define NO_LIBBABELTRACE if you do not want libbabeltrace support
 # for CTF data format.
+#
+# Define NO_LZMA if you do not want to support compressed (xz) kernel modules
 
 ifeq ($(srctree),)
 srctree := $(patsubst %/,%,$(dir $(shell pwd)))
tools/perf/config/Makefile +10 -0
index d77bcea691dd..ba9f9811812e 100644
@@ -660,6 +660,16 @@ ifndef NO_ZLIB
   endif
 endif
 
+ifndef NO_LZMA
+  ifeq ($(feature-lzma), 1)
+    CFLAGS += -DHAVE_LZMA_SUPPORT
+    EXTLIBS += -llzma
+    $(call detected,CONFIG_LZMA)
+  else
+    NO_LZMA := 1
+  endif
+endif
+
 ifndef NO_BACKTRACE
   ifeq ($(feature-backtrace), 1)
     CFLAGS += -DHAVE_BACKTRACE_SUPPORT
tools/perf/util/Build +1 -0
index 972a6e0da7ad..797490a40075 100644
@@ -94,6 +94,7 @@ libperf-y += scripting-engines/
 
 libperf-$(CONFIG_PERF_REGS) += perf_regs.o
 libperf-$(CONFIG_ZLIB) += zlib.o
+libperf-$(CONFIG_LZMA) += lzma.o
 
 CFLAGS_config.o   += -DETC_PERFCONFIG="BUILD_STR($(ETC_PERFCONFIG_SQ))"
 CFLAGS_exec_cmd.o += -DPERF_EXEC_PATH="BUILD_STR($(perfexecdir_SQ))" -DPREFIX="BUILD_STR($(prefix_SQ))"
tools/perf/util/dso.c +3 -0
index 814554d1b857..be368414036c 100644
@@ -148,6 +148,9 @@ static const struct {
 #ifdef HAVE_ZLIB_SUPPORT
 	{ "gz", gzip_decompress_to_file },
 #endif
+#ifdef HAVE_LZMA_SUPPORT
+	{ "xz", lzma_decompress_to_file },
+#endif
 	{ NULL, NULL },
 };
 
tools/perf/util/lzma.c new file +95 -0

95 lines not on this page · raw message

tools/perf/util/util.h +4 -0
index fbd598afc606..1ff23e04ad27 100644
@@ -329,4 +329,8 @@ bool find_process(const char *name);
 int gzip_decompress_to_file(const char *input, int output_fd);
 #endif
 
+#ifdef HAVE_LZMA_SUPPORT
+int lzma_decompress_to_file(const char *input, int output_fd);
+#endif
+
 #endif /* GIT_COMPAT_UTIL_H */
Signature · 1 line
1.9.3
author reviewer
#7 lore raw

PATCH 4/14 perf tools: Add kmod_path__parse function

In reply to Jiri Olsa

Provides united way of parsing kernel module path into several components.

The new kmod_path__parse function and few defines:

int __kmod_path__parse(struct kmod_path *m, const char *path,

                         bool alloc_name, bool alloc_ext);
  #define kmod_path__parse(__m, __p)      __kmod_path__parse(__m, __p, false, false)
  #define kmod_path__parse_name(__m, __p) __kmod_path__parse(__m, __p, true , false)
  #define kmod_path__parse_ext(__m, __p)  __kmod_path__parse(__m, __p, false, true)

parse kernel module @path and updates @m argument like:

@comp - true if @path contains supported compression suffix,
false otherwise
@kmod - true if @path contains '.ko' suffix in right position,
false otherwise
@name - if (@alloc_name && @kmod) is true, it contains strdup-ed base name
of the kernel module without suffixes, otherwise strudup-ed base name of @path
@ext - if (@alloc_ext && @comp) is true, it contains strdup-ed string
the compression suffix

It returns 0 if there's no strdup error, -ENOMEM otherwise.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/tests/Build          |  1 +
 tools/perf/tests/builtin-test.c |  4 +++
 tools/perf/tests/kmod-path.c    | 73 +++++++++++++++++++++++++++++++++++++++++
 tools/perf/tests/tests.h        |  1 +
 tools/perf/util/util.c          | 66 +++++++++++++++++++++++++++++++++++++
 tools/perf/util/util.h          | 14 ++++++++
 6 files changed, 159 insertions(+)
 create mode 100644 tools/perf/tests/kmod-path.c
tools/perf/tests/Build +1 -0
index 2de01a4b4084..6a8801b32017 100644
@@ -30,6 +30,7 @@ perf-y += keep-tracking.o
 perf-y += code-reading.o
 perf-y += sample-parsing.o
 perf-y += parse-no-sample-id-all.o
+perf-y += kmod-path.o
 
 perf-$(CONFIG_X86) += perf-time-to-tsc.o
 
tools/perf/tests/builtin-test.c +4 -0
index 4b7d9ab0f049..31f9959c04c8 100644
@@ -167,6 +167,10 @@ static struct test {
 		.func = test__fdarray__add,
 	},
 	{
+		.desc = "Test kmod_path__parse function",
+		.func = test__kmod_path__parse,
+	},
+	{
 		.func = NULL,
 	},
 };
tools/perf/tests/kmod-path.c new file +73 -0

73 lines not on this page · raw message

tools/perf/tests/tests.h +1 -0
index 00e776a87a9c..52758a33f64c 100644
@@ -51,6 +51,7 @@ int test__hists_cumulate(void);
 int test__switch_tracking(void);
 int test__fdarray__filter(void);
 int test__fdarray__add(void);
+int test__kmod_path__parse(void);
 
 #if defined(__x86_64__) || defined(__i386__) || defined(__arm__)
 #ifdef HAVE_DWARF_UNWIND_SUPPORT
tools/perf/util/util.c +66 -0

69 lines not on this page · raw message

tools/perf/util/util.h +14 -0
index 1ff23e04ad27..ddca38a901e8 100644
@@ -292,6 +292,20 @@ struct parse_tag {
 
 unsigned long parse_tag_value(const char *str, struct parse_tag *tags);
 
+struct kmod_path {
+	char *name;
+	char *ext;
+	bool  comp;
+	bool  kmod;
+};
+
+int __kmod_path__parse(struct kmod_path *m, const char *path,
+		     bool alloc_name, bool alloc_ext);
+
+#define kmod_path__parse(__m, __p)      __kmod_path__parse(__m, __p, false, false)
+#define kmod_path__parse_name(__m, __p) __kmod_path__parse(__m, __p, true , false)
+#define kmod_path__parse_ext(__m, __p)  __kmod_path__parse(__m, __p, false, true)
+
 #define SRCLINE_UNKNOWN  ((char *) "??:0")
 
 static inline int path__join(char *bf, size_t size,
Signature · 1 line
1.9.3
author reviewer
#8 lore raw

PATCH 5/14 perf tools: Add dsos__new function

In reply to Jiri Olsa

Separate the creation of new dso object and its addition to the dsos list. It will be used in following patch.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/util/dso.c | 21 ++++++++++++---------
 tools/perf/util/dso.h |  1 +
 2 files changed, 13 insertions(+), 9 deletions(-)
tools/perf/util/dso.c +12 -9
index be368414036c..fc54cb81c7f8 100644
@@ -1005,21 +1005,24 @@ struct dso *dsos__find(const struct dsos *dsos, const char *name,
 	return dso__find_by_longname(&dsos->root, name);
 }
 
-struct dso *__dsos__findnew(struct dsos *dsos, const char *name)
+struct dso *dsos__new(struct dsos *dsos, const char *name)
 {
-	struct dso *dso = dsos__find(dsos, name, false);
+	struct dso *dso = dso__new(name);
 
-	if (!dso) {
-		dso = dso__new(name);
-		if (dso != NULL) {
-			dsos__add(dsos, dso);
-			dso__set_basename(dso);
-		}
+	if (dso != NULL) {
+		dsos__add(dsos, dso);
+		dso__set_basename(dso);
 	}
-
 	return dso;
 }
 
+struct dso *__dsos__findnew(struct dsos *dsos, const char *name)
+{
+	struct dso *dso = dsos__find(dsos, name, false);
+
+	return dso ? dso : dsos__new(dsos, name);
+}
+
 size_t __dsos__fprintf_buildid(struct list_head *head, FILE *fp,
 			       bool (skip)(struct dso *dso, int parm), int parm)
 {
tools/perf/util/dso.h +1 -0
index ced92841ff97..a712cbcd3a72 100644
@@ -249,6 +249,7 @@ struct dso *dso__kernel_findnew(struct machine *machine, const char *name,
 				const char *short_name, int dso_type);
 
 void dsos__add(struct dsos *dsos, struct dso *dso);
+struct dso *dsos__new(struct dsos *dsos, const char *name);
 struct dso *dsos__find(const struct dsos *dsos, const char *name,
 		       bool cmp_short);
 struct dso *__dsos__findnew(struct dsos *dsos, const char *name);
Signature · 1 line
1.9.3
#9 lore raw

Re: [PATCH 05/14] perf tools: Add dsos__new function

Em Tue, Mar 03, 2015 at 04:29:32PM +0100, Jiri Olsa escreveu:

Quoted 24 lines

Separate the creation of new dso object and its addition to the dsos list. It will be used in following patch.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/util/dso.c | 21 ++++++++++++---------
 tools/perf/util/dso.h |  1 +
 2 files changed, 13 insertions(+), 9 deletions(-)
tools/perf/util/dso.c +0 -0
index be368414036c..fc54cb81c7f8 100644

tools/perf/util/dso.c struct dso *dsos__find(const struct dsos *dsos, const char *name, · line 1008

Quoted 5 lines
tools/perf/util/dso.c +1 -1
@@ -1005,21 +1005,24 @@ struct dso *dsos__find(const struct dsos *dsos, const char *name,
 	return dso__find_by_longname(&dsos->root, name);
 }
 
-struct dso *__dsos__findnew(struct dsos *dsos, const char *name)
+struct dso *dsos__new(struct dsos *dsos, const char *name)

When I read dsos__new() I think that it will surely return a 'struct dsos' pointer, as it is the constructor ('__new') for the 'dsos' class. :-)

Right now we don't have a constructor for dsos, because its only used in machine where they are not m'allocated, i.e. we use just dsos__init(), but if we ever need a constructor for it, it will be called dsos__new().

So, to use the same rationale for dsos__findnew(), we can call it dsos__addnew(), because it will not find anything, just add a new dso.

- Arnaldo

Quoted 42 lines
tools/perf/util/dso.c +11 -8
 {
-	struct dso *dso = dsos__find(dsos, name, false);
+	struct dso *dso = dso__new(name);
 
-	if (!dso) {
-		dso = dso__new(name);
-		if (dso != NULL) {
-			dsos__add(dsos, dso);
-			dso__set_basename(dso);
-		}
+	if (dso != NULL) {
+		dsos__add(dsos, dso);
+		dso__set_basename(dso);
 	}
-
 	return dso;
 }
 
+struct dso *__dsos__findnew(struct dsos *dsos, const char *name)
+{
+	struct dso *dso = dsos__find(dsos, name, false);
+
+	return dso ? dso : dsos__new(dsos, name);
+}
+
 size_t __dsos__fprintf_buildid(struct list_head *head, FILE *fp,
 			       bool (skip)(struct dso *dso, int parm), int parm)
 {
tools/perf/util/dso.h +1 -0
index ced92841ff97..a712cbcd3a72 100644
@@ -249,6 +249,7 @@ struct dso *dso__kernel_findnew(struct machine *machine, const char *name,
 				const char *short_name, int dso_type);
 
 void dsos__add(struct dsos *dsos, struct dso *dso);
+struct dso *dsos__new(struct dsos *dsos, const char *name);
 struct dso *dsos__find(const struct dsos *dsos, const char *name,
 		       bool cmp_short);
 struct dso *__dsos__findnew(struct dsos *dsos, const char *name);
Signature · 1 line
1.9.3
#10 lore raw

Re: [PATCH 05/14] perf tools: Add dsos__new function

Quoted 34 lines · Arnaldo Carvalho de Melo

Em Tue, Mar 03, 2015 at 04:29:32PM +0100, Jiri Olsa escreveu:

Quoted 24 lines

Separate the creation of new dso object and its addition to the dsos list. It will be used in following patch.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/util/dso.c | 21 ++++++++++++---------
 tools/perf/util/dso.h |  1 +
 2 files changed, 13 insertions(+), 9 deletions(-)
tools/perf/util/dso.c +0 -0
index be368414036c..fc54cb81c7f8 100644

tools/perf/util/dso.c struct dso *dsos__find(const struct dsos *dsos, const char *name, · line 1008

Quoted 5 lines
tools/perf/util/dso.c +1 -1
@@ -1005,21 +1005,24 @@ struct dso *dsos__find(const struct dsos *dsos, const char *name,
 	return dso__find_by_longname(&dsos->root, name);
 }
 
-struct dso *__dsos__findnew(struct dsos *dsos, const char *name)
+struct dso *dsos__new(struct dsos *dsos, const char *name)

When I read dsos__new() I think that it will surely return a 'struct dsos' pointer, as it is the constructor ('__new') for the 'dsos' class. :-)

Quoted 6 lines

Right now we don't have a constructor for dsos, because its only used in machine where they are not m'allocated, i.e. we use just dsos__init(), but if we ever need a constructor for it, it will be called dsos__new().

So, to use the same rationale for dsos__findnew(), we can call it dsos__addnew(), because it will not find anything, just add a new dso.

ok, sounds good

thanks,
jirka

author reviewer
#11 lore raw

PATCH 6/14 perf tools: Add machine__module_dso function

In reply to Jiri Olsa

Separate the dso object addition and update when adding new kernel module.

Currently we update dso's symtab_type any time we find it in the list, because we can't distinguish between new and found dso from __dsos__findnew function.

Adding machine__module_dso that separates finding and adding new dso objects, so there's no superfluous update of dso.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/util/machine.c | 36 +++++++++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 11 deletions(-)
tools/perf/util/machine.c +25 -11
index 24f8c978cfd4..14f7caf4824a 100644
@@ -460,12 +460,35 @@ int machine__process_lost_event(struct machine *machine __maybe_unused,
 	return 0;
 }
 
+static struct dso *machine__module_dso(struct machine *machine, const char *filename)
+{
+	struct dso *dso;
+	bool compressed;
+
+	dso = dsos__find(&machine->kernel_dsos, filename, false);
+	if (!dso) {
+		dso = dsos__new(&machine->kernel_dsos, filename);
+		if (dso == NULL)
+			return NULL;
+
+		if (machine__is_host(machine))
+			dso->symtab_type = DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE;
+		else
+			dso->symtab_type = DSO_BINARY_TYPE__GUEST_KMODULE;
+
+		/* _KMODULE_COMP should be next to _KMODULE */
+		if (is_kernel_module(filename, &compressed) && compressed)
+			dso->symtab_type++;
+	}
+
+	return dso;
+}
+
 struct map *machine__new_module(struct machine *machine, u64 start,
 				const char *filename)
 {
 	struct map *map;
-	struct dso *dso = __dsos__findnew(&machine->kernel_dsos, filename);
-	bool compressed;
+	struct dso *dso = machine__module_dso(machine, filename);
 
 	if (dso == NULL)
 		return NULL;
@@ -474,15 +497,6 @@ struct map *machine__new_module(struct machine *machine, u64 start,
 	if (map == NULL)
 		return NULL;
 
-	if (machine__is_host(machine))
-		dso->symtab_type = DSO_BINARY_TYPE__SYSTEM_PATH_KMODULE;
-	else
-		dso->symtab_type = DSO_BINARY_TYPE__GUEST_KMODULE;
-
-	/* _KMODULE_COMP should be next to _KMODULE */
-	if (is_kernel_module(filename, &compressed) && compressed)
-		dso->symtab_type++;
-
 	map_groups__insert(&machine->kmaps, map);
 	return map;
 }
Signature · 1 line
1.9.3
author reviewer
#12 lore raw

PATCH 7/14 perf tools: Use kmod_path__parse for machine__new_dso

In reply to Jiri Olsa

Using kmod_path__parse to get the module name and update the dso short name within machine__new_dso function.

This way it's done only first time when dso is created, unlike the current way when we update it all the time we process memory map of the kernel module.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/util/machine.c | 57 +++++++++++++++++------------------------------
 1 file changed, 20 insertions(+), 37 deletions(-)
tools/perf/util/machine.c +20 -37
index 14f7caf4824a..5a58cdd141ae 100644
@@ -460,14 +460,15 @@ int machine__process_lost_event(struct machine *machine __maybe_unused,
 	return 0;
 }
 
-static struct dso *machine__module_dso(struct machine *machine, const char *filename)
+static struct dso*
+machine__module_dso(struct machine *machine, struct kmod_path *m,
+		    const char *filename)
 {
 	struct dso *dso;
-	bool compressed;
 
-	dso = dsos__find(&machine->kernel_dsos, filename, false);
+	dso = dsos__find(&machine->kernel_dsos, m->name, true);
 	if (!dso) {
-		dso = dsos__new(&machine->kernel_dsos, filename);
+		dso = dsos__new(&machine->kernel_dsos, m->name);
 		if (dso == NULL)
 			return NULL;
 
@@ -477,8 +478,11 @@ static struct dso *machine__module_dso(struct machine *machine, const char *file
 			dso->symtab_type = DSO_BINARY_TYPE__GUEST_KMODULE;
 
 		/* _KMODULE_COMP should be next to _KMODULE */
-		if (is_kernel_module(filename, &compressed) && compressed)
+		if (m->kmod && m->comp)
 			dso->symtab_type++;
+
+		dso__set_short_name(dso, strdup(m->name), true);
+		dso__set_long_name(dso, strdup(filename), true);
 	}
 
 	return dso;
@@ -488,16 +492,24 @@ struct map *machine__new_module(struct machine *machine, u64 start,
 				const char *filename)
 {
 	struct map *map;
-	struct dso *dso = machine__module_dso(machine, filename);
+	struct dso *dso;
+	struct kmod_path m;
 
-	if (dso == NULL)
+	if (kmod_path__parse_name(&m, filename))
 		return NULL;
 
+	dso = machine__module_dso(machine, &m, filename);
+	if (dso == NULL)
+		goto out;
+
 	map = map__new2(start, dso, MAP__FUNCTION);
 	if (map == NULL)
-		return NULL;
+		goto out;
 
 	map_groups__insert(&machine->kmaps, map);
+
+out:
+	free(m.name);
 	return map;
 }
 
@@ -1058,40 +1070,11 @@ static int machine__process_kernel_mmap_event(struct machine *machine,
 				strlen(kmmap_prefix) - 1) == 0;
 	if (event->mmap.filename[0] == '/' ||
 	    (!is_kernel_mmap && event->mmap.filename[0] == '[')) {
-
-		char short_module_name[1024];
-		char *name, *dot;
-
-		if (event->mmap.filename[0] == '/') {
-			name = strrchr(event->mmap.filename, '/');
-			if (name == NULL)
-				goto out_problem;
-
-			++name; /* skip / */
-			dot = strrchr(name, '.');
-			if (dot == NULL)
-				goto out_problem;
-			/* On some system, modules are compressed like .ko.gz */
-			if (is_supported_compression(dot + 1))
-				dot -= 3;
-			if (!is_kmodule_extension(dot + 1))
-				goto out_problem;
-			snprintf(short_module_name, sizeof(short_module_name),
-					"[%.*s]", (int)(dot - name), name);
-			strxfrchar(short_module_name, '-', '_');
-		} else
-			strcpy(short_module_name, event->mmap.filename);
-
 		map = machine__new_module(machine, event->mmap.start,
 					  event->mmap.filename);
 		if (map == NULL)
 			goto out_problem;
 
-		name = strdup(short_module_name);
-		if (name == NULL)
-			goto out_problem;
-
-		dso__set_short_name(map->dso, name, true);
 		map->end = map->start + event->mmap.len;
 	} else if (is_kernel_mmap) {
 		const char *symbol_name = (event->mmap.filename +
Signature · 1 line
1.9.3
author reviewer
#13 lore raw

PATCH 8/14 perf tools: Use kmod_path__parse in map_groups__set_modules_path_dir

In reply to Jiri Olsa

Replacing the file name parsing with kmod_path__parse and moving the dso update into new separate function.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/util/machine.c | 54 ++++++++++++++++++++++++++---------------------
 1 file changed, 30 insertions(+), 24 deletions(-)
tools/perf/util/machine.c +30 -24
index 5a58cdd141ae..0c2d9e68996b 100644
@@ -851,6 +851,28 @@ static char *get_kernel_version(const char *root_dir)
 	return strdup(name);
 }
 
+static int map_groups__set_module_path(struct map_groups *mg, const char *path,
+				       struct kmod_path *m)
+{
+	struct map *map;
+	char *long_name;
+
+	map = map_groups__find_by_name(mg, MAP__FUNCTION, m->name);
+	if (map == NULL)
+		return 0;
+
+	long_name = strdup(path);
+	if (long_name == NULL)
+		return -ENOMEM;
+
+	dso__set_long_name(map->dso, long_name, true);
+	dso__kernel_module_get_build_id(map->dso, "");
+
+	if (m->comp)
+		map->dso->symtab_type++;
+	return 0;
+}
+
 static int map_groups__set_modules_path_dir(struct map_groups *mg,
 				const char *dir_name, int depth)
 {
@@ -889,35 +911,19 @@ static int map_groups__set_modules_path_dir(struct map_groups *mg,
 			if (ret < 0)
 				goto out;
 		} else {
-			char *dot = strrchr(dent->d_name, '.'),
-			     dso_name[PATH_MAX];
-			struct map *map;
-			char *long_name;
-
-			if (dot == NULL)
-				continue;
+			struct kmod_path m;
 
-			/* On some system, modules are compressed like .ko.gz */
-			if (is_supported_compression(dot + 1) &&
-			    is_kmodule_extension(dot - 2))
-				dot -= 3;
+			ret = kmod_path__parse_name(&m, dent->d_name);
+			if (ret)
+				goto out;
 
-			snprintf(dso_name, sizeof(dso_name), "[%.*s]",
-				 (int)(dot - dent->d_name), dent->d_name);
+			if (m.kmod)
+				ret = map_groups__set_module_path(mg, path, &m);
 
-			strxfrchar(dso_name, '-', '_');
-			map = map_groups__find_by_name(mg, MAP__FUNCTION,
-						       dso_name);
-			if (map == NULL)
-				continue;
+			free(m.name);
 
-			long_name = strdup(path);
-			if (long_name == NULL) {
-				ret = -1;
+			if (ret)
 				goto out;
-			}
-			dso__set_long_name(map->dso, long_name, true);
-			dso__kernel_module_get_build_id(map->dso, "");
 		}
 	}
 
Signature · 1 line
1.9.3
#14 lore raw
In reply to Jiri Olsa
Quoted 9 lines · Jiri Olsa

hi,
adding support to decompress kernel modules on Fedora 21 (lzma compression) vaguely continuing in previous RFC:
http://marc.info/?l=linux-kernel&m=142261340327588&w=2

I'm basically extending he decompression framework Namhyung introduced earlier plus other fixes for kernel modules dso objects handling.

Quoted 6 lines

Last patch adds decompression support to annotate command, so it now displays annotation for compressed kernel modules.

Current work available at:
git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
perf/lzma

ugh.. the subject should be '00/14', sry ;-)

jirka

author reviewer
#15 lore raw

PATCH 9/14 perf tools: Use kmod_path__parse in decompress_kmodule

In reply to Jiri Olsa

Replacing the file name parsing with kmod_path__parse.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/util/symbol-elf.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)
tools/perf/util/symbol-elf.c +17 -8
index ada16762fac2..8dd530ce20c5 100644
@@ -579,32 +579,41 @@ static int dso__swap_init(struct dso *dso, unsigned char eidata)
 static int decompress_kmodule(struct dso *dso, const char *name,
 			      enum dso_binary_type type)
 {
-	int fd;
-	const char *ext = strrchr(name, '.');
+	int fd = -1, try = 2;
 	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 (!ext || !is_supported_compression(ext + 1)) {
-		ext = strrchr(dso->name, '.');
-		if (!ext || !is_supported_compression(ext + 1))
-			return -1;
+again:
+	if (kmod_path__parse_ext(&m, name))
+		return -1;
+
+	if (!m.comp) {
+		if (!--try)
+			goto out;
+
+		free(m.ext);
+		name = dso->name;
+		goto again;
 	}
 
 	fd = mkstemp(tmpbuf);
 	if (fd < 0)
-		return -1;
+		goto out;
 
-	if (!decompress_to_file(ext + 1, name, fd)) {
+	if (!decompress_to_file(m.ext, name, fd)) {
 		close(fd);
 		fd = -1;
 	}
 
 	unlink(tmpbuf);
 
+out:
+	free(m.ext);
 	return fd;
 }
 
Signature · 1 line
1.9.3
maintainer
#16 lore raw

Re: [PATCH 09/14] perf tools: Use kmod_path__parse in decompress_kmodule

Hi Jiri,

Quoted 42 lines · Jiri Olsa

Replacing the file name parsing with kmod_path__parse.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/util/symbol-elf.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)
tools/perf/util/symbol-elf.c +4 -6
index ada16762fac2..8dd530ce20c5 100644
@@ -579,32 +579,41 @@ static int dso__swap_init(struct dso *dso, unsigned char eidata)
 static int decompress_kmodule(struct dso *dso, const char *name,
 			      enum dso_binary_type type)
 {
-	int fd;
-	const char *ext = strrchr(name, '.');
+	int fd = -1, try = 2;
 	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 (!ext || !is_supported_compression(ext + 1)) {
-		ext = strrchr(dso->name, '.');
-		if (!ext || !is_supported_compression(ext + 1))
-			return -1;
+again:
+	if (kmod_path__parse_ext(&m, name))

tools/perf/util/symbol-elf.c static int dso__swap_init(struct dso *dso, unsigned char eidata) · line 602

Quoted 10 lines
tools/perf/util/symbol-elf.c +9 -0
+		return -1;
+
+	if (!m.comp) {
+		if (!--try)
+			goto out;
+
+		free(m.ext);
+		name = dso->name;
+		goto again;
 	}

Maybe we can do like this instead:

	if (type == DSO_BINARY_TYPE__BUILD_ID_CACHE)
		name = dso->name;

	if (kmod_path__parse_ext(&m, name))
		return -1;

Thanks,
Namhyung

Quoted 22 lines
tools/perf/util/symbol-elf.c +4 -2
 	fd = mkstemp(tmpbuf);
 	if (fd < 0)
-		return -1;
+		goto out;
 
-	if (!decompress_to_file(ext + 1, name, fd)) {
+	if (!decompress_to_file(m.ext, name, fd)) {
 		close(fd);
 		fd = -1;
 	}
 
 	unlink(tmpbuf);
 
+out:
+	free(m.ext);
 	return fd;
 }
 
Signature · 1 line
1.9.3
#17 lore raw

Re: [PATCH 09/14] perf tools: Use kmod_path__parse in decompress_kmodule

Quoted 58 lines · Namhyung Kim

Hi Jiri,

Quoted 42 lines · Jiri Olsa

Replacing the file name parsing with kmod_path__parse.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/util/symbol-elf.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)
tools/perf/util/symbol-elf.c +4 -6
index ada16762fac2..8dd530ce20c5 100644
@@ -579,32 +579,41 @@ static int dso__swap_init(struct dso *dso, unsigned char eidata)
 static int decompress_kmodule(struct dso *dso, const char *name,
 			      enum dso_binary_type type)
 {
-	int fd;
-	const char *ext = strrchr(name, '.');
+	int fd = -1, try = 2;
 	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 (!ext || !is_supported_compression(ext + 1)) {
-		ext = strrchr(dso->name, '.');
-		if (!ext || !is_supported_compression(ext + 1))
-			return -1;
+again:
+	if (kmod_path__parse_ext(&m, name))

tools/perf/util/symbol-elf.c static int dso__swap_init(struct dso *dso, unsigned char eidata) · line 602

Quoted 10 lines
tools/perf/util/symbol-elf.c +9 -0
+		return -1;
+
+	if (!m.comp) {
+		if (!--try)
+			goto out;
+
+		free(m.ext);
+		name = dso->name;
+		goto again;
 	}

Maybe we can do like this instead:

Quoted 5 lines
	if (type == DSO_BINARY_TYPE__BUILD_ID_CACHE)
		name = dso->name;

	if (kmod_path__parse_ext(&m, name))
		return -1;

right, looks lot simpler ;-) I'll check

thanks,
jirka

author reviewer
#18 lore raw

PATCH 10/14 perf tools: Use kmod_path__parse in is_kernel_module

In reply to Jiri Olsa

Replacing the current parsing code with kmod_path__parse function call.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/util/dso.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)
tools/perf/util/dso.c +6 -10
index fc54cb81c7f8..a17e5301af29 100644
@@ -178,19 +178,15 @@ bool is_kmodule_extension(const char *ext)
 
 bool is_kernel_module(const char *pathname, bool *compressed)
 {
-	const char *ext = strrchr(pathname, '.');
+	struct kmod_path m;
 
-	if (ext == NULL)
-		return false;
+	if (kmod_path__parse(&m, pathname))
+		return NULL;
 
-	if (is_supported_compression(ext + 1)) {
-		if (compressed)
-			*compressed = true;
-		ext -= 3;
-	} else if (compressed)
-		*compressed = false;
+	if (compressed)
+		*compressed = m.comp;
 
-	return is_kmodule_extension(ext + 1);
+	return m.kmod;
 }
 
 bool decompress_to_file(const char *ext, const char *filename, int output_fd)
Signature · 1 line
1.9.3
author reviewer
#19 lore raw

PATCH 11/14 perf tools: Remove compressed argument from is_kernel_module

In reply to Jiri Olsa

We no longer need the 'compressed' argument, because all current users use 'NULL' for it.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/util/dso.c     | 5 +----
 tools/perf/util/dso.h     | 2 +-
 tools/perf/util/header.c  | 2 +-
 tools/perf/util/machine.c | 2 +-
 4 files changed, 4 insertions(+), 7 deletions(-)
tools/perf/util/dso.c +1 -4
index a17e5301af29..e2eeadfad275 100644
@@ -176,16 +176,13 @@ bool is_kmodule_extension(const char *ext)
 	return false;
 }
 
-bool is_kernel_module(const char *pathname, bool *compressed)
+bool is_kernel_module(const char *pathname)
 {
 	struct kmod_path m;
 
 	if (kmod_path__parse(&m, pathname))
 		return NULL;
 
-	if (compressed)
-		*compressed = m.comp;
-
 	return m.kmod;
 }
 
tools/perf/util/dso.h +1 -1
index a712cbcd3a72..357d319a87d1 100644
@@ -190,7 +190,7 @@ int dso__read_binary_type_filename(const struct dso *dso, enum dso_binary_type t
 				   char *root_dir, char *filename, size_t size);
 bool is_supported_compression(const char *ext);
 bool is_kmodule_extension(const char *ext);
-bool is_kernel_module(const char *pathname, bool *compressed);
+bool is_kernel_module(const char *pathname);
 bool decompress_to_file(const char *ext, const char *filename, int output_fd);
 bool dso__needs_decompress(struct dso *dso);
 
tools/perf/util/header.c +1 -1
index 1f407f7352a7..fb432153e2aa 100644
@@ -1266,7 +1266,7 @@ static int __event_process_build_id(struct build_id_event *bev,
 
 		dso__set_build_id(dso, &bev->build_id);
 
-		if (!is_kernel_module(filename, NULL))
+		if (!is_kernel_module(filename))
 			dso->kernel = dso_type;
 
 		build_id__sprintf(dso->build_id, sizeof(dso->build_id),
tools/perf/util/machine.c +1 -1
index 0c2d9e68996b..03b73261ec3e 100644
@@ -1093,7 +1093,7 @@ static int machine__process_kernel_mmap_event(struct machine *machine,
 		struct dso *dso;
 
 		list_for_each_entry(dso, &machine->kernel_dsos.head, node) {
-			if (is_kernel_module(dso->long_name, NULL))
+			if (is_kernel_module(dso->long_name))
 				continue;
 
 			kernel = dso;
Signature · 1 line
1.9.3
author reviewer
#20 lore raw

PATCH 12/14 perf tools: Remove is_kmodule_extension function

In reply to Jiri Olsa

Because it's no longer needed.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/util/dso.c | 11 -----------
 tools/perf/util/dso.h |  1 -
 2 files changed, 12 deletions(-)
tools/perf/util/dso.c +0 -11
index e2eeadfad275..34b9ad0ff9da 100644
@@ -165,17 +165,6 @@ bool is_supported_compression(const char *ext)
 	return false;
 }
 
-bool is_kmodule_extension(const char *ext)
-{
-	if (strncmp(ext, "ko", 2))
-		return false;
-
-	if (ext[2] == '\0' || (ext[2] == '.' && is_supported_compression(ext+3)))
-		return true;
-
-	return false;
-}
-
 bool is_kernel_module(const char *pathname)
 {
 	struct kmod_path m;
tools/perf/util/dso.h +0 -1
index 357d319a87d1..ec58641ce5dc 100644
@@ -189,7 +189,6 @@ char dso__symtab_origin(const struct dso *dso);
 int dso__read_binary_type_filename(const struct dso *dso, enum dso_binary_type type,
 				   char *root_dir, char *filename, size_t size);
 bool is_supported_compression(const char *ext);
-bool is_kmodule_extension(const char *ext);
 bool is_kernel_module(const char *pathname);
 bool decompress_to_file(const char *ext, const char *filename, int output_fd);
 bool dso__needs_decompress(struct dso *dso);
Signature · 1 line
1.9.3
author reviewer
#21 lore raw

PATCH 13/14 perf tools: Try to lookup kernel module map before creating one

In reply to Jiri Olsa

Currently we assume machine__new_module is called only once for each module so we create its map&dso unconditionally.

However it's possible that it's called multiple times for same module. Like for perf record:
1) via machine__create_module during machine init
2) via kernel MMAP event processing

Trying to lookup kernel module map before creating one.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/util/machine.c | 5 +++++
 1 file changed, 5 insertions(+)
tools/perf/util/machine.c +5 -0
index 03b73261ec3e..5acfb11fa23f 100644
@@ -498,6 +498,11 @@ struct map *machine__new_module(struct machine *machine, u64 start,
 	if (kmod_path__parse_name(&m, filename))
 		return NULL;
 
+	map = map_groups__find_by_name(&machine->kmaps, MAP__FUNCTION,
+				       m.name);
+	if (map)
+		goto out;
+
 	dso = machine__module_dso(machine, &m, filename);
 	if (dso == NULL)
 		goto out;
Signature · 1 line
1.9.3
author reviewer
#22 lore raw

PATCH 14/14 perf annotate: Allow annotation for decompressed kernel modules

In reply to Jiri Olsa

Decompressing kernel module file for objdump command if needed. Annotation commands now display annotation for compressed kernel modules.

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/util/annotate.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)
tools/perf/util/annotate.c +31 -1
index 61bf9128e1f2..b72086eca943 100644
@@ -1008,6 +1008,32 @@ fallback:
 			}
 			filename = symfs_filename;
 		}
+	} else if (dso__needs_decompress(dso)) {
+		char tmp[PATH_MAX];
+		struct kmod_path m;
+		int fd;
+		bool ret;
+
+		if (kmod_path__parse_ext(&m, symfs_filename))
+			goto out_free_filename;
+
+		snprintf(tmp, PATH_MAX, "/tmp/perf-kmod-XXXXXX");
+
+		fd = mkstemp(tmp);
+		if (fd < 0) {
+			free(m.ext);
+			goto out_free_filename;
+		}
+
+		ret = decompress_to_file(m.ext, symfs_filename, fd);
+
+		free(m.ext);
+		close(fd);
+
+		if (!ret)
+			goto out_free_filename;
+
+		strcpy(symfs_filename, tmp);
 	}
 
 	snprintf(command, sizeof(command),
@@ -1027,7 +1053,7 @@ fallback:
 
 	file = popen(command, "r");
 	if (!file)
-		goto out_free_filename;
+		goto out_remove_tmp;
 
 	while (!feof(file))
 		if (symbol__parse_objdump_line(sym, map, file, privsize,
@@ -1042,6 +1068,10 @@ fallback:
 		delete_last_nop(sym);
 
 	pclose(file);
+
+out_remove_tmp:
+	if (dso__needs_decompress(dso))
+		unlink(symfs_filename);
 out_free_filename:
 	if (delete_extract)
 		kcore_extract__delete(&kce);
Signature · 1 line
1.9.3
maintainer
#23 lore raw
In reply to Jiri Olsa
Quoted 11 lines · Jiri Olsa

hi,
adding support to decompress kernel modules on Fedora 21 (lzma compression) vaguely continuing in previous RFC:
http://marc.info/?l=linux-kernel&m=142261340327588&w=2

I'm basically extending he decompression framework Namhyung introduced earlier plus other fixes for kernel modules dso objects handling.

Last patch adds decompression support to annotate command, so it now displays annotation for compressed kernel modules.

Nice work! I went through the patches and liked the kmod_path__parse change. :) Please feel free to add my Ack's to this series when you send v2.

Thanks,
Namhyung

Quoted 60 lines

Current work available at:
git://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
perf/lzma

thanks,
jirka

Cc Adrian Hunter <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Corey Ashford <[email protected]>
Cc David Ahern <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>

---
Jiri Olsa (14):
perf tools: Remove superfluous thread->comm_set setting perf tools: Add feature check for lzma library
perf tools: Add lzma decompression support for kernel module perf tools: Add kmod_path__parse function
perf tools: Add dsos__new function
perf tools: Add machine__module_dso function
perf tools: Use kmod_path__parse for machine__new_dso perf tools: Use kmod_path__parse in map_groups__set_modules_path_dir perf tools: Use kmod_path__parse in decompress_kmodule perf tools: Use kmod_path__parse in is_kernel_module perf tools: Remove compressed argument from is_kernel_module perf tools: Remove is_kmodule_extension function
perf tools: Try to lookup kernel module map before creating one perf annotate: Allow annotation for decompressed kernel modules

 tools/perf/Makefile.perf                     |   2 ++
 tools/perf/config/Makefile                   |  16 ++++++++--
 tools/perf/config/feature-checks/Makefile    |   6 +++-
 tools/perf/config/feature-checks/test-lzma.c |  10 ++++++
 tools/perf/tests/Build                       |   1 +
 tools/perf/tests/builtin-test.c              |   4 +++
 tools/perf/tests/kmod-path.c                 |  73 ++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/tests/tests.h                     |   1 +
 tools/perf/util/Build                        |   1 +
 tools/perf/util/annotate.c                   |  32 +++++++++++++++++++-
 tools/perf/util/dso.c                        |  52 ++++++++++++--------------------
 tools/perf/util/dso.h                        |   4 +--
 tools/perf/util/header.c                     |   2 +-
 tools/perf/util/lzma.c                       |  95 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/machine.c                    | 142 +++++++++++++++++++++++++++++++++++++++++++++-----------------------------------------
 tools/perf/util/symbol-elf.c                 |  25 ++++++++++-----
 tools/perf/util/thread.c                     |   1 -
 tools/perf/util/util.c                       |  66 ++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/util.h                       |  18 +++++++++++
 19 files changed, 436 insertions(+), 115 deletions(-)
 create mode 100644 tools/perf/config/feature-checks/test-lzma.c
 create mode 100644 tools/perf/tests/kmod-path.c
 create mode 100644 tools/perf/util/lzma.c
#24 lore raw
Quoted 10 lines · Namhyung Kim
Quoted 9 lines · Jiri Olsa

hi,
adding support to decompress kernel modules on Fedora 21 (lzma compression) vaguely continuing in previous RFC:
http://marc.info/?l=linux-kernel&m=142261340327588&w=2

I'm basically extending he decompression framework Namhyung introduced earlier plus other fixes for kernel modules dso objects handling.

Quoted 6 lines
Quoted 2 lines

Last patch adds decompression support to annotate command, so it now displays annotation for compressed kernel modules.

Nice work! I went through the patches and liked the kmod_path__parse change. :) Please feel free to add my Ack's to this series when you send v2.

thanks! will send v2 shortly..

jirka