perf/urgent fixes

Hi Ingo,

Please consider pulling,

- Arnaldo

Test results at the end of this message, as usual.

The following changes since commit 3e411b0ee7c7bf0cbe2bd5961f84a02f0451ad57:

Merge tag 'perf-urgent-for-mingo-4.12-20170606' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/urgent (2017-06-07 17:50:19 +0200)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-urgent-for-mingo-4.12-20170608

for you to fetch changes up to b89fe63fbafe307fb72546f7a2320380bf41bdd4:

perf symbols: Kill dso__build_id_is_kmod() (2017-06-08 15:39:34 -0300)

----------------------------------------------------------------
perf/urgent fixes:

- Fixes for handling compressed kernel modules (Namhyung Kim)

- Fix handling old style build-id cache ($HOME/.debug/) (Namhyung Kim)

- 'perf script' python/perl documentation fixes: outdated comments, invalid code snippets, etc (SeongJae Park)

----------------------------------------------------------------
Namhyung Kim (9):
perf annotate: Fix symbolic link of build-id cache perf tools: Fix a memory leak in __open_dso() perf tools: Introduce dso__decompress_kmodule_{fd,path} perf annotate: Use dso__decompress_kmodule_path() perf tools: Decompress kernel module when reading DSO data perf tools: Consolidate error path in __open_dso() perf tests: Decompress kernel module before objdump perf symbols: Keep DSO->symtab_type after decompress perf symbols: Kill dso__build_id_is_kmod()

SeongJae Park (6):
perf probe: Fix examples section of documentation perf script: Fix outdated comment for perf-trace-python perf script: Fix documentation errors
perf script python: Fix wrong code snippets in documentation perf script python: Updated trace_unhandled() signature perf script python: Remove dups in documentation examples

 tools/perf/Documentation/perf-probe.txt            |  8 +-
 tools/perf/Documentation/perf-script-perl.txt      |  2 +-
 tools/perf/Documentation/perf-script-python.txt    | 23 +++---
 tools/perf/tests/code-reading.c                    | 20 ++++-
 tools/perf/util/annotate.c                         | 37 +++-------
 tools/perf/util/build-id.c                         | 45 ------------
 tools/perf/util/build-id.h                         |  1 -
 tools/perf/util/dso.c                              | 85 ++++++++++++++++++++--
 tools/perf/util/dso.h                              |  6 ++
 .../util/scripting-engines/trace-event-python.c    |  2 +-
 tools/perf/util/symbol-elf.c                       | 38 +---------
 tools/perf/util/symbol.c                           |  4 -
 12 files changed, 136 insertions(+), 135 deletions(-)

Test results:

The first ones are container (docker) based builds of tools/perf with and without libelf support, objtool where it is supported and samples/bpf/, ditto. Where clang is available, it is also used to build perf with/without libelf.

Several are cross builds, the ones with -x-ARCH, and the android one, and those may not have all the features built, due to lack of multi-arch devel packages, available and being used so far on just a few, like debian:experimental-x-{arm64,mipsel}.

The 'perf test' one will perform a variety of tests exercising tools/perf/util/, tools/lib/{bpf,traceevent,etc}, as well as run perf commands with a variety of command line event specifications to then intercept the sys_perf_event syscall to check that the perf_event_attr fields are set up as expected, among a variety of other unit tests.

Then there is the 'make -C tools/perf build-test' ones, that build tools/perf/ with a variety of feature sets, exercising the build with an incomplete set of features as well as with a complete one. It is planned to have it run on each of the containers mentioned above, using some container orchestration infrastructure. Get in contact if interested in helping having this in place.

[root@jouet ~]# dm
1 alpine:3.4: Ok
2 alpine:3.5: Ok
3 alpine:3.6: Ok
4 alpine:edge: Ok
5 android-ndk:r12b-arm: Ok
6 archlinux:latest: Ok
7 centos:5: Ok
8 centos:6: Ok
9 centos:7: Ok
10 debian:7: Ok
11 debian:8: Ok
12 debian:9: Ok
13 debian:experimental: Ok
14 debian:experimental-x-arm64: Ok
15 debian:experimental-x-mips: Ok
16 debian:experimental-x-mips64: Ok
17 debian:experimental-x-mipsel: Ok
18 fedora:20: Ok
19 fedora:21: Ok
20 fedora:22: Ok
21 fedora:23: Ok
22 fedora:24: Ok
23 fedora:24-x-ARC-uClibc: Ok
24 fedora:25: Ok
25 fedora:rawhide: Ok
26 mageia:5: Ok
27 opensuse:13.2: Ok
28 opensuse:42.1: Ok
29 opensuse:tumbleweed: Ok
30 ubuntu:12.04.5: Ok
31 ubuntu:14.04.4: Ok
32 ubuntu:14.04.4-x-linaro-arm64: Ok
33 ubuntu:15.10: Ok
34 ubuntu:16.04: Ok
35 ubuntu:16.04-x-arm: Ok
36 ubuntu:16.04-x-arm64: Ok
37 ubuntu:16.04-x-powerpc: Ok
38 ubuntu:16.04-x-powerpc64: Ok
39 ubuntu:16.04-x-powerpc64el: Ok
40 ubuntu:16.04-x-s390: Ok
41 ubuntu:16.10: Ok
41 ubuntu:16.10: Ok
42 ubuntu:17.04: Ok
#

[root@jouet ~]# uname -a
Linux jouet 4.11.0-rc8+ #8 SMP Wed Apr 26 08:50:41 -03 2017 x86_64 x86_64 x86_64 GNU/Linux [root@jouet ~]# perf test

   1: vmlinux symtab matches kallsyms            : Ok
   2: Detect openat syscall event                : Ok
   3: Detect openat syscall event on all cpus    : Ok
   4: Read samples using the mmap interface      : Ok
   5: Parse event definition strings             : Ok
   6: Simple expression parser                   : Ok
   7: PERF_RECORD_* events & perf_sample fields  : Ok
   8: Parse perf pmu format                      : Ok
   9: DSO data read                              : Ok
  10: DSO data cache                             : Ok
  11: DSO data reopen                            : Ok
  12: Roundtrip evsel->name                      : Ok
  13: Parse sched tracepoints fields             : Ok
  14: syscalls:sys_enter_openat event fields     : Ok
  15: Setup struct perf_event_attr               : Ok
  16: Match and link multiple hists              : Ok
  17: 'import perf' in python                    : Ok
  18: Breakpoint overflow signal handler         : Ok
  19: Breakpoint overflow sampling               : Ok
  20: Number of exit events of a simple workload : Ok
  21: Software clock events period values        : Ok
  22: Object code reading                        : Ok
  23: Sample parsing                             : Ok
  24: Use a dummy software event to keep tracking: Ok
  25: Parse with no sample_id_all bit set        : Ok
  26: Filter hist entries                        : Ok
  27: Lookup mmap thread                         : Ok
  28: Share thread mg                            : Ok
  29: Sort output of hist entries                : Ok
  30: Cumulate child hist entries                : Ok
  31: Track with sched_switch                    : Ok
  32: Filter fds with revents mask in a fdarray  : Ok
  33: Add fd to a fdarray, making it autogrow    : Ok
  34: kmod_path__parse                           : Ok
  35: Thread map                                 : Ok
  36: LLVM search and compile                    :
  36.1: Basic BPF llvm compile                    : Ok
  36.2: kbuild searching                          : Ok
  36.3: Compile source for BPF prologue generation: Ok
  36.4: Compile source for BPF relocation         : Ok
  37: Session topology                           : Ok
  38: BPF filter                                 :
  38.1: Basic BPF filtering                      : Ok
  38.2: BPF pinning                              : Ok
  38.3: BPF prologue generation                  : Ok
  38.4: BPF relocation checker                   : Ok
  39: Synthesize thread map                      : Ok
  40: Remove thread map                          : Ok
  41: Synthesize cpu map                         : Ok
  42: Synthesize stat config                     : Ok
  43: Synthesize stat                            : Ok
  44: Synthesize stat round                      : Ok
  45: Synthesize attr update                     : Ok
  46: Event times                                : Ok
  47: Read backward ring buffer                  : Ok
  48: Print cpu map                              : Ok
  49: Probe SDT events                           : Ok
  50: is_printable_array                         : Ok
  51: Print bitmap                               : Ok
  52: perf hooks                                 : Ok
  53: builtin clang support                      : Skip (not compiled in)
  54: unit_number__scnprintf                     : Ok
  55: x86 rdpmc                                  : Ok
  56: Convert perf time to TSC                   : Ok
  57: DWARF unwind                               : Ok

58: x86 instruction decoder - new instructions : Ok
59: Intel cqm nmi context read : Skip
[root@jouet ~]#

[acme@seventh linux]$ make -C tools/perf build-test
make: Entering directory `/home/acme/git/linux/tools/perf'
- tarpkg: ./tests/perf-targz-src-pkg .
make_no_libbpf_O: make NO_LIBBPF=1
make_no_libaudit_O: make NO_LIBAUDIT=1
make_with_clangllvm_O: make LIBCLANGLLVM=1
make_no_auxtrace_O: make NO_AUXTRACE=1
make_static_O: make LDFLAGS=-static
make_debug_O: make DEBUG=1
make_tags_O: make tags
make_install_bin_O: make install-bin
make_no_libpython_O: make NO_LIBPYTHON=1
make_no_backtrace_O: make NO_BACKTRACE=1
make_help_O: make help
make_no_demangle_O: make NO_DEMANGLE=1
make_util_map_o_O: make util/map.o
make_doc_O: make doc
make_no_ui_O: make NO_NEWT=1 NO_SLANG=1 NO_GTK2=1
make_with_babeltrace_O: make LIBBABELTRACE=1
make_no_libperl_O: make NO_LIBPERL=1
make_no_libelf_O: make NO_LIBELF=1
make_minimal_O: make NO_LIBPERL=1 NO_LIBPYTHON=1 NO_NEWT=1 NO_GTK2=1 NO_DEMANGLE=1 NO_LIBELF=1 NO_LIBUNWIND=1 NO_BACKTRACE=1 NO_LIBNUMA=1 NO_LIBAUDIT=1 NO_LIBBIONIC=1 NO_LIBDW_DWARF_UNWIND=1 NO_AUXTRACE=1 NO_LIBBPF=1 NO_LIBCRYPTO=1 NO_SDT=1 NO_JVMTI=1
make_no_libdw_dwarf_unwind_O: make NO_LIBDW_DWARF_UNWIND=1
make_install_prefix_slash_O: make install prefix=/tmp/krava/
make_no_libunwind_O: make NO_LIBUNWIND=1
make_install_O: make install
make_no_libnuma_O: make NO_LIBNUMA=1
make_no_gtk2_O: make NO_GTK2=1
make_cscope_O: make cscope
make_no_slang_O: make NO_SLANG=1
make_pure_O: make
make_no_libbionic_O: make NO_LIBBIONIC=1
make_util_pmu_bison_o_O: make util/pmu-bison.o
make_no_newt_O: make NO_NEWT=1
make_no_scripts_O: make NO_LIBPYTHON=1 NO_LIBPERL=1
make_clean_all_O: make clean all
make_perf_o_O: make perf.o
make_install_prefix_O: make install prefix=/tmp/krava
OK
make: Leaving directory `/home/acme/git/linux/tools/perf'
[acme@seventh linux]$

From: SeongJae Park <[email protected]>

An example in perf-probe documentation for pattern of function name based probe addition is not providing example command for that case.

This commit fixes the example to give appropriate example command.

---
 tools/perf/Documentation/perf-probe.txt | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

index e6c9902c6d82..165c2b1d4317 100644
@@ -240,9 +240,13 @@ Add a probe on schedule() function 12th line with recording cpu local variable:
  or
  ./perf probe --add='schedule:12 cpu'
 
- this will add one or more probes which has the name start with "schedule".
+Add one or more probes which has the name start with "schedule".
 
- Add probes on lines in schedule() function which calls update_rq_clock().
+ ./perf probe schedule*
+ or
+ ./perf probe --add='schedule*'
+
+Add probes on lines in schedule() function which calls update_rq_clock().
 
  ./perf probe 'schedule;update_rq_clock*'
  or

2.9.4

From: SeongJae Park <[email protected]>

Script generated by the '--gen-script' option contains an outdated comment. It mentions a 'perf-trace-python' document while it has been renamed to 'perf-script-python'. Fix it.

---
 tools/perf/util/scripting-engines/trace-event-python.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 9d92af7d0718..40de3cb40d21 100644
@@ -1219,7 +1219,7 @@ static int python_generate_script(struct pevent *pevent, const char *outfile)
 	fprintf(ofp, "# be retrieved using Python functions of the form "
 		"common_*(context).\n");
 
-	fprintf(ofp, "# See the perf-trace-python Documentation for the list "
+	fprintf(ofp, "# See the perf-script-python Documentation for the list "
 		"of available functions.\n\n");
 
 	fprintf(ofp, "import os\n");

2.9.4

From: SeongJae Park <[email protected]>

This commit fixes two errors in documents for perf-script-python and perf-script-perl as below:

- /sys/kernel/debug/tracing events -> /sys/kernel/debug/tracing/events/
- trace_handled -> trace_unhandled

---
 tools/perf/Documentation/perf-script-perl.txt   | 2 +-
 tools/perf/Documentation/perf-script-python.txt | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

index dfbb506d2c34..142606c0ec9c 100644
@@ -39,7 +39,7 @@ EVENT HANDLERS
 When perf script is invoked using a trace script, a user-defined
 'handler function' is called for each event in the trace.  If there's
 no handler function defined for a given event type, the event is
-ignored (or passed to a 'trace_handled' function, see below) and the
+ignored (or passed to a 'trace_unhandled' function, see below) and the
 next event is processed.
 
 Most of the event's field values are passed as arguments to the

index 54acba221558..087b87c956ba 100644
@@ -321,7 +321,7 @@ So those are the essential steps in writing and running a script.  The
 process can be generalized to any tracepoint or set of tracepoints
 you're interested in - basically find the tracepoint(s) you're
 interested in by looking at the list of available events shown by
-'perf list' and/or look in /sys/kernel/debug/tracing events for
+'perf list' and/or look in /sys/kernel/debug/tracing/events/ for
 detailed event and field info, record the corresponding trace data
 using 'perf record', passing it the list of interesting events,
 generate a skeleton script using 'perf script -g python' and modify the

@@ -437,7 +437,7 @@ EVENT HANDLERS
 When perf script is invoked using a trace script, a user-defined
 'handler function' is called for each event in the trace.  If there's
 no handler function defined for a given event type, the event is
-ignored (or passed to a 'trace_handled' function, see below) and the
+ignored (or passed to a 'trace_unhandled' function, see below) and the
 next event is processed.
 
 Most of the event's field values are passed as arguments to the

2.9.4

From: SeongJae Park <[email protected]>

This commit fixes wrong code snippets for trace_begin() and trace_end() function example definition.

---
 tools/perf/Documentation/perf-script-python.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

index 087b87c956ba..bad6e93cceff 100644
@@ -532,7 +532,7 @@ can implement a set of optional functions:
 gives scripts a chance to do setup tasks:
 
 ----
-def trace_begin:
+def trace_begin():
     pass
 ----
 

@@ -541,7 +541,7 @@ def trace_begin:
  as display results:
 
 ----
-def trace_end:
+def trace_end():
     pass
 ----
 

2.9.4

From: SeongJae Park <[email protected]>

Default function signature of trace_unhandled() got changed to include a field dict, but its documentation, perf-script-python.txt has not been updated. Fix it.

---
 tools/perf/Documentation/perf-script-python.txt | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

index bad6e93cceff..378783fe2b6b 100644
@@ -149,10 +149,8 @@ def raw_syscalls__sys_enter(event_name, context, common_cpu,
 		print "id=%d, args=%s\n" % \
 		(id, args),
 
-def trace_unhandled(event_name, context, common_cpu, common_secs, common_nsecs,
-		common_pid, common_comm):
-		print_header(event_name, common_cpu, common_secs, common_nsecs,
-		common_pid, common_comm)
+def trace_unhandled(event_name, context, event_fields_dict):
+		print ' '.join(['%s=%s'%(k,str(v))for k,v in sorted(event_fields_dict.items())])
 
 def print_header(event_name, cpu, secs, nsecs, pid, comm):
 	print "%-20s %5u %05u.%09u %8u %-20s " % \

@@ -550,8 +548,7 @@ def trace_end():
  of common arguments are passed into it:
 
 ----
-def trace_unhandled(event_name, context, common_cpu, common_secs,
-        common_nsecs, common_pid, common_comm):
+def trace_unhandled(event_name, context, event_fields_dict):
     pass
 ----
 

2.9.4

From: SeongJae Park <[email protected]>

Few shell command examples in perf-script-python.txt has few nitpicks include:

- tools/perf/scripts/python directory listing command is unnecessarily repeated.
- few examples contain additional information in command prompt unnecessarily and inconsistently.

This commit fixes them to enhance readability of the document.

---
 tools/perf/Documentation/perf-script-python.txt | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

index 378783fe2b6b..51ec2d20068a 100644
@@ -332,7 +332,7 @@ right place, you can have your script listed alongside the other
 scripts listed by the 'perf script -l' command e.g.:
 
 ----
-root@tropicana:~# perf script -l
+# perf script -l
 List of available trace scripts:
   wakeup-latency                       system-wide min/max/avg wakeup latency
   rw-by-file <comm>                    r/w activity for a program, by file

@@ -381,8 +381,6 @@ source tree:
 
 ----
 # ls -al kernel-source/tools/perf/scripts/python
-
-root@tropicana:/home/trz/src/tip# ls -al tools/perf/scripts/python
 total 32
 drwxr-xr-x 4 trz trz 4096 2010-01-26 22:30 .
 drwxr-xr-x 4 trz trz 4096 2010-01-26 22:29 ..

@@ -397,7 +395,7 @@ otherwise your script won't show up at run-time), 'perf script -l'
 should show a new entry for your script:
 
 ----
-root@tropicana:~# perf script -l
+# perf script -l
 List of available trace scripts:
   wakeup-latency                       system-wide min/max/avg wakeup latency
   rw-by-file <comm>                    r/w activity for a program, by file

2.9.4

From: Namhyung Kim <[email protected]>

The commit 6ebd2547dd24 ("perf annotate: Fix a bug following symbolic link of a build-id file") changed to use dirname to follow the symlink. But it only considers new-style build-id cache names so old names fail on readlink() and force to use system path which might not available.

---
 tools/perf/util/annotate.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

index 1367d7e35242..df4486c3a2fa 100644
@@ -1321,6 +1321,7 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil
 	char linkname[PATH_MAX];
 	char *build_id_filename;
 	char *build_id_path = NULL;
+	char *pos;
 
 	if (dso->symtab_type == DSO_BINARY_TYPE__KALLSYMS &&
 	    !dso__is_kcore(dso))

@@ -1340,7 +1341,14 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil
 	if (!build_id_path)
 		return -1;
 
-	dirname(build_id_path);
+	/*
+	 * old style build-id cache has name of XX/XXXXXXX.. while
+	 * new style has XX/XXXXXXX../{elf,kallsyms,vdso}.
+	 * extract the build-id part of dirname in the new style only.
+	 */
+	pos = strrchr(build_id_path, '/');
+	if (pos && strlen(pos) < SBUILD_ID_SIZE - 2)
+		dirname(build_id_path);
 
 	if (dso__is_kcore(dso) ||
 	    readlink(build_id_path, linkname, sizeof(linkname)) < 0 ||

2.9.4

From: Namhyung Kim <[email protected]>

The 'name' variable should be freed on the error path.

---
 tools/perf/util/dso.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

index b27d127cdf68..1f29e4fe7af0 100644
@@ -412,8 +412,10 @@ static int __open_dso(struct dso *dso, struct machine *machine)
 		return -EINVAL;
 	}
 
-	if (!is_regular_file(name))
+	if (!is_regular_file(name)) {
+		free(name);
 		return -EINVAL;
+	}
 
 	fd = do_open(name);
 	free(name);

2.9.4

From: Namhyung Kim <[email protected]>

Move decompress_kmodule() to util/dso.c and split it into two functions returning fd and (decompressed) file path. The existing user only wants the fd version but the path version will be used soon.

---
 tools/perf/util/dso.c        | 58 ++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/dso.h        |  6 +++++
 tools/perf/util/symbol-elf.c | 36 +--------------------------
 3 files changed, 65 insertions(+), 35 deletions(-)

index 1f29e4fe7af0..b346b8eba65c 100644
@@ -248,6 +248,64 @@ bool dso__needs_decompress(struct dso *dso)
 		dso->symtab_type == DSO_BINARY_TYPE__GUEST_KMODULE_COMP;
 }
 
+static int decompress_kmodule(struct dso *dso, const char *name, char *tmpbuf)
+{
+	int fd = -1;
+	struct kmod_path m;
+
+	if (!dso__needs_decompress(dso))
+		return -1;
+
+	if (kmod_path__parse_ext(&m, dso->long_name))
+		return -1;
+
+	if (!m.comp)
+		goto out;
+
+	fd = mkstemp(tmpbuf);
+	if (fd < 0) {
+		dso->load_errno = errno;
+		goto out;
+	}
+
+	if (!decompress_to_file(m.ext, name, fd)) {
+		dso->load_errno = DSO_LOAD_ERRNO__DECOMPRESSION_FAILURE;
+		close(fd);
+		fd = -1;
+	}
+
+out:
+	free(m.ext);
+	return fd;
+}
+
+int dso__decompress_kmodule_fd(struct dso *dso, const char *name)
+{
+	char tmpbuf[] = KMOD_DECOMP_NAME;
+	int fd;
+
+	fd = decompress_kmodule(dso, name, tmpbuf);
+	unlink(tmpbuf);
+	return fd;
+}
+
+int dso__decompress_kmodule_path(struct dso *dso, const char *name,
+				 char *pathname, size_t len)
+{
+	char tmpbuf[] = KMOD_DECOMP_NAME;
+	int fd;
+
+	fd = decompress_kmodule(dso, name, tmpbuf);
+	if (fd < 0) {
+		unlink(tmpbuf);
+		return -1;
+	}
+
+	strncpy(pathname, tmpbuf, len);
+	close(fd);
+	return 0;
+}
+
 /*
  * Parses kernel module specified in @path and updates
  * @m argument like:

index 5fe2ab5877bd..bd061ba7b47c 100644
@@ -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;

index 1fb2efae4f02..d342e771dbad 100644
@@ -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.9.4

From: Namhyung Kim <[email protected]>

Convert open-coded decompress routine to use the function.

---
 tools/perf/util/annotate.c | 27 +++------------------------
 1 file changed, 3 insertions(+), 24 deletions(-)

index df4486c3a2fa..ddbd56df9187 100644
@@ -1431,31 +1431,10 @@ int symbol__disassemble(struct symbol *sym, struct map *map, const char *arch_na
 				sizeof(symfs_filename));
 		}
 	} else if (dso__needs_decompress(dso)) {
-		char tmp[PATH_MAX];
-		struct kmod_path m;
-		int fd;
-		bool ret;
+		char tmp[KMOD_DECOMP_LEN];
 
-		if (kmod_path__parse_ext(&m, symfs_filename))
-			goto out;
-
-		snprintf(tmp, PATH_MAX, "/tmp/perf-kmod-XXXXXX");
-
-		fd = mkstemp(tmp);
-		if (fd < 0) {
-			free(m.ext);
-			goto out;
-		}
-
-		ret = decompress_to_file(m.ext, symfs_filename, fd);
-
-		if (ret)
-			pr_err("Cannot decompress %s %s\n", m.ext, symfs_filename);
-
-		free(m.ext);
-		close(fd);
-
-		if (!ret)
+		if (dso__decompress_kmodule_path(dso, symfs_filename,
+						 tmp, sizeof(tmp)) < 0)
 			goto out;
 
 		strcpy(symfs_filename, tmp);

2.9.4

From: Namhyung Kim <[email protected]>

Currently perf decompresses kernel modules when loading the symbol table but it missed to do it when reading raw data.

---
 tools/perf/util/dso.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

index b346b8eba65c..c63525d845c5 100644
@@ -475,7 +475,23 @@ static int __open_dso(struct dso *dso, struct machine *machine)
 		return -EINVAL;
 	}
 
+	if (dso__needs_decompress(dso)) {
+		char newpath[KMOD_DECOMP_LEN];
+		size_t len = sizeof(newpath);
+
+		if (dso__decompress_kmodule_path(dso, name, newpath, len) < 0) {
+			free(name);
+			return -dso->load_errno;
+		}
+
+		strcpy(name, newpath);
+	}
+
 	fd = do_open(name);
+
+	if (dso__needs_decompress(dso))
+		unlink(name);
+
 	free(name);
 	return fd;
 }

2.9.4

From: Namhyung Kim <[email protected]>

On failure, it should free the 'name', so clean up the error path using goto.

---
 tools/perf/util/dso.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

index c63525d845c5..4e7ab611377a 100644
@@ -454,7 +454,7 @@ static int do_open(char *name)
 
 static int __open_dso(struct dso *dso, struct machine *machine)
 {
-	int fd;
+	int fd = -EINVAL;
 	char *root_dir = (char *)"";
 	char *name = malloc(PATH_MAX);
 

@@ -465,23 +465,19 @@ static int __open_dso(struct dso *dso, struct machine *machine)
 		root_dir = machine->root_dir;
 
 	if (dso__read_binary_type_filename(dso, dso->binary_type,
-					    root_dir, name, PATH_MAX)) {
-		free(name);
-		return -EINVAL;
-	}
+					    root_dir, name, PATH_MAX))
+		goto out;
 
-	if (!is_regular_file(name)) {
-		free(name);
-		return -EINVAL;
-	}
+	if (!is_regular_file(name))
+		goto out;
 
 	if (dso__needs_decompress(dso)) {
 		char newpath[KMOD_DECOMP_LEN];
 		size_t len = sizeof(newpath);
 
 		if (dso__decompress_kmodule_path(dso, name, newpath, len) < 0) {
-			free(name);
-			return -dso->load_errno;
+			fd = -dso->load_errno;
+			goto out;
 		}
 
 		strcpy(name, newpath);

@@ -492,6 +488,7 @@ static int __open_dso(struct dso *dso, struct machine *machine)
 	if (dso__needs_decompress(dso))
 		unlink(name);
 
+out:
 	free(name);
 	return fd;
 }

2.9.4

From: Namhyung Kim <[email protected]>

If a kernel modules is compressed, it should be decompressed before running objdump to parse binary data correctly. This fixes a failure of object code reading test for me.

---
 tools/perf/tests/code-reading.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

index 1f14e7612cbb..94b7c7b02bde 100644
@@ -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.9.4

From: Namhyung Kim <[email protected]>

The symsrc__init() overwrites dso->symtab_type as symsrc->type in dso__load_sym(). But for compressed kernel modules in the build-id cache, it should have original symtab type to be decompressed as needed.

This fixes perf annotate to show disassembly of the function properly.

---
 tools/perf/util/symbol-elf.c | 2 ++
 1 file changed, 2 insertions(+)

index d342e771dbad..502505cf236a 100644
@@ -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.9.4

From: Namhyung Kim <[email protected]>

The commit e7ee40475760 ("perf symbols: Fix symbols searching for module in buildid-cache") added the function to check kernel modules reside in the build-id cache. This was because there's no way to identify a DSO which is actually a kernel module. So it searched linkname of the file and find ".ko" suffix.

But this does not work for compressed kernel modules and now such DSOs hCcave correct symtab_type now. So no need to check it anymore. This patch essentially reverts the commit.

---
 tools/perf/util/build-id.c | 45 ---------------------------------------------
 tools/perf/util/build-id.h |  1 -
 tools/perf/util/symbol.c   |  4 ----
 3 files changed, 50 deletions(-)

index 168cc49654e7..e0148b081bdf 100644
@@ -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)		\

index 8a89b195c1fc..96690a55c62c 100644
@@ -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,

index 8f2b068ff756..e7a98dbd2aed 100644
@@ -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.9.4

Pulled, thanks a lot Arnaldo!

Ingo