perf: Assorted fixes

There's no reason anymore to treat babel trace in a special way, because a) we no longer display its state b) the needed babeltrace library is now out and well adopted among distros.

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/perf/Makefile.config | 2 +-
 tools/perf/Makefile.perf   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

tools/perf/Makefile.config +1 -1

index f050f38d8fa3..12dec6ea5ed2 100644
@@ -780,7 +780,7 @@ else
   NO_PERF_READ_VDSOX32 := 1
 endif
 
-ifdef LIBBABELTRACE
+ifndef NO_LIBBABELTRACE
   $(call feature_check,libbabeltrace)
   ifeq ($(feature-libbabeltrace), 1)
     CFLAGS += -DHAVE_LIBBABELTRACE_SUPPORT $(LIBBABELTRACE_CFLAGS)

tools/perf/Makefile.perf +1 -1

index 68cf1360a3f3..9fdefd748e2e 100644
@@ -77,7 +77,7 @@ include ../scripts/utilities.mak
 #
 # Define NO_ZLIB if you do not want to support compressed kernel modules
 #
-# Define LIBBABELTRACE if you DO want libbabeltrace support
+# 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

Signature · 1 line

2.13.6

Em Sun, Jan 07, 2018 at 05:03:45PM +0100, Jiri Olsa escreveu:

Quoted 3 lines

There's no reason anymore to treat babel trace in a special way, because a) we no longer display its state b) the needed babeltrace library is now out and well adopted among distros.

Humm, after I applied this I got:

[acme@jouet perf]$ perf
perf: error while loading shared libraries: libbabeltrace-ctf.so.2: cannot open shared object file: No such file or directory [acme@jouet perf]$

So it detects it somehow, builds with it, then fails to find it in the LD_LIBRARY_PATH somehow?

- Arnaldo

Quoted 35 lines

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/perf/Makefile.config | 2 +-
 tools/perf/Makefile.perf   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

tools/perf/Makefile.config +1 -1

index f050f38d8fa3..12dec6ea5ed2 100644
@@ -780,7 +780,7 @@ else
   NO_PERF_READ_VDSOX32 := 1
 endif
 
-ifdef LIBBABELTRACE
+ifndef NO_LIBBABELTRACE
   $(call feature_check,libbabeltrace)
   ifeq ($(feature-libbabeltrace), 1)
     CFLAGS += -DHAVE_LIBBABELTRACE_SUPPORT $(LIBBABELTRACE_CFLAGS)

tools/perf/Makefile.perf +1 -1

index 68cf1360a3f3..9fdefd748e2e 100644
@@ -77,7 +77,7 @@ include ../scripts/utilities.mak
 #
 # Define NO_ZLIB if you do not want to support compressed kernel modules
 #
-# Define LIBBABELTRACE if you DO want libbabeltrace support
+# 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

Signature · 1 line

2.13.6

Em Mon, Jan 08, 2018 at 12:17:09PM -0300, Arnaldo Carvalho de Melo escreveu:

Quoted 5 lines

Em Sun, Jan 07, 2018 at 05:03:45PM +0100, Jiri Olsa escreveu:

Quoted 3 lines

There's no reason anymore to treat babel trace in a special way, because a) we no longer display its state b) the needed babeltrace library is now out and well adopted among distros.

Quoted 8 lines

Humm, after I applied this I got:

[acme@jouet perf]$ perf
perf: error while loading shared libraries: libbabeltrace-ctf.so.2: cannot open shared object file: No such file or directory [acme@jouet perf]$

So it detects it somehow, builds with it, then fails to find it in the LD_LIBRARY_PATH somehow?

[acme@jouet perf]$ ldd ~/bin/perf | grep babel
libbabeltrace-ctf.so.2 => not found
[acme@jouet perf]$

[acme@jouet perf]$ tail -5 /tmp/build/perf/feature/test-libbabeltrace.d
/usr/local/include/babeltrace/ctf-ir/visitor.h \
/usr/local/include/babeltrace/values.h \
/usr/local/include/babeltrace/ref.h \
/usr/local/include/babeltrace/graph/notification.h \
/usr/local/include/babeltrace/ctf-ir/stream-class.h
[acme@jouet perf]$

so I don't have it in LD_LIBRARY_PATH:

[acme@jouet perf]$ export LD_LIBRARY_PATH=/usr/local/lib/ [acme@jouet perf]$ ldd ~/bin/perf | grep babel

	libbabeltrace-ctf.so.2 => /usr/local/lib/libbabeltrace-ctf.so.2 (0x00007f6322e83000)

[acme@jouet perf]$ perf stat -e cycles sleep 1

Performance counter stats for 'sleep 1':

           384,636      cycles:u                                                    

       1.001294682 seconds time elapsed

[acme@jouet perf]$

Will try removing this local install and use what is in fedora27 instead...

Works:

[acme@jouet perf]$ ldd ~/bin/perf | grep babel

	libbabeltrace-ctf.so.1 => /lib64/libbabeltrace-ctf.so.1 (0x00007fcb688d0000)
	libbabeltrace.so.1 => /lib64/libbabeltrace.so.1 (0x00007fcb67e92000)

[acme@jouet perf]$ rpm -q libbabeltrace-devel
libbabeltrace-devel-1.5.3-1.fc27.x86_64
[acme@jouet perf]$

But then, if we show:

Auto-detecting system features:

...                         dwarf: [ on  ]
...            dwarf_getlocations: [ on  ]
...                         glibc: [ on  ]
...                          gtk2: [ on  ]
...                      libaudit: [ on  ]
...                        libbfd: [ on  ]
...                        libelf: [ on  ]
...                       libnuma: [ on  ]
...        numa_num_possible_cpus: [ on  ]
...                       libperl: [ on  ]
...                     libpython: [ on  ]
...                      libslang: [ on  ]
...                     libcrypto: [ on  ]
...                     libunwind: [ on  ]
...            libdw-dwarf-unwind: [ on  ]
...                          zlib: [ on  ]
...                          lzma: [ on  ]
...                     get_cpuid: [ on  ]
...                           bpf: [ on  ]

Shoudln't we tell the user that babeltrace was linked too?

On Mon, Jan 08, 2018 at 12:24:33PM -0300, Arnaldo Carvalho de Melo wrote:

SNIP

Quoted 33 lines

Works:

[acme@jouet perf]$ ldd ~/bin/perf | grep babel

	libbabeltrace-ctf.so.1 => /lib64/libbabeltrace-ctf.so.1 (0x00007fcb688d0000)
	libbabeltrace.so.1 => /lib64/libbabeltrace.so.1 (0x00007fcb67e92000)

[acme@jouet perf]$ rpm -q libbabeltrace-devel
libbabeltrace-devel-1.5.3-1.fc27.x86_64
[acme@jouet perf]$

But then, if we show:

Auto-detecting system features:

...                         dwarf: [ on  ]
...            dwarf_getlocations: [ on  ]
...                         glibc: [ on  ]
...                          gtk2: [ on  ]
...                      libaudit: [ on  ]
...                        libbfd: [ on  ]
...                        libelf: [ on  ]
...                       libnuma: [ on  ]
...        numa_num_possible_cpus: [ on  ]
...                       libperl: [ on  ]
...                     libpython: [ on  ]
...                      libslang: [ on  ]
...                     libcrypto: [ on  ]
...                     libunwind: [ on  ]
...            libdw-dwarf-unwind: [ on  ]
...                          zlib: [ on  ]
...                          lzma: [ on  ]
...                     get_cpuid: [ on  ]
...                           bpf: [ on  ]

Quoted 1 line

Shoudln't we tell the user that babeltrace was linked too?

the logic is just to simply display the state of features which we provide in FEATURE_DISPLAY variable, which is believed to be common and important enough to display status about

maybe we could display it with 'make VF=1' output?

Quoted 39 lines · Jiri Olsa

On Mon, Jan 08, 2018 at 12:24:33PM -0300, Arnaldo Carvalho de Melo wrote:

SNIP

Quoted 34 lines

Works:

[acme@jouet perf]$ ldd ~/bin/perf | grep babel

	libbabeltrace-ctf.so.1 => /lib64/libbabeltrace-ctf.so.1 (0x00007fcb688d0000)
	libbabeltrace.so.1 => /lib64/libbabeltrace.so.1 (0x00007fcb67e92000)

[acme@jouet perf]$ rpm -q libbabeltrace-devel
libbabeltrace-devel-1.5.3-1.fc27.x86_64
[acme@jouet perf]$

But then, if we show:

Auto-detecting system features:

...                         dwarf: [ on  ]
...            dwarf_getlocations: [ on  ]
...                         glibc: [ on  ]
...                          gtk2: [ on  ]
...                      libaudit: [ on  ]
...                        libbfd: [ on  ]
...                        libelf: [ on  ]
...                       libnuma: [ on  ]
...        numa_num_possible_cpus: [ on  ]
...                       libperl: [ on  ]
...                     libpython: [ on  ]
...                      libslang: [ on  ]
...                     libcrypto: [ on  ]
...                     libunwind: [ on  ]
...            libdw-dwarf-unwind: [ on  ]
...                          zlib: [ on  ]
...                          lzma: [ on  ]
...                     get_cpuid: [ on  ]
...                           bpf: [ on  ]

Shoudln't we tell the user that babeltrace was linked too?

Quoted 6 lines

the logic is just to simply display the state of features which we provide in FEATURE_DISPLAY variable, which is believed to be common and important enough to display status about

maybe we could display it with 'make VF=1' output?

like in patch attached ;-)

Quoted 45 lines · Jiri Olsa

Quoted 44 lines · Jiri Olsa

On Mon, Jan 08, 2018 at 12:24:33PM -0300, Arnaldo Carvalho de Melo wrote:

SNIP

Quoted 33 lines

Works:

[acme@jouet perf]$ ldd ~/bin/perf | grep babel

	libbabeltrace-ctf.so.1 => /lib64/libbabeltrace-ctf.so.1 (0x00007fcb688d0000)
	libbabeltrace.so.1 => /lib64/libbabeltrace.so.1 (0x00007fcb67e92000)

[acme@jouet perf]$ rpm -q libbabeltrace-devel
libbabeltrace-devel-1.5.3-1.fc27.x86_64
[acme@jouet perf]$

But then, if we show:

Auto-detecting system features:

...                         dwarf: [ on  ]
...            dwarf_getlocations: [ on  ]
...                         glibc: [ on  ]
...                          gtk2: [ on  ]
...                      libaudit: [ on  ]
...                        libbfd: [ on  ]
...                        libelf: [ on  ]
...                       libnuma: [ on  ]
...        numa_num_possible_cpus: [ on  ]
...                       libperl: [ on  ]
...                     libpython: [ on  ]
...                      libslang: [ on  ]
...                     libcrypto: [ on  ]
...                     libunwind: [ on  ]
...            libdw-dwarf-unwind: [ on  ]
...                          zlib: [ on  ]
...                          lzma: [ on  ]
...                     get_cpuid: [ on  ]
...                           bpf: [ on  ]

Quoted 1 line

Shoudln't we tell the user that babeltrace was linked too?

the logic is just to simply display the state of features which we provide in FEATURE_DISPLAY variable, which is believed to be common and important enough to display status about

Quoted 3 lines

Quoted 1 line

maybe we could display it with 'make VF=1' output?

like in patch attached ;-)

with changelog

thanks,
jirka

---
Display state of the rest of the features (FEATURE_TESTS_EXTRA) for 'make VF=1' build. These features are detected manually by perf's Makefile.config so they can't be displayed with the main list, but only after we're done in Makefile.config.

  $ make VF=1
    BUILD:   Doing 'make -j4' parallel build

Auto-detecting system features:

  ...                         dwarf: [ on  ]
  ...            dwarf_getlocations: [ on  ]
  ...                         glibc: [ on  ]
  ...                          gtk2: [ on  ]

SNIP

  ...                       timerfd: [ on  ]
  ...                  sched_getcpu: [ on  ]
  ...                           sdt: [ on  ]
  ...                         setns: [ on  ]

extra features:
... bionic: [ OFF ]
... compile-32: [ on ]
... compile-x32: [ OFF ]
... cplus-demangle: [ on ]
... hello: [ OFF ]

  ...                 libbabeltrace: [ on  ]
  ...                       liberty: [ on  ]
  ...                     liberty-z: [ on  ]

... libunwind-debug-frame: [ OFF ]
... libunwind-debug-frame-arm: [ OFF ]
... libunwind-debug-frame-aarch64: [ OFF ]

SNIP

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/perf/Makefile.config | 4 ++++
 1 file changed, 4 insertions(+)

tools/perf/Makefile.config +4 -0

index 12dec6ea5ed2..92265b32dddd 100644
@@ -947,6 +947,10 @@ define print_var_code
 endef
 
 ifeq ($(VF),1)
+  # Display EXTRA features which are detected manualy
+  # from here with feature_check call and thus cannot
+  # be partof global state output.
+  $(foreach feat,$(FEATURE_TESTS_EXTRA),$(call feature_print_status,$(feat),))
   $(call print_var,prefix)
   $(call print_var,bindir)
   $(call print_var,libdir)

Signature · 1 line

2.13.6

Quoted 9 lines · Jiri Olsa

Quoted 8 lines · Jiri Olsa

Quoted 7 lines · Arnaldo Carvalho de Melo

Quoted 1 line

Shoudln't we tell the user that babeltrace was linked too?

the logic is just to simply display the state of features which we provide in FEATURE_DISPLAY variable, which is believed to be common and important enough to display status about

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

maybe we could display it with 'make VF=1' output?

like in patch attached ;-)

with changelog

Arnaldo,
could you please check this one?

Currently we use perf_event_context::task_ctx_data to save and restore the LBR status when the task is scheduled out and in.

We don't allocate it for child contexts, which results in shorter task's LBR stack, because we don't save the history from previous run and start over every time we schedule the task in.

I made a test to generate samples with LBR call stack and got higher numbers on bigger chain depths:

before: after:
LBR call chain: nr: 1 60561 498127
LBR call chain: nr: 2 0 0
LBR call chain: nr: 3 107030 2172
LBR call chain: nr: 4 466685 62758
LBR call chain: nr: 5 2307319 878046
LBR call chain: nr: 6 48713 495218
LBR call chain: nr: 7 1040 4551

  LBR call chain: nr: 8         481        172
  LBR call chain: nr: 9         878        120
  LBR call chain: nr: 10       2377       6698
  LBR call chain: nr: 11      28830     151487
  LBR call chain: nr: 12      29347     339867
  LBR call chain: nr: 13          4         22
  LBR call chain: nr: 14          3         53

Cc Andi Kleen <[email protected]>

Signed-off-by Jiri Olsa <[email protected]>

---
 kernel/events/core.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

kernel/events/core.c +14 -0

index 4df5b695bf0d..55fb648a32b0 100644
@@ -10703,6 +10703,19 @@ inherit_event(struct perf_event *parent_event,
 	if (IS_ERR(child_event))
 		return child_event;
 
+
+	if ((child_event->attach_state & PERF_ATTACH_TASK_DATA) &&
+	    !child_ctx->task_ctx_data) {
+		struct pmu *pmu = child_event->pmu;
+
+		child_ctx->task_ctx_data = kzalloc(pmu->task_ctx_size,
+						   GFP_KERNEL);
+		if (!child_ctx->task_ctx_data) {
+			free_event(child_event);
+			return NULL;
+		}
+	}
+
 	/*
 	 * is_orphaned_event() and list_add_tail(&parent_event->child_list)
 	 * must be under the same lock in order to serialize against

@@ -10713,6 +10726,7 @@ inherit_event(struct perf_event *parent_event,
 	if (is_orphaned_event(parent_event) ||
 	    !atomic_long_inc_not_zero(&parent_event->refcount)) {
 		mutex_unlock(&parent_event->child_mutex);
+		/* task_ctx_data is freed with child_ctx */
 		free_event(child_event);
 		return NULL;
 	}

Signature · 1 line

2.13.6

It simplifies the code a bit, because we dump the callchain even if it's empty. With 'empty' callchain we can remove all the NULL-checking code paths.

Original-patch-from: Peter Zijlstra <[email protected]>

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>

---
 kernel/events/core.c | 30 ++++++++++++------------------
 1 file changed, 12 insertions(+), 18 deletions(-)

kernel/events/core.c +12 -18

index 5fc1ded4b450..4e1a1bf8d867 100644
@@ -5815,19 +5815,11 @@ void perf_output_sample(struct perf_output_handle *handle,
 		perf_output_read(handle, event);
 
 	if (sample_type & PERF_SAMPLE_CALLCHAIN) {
-		if (data->callchain) {
-			int size = 1;
-
-			if (data->callchain)
-				size += data->callchain->nr;
-
-			size *= sizeof(u64);
+		int size = 1;
 
-			__output_copy(handle, data->callchain, size);
-		} else {
-			u64 nr = 0;
-			perf_output_put(handle, nr);
-		}
+		size += data->callchain->nr;
+		size *= sizeof(u64);
+		__output_copy(handle, data->callchain, size);
 	}
 
 	if (sample_type & PERF_SAMPLE_RAW) {

@@ -5980,6 +5972,8 @@ static u64 perf_virt_to_phys(u64 virt)
 	return phys_addr;
 }
 
+static struct perf_callchain_entry __empty_callchain = { .nr = 0, };
+
 static struct perf_callchain_entry *
 perf_callchain(struct perf_event *event, struct pt_regs *regs)
 {

@@ -5988,12 +5982,14 @@ perf_callchain(struct perf_event *event, struct pt_regs *regs)
 	/* Disallow cross-task user callchains. */
 	bool crosstask = event->ctx->task && event->ctx->task != current;
 	const u32 max_stack = event->attr.sample_max_stack;
+	struct perf_callchain_entry *callchain;
 
 	if (!kernel && !user)
-		return NULL;
+		return &__empty_callchain;
 
-	return get_perf_callchain(regs, 0, kernel, user,
-				  max_stack, crosstask, true);
+	callchain = get_perf_callchain(regs, 0, kernel, user,
+				       max_stack, crosstask, true);
+	return callchain ?: &__empty_callchain;
 }
 
 void perf_prepare_sample(struct perf_event_header *header,

@@ -6018,9 +6014,7 @@ void perf_prepare_sample(struct perf_event_header *header,
 		int size = 1;
 
 		data->callchain = perf_callchain(event, regs);
-
-		if (data->callchain)
-			size += data->callchain->nr;
+		size += data->callchain->nr;
 
 		header->size += size * sizeof(u64);
 	}

Signature · 1 line

2.13.6

Em Sun, Jan 07, 2018 at 05:03:53PM +0100, Jiri Olsa escreveu:

tools/perf/util/event.c size_t perf_event__fprintf_switch(union perf_event *event, FILE *fp) · line 1441

Quoted 5 lines

+++ b/tools/perf/util/event.c

tools/perf/util/event.c +4 -0

+static size_t perf_event__fprintf_lost(union perf_event *event, FILE *fp)
+{
+	return fprintf(fp, " lost %lu\n", event->lost.lost);
+}

Had to make this:

	return fprintf(fp, " lost %" PRIu64 "\n", event->lost.lost);

To fix this:

CC /tmp/build/perf/util/event.o
util/event.c: In function 'perf_event__fprintf_lost':
util/event.c:1440:21: error: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'u64 {aka long long unsigned int}' [-Werror=format=]

  return fprintf(fp, " lost %lu\n", event->lost.lost);
                     ^

CC /tmp/build/perf/builtin-script.o
CC /tmp/build/perf/builtin-kmem.o
cc1: all warnings being treated as errors

on these systems:

17 debian:experimental-x-mips : FAIL mips-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 19 debian:experimental-x-mipsel: FAIL mipsel-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0

  25 fedora:24-x-ARC-uClibc      : FAIL arc-linux-gcc (ARCompact ISA Linux uClibc toolchain 2017.09-rc2) 7.1.1 20170710
  43 ubuntu:16.04-x-arm          : FAIL arm-linux-gnueabihf-gcc (Ubuntu/Linaro 5.4.0-6ubuntu1~16.04.4) 5.4.0 20160609
  45 ubuntu:16.04-x-powerpc      : FAIL powerpc-linux-gnu-gcc (Ubuntu 5.4.0-6ubuntu1~16.04.4) 5.4.0 20160609

- Arnaldo

Adding --task option to display monitored tasks stored in perf.data. Displaying pid/tid/ppid plus the command string aligned to distinguish parent and child tasks.

  $ perf record -a
  ...
  $ perf report --task
  #     pid     tid    ppid  comm
          0       0      -1 |swapper
          2       2       0 | kthreadd
      14080   14080       2 |  kworker/u17:1
          4       4       2 |  kworker/0:0H
          6       6       2 |  mm_percpu_wq
  ...
          1       1       0 | systemd
      23242   23242       1 |  firefox
      23242   23298   23242 |   Cache2 I/O
      23242   23304   23242 |   GMPThread
  ...
       1195    1195       1 |  login
       1611    1611    1195 |   bash
       1639    1639    1611 |    startx
       1663    1663    1639 |     xinit
       1673    1673    1663 |      xmonad-x86_64-l
      23939   23939    1673 |       xterm
      23941   23941   23939 |        bash
      23963   23963   23941 |         mutt
      24954   24954   23963 |          offlineimap

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/perf/Documentation/perf-report.txt |   4 +
 tools/perf/builtin-report.c              | 136 ++++++++++++++++++++++++++++++-
 2 files changed, 138 insertions(+), 2 deletions(-)

tools/perf/Documentation/perf-report.txt +4 -0

index b0d70bf4ddfe..dcf0d154918f 100644
@@ -441,6 +441,10 @@ include::itrace.txt[]
 	Display overall events statistics without any further processing.
 	(like the one at the end of the perf report -D command)
 
+--task::
+	Display monitored tasks stored in perf data. Displaying pid/tid/ppid
+	plus the command string aligned to distinguish parent and child tasks.
+
 include::callchain-overhead-calculation.txt[]
 
 SEE ALSO

tools/perf/builtin-report.c +134 -2

index 11d303494b0c..f2ca2cbe63c1 100644
@@ -15,6 +15,7 @@
 #include "util/color.h"
 #include <linux/list.h>
 #include <linux/rbtree.h>
+#include <linux/err.h>
 #include "util/symbol.h"
 #include "util/callchain.h"
 #include "util/values.h"

@@ -61,6 +62,7 @@ struct report {
 	bool			inverted_callchain;
 	bool			mem_mode;
 	bool			stat_mode;
+	bool			task_mode;
 	bool			header;
 	bool			header_only;
 	bool			nonany_branch_mode;

@@ -598,6 +600,124 @@ static int stat_print(struct report *rep)
 	return 0;
 }
 
+static void task_setup(struct report *rep)
+{
+	memset(&rep->tool, 0, sizeof(rep->tool));
+	rep->tool.comm = perf_event__process_comm;
+	rep->tool.exit = perf_event__process_exit;
+	rep->tool.fork = perf_event__process_fork;
+	rep->tool.no_warn = true;
+}
+
+struct task {
+	struct thread		*thread;
+	struct list_head	 list;
+	struct list_head	 children;
+};
+
+static struct task *task_list(struct task *task, struct machine *machine)
+{
+	struct thread *parent_thread, *thread = task->thread;
+	struct task   *parent_task;
+
+	/* Already listed. */
+	if (!list_empty(&task->list))
+		return NULL;
+
+	/* Last one in the chain. */
+	if (thread->ppid == -1)
+		return task;
+
+	parent_thread = machine__findnew_thread(machine, -1, thread->ppid);
+	if (!parent_thread)
+		return ERR_PTR(-ENOMEM);
+
+	parent_task = thread__priv(parent_thread);
+	list_add_tail(&task->list, &parent_task->children);
+	return task_list(parent_task, machine);
+}
+
+static void task__print_level(struct task *task, FILE *fp, int level)
+{
+	struct thread *thread = task->thread;
+	struct task *child;
+
+	fprintf(fp, "  %8d %8d %8d |%*s%s\n",
+		thread->pid_, thread->tid, thread->ppid,
+		level, "", thread__comm_str(thread));
+
+	if (!list_empty(&task->children)) {
+		list_for_each_entry(child, &task->children, list)
+			task__print_level(child, fp, level + 1);
+	}
+}
+
+static int task_print(struct report *rep, FILE *fp)
+{
+	struct perf_session *session = rep->session;
+	struct machine      *machine = &session->machines.host;
+	struct task *tasks, *task;
+	unsigned int nr = 0, itask = 0, i;
+	struct rb_node *nd;
+	LIST_HEAD(list);
+
+	/*
+	 * No locking needed while accessing machine->threads,
+	 * because --task is single threaded command.
+	 */
+
+	/* Count all the threads. */
+	for (i = 0; i < THREADS__TABLE_SIZE; i++)
+		nr += machine->threads[i].nr;
+
+	tasks = malloc(sizeof(*tasks) * nr);
+	if (!tasks)
+		return -ENOMEM;
+
+	for (i = 0; i < THREADS__TABLE_SIZE; i++) {
+		struct threads *threads = &machine->threads[i];
+
+		for (nd = rb_first(&threads->entries); nd; nd = rb_next(nd)) {
+			task = tasks + itask++;
+
+			task->thread = rb_entry(nd, struct thread, rb_node);
+			INIT_LIST_HEAD(&task->children);
+			INIT_LIST_HEAD(&task->list);
+			thread__set_priv(task->thread, task);
+		}
+	}
+
+	/*
+	 * Iterate every task down to the unprocessed parent
+	 * and link all in task children list. Task with no
+	 * parent is added into 'list'.
+	 */
+	for (itask = 0; itask < nr; itask++) {
+		task = tasks + itask;
+
+		if (!list_empty(&task->list))
+			continue;
+
+		task = task_list(task, machine);
+		if (IS_ERR(task)) {
+			pr_err("Error: failed to process tasks\n");
+			free(tasks);
+			return -ENOMEM;
+		}
+
+		if (task)
+			list_add_tail(&task->list, &list);
+	}
+
+	fprintf(fp, "# %8s %8s %8s  %s\n", "pid", "tid", "ppid", "comm");
+
+	list_for_each_entry(task, &list, list)
+		task__print_level(task, fp, 0);
+
+	free(tasks);
+	return 0;
+}
+
 static int __cmd_report(struct report *rep)
 {
 	int ret;

@@ -632,6 +752,9 @@ static int __cmd_report(struct report *rep)
 	if (rep->stat_mode)
 		stat_setup(rep);
 
+	if (rep->task_mode)
+		task_setup(rep);
+
 	ret = perf_session__process_events(session);
 	if (ret) {
 		ui__error("failed to process sample\n");

@@ -641,6 +764,9 @@ static int __cmd_report(struct report *rep)
 	if (rep->stat_mode)
 		return stat_print(rep);
 
+	if (rep->task_mode)
+		return task_print(rep, stdout);
+
 	report__warn_kptr_restrict(rep);
 
 	evlist__for_each_entry(session->evlist, pos)

@@ -798,6 +924,7 @@ int cmd_report(int argc, const char **argv)
 	OPT_BOOLEAN('D', "dump-raw-trace", &dump_trace,
 		    "dump raw trace in ASCII"),
 	OPT_BOOLEAN(0, "stat", &report.stat_mode, "Display event stats"),
+	OPT_BOOLEAN(0, "task", &report.task_mode, "Display recorded tasks"),
 	OPT_STRING('k', "vmlinux", &symbol_conf.vmlinux_name,
 		   "file", "vmlinux pathname"),
 	OPT_STRING(0, "kallsyms", &symbol_conf.kallsyms_name,

@@ -1059,8 +1186,12 @@ int cmd_report(int argc, const char **argv)
 		report.tool.show_feat_hdr = SHOW_FEAT_HEADER;
 	if (report.show_full_info)
 		report.tool.show_feat_hdr = SHOW_FEAT_HEADER_FULL_INFO;
-	if (report.stat_mode)
+	if (report.stat_mode || report.task_mode)
 		use_browser = 0;
+	if (report.stat_mode && report.task_mode) {
+		pr_err("Error: --task and --stat options cannot be used together\n");
+		goto error;
+	}
 
 	if (strcmp(input_name, "-") != 0)
 		setup_browser(true);

@@ -1083,7 +1214,8 @@ int cmd_report(int argc, const char **argv)
 			ret = 0;
 			goto error;
 		}
-	} else if (use_browser == 0 && !quiet && !report.stat_mode) {
+	} else if (use_browser == 0 && !quiet &&
+		   !report.stat_mode && !report.task_mode) {
 		fputs("# To display the perf.data header info, please use --header/--header-only options.\n#\n",
 		      stdout);
 	}

Signature · 1 line

2.13.6

Em Sun, Jan 07, 2018 at 05:03:56PM +0100, Jiri Olsa escreveu:

Quoted 3 lines

Adding --task option to display monitored tasks stored in perf.data. Displaying pid/tid/ppid plus the command string aligned to distinguish parent and child tasks.

I'm making this --tasks, plural, but then you can also use --task :-)

- Arnaldo

Quoted 246 lines

  $ perf record -a
  ...
  $ perf report --task
  #     pid     tid    ppid  comm
          0       0      -1 |swapper
          2       2       0 | kthreadd
      14080   14080       2 |  kworker/u17:1
          4       4       2 |  kworker/0:0H
          6       6       2 |  mm_percpu_wq
  ...
          1       1       0 | systemd
      23242   23242       1 |  firefox
      23242   23298   23242 |   Cache2 I/O
      23242   23304   23242 |   GMPThread
  ...
       1195    1195       1 |  login
       1611    1611    1195 |   bash
       1639    1639    1611 |    startx
       1663    1663    1639 |     xinit
       1673    1673    1663 |      xmonad-x86_64-l
      23939   23939    1673 |       xterm
      23941   23941   23939 |        bash
      23963   23963   23941 |         mutt
      24954   24954   23963 |          offlineimap

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/perf/Documentation/perf-report.txt |   4 +
 tools/perf/builtin-report.c              | 136 ++++++++++++++++++++++++++++++-
 2 files changed, 138 insertions(+), 2 deletions(-)

tools/perf/Documentation/perf-report.txt +4 -0

index b0d70bf4ddfe..dcf0d154918f 100644
@@ -441,6 +441,10 @@ include::itrace.txt[]
 	Display overall events statistics without any further processing.
 	(like the one at the end of the perf report -D command)
 
+--task::
+	Display monitored tasks stored in perf data. Displaying pid/tid/ppid
+	plus the command string aligned to distinguish parent and child tasks.
+
 include::callchain-overhead-calculation.txt[]
 
 SEE ALSO

tools/perf/builtin-report.c +134 -2

index 11d303494b0c..f2ca2cbe63c1 100644
@@ -15,6 +15,7 @@
 #include "util/color.h"
 #include <linux/list.h>
 #include <linux/rbtree.h>
+#include <linux/err.h>
 #include "util/symbol.h"
 #include "util/callchain.h"
 #include "util/values.h"

@@ -61,6 +62,7 @@ struct report {
 	bool			inverted_callchain;
 	bool			mem_mode;
 	bool			stat_mode;
+	bool			task_mode;
 	bool			header;
 	bool			header_only;
 	bool			nonany_branch_mode;

@@ -598,6 +600,124 @@ static int stat_print(struct report *rep)
 	return 0;
 }
 
+static void task_setup(struct report *rep)
+{
+	memset(&rep->tool, 0, sizeof(rep->tool));
+	rep->tool.comm = perf_event__process_comm;
+	rep->tool.exit = perf_event__process_exit;
+	rep->tool.fork = perf_event__process_fork;
+	rep->tool.no_warn = true;
+}
+
+struct task {
+	struct thread		*thread;
+	struct list_head	 list;
+	struct list_head	 children;
+};
+
+static struct task *task_list(struct task *task, struct machine *machine)
+{
+	struct thread *parent_thread, *thread = task->thread;
+	struct task   *parent_task;
+
+	/* Already listed. */
+	if (!list_empty(&task->list))
+		return NULL;
+
+	/* Last one in the chain. */
+	if (thread->ppid == -1)
+		return task;
+
+	parent_thread = machine__findnew_thread(machine, -1, thread->ppid);
+	if (!parent_thread)
+		return ERR_PTR(-ENOMEM);
+
+	parent_task = thread__priv(parent_thread);
+	list_add_tail(&task->list, &parent_task->children);
+	return task_list(parent_task, machine);
+}
+
+static void task__print_level(struct task *task, FILE *fp, int level)
+{
+	struct thread *thread = task->thread;
+	struct task *child;
+
+	fprintf(fp, "  %8d %8d %8d |%*s%s\n",
+		thread->pid_, thread->tid, thread->ppid,
+		level, "", thread__comm_str(thread));
+
+	if (!list_empty(&task->children)) {
+		list_for_each_entry(child, &task->children, list)
+			task__print_level(child, fp, level + 1);
+	}
+}
+
+static int task_print(struct report *rep, FILE *fp)
+{
+	struct perf_session *session = rep->session;
+	struct machine      *machine = &session->machines.host;
+	struct task *tasks, *task;
+	unsigned int nr = 0, itask = 0, i;
+	struct rb_node *nd;
+	LIST_HEAD(list);
+
+	/*
+	 * No locking needed while accessing machine->threads,
+	 * because --task is single threaded command.
+	 */
+
+	/* Count all the threads. */
+	for (i = 0; i < THREADS__TABLE_SIZE; i++)
+		nr += machine->threads[i].nr;
+
+	tasks = malloc(sizeof(*tasks) * nr);
+	if (!tasks)
+		return -ENOMEM;
+
+	for (i = 0; i < THREADS__TABLE_SIZE; i++) {
+		struct threads *threads = &machine->threads[i];
+
+		for (nd = rb_first(&threads->entries); nd; nd = rb_next(nd)) {
+			task = tasks + itask++;
+
+			task->thread = rb_entry(nd, struct thread, rb_node);
+			INIT_LIST_HEAD(&task->children);
+			INIT_LIST_HEAD(&task->list);
+			thread__set_priv(task->thread, task);
+		}
+	}
+
+	/*
+	 * Iterate every task down to the unprocessed parent
+	 * and link all in task children list. Task with no
+	 * parent is added into 'list'.
+	 */
+	for (itask = 0; itask < nr; itask++) {
+		task = tasks + itask;
+
+		if (!list_empty(&task->list))
+			continue;
+
+		task = task_list(task, machine);
+		if (IS_ERR(task)) {
+			pr_err("Error: failed to process tasks\n");
+			free(tasks);
+			return -ENOMEM;
+		}
+
+		if (task)
+			list_add_tail(&task->list, &list);
+	}
+
+	fprintf(fp, "# %8s %8s %8s  %s\n", "pid", "tid", "ppid", "comm");
+
+	list_for_each_entry(task, &list, list)
+		task__print_level(task, fp, 0);
+
+	free(tasks);
+	return 0;
+}
+
 static int __cmd_report(struct report *rep)
 {
 	int ret;

@@ -632,6 +752,9 @@ static int __cmd_report(struct report *rep)
 	if (rep->stat_mode)
 		stat_setup(rep);
 
+	if (rep->task_mode)
+		task_setup(rep);
+
 	ret = perf_session__process_events(session);
 	if (ret) {
 		ui__error("failed to process sample\n");

@@ -641,6 +764,9 @@ static int __cmd_report(struct report *rep)
 	if (rep->stat_mode)
 		return stat_print(rep);
 
+	if (rep->task_mode)
+		return task_print(rep, stdout);
+
 	report__warn_kptr_restrict(rep);
 
 	evlist__for_each_entry(session->evlist, pos)

@@ -798,6 +924,7 @@ int cmd_report(int argc, const char **argv)
 	OPT_BOOLEAN('D', "dump-raw-trace", &dump_trace,
 		    "dump raw trace in ASCII"),
 	OPT_BOOLEAN(0, "stat", &report.stat_mode, "Display event stats"),
+	OPT_BOOLEAN(0, "task", &report.task_mode, "Display recorded tasks"),
 	OPT_STRING('k', "vmlinux", &symbol_conf.vmlinux_name,
 		   "file", "vmlinux pathname"),
 	OPT_STRING(0, "kallsyms", &symbol_conf.kallsyms_name,

@@ -1059,8 +1186,12 @@ int cmd_report(int argc, const char **argv)
 		report.tool.show_feat_hdr = SHOW_FEAT_HEADER;
 	if (report.show_full_info)
 		report.tool.show_feat_hdr = SHOW_FEAT_HEADER_FULL_INFO;
-	if (report.stat_mode)
+	if (report.stat_mode || report.task_mode)
 		use_browser = 0;
+	if (report.stat_mode && report.task_mode) {
+		pr_err("Error: --task and --stat options cannot be used together\n");
+		goto error;
+	}
 
 	if (strcmp(input_name, "-") != 0)
 		setup_browser(true);

@@ -1083,7 +1214,8 @@ int cmd_report(int argc, const char **argv)
 			ret = 0;
 			goto error;
 		}
-	} else if (use_browser == 0 && !quiet && !report.stat_mode) {
+	} else if (use_browser == 0 && !quiet &&
+		   !report.stat_mode && !report.task_mode) {
 		fputs("# To display the perf.data header info, please use --header/--header-only options.\n#\n",
 		      stdout);
 	}

Signature · 1 line

2.13.6

Adding --task option to display monitored tasks stored in perf.data. Displaying pid/tid/ppid plus the command string aligned to distinguish parent and child tasks.

  $ perf record -a
  ...
  $ perf report --task
  #     pid     tid    ppid  comm
          0       0      -1 |swapper
          2       2       0 | kthreadd
      14080   14080       2 |  kworker/u17:1
          4       4       2 |  kworker/0:0H
          6       6       2 |  mm_percpu_wq
  ...
          1       1       0 | systemd
      23242   23242       1 |  firefox
      23242   23298   23242 |   Cache2 I/O
      23242   23304   23242 |   GMPThread
  ...
       1195    1195       1 |  login

       1611    1611    1195 |   bash
       1639    1639    1611 |    startx
       1663    1663    1639 |     xinit
       1673    1673    1663 |      xmonad-x86_64-l
      23939   23939    1673 |       xterm
      23941   23941   23939 |        bash
      23963   23963   23941 |         mutt
      24954   24954   23963 |          offlineimap

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Jiri Olsa <[email protected]>

---
 tools/perf/Documentation/perf-report.txt |   4 +
 tools/perf/builtin-report.c              | 136 ++++++++++++++++++++++++++++++-
 2 files changed, 138 insertions(+), 2 deletions(-)

tools/perf/Documentation/perf-report.txt +4 -0

index b0d70bf4ddfe..dcf0d154918f 100644
@@ -441,6 +441,10 @@ include::itrace.txt[]
 	Display overall events statistics without any further processing.
 	(like the one at the end of the perf report -D command)
 
+--task::
+	Display monitored tasks stored in perf data. Displaying pid/tid/ppid
+	plus the command string aligned to distinguish parent and child tasks.
+
 include::callchain-overhead-calculation.txt[]
 
 SEE ALSO

tools/perf/builtin-report.c +21 -0

index 11d303494b0c..f2ca2cbe63c1 100644
@@ -15,6 +15,7 @@
 #include "util/color.h"
 #include <linux/list.h>
 #include <linux/rbtree.h>
+#include <linux/err.h>
 #include "util/symbol.h"
 #include "util/callchain.h"
 #include "util/values.h"

@@ -61,6 +62,7 @@ struct report {
 	bool			inverted_callchain;
 	bool			mem_mode;
 	bool			stat_mode;
+	bool			task_mode;
 	bool			header;
 	bool			header_only;
 	bool			nonany_branch_mode;

@@ -598,6 +600,124 @@ static int stat_print(struct report *rep)
 	return 0;
 }
 
+static void task_setup(struct report *rep)
+{
+	memset(&rep->tool, 0, sizeof(rep->tool));
+	rep->tool.comm = perf_event__process_comm;
+	rep->tool.exit = perf_event__process_exit;
+	rep->tool.fork = perf_event__process_fork;
+	rep->tool.no_warn = true;
+}
+
+struct task {
+	struct thread		*thread;
+	struct list_head	 list;
+	struct list_head	 children;
+};
+
+static struct task *task_list(struct task *task, struct machine *machine)
+{
+	struct thread *parent_thread, *thread = task->thread;
+	struct task   *parent_task;

tools/perf/builtin-report.c +7 -0

+	if (!parent_thread)
+		return ERR_PTR(-ENOMEM);
+
+	parent_task = thread__priv(parent_thread);
+	list_add_tail(&task->list, &parent_task->children);
+	return task_list(parent_task, machine);
+}

Quoted 9 lines

tools/perf/builtin-report.c +9 -0

+static struct task *task_list(struct task *task, struct machine *machine)
+{
+	struct thread *parent_thread, *thread = task->thread;
+	struct task   *parent_task;
+
+	/* Already listed. */
+	if (!list_empty(&task->list))
+		return NULL;
+

tools/perf/builtin-report.c static int stat_print(struct report *rep) · line 631

Quoted 5 lines

tools/perf/builtin-report.c +5 -0

+	/* Last one in the chain. */
+	if (thread->ppid == -1)
+		return task;
+
+	parent_thread = machine__findnew_thread(machine, -1, thread->ppid);

I think it should be machine__find_thread() since creating a new thread at this stage would lack thread->priv anyway.

SNIP

Quoted 9 lines

Quoted 9 lines

(quoted diff) +9 -0

+static struct task *task_list(struct task *task, struct machine *machine)
+{
+	struct thread *parent_thread, *thread = task->thread;
+	struct task   *parent_task;
+
+	/* Already listed. */
+	if (!list_empty(&task->list))
+		return NULL;
+

Quoted 8 lines

Review

Quoted 5 lines

(quoted diff) +5 -0

+	/* Last one in the chain. */
+	if (thread->ppid == -1)
+		return task;
+
+	parent_thread = machine__findnew_thread(machine, -1, thread->ppid);

I think it should be machine__find_thread() since creating a new thread at this stage would lack thread->priv anyway.

ugh, that's right.. I tried to stay safe, but the NULL in priv would bring it down anyway

Arnaldo,
I already see changed version on top of your branch, please let me know if you make also this change or I should send v2

Em Tue, Jan 09, 2018 at 10:15:51AM +0100, Jiri Olsa escreveu:

Quoted 22 lines · Namhyung Kim

SNIP

Quoted 9 lines

Quoted 9 lines

(quoted diff) +9 -0

+static struct task *task_list(struct task *task, struct machine *machine)
+{
+	struct thread *parent_thread, *thread = task->thread;
+	struct task   *parent_task;
+
+	/* Already listed. */
+	if (!list_empty(&task->list))
+		return NULL;
+

Quoted 8 lines

Review

Quoted 5 lines

(quoted diff) +5 -0

+	/* Last one in the chain. */
+	if (thread->ppid == -1)
+		return task;
+
+	parent_thread = machine__findnew_thread(machine, -1, thread->ppid);

I think it should be machine__find_thread() since creating a new thread at this stage would lack thread->priv anyway.

Quoted 7 lines

ugh, that's right.. I tried to stay safe, but the NULL in priv would bring it down anyway

Arnaldo,
I already see changed version on top of your branch, please let me know if you make also this change or I should send v2

I can make the change, thanks!

Just to make sure we're on the same page, just this, right?

tools/perf/builtin-report.c +0 -0

index 3685ac101b16..e60709fe31ed 100644