perf/core improvements and fixes

Hi Ingo,

Please consider pulling,

- Arnaldo

The following changes since commit 691286b5561aab2e1b00119bc328598c01250548:

kprobes/x86: Remove stale ARCH_SUPPORTS_KPROBES_ON_FTRACE define (2014-10-17 07:18:34 +0200)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-core-for-mingo

for you to fetch changes up to e8564b710c6df2c3aeb56c507c22f4bcfa4c0b2d:

perf script: Add period as a default output column (2014-10-17 15:22:19 -0300)

----------------------------------------------------------------
perf/core improvements and fixes:

User visible:

o Add period data column and make it default in 'perf script' (Jiri Olsa)

Infrastructure:

* Move exit stuff from perf_evsel__delete to perf_evsel__exit, delete should be just a front end for exit + free (Arnaldo Carvalho de Melo)

* Add missing 'struct option' forward declaration (Arnaldo Carvalho de Melo)

* No need to drag util/cgroup.h into evsel.h (Arnaldo Carvalho de Melo)

----------------------------------------------------------------
Arnaldo Carvalho de Melo (3):
perf evsel: Move exit stuff from __delete to __exit perf evlist: Add missing 'struct option' forward declaration perf evsel: No need to drag util/cgroup.h

Jiri Olsa (2):
perf script: Add period data column
perf script: Add period as a default output column

 tools/perf/Documentation/perf-script.txt |  2 +-
 tools/perf/builtin-record.c              |  1 +
 tools/perf/builtin-script.c              | 21 +++++++++++++++++----
 tools/perf/builtin-stat.c                |  1 +
 tools/perf/util/evlist.h                 |  2 ++
 tools/perf/util/evsel.c                  | 11 ++++++-----
 tools/perf/util/evsel.h                  |  3 ++-
 7 files changed, 30 insertions(+), 11 deletions(-)

From: Arnaldo Carvalho de Melo <[email protected]>

So that when an evsel is embedded into other struct it can free up resources calling perf_evsel__exit().

---
 tools/perf/util/evsel.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

index d1ecde0fd56c..786ea557af26 100644
@@ -850,17 +850,17 @@ void perf_evsel__exit(struct perf_evsel *evsel)
 	assert(list_empty(&evsel->node));
 	perf_evsel__free_fd(evsel);
 	perf_evsel__free_id(evsel);
+	close_cgroup(evsel->cgrp);
+	zfree(&evsel->group_name);
+	if (evsel->tp_format)
+		pevent_free_format(evsel->tp_format);
+	zfree(&evsel->name);
 	perf_evsel__object.fini(evsel);
 }
 
 void perf_evsel__delete(struct perf_evsel *evsel)
 {
 	perf_evsel__exit(evsel);
-	close_cgroup(evsel->cgrp);
-	zfree(&evsel->group_name);
-	if (evsel->tp_format)
-		pevent_free_format(evsel->tp_format);
-	zfree(&evsel->name);
 	free(evsel);
 }
 

1.9.3

From: Arnaldo Carvalho de Melo <[email protected]>

It was being found, by chance, because evsel.h needlessly includes util/cgroup.h, which will be sorted out in a following patch.

---
 tools/perf/util/evlist.h | 2 ++
 1 file changed, 2 insertions(+)

index bd312b01e876..649b0c597283 100644
@@ -117,6 +117,8 @@ int perf_evlist__prepare_workload(struct perf_evlist *evlist,
 						     void *ucontext));
 int perf_evlist__start_workload(struct perf_evlist *evlist);
 
+struct option;
+
 int perf_evlist__parse_mmap_pages(const struct option *opt,
 				  const char *str,
 				  int unset);

1.9.3

From: Arnaldo Carvalho de Melo <[email protected]>

The only thing we need is a forward declaration for 'struct cgroup_sel', that is inside 'struct perf_evsel'.

Include cgroup.h instead on the tools that support cgroups.

---
 tools/perf/builtin-record.c | 1 +
 tools/perf/builtin-stat.c   | 1 +
 tools/perf/util/evsel.c     | 1 +
 tools/perf/util/evsel.h     | 3 ++-
 4 files changed, 5 insertions(+), 1 deletion(-)

index a6b2132c666f..2583a9b04317 100644
@@ -15,6 +15,7 @@
 #include "util/parse-events.h"
 
 #include "util/callchain.h"
+#include "util/cgroup.h"
 #include "util/header.h"
 #include "util/event.h"
 #include "util/evlist.h"

index b22c62f80078..055ce9232c9e 100644
@@ -43,6 +43,7 @@
 
 #include "perf.h"
 #include "builtin.h"
+#include "util/cgroup.h"
 #include "util/util.h"
 #include "util/parse-options.h"
 #include "util/parse-events.h"

index 786ea557af26..2f9e68025ede 100644
@@ -16,6 +16,7 @@
 #include <sys/resource.h>
 #include "asm/bug.h"
 #include "callchain.h"
+#include "cgroup.h"
 #include "evsel.h"
 #include "evlist.h"
 #include "util.h"

index 1d5c754aebc4..163c5604e5d1 100644
@@ -7,7 +7,6 @@
 #include <linux/perf_event.h>
 #include <linux/types.h>
 #include "xyarray.h"
-#include "cgroup.h"
 #include "symbol.h"
 
 struct perf_counts_values {

@@ -42,6 +41,8 @@ struct perf_sample_id {
 	u64			period;
 };
 
+struct cgroup_sel;
+
 /** struct perf_evsel - event selector
  *
  * @name - Can be set to retain the original event name passed by the user,

1.9.3

From: Jiri Olsa <[email protected]>

Adding period data column to be displayed in perf script. It's possible to get period values using -f option, like:

  $ perf script -f comm,tid,time,period,ip,sym,dso
          :26019 26019 52414.329088:       3707  ffffffff8105443a native_write_msr_safe ([kernel.kallsyms])
          :26019 26019 52414.329088:         44  ffffffff8105443a native_write_msr_safe ([kernel.kallsyms])
          :26019 26019 52414.329093:       1987  ffffffff8105443a native_write_msr_safe ([kernel.kallsyms])
          :26019 26019 52414.329093:          6  ffffffff8105443a native_write_msr_safe ([kernel.kallsyms])
              ls 26019 52414.329442:     537558        3407c0639c _dl_map_object_from_fd (/usr/lib64/ld-2.17.so)
              ls 26019 52414.329442:       2099        3407c0639c _dl_map_object_from_fd (/usr/lib64/ld-2.17.so)
              ls 26019 52414.330181:    1242100        34080917bb get_next_seq (/usr/lib64/libc-2.17.so)
              ls 26019 52414.330181:       3774        34080917bb get_next_seq (/usr/lib64/libc-2.17.so)
              ls 26019 52414.331427:    1083662  ffffffff810c7dc2 update_curr ([kernel.kallsyms])
              ls 26019 52414.331427:        360  ffffffff810c7dc2 update_curr ([kernel.kallsyms])

---
 tools/perf/Documentation/perf-script.txt |  2 +-
 tools/perf/builtin-script.c              | 12 +++++++++++-
 2 files changed, 12 insertions(+), 2 deletions(-)

index 5a0160d359a1..21494806c0ab 100644
@@ -115,7 +115,7 @@ OPTIONS
 -f::
 --fields::
         Comma separated list of fields to print. Options are:
-        comm, tid, pid, time, cpu, event, trace, ip, sym, dso, addr, symoff, srcline.
+        comm, tid, pid, time, cpu, event, trace, ip, sym, dso, addr, symoff, srcline, period.
         Field list can be prepended with the type, trace, sw or hw,
         to indicate to which event type the field list applies.
         e.g., -f sw:comm,tid,time,ip,sym  and -f trace:time,cpu,trace

index 6b4925f65bf0..0659dff4a21d 100644
@@ -44,6 +44,7 @@ enum perf_output_field {
 	PERF_OUTPUT_ADDR            = 1U << 10,
 	PERF_OUTPUT_SYMOFFSET       = 1U << 11,
 	PERF_OUTPUT_SRCLINE         = 1U << 12,
+	PERF_OUTPUT_PERIOD          = 1U << 13,
 };
 
 struct output_option {

@@ -63,6 +64,7 @@ struct output_option {
 	{.str = "addr",  .field = PERF_OUTPUT_ADDR},
 	{.str = "symoff", .field = PERF_OUTPUT_SYMOFFSET},
 	{.str = "srcline", .field = PERF_OUTPUT_SRCLINE},
+	{.str = "period", .field = PERF_OUTPUT_PERIOD},
 };
 
 /* default set to maintain compatibility with current format */

@@ -229,6 +231,11 @@ static int perf_evsel__check_attr(struct perf_evsel *evsel,
 					PERF_OUTPUT_CPU))
 		return -EINVAL;
 
+	if (PRINT_FIELD(PERIOD) &&
+		perf_evsel__check_stype(evsel, PERF_SAMPLE_PERIOD, "PERIOD",
+					PERF_OUTPUT_PERIOD))
+		return -EINVAL;
+
 	return 0;
 }
 

@@ -448,6 +455,9 @@ static void process_event(union perf_event *event, struct perf_sample *sample,
 
 	print_sample_start(sample, thread, evsel);
 
+	if (PRINT_FIELD(PERIOD))
+		printf("%10" PRIu64 " ", sample->period);
+
 	if (PRINT_FIELD(EVNAME)) {
 		const char *evname = perf_evsel__name(evsel);
 		printf("%s: ", evname ? evname : "[unknown]");

@@ -1543,7 +1553,7 @@ int cmd_script(int argc, const char **argv, const char *prefix __maybe_unused)
 		     "comma separated output fields prepend with 'type:'. "
 		     "Valid types: hw,sw,trace,raw. "
 		     "Fields: comm,tid,pid,time,cpu,event,trace,ip,sym,dso,"
-		     "addr,symoff", parse_output_fields),
+		     "addr,symoff,period", parse_output_fields),
 	OPT_BOOLEAN('a', "all-cpus", &system_wide,
 		    "system-wide collection from all CPUs"),
 	OPT_STRING('S', "symbols", &symbol_conf.sym_list_str, "symbol[,symbol...]",

1.9.3

From: Jiri Olsa <[email protected]>

Adding period as a default output column in script command fo hardware, software and raw events.

If PERF_SAMPLE_PERIOD sample type is defined in perf.data, following will be displayed in perf script output:

  $ perf script
              ls  8034 57477.887209:     250000 task-clock:  ffffffff81361d72 memset ([kernel.kallsyms])
              ls  8034 57477.887464:     250000 task-clock:  ffffffff816f6d92 _raw_spin_unlock_irqrestore ([kernel.kallsyms])
              ls  8034 57477.887708:     250000 task-clock:  ffffffff811a94f0 do_munmap ([kernel.kallsyms])
              ls  8034 57477.887959:     250000 task-clock:        34080916c6 get_next_seq (/usr/lib64/libc-2.17.so)
              ls  8034 57477.888208:     250000 task-clock:        3408079230 _IO_doallocbuf (/usr/lib64/libc-2.17.so)
              ls  8034 57477.888717:     250000 task-clock:  ffffffff814242c8 n_tty_write ([kernel.kallsyms])
              ls  8034 57477.889285:     250000 task-clock:        3408076402 fwrite_unlocked (/usr/lib64/libc-2.17.so)

---
 tools/perf/builtin-script.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

index 0659dff4a21d..9708a1290571 100644
@@ -82,7 +82,8 @@ static struct {
 		.fields = PERF_OUTPUT_COMM | PERF_OUTPUT_TID |
 			      PERF_OUTPUT_CPU | PERF_OUTPUT_TIME |
 			      PERF_OUTPUT_EVNAME | PERF_OUTPUT_IP |
-				  PERF_OUTPUT_SYM | PERF_OUTPUT_DSO,
+			      PERF_OUTPUT_SYM | PERF_OUTPUT_DSO |
+			      PERF_OUTPUT_PERIOD,
 
 		.invalid_fields = PERF_OUTPUT_TRACE,
 	},

@@ -93,7 +94,8 @@ static struct {
 		.fields = PERF_OUTPUT_COMM | PERF_OUTPUT_TID |
 			      PERF_OUTPUT_CPU | PERF_OUTPUT_TIME |
 			      PERF_OUTPUT_EVNAME | PERF_OUTPUT_IP |
-				  PERF_OUTPUT_SYM | PERF_OUTPUT_DSO,
+			      PERF_OUTPUT_SYM | PERF_OUTPUT_DSO |
+			      PERF_OUTPUT_PERIOD,
 
 		.invalid_fields = PERF_OUTPUT_TRACE,
 	},

@@ -112,7 +114,8 @@ static struct {
 		.fields = PERF_OUTPUT_COMM | PERF_OUTPUT_TID |
 			      PERF_OUTPUT_CPU | PERF_OUTPUT_TIME |
 			      PERF_OUTPUT_EVNAME | PERF_OUTPUT_IP |
-				  PERF_OUTPUT_SYM | PERF_OUTPUT_DSO,
+			      PERF_OUTPUT_SYM | PERF_OUTPUT_DSO |
+			      PERF_OUTPUT_PERIOD,
 
 		.invalid_fields = PERF_OUTPUT_TRACE,
 	},

1.9.3

Pulled, thanks a lot Arnaldo!

Ingo