linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [GIT PULL 00/15] perf/core improvements and fixes
@ 2016-03-07 19:44 Arnaldo Carvalho de Melo
  2016-03-07 19:44 ` [PATCH 01/15] perf tools: Explicitly declare inc_group_count as a void function Arnaldo Carvalho de Melo
                   ` (15 more replies)
  0 siblings, 16 replies; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-03-07 19:44 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Adrian Hunter,
	Alexander Shishkin, Andi Kleen, Borislav Petkov, Colin Ian King,
	David Ahern, Davidlohr Bueso, He Kuang, Jiri Olsa, Mel Gorman,
	Namhyung Kim, Peter Zijlstra, Stephane Eranian, Steven Rostedt,
	Wang Nan, Arnaldo Carvalho de Melo

Hi Ingo,

	Please consider pulling,

- Arnaldo

The following changes since commit 009668520ae00d52026ccdb3884864e3473c6b65:

  Merge tag 'perf-core-for-mingo-20160303' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core (2016-03-04 12:19:21 +0100)

are available in the git repository at:

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

for you to fetch changes up to b03ae342d9bec460a6c9c327c3f5f758263b0932:

  perf report: Use hierarchy hpp list on gtk (2016-03-07 15:10:41 -0300)

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

User visible:

- Allow grouping multiple sort keys per 'perf report/top --hierarchy'
  level (Namhyung Kim)

- Document 'perf stat --detailed' option (Borislav Petkov)

Infrastructure:

- jitdump prep work for supporting it with Intel PT (Adrian Hunter)

- Use 64-bit shifts with (TSC) time conversion (Adrian Hunter)

Trivial:

- Explicitly declare inc_group_count as a void function (Colin Ian King)

Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

----------------------------------------------------------------
Adrian Hunter (5):
      perf inject: Hit all DSOs for AUX data in JIT and other cases
      perf session: Simplify tool stubs
      perf jit: Let jit_process() return errors
      perf jit: Move clockid validation
      perf tools: Use 64-bit shifts with (TSC) time conversion

Borislav Petkov (1):
      perf stat: Document --detailed option

Colin Ian King (1):
      perf tools: Explicitly declare inc_group_count as a void function

Namhyung Kim (8):
      perf hists: Add level field to struct perf_hpp_fmt
      perf hists: Introduce perf_hpp__setup_hists_formats()
      perf hists: Use own hpp_list for hierarchy mode
      perf hists: Support multiple sort keys in a hierarchy level
      perf hists: Fix indent for multiple hierarchy sort key
      perf report: Use hierarchy hpp list on stdio
      perf hists browser: Use hierarchy hpp list
      perf report: Use hierarchy hpp list on gtk

 tools/perf/Documentation/perf-stat.txt |   8 ++
 tools/perf/arch/x86/tests/rdpmc.c      |   2 +-
 tools/perf/builtin-inject.c            |  52 ++++------
 tools/perf/ui/browsers/hists.c         | 147 +++++++++++++++-------------
 tools/perf/ui/gtk/hists.c              |  73 ++++++++------
 tools/perf/ui/hist.c                   |  69 +++++++++++++
 tools/perf/ui/stdio/hist.c             | 171 +++++++++++++++++----------------
 tools/perf/util/hist.c                 |  72 +++++++++-----
 tools/perf/util/hist.h                 |  14 +++
 tools/perf/util/jitdump.c              |  29 +++++-
 tools/perf/util/parse-events.y         |   2 +-
 tools/perf/util/session.c              |  40 ++------
 tools/perf/util/sort.c                 | 146 ++++++++++++++++++++--------
 tools/perf/util/sort.h                 |   1 +
 tools/perf/util/tsc.c                  |   2 +-
 15 files changed, 514 insertions(+), 314 deletions(-)

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 01/15] perf tools: Explicitly declare inc_group_count as a void function
  2016-03-07 19:44 [GIT PULL 00/15] perf/core improvements and fixes Arnaldo Carvalho de Melo
@ 2016-03-07 19:44 ` Arnaldo Carvalho de Melo
  2016-03-07 19:44 ` [PATCH 02/15] perf inject: Hit all DSOs for AUX data in JIT and other cases Arnaldo Carvalho de Melo
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-03-07 19:44 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Colin Ian King, Alexander Shishkin, He Kuang,
	Peter Zijlstra, Wang Nan, Arnaldo Carvalho de Melo

From: Colin Ian King <colin.king@canonical.com>

The return type is not defined, so it defaults to int, however, the
function is not returning anything, so this is clearly not correct. Make
it a void function.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: He Kuang <hekuang@huawei.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1457008214-14393-1-git-send-email-colin.king@canonical.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/parse-events.y | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index 85c44ba79cad..5be4a5f216d6 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -28,7 +28,7 @@ do { \
 	INIT_LIST_HEAD(list);         \
 } while (0)
 
-static inc_group_count(struct list_head *list,
+static void inc_group_count(struct list_head *list,
 		       struct parse_events_evlist *data)
 {
 	/* Count groups only have more than 1 members */
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 02/15] perf inject: Hit all DSOs for AUX data in JIT and other cases
  2016-03-07 19:44 [GIT PULL 00/15] perf/core improvements and fixes Arnaldo Carvalho de Melo
  2016-03-07 19:44 ` [PATCH 01/15] perf tools: Explicitly declare inc_group_count as a void function Arnaldo Carvalho de Melo
@ 2016-03-07 19:44 ` Arnaldo Carvalho de Melo
  2016-03-07 19:44 ` [PATCH 03/15] perf session: Simplify tool stubs Arnaldo Carvalho de Melo
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-03-07 19:44 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Adrian Hunter, Jiri Olsa, Stephane Eranian,
	Arnaldo Carvalho de Melo

From: Adrian Hunter <adrian.hunter@intel.com>

Currently, when injecting build ids, if there is AUX data then 'perf
inject' hits all DSOs because it is not known which DSOs the trace data
would hit.

That needs to be done for JIT injection also, and in fact there is no
reason to distinguish what kind of injection is being done.  That is,
any time there is AUX data and the HEADER_BUID_ID feature flag is set,
and the AUX data is not being processed, then hit all DSOs.  This patch
does that.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1457005856-6143-2-git-send-email-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-inject.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index b38445f08c2f..c6a4f2f94ab1 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -679,12 +679,16 @@ static int __cmd_inject(struct perf_inject *inject)
 	ret = perf_session__process_events(session);
 
 	if (!file_out->is_pipe) {
-		if (inject->build_ids) {
+		if (inject->build_ids)
 			perf_header__set_feat(&session->header,
 					      HEADER_BUILD_ID);
-			if (inject->have_auxtrace)
-				dsos__hit_all(session);
-		}
+		/*
+		 * Keep all buildids when there is unprocessed AUX data because
+		 * it is not known which ones the AUX trace hits.
+		 */
+		if (perf_header__has_feat(&session->header, HEADER_BUILD_ID) &&
+		    inject->have_auxtrace && !inject->itrace_synth_opts.set)
+			dsos__hit_all(session);
 		/*
 		 * The AUX areas have been removed and replaced with
 		 * synthesized hardware events, so clear the feature flag and
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 03/15] perf session: Simplify tool stubs
  2016-03-07 19:44 [GIT PULL 00/15] perf/core improvements and fixes Arnaldo Carvalho de Melo
  2016-03-07 19:44 ` [PATCH 01/15] perf tools: Explicitly declare inc_group_count as a void function Arnaldo Carvalho de Melo
  2016-03-07 19:44 ` [PATCH 02/15] perf inject: Hit all DSOs for AUX data in JIT and other cases Arnaldo Carvalho de Melo
@ 2016-03-07 19:44 ` Arnaldo Carvalho de Melo
  2016-03-08  7:45   ` Adrian Hunter
  2016-03-07 19:44 ` [PATCH 04/15] perf jit: Let jit_process() return errors Arnaldo Carvalho de Melo
                   ` (12 subsequent siblings)
  15 siblings, 1 reply; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-03-07 19:44 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Adrian Hunter, Jiri Olsa, Stephane Eranian,
	Arnaldo Carvalho de Melo

From: Adrian Hunter <ajhunter@gmail.com>

Some of the stubs are identical so just have one function for them.

Signed-off-by: Adrian Hunter <ajhunter@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1457005856-6143-3-git-send-email-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/session.c | 40 +++++++---------------------------------
 1 file changed, 7 insertions(+), 33 deletions(-)

diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 40b7a0d0905b..60b3593d210d 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -240,14 +240,6 @@ static int process_event_stub(struct perf_tool *tool __maybe_unused,
 	return 0;
 }
 
-static int process_build_id_stub(struct perf_tool *tool __maybe_unused,
-				 union perf_event *event __maybe_unused,
-				 struct perf_session *session __maybe_unused)
-{
-	dump_printf(": unhandled!\n");
-	return 0;
-}
-
 static int process_finished_round_stub(struct perf_tool *tool __maybe_unused,
 				       union perf_event *event __maybe_unused,
 				       struct ordered_events *oe __maybe_unused)
@@ -260,23 +252,6 @@ static int process_finished_round(struct perf_tool *tool,
 				  union perf_event *event,
 				  struct ordered_events *oe);
 
-static int process_id_index_stub(struct perf_tool *tool __maybe_unused,
-				 union perf_event *event __maybe_unused,
-				 struct perf_session *perf_session
-				 __maybe_unused)
-{
-	dump_printf(": unhandled!\n");
-	return 0;
-}
-
-static int process_event_auxtrace_info_stub(struct perf_tool *tool __maybe_unused,
-				union perf_event *event __maybe_unused,
-				struct perf_session *session __maybe_unused)
-{
-	dump_printf(": unhandled!\n");
-	return 0;
-}
-
 static int skipn(int fd, off_t n)
 {
 	char buf[4096];
@@ -303,10 +278,9 @@ static s64 process_event_auxtrace_stub(struct perf_tool *tool __maybe_unused,
 	return event->auxtrace.size;
 }
 
-static
-int process_event_auxtrace_error_stub(struct perf_tool *tool __maybe_unused,
-				      union perf_event *event __maybe_unused,
-				      struct perf_session *session __maybe_unused)
+static int process_event_op2_stub(struct perf_tool *tool __maybe_unused,
+				  union perf_event *event __maybe_unused,
+				  struct perf_session *session __maybe_unused)
 {
 	dump_printf(": unhandled!\n");
 	return 0;
@@ -410,7 +384,7 @@ void perf_tool__fill_defaults(struct perf_tool *tool)
 	if (tool->tracing_data == NULL)
 		tool->tracing_data = process_event_synth_tracing_data_stub;
 	if (tool->build_id == NULL)
-		tool->build_id = process_build_id_stub;
+		tool->build_id = process_event_op2_stub;
 	if (tool->finished_round == NULL) {
 		if (tool->ordered_events)
 			tool->finished_round = process_finished_round;
@@ -418,13 +392,13 @@ void perf_tool__fill_defaults(struct perf_tool *tool)
 			tool->finished_round = process_finished_round_stub;
 	}
 	if (tool->id_index == NULL)
-		tool->id_index = process_id_index_stub;
+		tool->id_index = process_event_op2_stub;
 	if (tool->auxtrace_info == NULL)
-		tool->auxtrace_info = process_event_auxtrace_info_stub;
+		tool->auxtrace_info = process_event_op2_stub;
 	if (tool->auxtrace == NULL)
 		tool->auxtrace = process_event_auxtrace_stub;
 	if (tool->auxtrace_error == NULL)
-		tool->auxtrace_error = process_event_auxtrace_error_stub;
+		tool->auxtrace_error = process_event_op2_stub;
 	if (tool->thread_map == NULL)
 		tool->thread_map = process_event_thread_map_stub;
 	if (tool->cpu_map == NULL)
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 04/15] perf jit: Let jit_process() return errors
  2016-03-07 19:44 [GIT PULL 00/15] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (2 preceding siblings ...)
  2016-03-07 19:44 ` [PATCH 03/15] perf session: Simplify tool stubs Arnaldo Carvalho de Melo
@ 2016-03-07 19:44 ` Arnaldo Carvalho de Melo
  2016-03-07 19:44 ` [PATCH 05/15] perf jit: Move clockid validation Arnaldo Carvalho de Melo
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-03-07 19:44 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Adrian Hunter, Jiri Olsa, Stephane Eranian,
	Arnaldo Carvalho de Melo

From: Adrian Hunter <adrian.hunter@intel.com>

In preparation for moving clockid validation into jit_process().

Previously a return value of zero meant the processing had been done and
non-zero meant either the processing was not done (i.e. not the jitdump
file mmap event) or an error occurred.

Change it so that zero means the processing was not done, one means the
processing was done and successful, and negative values are an error.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1457005856-6143-5-git-send-email-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-inject.c | 16 ++++++++++++----
 tools/perf/util/jitdump.c   |  6 ++++--
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index c6a4f2f94ab1..2512d71ca386 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -253,12 +253,16 @@ static int perf_event__jit_repipe_mmap(struct perf_tool *tool,
 {
 	struct perf_inject *inject = container_of(tool, struct perf_inject, tool);
 	u64 n = 0;
+	int ret;
 
 	/*
 	 * if jit marker, then inject jit mmaps and generate ELF images
 	 */
-	if (!jit_process(inject->session, &inject->output, machine,
-			 event->mmap.filename, sample->pid, &n)) {
+	ret = jit_process(inject->session, &inject->output, machine,
+			  event->mmap.filename, sample->pid, &n);
+	if (ret < 0)
+		return ret;
+	if (ret) {
 		inject->bytes_written += n;
 		return 0;
 	}
@@ -287,12 +291,16 @@ static int perf_event__jit_repipe_mmap2(struct perf_tool *tool,
 {
 	struct perf_inject *inject = container_of(tool, struct perf_inject, tool);
 	u64 n = 0;
+	int ret;
 
 	/*
 	 * if jit marker, then inject jit mmaps and generate ELF images
 	 */
-	if (!jit_process(inject->session, &inject->output, machine,
-			  event->mmap2.filename, sample->pid, &n)) {
+	ret = jit_process(inject->session, &inject->output, machine,
+			  event->mmap2.filename, sample->pid, &n);
+	if (ret < 0)
+		return ret;
+	if (ret) {
 		inject->bytes_written += n;
 		return 0;
 	}
diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
index 99fa5eee9fe0..bd9e44f9fff2 100644
--- a/tools/perf/util/jitdump.c
+++ b/tools/perf/util/jitdump.c
@@ -647,7 +647,7 @@ jit_process(struct perf_session *session,
 	 * first, detect marker mmap (i.e., the jitdump mmap)
 	 */
 	if (jit_detect(filename, pid))
-		return -1;
+		return 0;
 
 	memset(&jd, 0, sizeof(jd));
 
@@ -665,8 +665,10 @@ jit_process(struct perf_session *session,
 	*nbytes = 0;
 
 	ret = jit_inject(&jd, filename);
-	if (!ret)
+	if (!ret) {
 		*nbytes = jd.bytes_written;
+		ret = 1;
+	}
 
 	return ret;
 }
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 05/15] perf jit: Move clockid validation
  2016-03-07 19:44 [GIT PULL 00/15] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (3 preceding siblings ...)
  2016-03-07 19:44 ` [PATCH 04/15] perf jit: Let jit_process() return errors Arnaldo Carvalho de Melo
@ 2016-03-07 19:44 ` Arnaldo Carvalho de Melo
  2016-03-07 19:44 ` [PATCH 06/15] perf tools: Use 64-bit shifts with (TSC) time conversion Arnaldo Carvalho de Melo
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-03-07 19:44 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Adrian Hunter, Jiri Olsa, Stephane Eranian,
	Arnaldo Carvalho de Melo

From: Adrian Hunter <ajhunter@gmail.com>

Move clockid validation into jit_process() so it can later be made
conditional.

Signed-off-by: Adrian Hunter <ajhunter@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1457005856-6143-6-git-send-email-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-inject.c | 24 ------------------------
 tools/perf/util/jitdump.c   | 23 +++++++++++++++++++++++
 2 files changed, 23 insertions(+), 24 deletions(-)

diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 2512d71ca386..b2885776b602 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -729,23 +729,6 @@ static int __cmd_inject(struct perf_inject *inject)
 	return ret;
 }
 
-#ifdef HAVE_LIBELF_SUPPORT
-static int
-jit_validate_events(struct perf_session *session)
-{
-	struct perf_evsel *evsel;
-
-	/*
-	 * check that all events use CLOCK_MONOTONIC
-	 */
-	evlist__for_each(session->evlist, evsel) {
-		if (evsel->attr.use_clockid == 0 || evsel->attr.clockid != CLOCK_MONOTONIC)
-			return -1;
-	}
-	return 0;
-}
-#endif
-
 int cmd_inject(int argc, const char **argv, const char *prefix __maybe_unused)
 {
 	struct perf_inject inject = {
@@ -852,13 +835,6 @@ int cmd_inject(int argc, const char **argv, const char *prefix __maybe_unused)
 	}
 #ifdef HAVE_LIBELF_SUPPORT
 	if (inject.jit_mode) {
-		/*
-		 * validate event is using the correct clockid
-		 */
-		if (jit_validate_events(inject.session)) {
-			fprintf(stderr, "error, jitted code must be sampled with perf record -k 1\n");
-			return -1;
-		}
 		inject.tool.mmap2	   = perf_event__jit_repipe_mmap2;
 		inject.tool.mmap	   = perf_event__jit_repipe_mmap;
 		inject.tool.ordered_events = true;
diff --git a/tools/perf/util/jitdump.c b/tools/perf/util/jitdump.c
index bd9e44f9fff2..cd272cc21e05 100644
--- a/tools/perf/util/jitdump.c
+++ b/tools/perf/util/jitdump.c
@@ -99,6 +99,21 @@ jit_close(struct jit_buf_desc *jd)
 }
 
 static int
+jit_validate_events(struct perf_session *session)
+{
+	struct perf_evsel *evsel;
+
+	/*
+	 * check that all events use CLOCK_MONOTONIC
+	 */
+	evlist__for_each(session->evlist, evsel) {
+		if (evsel->attr.use_clockid == 0 || evsel->attr.clockid != CLOCK_MONOTONIC)
+			return -1;
+	}
+	return 0;
+}
+
+static int
 jit_open(struct jit_buf_desc *jd, const char *name)
 {
 	struct jitheader header;
@@ -157,6 +172,14 @@ jit_open(struct jit_buf_desc *jd, const char *name)
 		goto error;
 	}
 
+	/*
+	 * validate event is using the correct clockid
+	 */
+	if (jit_validate_events(jd->session)) {
+		pr_err("error, jitted code must be sampled with perf record -k 1\n");
+		goto error;
+	}
+
 	bs = header.total_size - sizeof(header);
 
 	if (bs > bsz) {
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 06/15] perf tools: Use 64-bit shifts with (TSC) time conversion
  2016-03-07 19:44 [GIT PULL 00/15] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (4 preceding siblings ...)
  2016-03-07 19:44 ` [PATCH 05/15] perf jit: Move clockid validation Arnaldo Carvalho de Melo
@ 2016-03-07 19:44 ` Arnaldo Carvalho de Melo
  2016-03-07 19:44 ` [PATCH 07/15] perf hists: Add level field to struct perf_hpp_fmt Arnaldo Carvalho de Melo
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-03-07 19:44 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Adrian Hunter, Jiri Olsa, Stephane Eranian,
	Arnaldo Carvalho de Melo

From: Adrian Hunter <adrian.hunter@intel.com>

Commit b9511cd761fa ("perf/x86: Fix time_shift in perf_event_mmap_page")
altered the time conversion algorithms documented in the perf_event.h
header file, to use 64-bit shifts.  That was done to make the code more
future-proof (i.e. some time in the future a 32-bit shift could be
allowed).  Reflect those changes in perf tools.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1457005856-6143-9-git-send-email-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/arch/x86/tests/rdpmc.c | 2 +-
 tools/perf/util/tsc.c             | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/arch/x86/tests/rdpmc.c b/tools/perf/arch/x86/tests/rdpmc.c
index 7945462851a4..72193f19d6d7 100644
--- a/tools/perf/arch/x86/tests/rdpmc.c
+++ b/tools/perf/arch/x86/tests/rdpmc.c
@@ -59,7 +59,7 @@ static u64 mmap_read_self(void *addr)
 		u64 quot, rem;
 
 		quot = (cyc >> time_shift);
-		rem = cyc & ((1 << time_shift) - 1);
+		rem = cyc & (((u64)1 << time_shift) - 1);
 		delta = time_offset + quot * time_mult +
 			((rem * time_mult) >> time_shift);
 
diff --git a/tools/perf/util/tsc.c b/tools/perf/util/tsc.c
index 4d4210d4e13d..1b741646eed0 100644
--- a/tools/perf/util/tsc.c
+++ b/tools/perf/util/tsc.c
@@ -19,7 +19,7 @@ u64 tsc_to_perf_time(u64 cyc, struct perf_tsc_conversion *tc)
 	u64 quot, rem;
 
 	quot = cyc >> tc->time_shift;
-	rem  = cyc & ((1 << tc->time_shift) - 1);
+	rem  = cyc & (((u64)1 << tc->time_shift) - 1);
 	return tc->time_zero + quot * tc->time_mult +
 	       ((rem * tc->time_mult) >> tc->time_shift);
 }
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 07/15] perf hists: Add level field to struct perf_hpp_fmt
  2016-03-07 19:44 [GIT PULL 00/15] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (5 preceding siblings ...)
  2016-03-07 19:44 ` [PATCH 06/15] perf tools: Use 64-bit shifts with (TSC) time conversion Arnaldo Carvalho de Melo
@ 2016-03-07 19:44 ` Arnaldo Carvalho de Melo
  2016-03-07 19:44 ` [PATCH 08/15] perf stat: Document --detailed option Arnaldo Carvalho de Melo
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-03-07 19:44 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Namhyung Kim, Andi Kleen, David Ahern, Jiri Olsa,
	Peter Zijlstra, Stephane Eranian, Wang Nan,
	Arnaldo Carvalho de Melo

From: Namhyung Kim <namhyung@kernel.org>

The level field is to distinguish levels in the hierarchy mode.
Currently each column (perf_hpp_fmt) has a different level.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1457103582-28396-2-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/hist.h |  1 +
 tools/perf/util/sort.c | 74 ++++++++++++++++++++++++++++----------------------
 2 files changed, 42 insertions(+), 33 deletions(-)

diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index da5e50586bfd..f4ef513527ba 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -233,6 +233,7 @@ struct perf_hpp_fmt {
 	int len;
 	int user_len;
 	int idx;
+	int level;
 };
 
 struct perf_hpp_list {
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 4380a2858802..ab6eb7ca8c60 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -1544,7 +1544,7 @@ static void hse_free(struct perf_hpp_fmt *fmt)
 }
 
 static struct hpp_sort_entry *
-__sort_dimension__alloc_hpp(struct sort_dimension *sd)
+__sort_dimension__alloc_hpp(struct sort_dimension *sd, int level)
 {
 	struct hpp_sort_entry *hse;
 
@@ -1572,6 +1572,7 @@ __sort_dimension__alloc_hpp(struct sort_dimension *sd)
 	hse->hpp.elide = false;
 	hse->hpp.len = 0;
 	hse->hpp.user_len = 0;
+	hse->hpp.level = level;
 
 	return hse;
 }
@@ -1581,7 +1582,8 @@ static void hpp_free(struct perf_hpp_fmt *fmt)
 	free(fmt);
 }
 
-static struct perf_hpp_fmt *__hpp_dimension__alloc_hpp(struct hpp_dimension *hd)
+static struct perf_hpp_fmt *__hpp_dimension__alloc_hpp(struct hpp_dimension *hd,
+						       int level)
 {
 	struct perf_hpp_fmt *fmt;
 
@@ -1590,6 +1592,7 @@ static struct perf_hpp_fmt *__hpp_dimension__alloc_hpp(struct hpp_dimension *hd)
 		INIT_LIST_HEAD(&fmt->list);
 		INIT_LIST_HEAD(&fmt->sort_list);
 		fmt->free = hpp_free;
+		fmt->level = level;
 	}
 
 	return fmt;
@@ -1611,9 +1614,9 @@ int hist_entry__filter(struct hist_entry *he, int type, const void *arg)
 	return hse->se->se_filter(he, type, arg);
 }
 
-static int __sort_dimension__add_hpp_sort(struct sort_dimension *sd)
+static int __sort_dimension__add_hpp_sort(struct sort_dimension *sd, int level)
 {
-	struct hpp_sort_entry *hse = __sort_dimension__alloc_hpp(sd);
+	struct hpp_sort_entry *hse = __sort_dimension__alloc_hpp(sd, level);
 
 	if (hse == NULL)
 		return -1;
@@ -1625,7 +1628,7 @@ static int __sort_dimension__add_hpp_sort(struct sort_dimension *sd)
 static int __sort_dimension__add_hpp_output(struct perf_hpp_list *list,
 					    struct sort_dimension *sd)
 {
-	struct hpp_sort_entry *hse = __sort_dimension__alloc_hpp(sd);
+	struct hpp_sort_entry *hse = __sort_dimension__alloc_hpp(sd, 0);
 
 	if (hse == NULL)
 		return -1;
@@ -1868,7 +1871,8 @@ static void hde_free(struct perf_hpp_fmt *fmt)
 }
 
 static struct hpp_dynamic_entry *
-__alloc_dynamic_entry(struct perf_evsel *evsel, struct format_field *field)
+__alloc_dynamic_entry(struct perf_evsel *evsel, struct format_field *field,
+		      int level)
 {
 	struct hpp_dynamic_entry *hde;
 
@@ -1899,6 +1903,7 @@ __alloc_dynamic_entry(struct perf_evsel *evsel, struct format_field *field)
 	hde->hpp.elide = false;
 	hde->hpp.len = 0;
 	hde->hpp.user_len = 0;
+	hde->hpp.level = level;
 
 	return hde;
 }
@@ -1974,11 +1979,11 @@ static struct perf_evsel *find_evsel(struct perf_evlist *evlist, char *event_nam
 
 static int __dynamic_dimension__add(struct perf_evsel *evsel,
 				    struct format_field *field,
-				    bool raw_trace)
+				    bool raw_trace, int level)
 {
 	struct hpp_dynamic_entry *hde;
 
-	hde = __alloc_dynamic_entry(evsel, field);
+	hde = __alloc_dynamic_entry(evsel, field, level);
 	if (hde == NULL)
 		return -ENOMEM;
 
@@ -1988,14 +1993,14 @@ static int __dynamic_dimension__add(struct perf_evsel *evsel,
 	return 0;
 }
 
-static int add_evsel_fields(struct perf_evsel *evsel, bool raw_trace)
+static int add_evsel_fields(struct perf_evsel *evsel, bool raw_trace, int level)
 {
 	int ret;
 	struct format_field *field;
 
 	field = evsel->tp_format->format.fields;
 	while (field) {
-		ret = __dynamic_dimension__add(evsel, field, raw_trace);
+		ret = __dynamic_dimension__add(evsel, field, raw_trace, level);
 		if (ret < 0)
 			return ret;
 
@@ -2004,7 +2009,8 @@ static int add_evsel_fields(struct perf_evsel *evsel, bool raw_trace)
 	return 0;
 }
 
-static int add_all_dynamic_fields(struct perf_evlist *evlist, bool raw_trace)
+static int add_all_dynamic_fields(struct perf_evlist *evlist, bool raw_trace,
+				  int level)
 {
 	int ret;
 	struct perf_evsel *evsel;
@@ -2013,7 +2019,7 @@ static int add_all_dynamic_fields(struct perf_evlist *evlist, bool raw_trace)
 		if (evsel->attr.type != PERF_TYPE_TRACEPOINT)
 			continue;
 
-		ret = add_evsel_fields(evsel, raw_trace);
+		ret = add_evsel_fields(evsel, raw_trace, level);
 		if (ret < 0)
 			return ret;
 	}
@@ -2021,7 +2027,7 @@ static int add_all_dynamic_fields(struct perf_evlist *evlist, bool raw_trace)
 }
 
 static int add_all_matching_fields(struct perf_evlist *evlist,
-				   char *field_name, bool raw_trace)
+				   char *field_name, bool raw_trace, int level)
 {
 	int ret = -ESRCH;
 	struct perf_evsel *evsel;
@@ -2035,14 +2041,15 @@ static int add_all_matching_fields(struct perf_evlist *evlist,
 		if (field == NULL)
 			continue;
 
-		ret = __dynamic_dimension__add(evsel, field, raw_trace);
+		ret = __dynamic_dimension__add(evsel, field, raw_trace, level);
 		if (ret < 0)
 			break;
 	}
 	return ret;
 }
 
-static int add_dynamic_entry(struct perf_evlist *evlist, const char *tok)
+static int add_dynamic_entry(struct perf_evlist *evlist, const char *tok,
+			     int level)
 {
 	char *str, *event_name, *field_name, *opt_name;
 	struct perf_evsel *evsel;
@@ -2072,12 +2079,12 @@ static int add_dynamic_entry(struct perf_evlist *evlist, const char *tok)
 	}
 
 	if (!strcmp(field_name, "trace_fields")) {
-		ret = add_all_dynamic_fields(evlist, raw_trace);
+		ret = add_all_dynamic_fields(evlist, raw_trace, level);
 		goto out;
 	}
 
 	if (event_name == NULL) {
-		ret = add_all_matching_fields(evlist, field_name, raw_trace);
+		ret = add_all_matching_fields(evlist, field_name, raw_trace, level);
 		goto out;
 	}
 
@@ -2095,7 +2102,7 @@ static int add_dynamic_entry(struct perf_evlist *evlist, const char *tok)
 	}
 
 	if (!strcmp(field_name, "*")) {
-		ret = add_evsel_fields(evsel, raw_trace);
+		ret = add_evsel_fields(evsel, raw_trace, level);
 	} else {
 		field = pevent_find_any_field(evsel->tp_format, field_name);
 		if (field == NULL) {
@@ -2104,7 +2111,7 @@ static int add_dynamic_entry(struct perf_evlist *evlist, const char *tok)
 			return -ENOENT;
 		}
 
-		ret = __dynamic_dimension__add(evsel, field, raw_trace);
+		ret = __dynamic_dimension__add(evsel, field, raw_trace, level);
 	}
 
 out:
@@ -2112,12 +2119,12 @@ out:
 	return ret;
 }
 
-static int __sort_dimension__add(struct sort_dimension *sd)
+static int __sort_dimension__add(struct sort_dimension *sd, int level)
 {
 	if (sd->taken)
 		return 0;
 
-	if (__sort_dimension__add_hpp_sort(sd) < 0)
+	if (__sort_dimension__add_hpp_sort(sd, level) < 0)
 		return -1;
 
 	if (sd->entry->se_collapse)
@@ -2128,14 +2135,14 @@ static int __sort_dimension__add(struct sort_dimension *sd)
 	return 0;
 }
 
-static int __hpp_dimension__add(struct hpp_dimension *hd)
+static int __hpp_dimension__add(struct hpp_dimension *hd, int level)
 {
 	struct perf_hpp_fmt *fmt;
 
 	if (hd->taken)
 		return 0;
 
-	fmt = __hpp_dimension__alloc_hpp(hd);
+	fmt = __hpp_dimension__alloc_hpp(hd, level);
 	if (!fmt)
 		return -1;
 
@@ -2165,7 +2172,7 @@ static int __hpp_dimension__add_output(struct perf_hpp_list *list,
 	if (hd->taken)
 		return 0;
 
-	fmt = __hpp_dimension__alloc_hpp(hd);
+	fmt = __hpp_dimension__alloc_hpp(hd, 0);
 	if (!fmt)
 		return -1;
 
@@ -2180,8 +2187,8 @@ int hpp_dimension__add_output(unsigned col)
 	return __hpp_dimension__add_output(&perf_hpp_list, &hpp_sort_dimensions[col]);
 }
 
-static int sort_dimension__add(const char *tok,
-			       struct perf_evlist *evlist __maybe_unused)
+static int sort_dimension__add(const char *tok, struct perf_evlist *evlist,
+			       int level)
 {
 	unsigned int i;
 
@@ -2220,7 +2227,7 @@ static int sort_dimension__add(const char *tok,
 			sort__has_thread = 1;
 		}
 
-		return __sort_dimension__add(sd);
+		return __sort_dimension__add(sd, level);
 	}
 
 	for (i = 0; i < ARRAY_SIZE(hpp_sort_dimensions); i++) {
@@ -2229,7 +2236,7 @@ static int sort_dimension__add(const char *tok,
 		if (strncasecmp(tok, hd->name, strlen(tok)))
 			continue;
 
-		return __hpp_dimension__add(hd);
+		return __hpp_dimension__add(hd, level);
 	}
 
 	for (i = 0; i < ARRAY_SIZE(bstack_sort_dimensions); i++) {
@@ -2244,7 +2251,7 @@ static int sort_dimension__add(const char *tok,
 		if (sd->entry == &sort_sym_from || sd->entry == &sort_sym_to)
 			sort__has_sym = 1;
 
-		__sort_dimension__add(sd);
+		__sort_dimension__add(sd, level);
 		return 0;
 	}
 
@@ -2260,11 +2267,11 @@ static int sort_dimension__add(const char *tok,
 		if (sd->entry == &sort_mem_daddr_sym)
 			sort__has_sym = 1;
 
-		__sort_dimension__add(sd);
+		__sort_dimension__add(sd, level);
 		return 0;
 	}
 
-	if (!add_dynamic_entry(evlist, tok))
+	if (!add_dynamic_entry(evlist, tok, level))
 		return 0;
 
 	return -ESRCH;
@@ -2274,10 +2281,11 @@ static int setup_sort_list(char *str, struct perf_evlist *evlist)
 {
 	char *tmp, *tok;
 	int ret = 0;
+	int level = 0;
 
 	for (tok = strtok_r(str, ", ", &tmp);
 			tok; tok = strtok_r(NULL, ", ", &tmp)) {
-		ret = sort_dimension__add(tok, evlist);
+		ret = sort_dimension__add(tok, evlist, level++);
 		if (ret == -EINVAL) {
 			error("Invalid --sort key: `%s'", tok);
 			break;
@@ -2667,7 +2675,7 @@ int setup_sorting(struct perf_evlist *evlist)
 		return err;
 
 	if (parent_pattern != default_parent_pattern) {
-		err = sort_dimension__add("parent", evlist);
+		err = sort_dimension__add("parent", evlist, -1);
 		if (err < 0)
 			return err;
 	}
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 08/15] perf stat: Document --detailed option
  2016-03-07 19:44 [GIT PULL 00/15] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (6 preceding siblings ...)
  2016-03-07 19:44 ` [PATCH 07/15] perf hists: Add level field to struct perf_hpp_fmt Arnaldo Carvalho de Melo
@ 2016-03-07 19:44 ` Arnaldo Carvalho de Melo
  2016-03-08  9:10   ` Ingo Molnar
  2016-03-07 19:44 ` [PATCH 09/15] perf hists: Introduce perf_hpp__setup_hists_formats() Arnaldo Carvalho de Melo
                   ` (7 subsequent siblings)
  15 siblings, 1 reply; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-03-07 19:44 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Borislav Petkov, Davidlohr Bueso, Jiri Olsa,
	Mel Gorman, Peter Zijlstra, Steven Rostedt,
	Arnaldo Carvalho de Melo

From: Borislav Petkov <bp@suse.de>

I'm surprised this remained undocumented since at least 2011. And it is
actually a very useful switch, as Steve and I came to realize recently.

Add the text from

  2cba3ffb9a9d ("perf stat: Add -d -d and -d -d -d options to show more CPU events")

which added the incrementing aspect to -d.

Signed-off-by: Borislav Petkov <bp@suse.de>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Davidlohr Bueso <dbueso@suse.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mel Gorman <mgorman@suse.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Steven Rostedt <rostedt@goodmis.org>
Fixes: 2cba3ffb9a9d ("perf stat: Add -d -d and -d -d -d options to show more CPU events")
Link: http://lkml.kernel.org/r/1457347294-32546-1-git-send-email-bp@alien8.de
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Documentation/perf-stat.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
index 52ef7a9d50aa..14d9e8ffaff7 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -69,6 +69,14 @@ report::
 --scale::
 	scale/normalize counter values
 
+-d::
+--detailed::
+	print more detailed statistics, can be specified up to 3 times
+
+	   -d:          detailed events, L1 and LLC data cache
+        -d -d:     more detailed events, dTLB and iTLB events
+     -d -d -d:     very detailed events, adding prefetch events
+
 -r::
 --repeat=<n>::
 	repeat command and print average + stddev (max: 100). 0 means forever.
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 09/15] perf hists: Introduce perf_hpp__setup_hists_formats()
  2016-03-07 19:44 [GIT PULL 00/15] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (7 preceding siblings ...)
  2016-03-07 19:44 ` [PATCH 08/15] perf stat: Document --detailed option Arnaldo Carvalho de Melo
@ 2016-03-07 19:44 ` Arnaldo Carvalho de Melo
  2016-03-07 19:44 ` [PATCH 10/15] perf hists: Use own hpp_list for hierarchy mode Arnaldo Carvalho de Melo
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-03-07 19:44 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Namhyung Kim, Andi Kleen, David Ahern, Jiri Olsa,
	Peter Zijlstra, Stephane Eranian, Wang Nan,
	Arnaldo Carvalho de Melo

From: Namhyung Kim <namhyung@kernel.org>

The perf_hpp__setup_hists_formats() is to build hists-specific output
formats (and sort keys).  Currently it's only used in order to build the
output format in a hierarchy with same sort keys, but it could be used
with different sort keys in non-hierarchy mode later.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1457361308-514-2-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/hist.c   | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/hist.c | 12 ++++++++++
 tools/perf/util/hist.h | 11 +++++++++
 tools/perf/util/sort.c | 32 +++++++++++++++++++++++++
 4 files changed, 118 insertions(+)

diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 7c0585c146e1..3a15e844f89a 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -5,6 +5,7 @@
 #include "../util/util.h"
 #include "../util/sort.h"
 #include "../util/evsel.h"
+#include "../util/evlist.h"
 
 /* hist period print (hpp) functions */
 
@@ -715,3 +716,65 @@ void perf_hpp__set_user_width(const char *width_list_str)
 			break;
 	}
 }
+
+static int add_hierarchy_fmt(struct hists *hists, struct perf_hpp_fmt *fmt)
+{
+	struct perf_hpp_list_node *node = NULL;
+	struct perf_hpp_fmt *fmt_copy;
+	bool found = false;
+
+	list_for_each_entry(node, &hists->hpp_formats, list) {
+		if (node->level == fmt->level) {
+			found = true;
+			break;
+		}
+	}
+
+	if (!found) {
+		node = malloc(sizeof(*node));
+		if (node == NULL)
+			return -1;
+
+		node->level = fmt->level;
+		perf_hpp_list__init(&node->hpp);
+
+		list_add_tail(&node->list, &hists->hpp_formats);
+	}
+
+	fmt_copy = perf_hpp_fmt__dup(fmt);
+	if (fmt_copy == NULL)
+		return -1;
+
+	list_add_tail(&fmt_copy->list, &node->hpp.fields);
+	list_add_tail(&fmt_copy->sort_list, &node->hpp.sorts);
+
+	return 0;
+}
+
+int perf_hpp__setup_hists_formats(struct perf_hpp_list *list,
+				  struct perf_evlist *evlist)
+{
+	struct perf_evsel *evsel;
+	struct perf_hpp_fmt *fmt;
+	struct hists *hists;
+	int ret;
+
+	if (!symbol_conf.report_hierarchy)
+		return 0;
+
+	evlist__for_each(evlist, evsel) {
+		hists = evsel__hists(evsel);
+
+		perf_hpp_list__for_each_sort_list(list, fmt) {
+			if (perf_hpp__is_dynamic_entry(fmt) &&
+			    !perf_hpp__defined_dynamic_entry(fmt, hists))
+				continue;
+
+			ret = add_hierarchy_fmt(hists, fmt);
+			if (ret < 0)
+				return ret;
+		}
+	}
+
+	return 0;
+}
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 4b8b67bc0cd8..fea92fcb6903 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -2105,6 +2105,7 @@ int __hists__init(struct hists *hists, struct perf_hpp_list *hpp_list)
 	pthread_mutex_init(&hists->lock, NULL);
 	hists->socket_filter = -1;
 	hists->hpp_list = hpp_list;
+	INIT_LIST_HEAD(&hists->hpp_formats);
 	return 0;
 }
 
@@ -2133,8 +2134,19 @@ static void hists__delete_all_entries(struct hists *hists)
 static void hists_evsel__exit(struct perf_evsel *evsel)
 {
 	struct hists *hists = evsel__hists(evsel);
+	struct perf_hpp_fmt *fmt, *pos;
+	struct perf_hpp_list_node *node, *tmp;
 
 	hists__delete_all_entries(hists);
+
+	list_for_each_entry_safe(node, tmp, &hists->hpp_formats, list) {
+		perf_hpp_list__for_each_format_safe(&node->hpp, fmt, pos) {
+			list_del(&fmt->list);
+			free(fmt);
+		}
+		list_del(&node->list);
+		free(node);
+	}
 }
 
 static int hists_evsel__init(struct perf_evsel *evsel)
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index f4ef513527ba..3cab9dc20822 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -78,6 +78,7 @@ struct hists {
 	u16			col_len[HISTC_NR_COLS];
 	int			socket_filter;
 	struct perf_hpp_list	*hpp_list;
+	struct list_head	hpp_formats;
 	int			nr_sort_keys;
 };
 
@@ -244,6 +245,12 @@ struct perf_hpp_list {
 
 extern struct perf_hpp_list perf_hpp_list;
 
+struct perf_hpp_list_node {
+	struct list_head	list;
+	struct perf_hpp_list	hpp;
+	int			level;
+};
+
 void perf_hpp_list__column_register(struct perf_hpp_list *list,
 				    struct perf_hpp_fmt *format);
 void perf_hpp_list__register_sort_field(struct perf_hpp_list *list,
@@ -299,6 +306,8 @@ void perf_hpp__cancel_cumulate(void);
 void perf_hpp__setup_output_field(struct perf_hpp_list *list);
 void perf_hpp__reset_output_field(struct perf_hpp_list *list);
 void perf_hpp__append_sort_keys(struct perf_hpp_list *list);
+int perf_hpp__setup_hists_formats(struct perf_hpp_list *list,
+				  struct perf_evlist *evlist);
 
 
 bool perf_hpp__is_sort_entry(struct perf_hpp_fmt *format);
@@ -308,6 +317,8 @@ bool perf_hpp__is_trace_entry(struct perf_hpp_fmt *fmt);
 bool perf_hpp__is_srcline_entry(struct perf_hpp_fmt *fmt);
 bool perf_hpp__is_srcfile_entry(struct perf_hpp_fmt *fmt);
 
+struct perf_hpp_fmt *perf_hpp_fmt__dup(struct perf_hpp_fmt *fmt);
+
 int hist_entry__filter(struct hist_entry *he, int type, const void *arg);
 
 static inline bool perf_hpp__should_skip(struct perf_hpp_fmt *format,
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index ab6eb7ca8c60..71d45d147376 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -1908,6 +1908,34 @@ __alloc_dynamic_entry(struct perf_evsel *evsel, struct format_field *field,
 	return hde;
 }
 
+struct perf_hpp_fmt *perf_hpp_fmt__dup(struct perf_hpp_fmt *fmt)
+{
+	struct perf_hpp_fmt *new_fmt = NULL;
+
+	if (perf_hpp__is_sort_entry(fmt)) {
+		struct hpp_sort_entry *hse, *new_hse;
+
+		hse = container_of(fmt, struct hpp_sort_entry, hpp);
+		new_hse = memdup(hse, sizeof(*hse));
+		if (new_hse)
+			new_fmt = &new_hse->hpp;
+	} else if (perf_hpp__is_dynamic_entry(fmt)) {
+		struct hpp_dynamic_entry *hde, *new_hde;
+
+		hde = container_of(fmt, struct hpp_dynamic_entry, hpp);
+		new_hde = memdup(hde, sizeof(*hde));
+		if (new_hde)
+			new_fmt = &new_hde->hpp;
+	} else {
+		new_fmt = memdup(fmt, sizeof(*fmt));
+	}
+
+	INIT_LIST_HEAD(&new_fmt->list);
+	INIT_LIST_HEAD(&new_fmt->sort_list);
+
+	return new_fmt;
+}
+
 static int parse_field_name(char *str, char **event, char **field, char **opt)
 {
 	char *event_name, *field_name, *opt_name;
@@ -2700,6 +2728,10 @@ int setup_sorting(struct perf_evlist *evlist)
 	/* and then copy output fields to sort keys */
 	perf_hpp__append_sort_keys(&perf_hpp_list);
 
+	/* setup hists-specific output fields */
+	if (perf_hpp__setup_hists_formats(&perf_hpp_list, evlist) < 0)
+		return -1;
+
 	return 0;
 }
 
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 10/15] perf hists: Use own hpp_list for hierarchy mode
  2016-03-07 19:44 [GIT PULL 00/15] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (8 preceding siblings ...)
  2016-03-07 19:44 ` [PATCH 09/15] perf hists: Introduce perf_hpp__setup_hists_formats() Arnaldo Carvalho de Melo
@ 2016-03-07 19:44 ` Arnaldo Carvalho de Melo
  2016-03-07 19:44 ` [PATCH 11/15] perf hists: Support multiple sort keys in a hierarchy level Arnaldo Carvalho de Melo
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-03-07 19:44 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Namhyung Kim, Andi Kleen, David Ahern, Jiri Olsa,
	Peter Zijlstra, Stephane Eranian, Wang Nan,
	Arnaldo Carvalho de Melo

From: Namhyung Kim <namhyung@kernel.org>

Now each hists has its own hpp lists in hierarchy.  So instead of having
a pointer to a single perf_hpp_fmt in a hist entry, make it point the
hpp_list for its level.  This will be used to support multiple sort keys
in a single hierarchy level.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1457361308-514-3-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c | 45 +++++++++++++++++--------------
 tools/perf/ui/gtk/hists.c      | 20 +++++++++-----
 tools/perf/ui/hist.c           |  5 ++++
 tools/perf/ui/stdio/hist.c     | 44 +++++++++++++++----------------
 tools/perf/util/hist.c         | 60 ++++++++++++++++++++++++------------------
 tools/perf/util/hist.h         |  1 +
 tools/perf/util/sort.h         |  1 +
 7 files changed, 103 insertions(+), 73 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 5ffffcb1e3c5..928b4825b752 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1388,25 +1388,26 @@ static int hist_browser__show_hierarchy_entry(struct hist_browser *browser,
 					      HE_COLORSET_NORMAL);
 		}
 
-		ui_browser__write_nstring(&browser->b, "", 2);
-		width -= 2;
+		perf_hpp_list__for_each_format(entry->hpp_list, fmt) {
+			ui_browser__write_nstring(&browser->b, "", 2);
+			width -= 2;
 
-		/*
-		 * No need to call hist_entry__snprintf_alignment()
-		 * since this fmt is always the last column in the
-		 * hierarchy mode.
-		 */
-		fmt = entry->fmt;
-		if (fmt->color) {
-			width -= fmt->color(fmt, &hpp, entry);
-		} else {
-			int i = 0;
+			/*
+			 * No need to call hist_entry__snprintf_alignment()
+			 * since this fmt is always the last column in the
+			 * hierarchy mode.
+			 */
+			if (fmt->color) {
+				width -= fmt->color(fmt, &hpp, entry);
+			} else {
+				int i = 0;
 
-			width -= fmt->entry(fmt, &hpp, entry);
-			ui_browser__printf(&browser->b, "%s", ltrim(s));
+				width -= fmt->entry(fmt, &hpp, entry);
+				ui_browser__printf(&browser->b, "%s", ltrim(s));
 
-			while (isspace(s[i++]))
-				width++;
+				while (isspace(s[i++]))
+					width++;
+			}
 		}
 	}
 
@@ -1934,7 +1935,7 @@ static int hist_browser__fprintf_hierarchy_entry(struct hist_browser *browser,
 	struct perf_hpp_fmt *fmt;
 	bool first = true;
 	int ret;
-	int hierarchy_indent = (nr_sort_keys + 1) * HIERARCHY_INDENT;
+	int hierarchy_indent = nr_sort_keys * HIERARCHY_INDENT;
 
 	printed = fprintf(fp, "%*s", level * HIERARCHY_INDENT, "");
 
@@ -1962,9 +1963,13 @@ static int hist_browser__fprintf_hierarchy_entry(struct hist_browser *browser,
 	ret = scnprintf(hpp.buf, hpp.size, "%*s", hierarchy_indent, "");
 	advance_hpp(&hpp, ret);
 
-	fmt = he->fmt;
-	ret = fmt->entry(fmt, &hpp, he);
-	advance_hpp(&hpp, ret);
+	perf_hpp_list__for_each_format(he->hpp_list, fmt) {
+		ret = scnprintf(hpp.buf, hpp.size, "  ");
+		advance_hpp(&hpp, ret);
+
+		ret = fmt->entry(fmt, &hpp, he);
+		advance_hpp(&hpp, ret);
+	}
 
 	printed += fprintf(fp, "%s\n", rtrim(s));
 
diff --git a/tools/perf/ui/gtk/hists.c b/tools/perf/ui/gtk/hists.c
index a5758fdfbe1f..4534e2d7669c 100644
--- a/tools/perf/ui/gtk/hists.c
+++ b/tools/perf/ui/gtk/hists.c
@@ -412,6 +412,7 @@ static void perf_gtk__add_hierarchy_entries(struct hists *hists,
 	for (node = rb_first(root); node; node = rb_next(node)) {
 		GtkTreeIter iter;
 		float percent;
+		char *bf;
 
 		he = rb_entry(node, struct hist_entry, rb_node);
 		if (he->filtered)
@@ -437,13 +438,20 @@ static void perf_gtk__add_hierarchy_entries(struct hists *hists,
 			gtk_tree_store_set(store, &iter, col_idx++, hpp->buf, -1);
 		}
 
-		fmt = he->fmt;
-		if (fmt->color)
-			fmt->color(fmt, hpp, he);
-		else
-			fmt->entry(fmt, hpp, he);
+		bf = hpp->buf;
+		perf_hpp_list__for_each_format(he->hpp_list, fmt) {
+			int ret;
+
+			if (fmt->color)
+				ret = fmt->color(fmt, hpp, he);
+			else
+				ret = fmt->entry(fmt, hpp, he);
+
+			snprintf(hpp->buf + ret, hpp->size - ret, "  ");
+			advance_hpp(hpp, ret + 2);
+		}
 
-		gtk_tree_store_set(store, &iter, col_idx, rtrim(hpp->buf), -1);
+		gtk_tree_store_set(store, &iter, col_idx, rtrim(bf), -1);
 
 		if (!he->leaf) {
 			perf_gtk__add_hierarchy_entries(hists, &he->hroot_out,
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 3a15e844f89a..95795ef4209b 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -722,6 +722,7 @@ static int add_hierarchy_fmt(struct hists *hists, struct perf_hpp_fmt *fmt)
 	struct perf_hpp_list_node *node = NULL;
 	struct perf_hpp_fmt *fmt_copy;
 	bool found = false;
+	bool skip = perf_hpp__should_skip(fmt, hists);
 
 	list_for_each_entry(node, &hists->hpp_formats, list) {
 		if (node->level == fmt->level) {
@@ -735,6 +736,7 @@ static int add_hierarchy_fmt(struct hists *hists, struct perf_hpp_fmt *fmt)
 		if (node == NULL)
 			return -1;
 
+		node->skip = skip;
 		node->level = fmt->level;
 		perf_hpp_list__init(&node->hpp);
 
@@ -745,6 +747,9 @@ static int add_hierarchy_fmt(struct hists *hists, struct perf_hpp_fmt *fmt)
 	if (fmt_copy == NULL)
 		return -1;
 
+	if (!skip)
+		node->skip = false;
+
 	list_add_tail(&fmt_copy->list, &node->hpp.fields);
 	list_add_tail(&fmt_copy->sort_list, &node->hpp.sorts);
 
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 6d06fbb365b6..073642a63cc9 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -451,33 +451,33 @@ static int hist_entry__hierarchy_fprintf(struct hist_entry *he,
 		advance_hpp(hpp, ret);
 	}
 
-	if (sep)
-		ret = scnprintf(hpp->buf, hpp->size, "%s", sep);
-	else
+	if (!sep)
 		ret = scnprintf(hpp->buf, hpp->size, "%*s",
-				(nr_sort_key - 1) * HIERARCHY_INDENT + 2, "");
+				(nr_sort_key - 1) * HIERARCHY_INDENT, "");
 	advance_hpp(hpp, ret);
 
 	printed += fprintf(fp, "%s", buf);
 
-	hpp->buf  = buf;
-	hpp->size = size;
-
-	/*
-	 * No need to call hist_entry__snprintf_alignment() since this
-	 * fmt is always the last column in the hierarchy mode.
-	 */
-	fmt = he->fmt;
-	if (perf_hpp__use_color() && fmt->color)
-		fmt->color(fmt, hpp, he);
-	else
-		fmt->entry(fmt, hpp, he);
-
-	/*
-	 * dynamic entries are right-aligned but we want left-aligned
-	 * in the hierarchy mode
-	 */
-	printed += fprintf(fp, "%s\n", ltrim(buf));
+	perf_hpp_list__for_each_format(he->hpp_list, fmt) {
+		hpp->buf  = buf;
+		hpp->size = size;
+
+		/*
+		 * No need to call hist_entry__snprintf_alignment() since this
+		 * fmt is always the last column in the hierarchy mode.
+		 */
+		if (perf_hpp__use_color() && fmt->color)
+			fmt->color(fmt, hpp, he);
+		else
+			fmt->entry(fmt, hpp, he);
+
+		/*
+		 * dynamic entries are right-aligned but we want left-aligned
+		 * in the hierarchy mode
+		 */
+		printed += fprintf(fp, "%s%s", sep ?: "  ", ltrim(buf));
+	}
+	printed += putc('\n', fp);
 
 	if (symbol_conf.use_callchain && he->leaf) {
 		u64 total = hists__total_period(hists);
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index fea92fcb6903..29da9e0d8db9 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -1091,18 +1091,25 @@ static void hists__apply_filters(struct hists *hists, struct hist_entry *he);
 static struct hist_entry *hierarchy_insert_entry(struct hists *hists,
 						 struct rb_root *root,
 						 struct hist_entry *he,
-						 struct perf_hpp_fmt *fmt)
+						 struct perf_hpp_list *hpp_list)
 {
 	struct rb_node **p = &root->rb_node;
 	struct rb_node *parent = NULL;
 	struct hist_entry *iter, *new;
+	struct perf_hpp_fmt *fmt;
 	int64_t cmp;
 
 	while (*p != NULL) {
 		parent = *p;
 		iter = rb_entry(parent, struct hist_entry, rb_node_in);
 
-		cmp = fmt->collapse(fmt, iter, he);
+		cmp = 0;
+		perf_hpp_list__for_each_sort_list(hpp_list, fmt) {
+			cmp = fmt->collapse(fmt, iter, he);
+			if (cmp)
+				break;
+		}
+
 		if (!cmp) {
 			he_stat__add_stat(&iter->stat, &he->stat);
 			return iter;
@@ -1121,24 +1128,26 @@ static struct hist_entry *hierarchy_insert_entry(struct hists *hists,
 	hists__apply_filters(hists, new);
 	hists->nr_entries++;
 
-	/* save related format for output */
-	new->fmt = fmt;
+	/* save related format list for output */
+	new->hpp_list = hpp_list;
 
 	/* some fields are now passed to 'new' */
-	if (perf_hpp__is_trace_entry(fmt) || perf_hpp__is_dynamic_entry(fmt))
-		he->trace_output = NULL;
-	else
-		new->trace_output = NULL;
+	perf_hpp_list__for_each_sort_list(hpp_list, fmt) {
+		if (perf_hpp__is_trace_entry(fmt) || perf_hpp__is_dynamic_entry(fmt))
+			he->trace_output = NULL;
+		else
+			new->trace_output = NULL;
 
-	if (perf_hpp__is_srcline_entry(fmt))
-		he->srcline = NULL;
-	else
-		new->srcline = NULL;
+		if (perf_hpp__is_srcline_entry(fmt))
+			he->srcline = NULL;
+		else
+			new->srcline = NULL;
 
-	if (perf_hpp__is_srcfile_entry(fmt))
-		he->srcfile = NULL;
-	else
-		new->srcfile = NULL;
+		if (perf_hpp__is_srcfile_entry(fmt))
+			he->srcfile = NULL;
+		else
+			new->srcfile = NULL;
+	}
 
 	rb_link_node(&new->rb_node_in, parent, p);
 	rb_insert_color(&new->rb_node_in, root);
@@ -1149,21 +1158,19 @@ static int hists__hierarchy_insert_entry(struct hists *hists,
 					 struct rb_root *root,
 					 struct hist_entry *he)
 {
-	struct perf_hpp_fmt *fmt;
+	struct perf_hpp_list_node *node;
 	struct hist_entry *new_he = NULL;
 	struct hist_entry *parent = NULL;
 	int depth = 0;
 	int ret = 0;
 
-	hists__for_each_sort_list(hists, fmt) {
-		if (!perf_hpp__is_sort_entry(fmt) &&
-		    !perf_hpp__is_dynamic_entry(fmt))
-			continue;
-		if (perf_hpp__should_skip(fmt, hists))
+	list_for_each_entry(node, &hists->hpp_formats, list) {
+		/* skip period (overhead) and elided columns */
+		if (node->level == 0 || node->skip)
 			continue;
 
 		/* insert copy of 'he' for each fmt into the hierarchy */
-		new_he = hierarchy_insert_entry(hists, root, he, fmt);
+		new_he = hierarchy_insert_entry(hists, root, he, &node->hpp);
 		if (new_he == NULL) {
 			ret = -1;
 			break;
@@ -1358,6 +1365,7 @@ static void hierarchy_insert_output_entry(struct rb_root *root,
 	struct rb_node **p = &root->rb_node;
 	struct rb_node *parent = NULL;
 	struct hist_entry *iter;
+	struct perf_hpp_fmt *fmt;
 
 	while (*p != NULL) {
 		parent = *p;
@@ -1373,8 +1381,10 @@ static void hierarchy_insert_output_entry(struct rb_root *root,
 	rb_insert_color(&he->rb_node, root);
 
 	/* update column width of dynamic entry */
-	if (perf_hpp__is_dynamic_entry(he->fmt))
-		he->fmt->sort(he->fmt, he, NULL);
+	perf_hpp_list__for_each_sort_list(he->hpp_list, fmt) {
+		if (perf_hpp__is_dynamic_entry(fmt))
+			fmt->sort(fmt, he, NULL);
+	}
 }
 
 static void hists__hierarchy_output_resort(struct hists *hists,
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 3cab9dc20822..2209188d729c 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -249,6 +249,7 @@ struct perf_hpp_list_node {
 	struct list_head	list;
 	struct perf_hpp_list	hpp;
 	int			level;
+	bool			skip;
 };
 
 void perf_hpp_list__column_register(struct perf_hpp_list *list,
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 25a5529a94e4..ea1f722cffea 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -130,6 +130,7 @@ struct hist_entry {
 	u32			raw_size;
 	void			*trace_output;
 	struct perf_hpp_fmt	*fmt;
+	struct perf_hpp_list	*hpp_list;
 	struct hist_entry	*parent_he;
 	union {
 		/* this is for hierarchical entry structure */
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 11/15] perf hists: Support multiple sort keys in a hierarchy level
  2016-03-07 19:44 [GIT PULL 00/15] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (9 preceding siblings ...)
  2016-03-07 19:44 ` [PATCH 10/15] perf hists: Use own hpp_list for hierarchy mode Arnaldo Carvalho de Melo
@ 2016-03-07 19:44 ` Arnaldo Carvalho de Melo
  2016-03-07 19:44 ` [PATCH 12/15] perf hists: Fix indent for multiple hierarchy sort key Arnaldo Carvalho de Melo
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-03-07 19:44 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Namhyung Kim, Andi Kleen, David Ahern, Jiri Olsa,
	Peter Zijlstra, Stephane Eranian, Wang Nan,
	Arnaldo Carvalho de Melo

From: Namhyung Kim <namhyung@kernel.org>

This implements having multiple sort keys in a single hierarchy level.
Originally only single sort key is supported for each level, but now
using the group syntax with '{ }', it can set more than one sort key in
one level.  Note that now it needs to quote in order to prevent shell
interpretation.

For example:

  $ perf report --hierarchy -s '{comm,dso},sym'
  ...
  #       Overhead  Command / Shared Object / Symbol
  # ..............  ..........................................
  #
      48.67%        swapper          [kernel.vmlinux]
         34.42%        [k] intel_idle
          1.30%        [k] __tick_nohz_idle_enter
          1.03%        [k] cpuidle_reflect
       8.87%        firefox          libpthread-2.22.so
          6.60%        [.] __GI___libc_recvmsg
          1.18%        [.] pthread_cond_signal@@GLIBC_2.3.2
          1.09%        [.] 0x000000000000ff4b
       6.11%        Xorg             libc-2.22.so
          5.27%        [.] __memcpy_sse2_unaligned

In the above example, the command name and the shared object name are
shown on the same line but the symbol name is on the different line.
Since the first two are grouped by '{}', they are in the same level.

Suggested-and-Tested=by: Arnaldo Carvalho de Melo <acme@kernel.org>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1457361308-514-4-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/sort.c | 42 ++++++++++++++++++++++++++++++++----------
 1 file changed, 32 insertions(+), 10 deletions(-)

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 71d45d147376..041f236379e0 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -2310,18 +2310,40 @@ static int setup_sort_list(char *str, struct perf_evlist *evlist)
 	char *tmp, *tok;
 	int ret = 0;
 	int level = 0;
+	int next_level = 1;
+	bool in_group = false;
+
+	do {
+		tok = str;
+		tmp = strpbrk(str, "{}, ");
+		if (tmp) {
+			if (in_group)
+				next_level = level;
+			else
+				next_level = level + 1;
+
+			if (*tmp == '{')
+				in_group = true;
+			else if (*tmp == '}')
+				in_group = false;
+
+			*tmp = '\0';
+			str = tmp + 1;
+		}
 
-	for (tok = strtok_r(str, ", ", &tmp);
-			tok; tok = strtok_r(NULL, ", ", &tmp)) {
-		ret = sort_dimension__add(tok, evlist, level++);
-		if (ret == -EINVAL) {
-			error("Invalid --sort key: `%s'", tok);
-			break;
-		} else if (ret == -ESRCH) {
-			error("Unknown --sort key: `%s'", tok);
-			break;
+		if (*tok) {
+			ret = sort_dimension__add(tok, evlist, level);
+			if (ret == -EINVAL) {
+				error("Invalid --sort key: `%s'", tok);
+				break;
+			} else if (ret == -ESRCH) {
+				error("Unknown --sort key: `%s'", tok);
+				break;
+			}
 		}
-	}
+
+		level = next_level;
+	} while (tmp);
 
 	return ret;
 }
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 12/15] perf hists: Fix indent for multiple hierarchy sort key
  2016-03-07 19:44 [GIT PULL 00/15] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (10 preceding siblings ...)
  2016-03-07 19:44 ` [PATCH 11/15] perf hists: Support multiple sort keys in a hierarchy level Arnaldo Carvalho de Melo
@ 2016-03-07 19:44 ` Arnaldo Carvalho de Melo
  2016-03-07 19:44 ` [PATCH 13/15] perf report: Use hierarchy hpp list on stdio Arnaldo Carvalho de Melo
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-03-07 19:44 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Namhyung Kim, Andi Kleen, David Ahern, Jiri Olsa,
	Peter Zijlstra, Stephane Eranian, Wang Nan,
	Arnaldo Carvalho de Melo

From: Namhyung Kim <namhyung@kernel.org>

When multiple sort keys are used in a single hierarchy, it should indent
using number of hierarchy levels instead of number of sort keys.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1457361308-514-5-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c | 23 ++++++++++-------------
 tools/perf/ui/hist.c           |  1 +
 tools/perf/ui/stdio/hist.c     | 26 +++++++++++---------------
 tools/perf/util/hist.h         |  1 +
 4 files changed, 23 insertions(+), 28 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 928b4825b752..2f02ce79bd9d 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1280,7 +1280,7 @@ static int hist_browser__show_entry(struct hist_browser *browser,
 static int hist_browser__show_hierarchy_entry(struct hist_browser *browser,
 					      struct hist_entry *entry,
 					      unsigned short row,
-					      int level, int nr_sort_keys)
+					      int level)
 {
 	int printed = 0;
 	int width = browser->b.width;
@@ -1294,7 +1294,7 @@ static int hist_browser__show_hierarchy_entry(struct hist_browser *browser,
 		.current_entry	= current_entry,
 	};
 	int column = 0;
-	int hierarchy_indent = (nr_sort_keys - 1) * HIERARCHY_INDENT;
+	int hierarchy_indent = (entry->hists->nr_hpp_node - 2) * HIERARCHY_INDENT;
 
 	if (current_entry) {
 		browser->he_selection = entry;
@@ -1436,8 +1436,7 @@ show_callchain:
 }
 
 static int hist_browser__show_no_entry(struct hist_browser *browser,
-				       unsigned short row,
-				       int level, int nr_sort_keys)
+				       unsigned short row, int level)
 {
 	int width = browser->b.width;
 	bool current_entry = ui_browser__is_current_entry(&browser->b, row);
@@ -1445,6 +1444,7 @@ static int hist_browser__show_no_entry(struct hist_browser *browser,
 	int column = 0;
 	int ret;
 	struct perf_hpp_fmt *fmt;
+	int indent = browser->hists->nr_hpp_node - 2;
 
 	if (current_entry) {
 		browser->he_selection = NULL;
@@ -1485,8 +1485,8 @@ static int hist_browser__show_no_entry(struct hist_browser *browser,
 		width -= ret;
 	}
 
-	ui_browser__write_nstring(&browser->b, "", nr_sort_keys * HIERARCHY_INDENT);
-	width -= nr_sort_keys * HIERARCHY_INDENT;
+	ui_browser__write_nstring(&browser->b, "", indent * HIERARCHY_INDENT);
+	width -= indent * HIERARCHY_INDENT;
 
 	if (column >= browser->b.horiz_scroll) {
 		char buf[32];
@@ -1553,7 +1553,7 @@ static int hists_browser__scnprintf_hierarchy_headers(struct hist_browser *brows
 	struct perf_hpp_fmt *fmt;
 	size_t ret = 0;
 	int column = 0;
-	int nr_sort_keys = hists->nr_sort_keys;
+	int indent = hists->nr_hpp_node - 2;
 	bool first = true;
 
 	ret = scnprintf(buf, size, " ");
@@ -1577,7 +1577,7 @@ static int hists_browser__scnprintf_hierarchy_headers(struct hist_browser *brows
 	}
 
 	ret = scnprintf(dummy_hpp.buf, dummy_hpp.size, "%*s",
-			(nr_sort_keys - 1) * HIERARCHY_INDENT, "");
+			indent * HIERARCHY_INDENT, "");
 	if (advance_hpp_check(&dummy_hpp, ret))
 		return ret;
 
@@ -1645,7 +1645,6 @@ static unsigned int hist_browser__refresh(struct ui_browser *browser)
 	u16 header_offset = 0;
 	struct rb_node *nd;
 	struct hist_browser *hb = container_of(browser, struct hist_browser, b);
-	int nr_sort = hb->hists->nr_sort_keys;
 
 	if (hb->show_headers) {
 		hist_browser__show_headers(hb);
@@ -1672,14 +1671,12 @@ static unsigned int hist_browser__refresh(struct ui_browser *browser)
 
 		if (symbol_conf.report_hierarchy) {
 			row += hist_browser__show_hierarchy_entry(hb, h, row,
-								  h->depth,
-								  nr_sort);
+								  h->depth);
 			if (row == browser->rows)
 				break;
 
 			if (h->has_no_entry) {
-				hist_browser__show_no_entry(hb, row, h->depth,
-							    nr_sort);
+				hist_browser__show_no_entry(hb, row, h->depth);
 				row++;
 			}
 		} else {
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 95795ef4209b..f03c4f70438f 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -740,6 +740,7 @@ static int add_hierarchy_fmt(struct hists *hists, struct perf_hpp_fmt *fmt)
 		node->level = fmt->level;
 		perf_hpp_list__init(&node->hpp);
 
+		hists->nr_hpp_node++;
 		list_add_tail(&node->list, &hists->hpp_formats);
 	}
 
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 073642a63cc9..543d7137cc0c 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -412,7 +412,7 @@ static int hist_entry__snprintf(struct hist_entry *he, struct perf_hpp *hpp)
 
 static int hist_entry__hierarchy_fprintf(struct hist_entry *he,
 					 struct perf_hpp *hpp,
-					 int nr_sort_key, struct hists *hists,
+					 struct hists *hists,
 					 FILE *fp)
 {
 	const char *sep = symbol_conf.field_sep;
@@ -453,7 +453,7 @@ static int hist_entry__hierarchy_fprintf(struct hist_entry *he,
 
 	if (!sep)
 		ret = scnprintf(hpp->buf, hpp->size, "%*s",
-				(nr_sort_key - 1) * HIERARCHY_INDENT, "");
+				(hists->nr_hpp_node - 2) * HIERARCHY_INDENT, "");
 	advance_hpp(hpp, ret);
 
 	printed += fprintf(fp, "%s", buf);
@@ -504,12 +504,8 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,
 	if (size == 0 || size > bfsz)
 		size = hpp.size = bfsz;
 
-	if (symbol_conf.report_hierarchy) {
-		int nr_sort = hists->nr_sort_keys;
-
-		return hist_entry__hierarchy_fprintf(he, &hpp, nr_sort,
-						     hists, fp);
-	}
+	if (symbol_conf.report_hierarchy)
+		return hist_entry__hierarchy_fprintf(he, &hpp, hists, fp);
 
 	hist_entry__snprintf(he, &hpp);
 
@@ -521,29 +517,29 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,
 	return ret;
 }
 
-static int print_hierarchy_indent(const char *sep, int nr_sort,
+static int print_hierarchy_indent(const char *sep, int indent,
 				  const char *line, FILE *fp)
 {
-	if (sep != NULL || nr_sort < 1)
+	if (sep != NULL || indent < 2)
 		return 0;
 
-	return fprintf(fp, "%-.*s", (nr_sort - 1) * HIERARCHY_INDENT, line);
+	return fprintf(fp, "%-.*s", (indent - 2) * HIERARCHY_INDENT, line);
 }
 
 static int print_hierarchy_header(struct hists *hists, struct perf_hpp *hpp,
 				  const char *sep, FILE *fp)
 {
 	bool first = true;
-	int nr_sort;
+	int indent;
 	int depth;
 	unsigned width = 0;
 	unsigned header_width = 0;
 	struct perf_hpp_fmt *fmt;
 
-	nr_sort = hists->nr_sort_keys;
+	indent = hists->nr_hpp_node;
 
 	/* preserve max indent depth for column headers */
-	print_hierarchy_indent(sep, nr_sort, spaces, fp);
+	print_hierarchy_indent(sep, indent, spaces, fp);
 
 	hists__for_each_format(hists, fmt) {
 		if (perf_hpp__is_sort_entry(fmt) || perf_hpp__is_dynamic_entry(fmt))
@@ -582,7 +578,7 @@ static int print_hierarchy_header(struct hists *hists, struct perf_hpp *hpp,
 	fprintf(fp, "\n# ");
 
 	/* preserve max indent depth for initial dots */
-	print_hierarchy_indent(sep, nr_sort, dots, fp);
+	print_hierarchy_indent(sep, indent, dots, fp);
 
 	first = true;
 	hists__for_each_format(hists, fmt) {
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 2209188d729c..2cb017f28f9e 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -80,6 +80,7 @@ struct hists {
 	struct perf_hpp_list	*hpp_list;
 	struct list_head	hpp_formats;
 	int			nr_sort_keys;
+	int			nr_hpp_node;
 };
 
 struct hist_entry_iter;
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 13/15] perf report: Use hierarchy hpp list on stdio
  2016-03-07 19:44 [GIT PULL 00/15] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (11 preceding siblings ...)
  2016-03-07 19:44 ` [PATCH 12/15] perf hists: Fix indent for multiple hierarchy sort key Arnaldo Carvalho de Melo
@ 2016-03-07 19:44 ` Arnaldo Carvalho de Melo
  2016-03-07 19:44 ` [PATCH 14/15] perf hists browser: Use hierarchy hpp list Arnaldo Carvalho de Melo
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-03-07 19:44 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Namhyung Kim, Andi Kleen, David Ahern, Jiri Olsa,
	Peter Zijlstra, Stephane Eranian, Wang Nan,
	Arnaldo Carvalho de Melo

From: Namhyung Kim <namhyung@kernel.org>

Now hpp formats are linked using perf_hpp_list_node when hierarchy is
enabled.  Use this info to print entries with multiple sort keys in a
single hierarchy properly.

For example, the below example shows using 4 sort keys with 2 levels.

  $ perf report --hierarchy -s '{prev_pid,prev_comm},{next_pid,next_comm}' \
   --percent-limit 1 -i perf.data.sched
  ...
  #    Overhead  prev_pid+prev_comm / next_pid+next_comm
  # ...........  .......................................
  #
      22.36%     0  swapper/0
          9.48%     17773  transmission-gt
          5.25%     109  kworker/0:1H
          1.53%     6524  Xephyr
      21.39%     17773  transmission-gt
          9.52%     0  swapper/0
          9.04%     0  swapper/2
          1.78%     0  swapper/3

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1457361308-514-6-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/stdio/hist.c | 103 +++++++++++++++++++++++++--------------------
 1 file changed, 57 insertions(+), 46 deletions(-)

diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 543d7137cc0c..7aff5acf3265 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -417,6 +417,7 @@ static int hist_entry__hierarchy_fprintf(struct hist_entry *he,
 {
 	const char *sep = symbol_conf.field_sep;
 	struct perf_hpp_fmt *fmt;
+	struct perf_hpp_list_node *fmt_node;
 	char *buf = hpp->buf;
 	size_t size = hpp->size;
 	int ret, printed = 0;
@@ -428,10 +429,10 @@ static int hist_entry__hierarchy_fprintf(struct hist_entry *he,
 	ret = scnprintf(hpp->buf, hpp->size, "%*s", he->depth * HIERARCHY_INDENT, "");
 	advance_hpp(hpp, ret);
 
-	hists__for_each_format(he->hists, fmt) {
-		if (perf_hpp__is_sort_entry(fmt) || perf_hpp__is_dynamic_entry(fmt))
-			break;
-
+	/* the first hpp_list_node is for overhead columns */
+	fmt_node = list_first_entry(&hists->hpp_formats,
+				    struct perf_hpp_list_node, list);
+	perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
 		/*
 		 * If there's no field_sep, we still need
 		 * to display initial '  '.
@@ -529,50 +530,49 @@ static int print_hierarchy_indent(const char *sep, int indent,
 static int print_hierarchy_header(struct hists *hists, struct perf_hpp *hpp,
 				  const char *sep, FILE *fp)
 {
-	bool first = true;
+	bool first_node, first_col;
 	int indent;
 	int depth;
 	unsigned width = 0;
 	unsigned header_width = 0;
 	struct perf_hpp_fmt *fmt;
+	struct perf_hpp_list_node *fmt_node;
 
 	indent = hists->nr_hpp_node;
 
 	/* preserve max indent depth for column headers */
 	print_hierarchy_indent(sep, indent, spaces, fp);
 
-	hists__for_each_format(hists, fmt) {
-		if (perf_hpp__is_sort_entry(fmt) || perf_hpp__is_dynamic_entry(fmt))
-			break;
-
-		if (!first)
-			fprintf(fp, "%s", sep ?: "  ");
-		else
-			first = false;
+	/* the first hpp_list_node is for overhead columns */
+	fmt_node = list_first_entry(&hists->hpp_formats,
+				    struct perf_hpp_list_node, list);
 
+	perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
 		fmt->header(fmt, hpp, hists_to_evsel(hists));
-		fprintf(fp, "%s", hpp->buf);
+		fprintf(fp, "%s%s", hpp->buf, sep ?: "  ");
 	}
 
 	/* combine sort headers with ' / ' */
-	first = true;
-	hists__for_each_format(hists, fmt) {
-		if (!perf_hpp__is_sort_entry(fmt) && !perf_hpp__is_dynamic_entry(fmt))
-			continue;
-		if (perf_hpp__should_skip(fmt, hists))
-			continue;
-
-		if (!first)
+	first_node = true;
+	list_for_each_entry_continue(fmt_node, &hists->hpp_formats, list) {
+		if (!first_node)
 			header_width += fprintf(fp, " / ");
-		else {
-			fprintf(fp, "%s", sep ?: "  ");
-			first = false;
-		}
+		first_node = false;
 
-		fmt->header(fmt, hpp, hists_to_evsel(hists));
-		rtrim(hpp->buf);
+		first_col = true;
+		perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
+			if (perf_hpp__should_skip(fmt, hists))
+				continue;
+
+			if (!first_col)
+				header_width += fprintf(fp, "+");
+			first_col = false;
+
+			fmt->header(fmt, hpp, hists_to_evsel(hists));
+			rtrim(hpp->buf);
 
-		header_width += fprintf(fp, "%s", ltrim(hpp->buf));
+			header_width += fprintf(fp, "%s", ltrim(hpp->buf));
+		}
 	}
 
 	fprintf(fp, "\n# ");
@@ -580,29 +580,35 @@ static int print_hierarchy_header(struct hists *hists, struct perf_hpp *hpp,
 	/* preserve max indent depth for initial dots */
 	print_hierarchy_indent(sep, indent, dots, fp);
 
-	first = true;
-	hists__for_each_format(hists, fmt) {
-		if (perf_hpp__is_sort_entry(fmt) || perf_hpp__is_dynamic_entry(fmt))
-			break;
+	/* the first hpp_list_node is for overhead columns */
+	fmt_node = list_first_entry(&hists->hpp_formats,
+				    struct perf_hpp_list_node, list);
 
-		if (!first)
-			fprintf(fp, "%s", sep ?: "  ");
-		else
-			first = false;
+	first_col = true;
+	perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
+		if (!first_col)
+			fprintf(fp, "%s", sep ?: "..");
+		first_col = false;
 
 		width = fmt->width(fmt, hpp, hists_to_evsel(hists));
 		fprintf(fp, "%.*s", width, dots);
 	}
 
 	depth = 0;
-	hists__for_each_format(hists, fmt) {
-		if (!perf_hpp__is_sort_entry(fmt) && !perf_hpp__is_dynamic_entry(fmt))
-			continue;
-		if (perf_hpp__should_skip(fmt, hists))
-			continue;
+	list_for_each_entry_continue(fmt_node, &hists->hpp_formats, list) {
+		first_col = true;
+		width = depth * HIERARCHY_INDENT;
 
-		width = fmt->width(fmt, hpp, hists_to_evsel(hists));
-		width += depth * HIERARCHY_INDENT;
+		perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
+			if (perf_hpp__should_skip(fmt, hists))
+				continue;
+
+			if (!first_col)
+				width++;  /* for '+' sign between column header */
+			first_col = false;
+
+			width += fmt->width(fmt, hpp, hists_to_evsel(hists));
+		}
 
 		if (width > header_width)
 			header_width = width;
@@ -621,6 +627,7 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,
 		      int max_cols, float min_pcnt, FILE *fp)
 {
 	struct perf_hpp_fmt *fmt;
+	struct perf_hpp_list_node *fmt_node;
 	struct rb_node *nd;
 	size_t ret = 0;
 	unsigned int width;
@@ -650,6 +657,10 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,
 	fprintf(fp, "# ");
 
 	if (symbol_conf.report_hierarchy) {
+		list_for_each_entry(fmt_node, &hists->hpp_formats, list) {
+			perf_hpp_list__for_each_format(&fmt_node->hpp, fmt)
+				perf_hpp__reset_width(fmt, hists);
+		}
 		nr_rows += print_hierarchy_header(hists, &dummy_hpp, sep, fp);
 		goto print_entries;
 	}
@@ -734,9 +745,9 @@ print_entries:
 		 * display "no entry >= x.xx%" message.
 		 */
 		if (!h->leaf && !hist_entry__has_hierarchy_children(h, min_pcnt)) {
-			int nr_sort = hists->nr_sort_keys;
+			int depth = hists->nr_hpp_node + h->depth + 1;
 
-			print_hierarchy_indent(sep, nr_sort + h->depth + 1, spaces, fp);
+			print_hierarchy_indent(sep, depth, spaces, fp);
 			fprintf(fp, "%*sno entry >= %.2f%%\n", indent, "", min_pcnt);
 
 			if (max_rows && ++nr_rows >= max_rows)
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 14/15] perf hists browser: Use hierarchy hpp list
  2016-03-07 19:44 [GIT PULL 00/15] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (12 preceding siblings ...)
  2016-03-07 19:44 ` [PATCH 13/15] perf report: Use hierarchy hpp list on stdio Arnaldo Carvalho de Melo
@ 2016-03-07 19:44 ` Arnaldo Carvalho de Melo
  2016-03-07 19:44 ` [PATCH 15/15] perf report: Use hierarchy hpp list on gtk Arnaldo Carvalho de Melo
  2016-03-10  9:32 ` [PATCH] perf tool: Build jitdump only on supported archs Jiri Olsa
  15 siblings, 0 replies; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-03-07 19:44 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Namhyung Kim, Andi Kleen, David Ahern, Jiri Olsa,
	Peter Zijlstra, Stephane Eranian, Wang Nan,
	Arnaldo Carvalho de Melo

From: Namhyung Kim <namhyung@kernel.org>

Now hpp formats are linked using perf_hpp_list_node when hierarchy is
enabled.  Like in stdio, use this info to print entries with multiple
sort keys in a single hierarchy properly.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1457361308-514-7-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/hists.c | 81 +++++++++++++++++++++++-------------------
 1 file changed, 45 insertions(+), 36 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 2f02ce79bd9d..e0e217ec856b 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1289,6 +1289,7 @@ static int hist_browser__show_hierarchy_entry(struct hist_browser *browser,
 	off_t row_offset = entry->row_offset;
 	bool first = true;
 	struct perf_hpp_fmt *fmt;
+	struct perf_hpp_list_node *fmt_node;
 	struct hpp_arg arg = {
 		.b		= &browser->b,
 		.current_entry	= current_entry,
@@ -1320,7 +1321,10 @@ static int hist_browser__show_hierarchy_entry(struct hist_browser *browser,
 	ui_browser__write_nstring(&browser->b, "", level * HIERARCHY_INDENT);
 	width -= level * HIERARCHY_INDENT;
 
-	hists__for_each_format(entry->hists, fmt) {
+	/* the first hpp_list_node is for overhead columns */
+	fmt_node = list_first_entry(&entry->hists->hpp_formats,
+				    struct perf_hpp_list_node, list);
+	perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
 		char s[2048];
 		struct perf_hpp hpp = {
 			.buf		= s,
@@ -1332,10 +1336,6 @@ static int hist_browser__show_hierarchy_entry(struct hist_browser *browser,
 		    column++ < browser->b.horiz_scroll)
 			continue;
 
-		if (perf_hpp__is_sort_entry(fmt) ||
-		    perf_hpp__is_dynamic_entry(fmt))
-			break;
-
 		if (current_entry && browser->b.navkeypressed) {
 			ui_browser__set_color(&browser->b,
 					      HE_COLORSET_SELECTED);
@@ -1444,6 +1444,7 @@ static int hist_browser__show_no_entry(struct hist_browser *browser,
 	int column = 0;
 	int ret;
 	struct perf_hpp_fmt *fmt;
+	struct perf_hpp_list_node *fmt_node;
 	int indent = browser->hists->nr_hpp_node - 2;
 
 	if (current_entry) {
@@ -1461,15 +1462,14 @@ static int hist_browser__show_no_entry(struct hist_browser *browser,
 	ui_browser__write_nstring(&browser->b, "", level * HIERARCHY_INDENT);
 	width -= level * HIERARCHY_INDENT;
 
-	hists__for_each_format(browser->hists, fmt) {
+	/* the first hpp_list_node is for overhead columns */
+	fmt_node = list_first_entry(&browser->hists->hpp_formats,
+				    struct perf_hpp_list_node, list);
+	perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
 		if (perf_hpp__should_skip(fmt, browser->hists) ||
 		    column++ < browser->b.horiz_scroll)
 			continue;
 
-		if (perf_hpp__is_sort_entry(fmt) ||
-		    perf_hpp__is_dynamic_entry(fmt))
-			break;
-
 		ret = fmt->width(fmt, NULL, hists_to_evsel(browser->hists));
 
 		if (first) {
@@ -1551,22 +1551,23 @@ static int hists_browser__scnprintf_hierarchy_headers(struct hist_browser *brows
 		.size   = size,
 	};
 	struct perf_hpp_fmt *fmt;
+	struct perf_hpp_list_node *fmt_node;
 	size_t ret = 0;
 	int column = 0;
 	int indent = hists->nr_hpp_node - 2;
-	bool first = true;
+	bool first_node, first_col;
 
 	ret = scnprintf(buf, size, " ");
 	if (advance_hpp_check(&dummy_hpp, ret))
 		return ret;
 
-	hists__for_each_format(hists, fmt) {
+	/* the first hpp_list_node is for overhead columns */
+	fmt_node = list_first_entry(&hists->hpp_formats,
+				    struct perf_hpp_list_node, list);
+	perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
 		if (column++ < browser->b.horiz_scroll)
 			continue;
 
-		if (perf_hpp__is_sort_entry(fmt) || perf_hpp__is_dynamic_entry(fmt))
-			break;
-
 		ret = fmt->header(fmt, &dummy_hpp, hists_to_evsel(hists));
 		if (advance_hpp_check(&dummy_hpp, ret))
 			break;
@@ -1581,34 +1582,42 @@ static int hists_browser__scnprintf_hierarchy_headers(struct hist_browser *brows
 	if (advance_hpp_check(&dummy_hpp, ret))
 		return ret;
 
-	hists__for_each_format(hists, fmt) {
-		char *start;
-
-		if (!perf_hpp__is_sort_entry(fmt) && !perf_hpp__is_dynamic_entry(fmt))
-			continue;
-		if (perf_hpp__should_skip(fmt, hists))
-			continue;
-
-		if (first) {
-			first = false;
-		} else {
+	first_node = true;
+	list_for_each_entry_continue(fmt_node, &hists->hpp_formats, list) {
+		if (!first_node) {
 			ret = scnprintf(dummy_hpp.buf, dummy_hpp.size, " / ");
 			if (advance_hpp_check(&dummy_hpp, ret))
 				break;
 		}
+		first_node = false;
 
-		ret = fmt->header(fmt, &dummy_hpp, hists_to_evsel(hists));
-		dummy_hpp.buf[ret] = '\0';
-		rtrim(dummy_hpp.buf);
+		first_col = true;
+		perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
+			char *start;
 
-		start = ltrim(dummy_hpp.buf);
-		ret = strlen(start);
+			if (perf_hpp__should_skip(fmt, hists))
+				continue;
 
-		if (start != dummy_hpp.buf)
-			memmove(dummy_hpp.buf, start, ret + 1);
+			if (!first_col) {
+				ret = scnprintf(dummy_hpp.buf, dummy_hpp.size, "+");
+				if (advance_hpp_check(&dummy_hpp, ret))
+					break;
+			}
+			first_col = false;
 
-		if (advance_hpp_check(&dummy_hpp, ret))
-			break;
+			ret = fmt->header(fmt, &dummy_hpp, hists_to_evsel(hists));
+			dummy_hpp.buf[ret] = '\0';
+			rtrim(dummy_hpp.buf);
+
+			start = ltrim(dummy_hpp.buf);
+			ret = strlen(start);
+
+			if (start != dummy_hpp.buf)
+				memmove(dummy_hpp.buf, start, ret + 1);
+
+			if (advance_hpp_check(&dummy_hpp, ret))
+				break;
+		}
 	}
 
 	return ret;
@@ -1676,7 +1685,7 @@ static unsigned int hist_browser__refresh(struct ui_browser *browser)
 				break;
 
 			if (h->has_no_entry) {
-				hist_browser__show_no_entry(hb, row, h->depth);
+				hist_browser__show_no_entry(hb, row, h->depth + 1);
 				row++;
 			}
 		} else {
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 15/15] perf report: Use hierarchy hpp list on gtk
  2016-03-07 19:44 [GIT PULL 00/15] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (13 preceding siblings ...)
  2016-03-07 19:44 ` [PATCH 14/15] perf hists browser: Use hierarchy hpp list Arnaldo Carvalho de Melo
@ 2016-03-07 19:44 ` Arnaldo Carvalho de Melo
  2016-03-10  9:32 ` [PATCH] perf tool: Build jitdump only on supported archs Jiri Olsa
  15 siblings, 0 replies; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-03-07 19:44 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Namhyung Kim, Andi Kleen, David Ahern, Jiri Olsa,
	Peter Zijlstra, Stephane Eranian, Wang Nan,
	Arnaldo Carvalho de Melo

From: Namhyung Kim <namhyung@kernel.org>

Now hpp formats are linked using perf_hpp_list_node when hierarchy is
enabled.  Like in stdio, use this info to print entries with multiple
sort keys in a single hierarchy properly.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1457361308-514-8-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/gtk/hists.c | 55 ++++++++++++++++++++++++++++-------------------
 1 file changed, 33 insertions(+), 22 deletions(-)

diff --git a/tools/perf/ui/gtk/hists.c b/tools/perf/ui/gtk/hists.c
index 4534e2d7669c..bd9bf7e343b1 100644
--- a/tools/perf/ui/gtk/hists.c
+++ b/tools/perf/ui/gtk/hists.c
@@ -407,7 +407,9 @@ static void perf_gtk__add_hierarchy_entries(struct hists *hists,
 	struct rb_node *node;
 	struct hist_entry *he;
 	struct perf_hpp_fmt *fmt;
+	struct perf_hpp_list_node *fmt_node;
 	u64 total = hists__total_period(hists);
+	int size;
 
 	for (node = rb_first(root); node; node = rb_next(node)) {
 		GtkTreeIter iter;
@@ -425,11 +427,11 @@ static void perf_gtk__add_hierarchy_entries(struct hists *hists,
 		gtk_tree_store_append(store, &iter, parent);
 
 		col_idx = 0;
-		hists__for_each_format(hists, fmt) {
-			if (perf_hpp__is_sort_entry(fmt) ||
-			    perf_hpp__is_dynamic_entry(fmt))
-				break;
 
+		/* the first hpp_list_node is for overhead columns */
+		fmt_node = list_first_entry(&hists->hpp_formats,
+					    struct perf_hpp_list_node, list);
+		perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
 			if (fmt->color)
 				fmt->color(fmt, hpp, he);
 			else
@@ -439,6 +441,7 @@ static void perf_gtk__add_hierarchy_entries(struct hists *hists,
 		}
 
 		bf = hpp->buf;
+		size = hpp->size;
 		perf_hpp_list__for_each_format(he->hpp_list, fmt) {
 			int ret;
 
@@ -451,9 +454,12 @@ static void perf_gtk__add_hierarchy_entries(struct hists *hists,
 			advance_hpp(hpp, ret + 2);
 		}
 
-		gtk_tree_store_set(store, &iter, col_idx, rtrim(bf), -1);
+		gtk_tree_store_set(store, &iter, col_idx, ltrim(rtrim(bf)), -1);
 
 		if (!he->leaf) {
+			hpp->buf = bf;
+			hpp->size = size;
+
 			perf_gtk__add_hierarchy_entries(hists, &he->hroot_out,
 							store, &iter, hpp,
 							min_pcnt);
@@ -486,6 +492,7 @@ static void perf_gtk__show_hierarchy(GtkWidget *window, struct hists *hists,
 				     float min_pcnt)
 {
 	struct perf_hpp_fmt *fmt;
+	struct perf_hpp_list_node *fmt_node;
 	GType col_types[MAX_COLUMNS];
 	GtkCellRenderer *renderer;
 	GtkTreeStore *store;
@@ -494,7 +501,7 @@ static void perf_gtk__show_hierarchy(GtkWidget *window, struct hists *hists,
 	int nr_cols = 0;
 	char s[512];
 	char buf[512];
-	bool first = true;
+	bool first_node, first_col;
 	struct perf_hpp hpp = {
 		.buf		= s,
 		.size		= sizeof(s),
@@ -514,11 +521,11 @@ static void perf_gtk__show_hierarchy(GtkWidget *window, struct hists *hists,
 	renderer = gtk_cell_renderer_text_new();
 
 	col_idx = 0;
-	hists__for_each_format(hists, fmt) {
-		if (perf_hpp__is_sort_entry(fmt) ||
-		    perf_hpp__is_dynamic_entry(fmt))
-			break;
 
+	/* the first hpp_list_node is for overhead columns */
+	fmt_node = list_first_entry(&hists->hpp_formats,
+				    struct perf_hpp_list_node, list);
+	perf_hpp_list__for_each_format(&fmt_node->hpp, fmt) {
 		gtk_tree_view_insert_column_with_attributes(GTK_TREE_VIEW(view),
 							    -1, fmt->name,
 							    renderer, "markup",
@@ -527,20 +534,24 @@ static void perf_gtk__show_hierarchy(GtkWidget *window, struct hists *hists,
 
 	/* construct merged column header since sort keys share single column */
 	buf[0] = '\0';
-	hists__for_each_format(hists ,fmt) {
-		if (!perf_hpp__is_sort_entry(fmt) &&
-		    !perf_hpp__is_dynamic_entry(fmt))
-			continue;
-		if (perf_hpp__should_skip(fmt, hists))
-			continue;
-
-		if (first)
-			first = false;
-		else
+	first_node = true;
+	list_for_each_entry_continue(fmt_node, &hists->hpp_formats, list) {
+		if (!first_node)
 			strcat(buf, " / ");
+		first_node = false;
 
-		fmt->header(fmt, &hpp, hists_to_evsel(hists));
-		strcat(buf, rtrim(hpp.buf));
+		first_col = true;
+		perf_hpp_list__for_each_format(&fmt_node->hpp ,fmt) {
+			if (perf_hpp__should_skip(fmt, hists))
+				continue;
+
+			if (!first_col)
+				strcat(buf, "+");
+			first_col = false;
+
+			fmt->header(fmt, &hpp, hists_to_evsel(hists));
+			strcat(buf, ltrim(rtrim(hpp.buf)));
+		}
 	}
 
 	gtk_tree_view_insert_column_with_attributes(GTK_TREE_VIEW(view),
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH 03/15] perf session: Simplify tool stubs
  2016-03-07 19:44 ` [PATCH 03/15] perf session: Simplify tool stubs Arnaldo Carvalho de Melo
@ 2016-03-08  7:45   ` Adrian Hunter
  2016-03-08  9:17     ` Ingo Molnar
  0 siblings, 1 reply; 25+ messages in thread
From: Adrian Hunter @ 2016-03-08  7:45 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Ingo Molnar
  Cc: linux-kernel, Jiri Olsa, Stephane Eranian, Arnaldo Carvalho de Melo

On 07/03/16 21:44, Arnaldo Carvalho de Melo wrote:
> From: Adrian Hunter <ajhunter@gmail.com>

Very sorry, but I just noticed that this and patch 5 (perf jit: Move clockid
validation) have the wrong email address for the "From" and "Signed-off-by".
 It should be adrian.hunter@intel.com.  Please consider changing it or
dropping these patches for now.

> 
> Some of the stubs are identical so just have one function for them.
> 
> Signed-off-by: Adrian Hunter <ajhunter@gmail.com>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Stephane Eranian <eranian@google.com>
> Link: http://lkml.kernel.org/r/1457005856-6143-3-git-send-email-adrian.hunter@intel.com
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 08/15] perf stat: Document --detailed option
  2016-03-07 19:44 ` [PATCH 08/15] perf stat: Document --detailed option Arnaldo Carvalho de Melo
@ 2016-03-08  9:10   ` Ingo Molnar
  0 siblings, 0 replies; 25+ messages in thread
From: Ingo Molnar @ 2016-03-08  9:10 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Borislav Petkov, Davidlohr Bueso, Jiri Olsa,
	Mel Gorman, Peter Zijlstra, Steven Rostedt,
	Arnaldo Carvalho de Melo


* Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> From: Borislav Petkov <bp@suse.de>
> 
> I'm surprised this remained undocumented since at least 2011. And it is
> actually a very useful switch, as Steve and I came to realize recently.
> 
> Add the text from
> 
>   2cba3ffb9a9d ("perf stat: Add -d -d and -d -d -d options to show more CPU events")
> 
> which added the incrementing aspect to -d.

Ah yes, my fault ...

> Signed-off-by: Borislav Petkov <bp@suse.de>
> Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Davidlohr Bueso <dbueso@suse.com>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Mel Gorman <mgorman@suse.com>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Fixes: 2cba3ffb9a9d ("perf stat: Add -d -d and -d -d -d options to show more CPU events")
> Link: http://lkml.kernel.org/r/1457347294-32546-1-git-send-email-bp@alien8.de
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
>  tools/perf/Documentation/perf-stat.txt | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
> index 52ef7a9d50aa..14d9e8ffaff7 100644
> --- a/tools/perf/Documentation/perf-stat.txt
> +++ b/tools/perf/Documentation/perf-stat.txt
> @@ -69,6 +69,14 @@ report::
>  --scale::
>  	scale/normalize counter values
>  
> +-d::
> +--detailed::
> +	print more detailed statistics, can be specified up to 3 times
> +
> +	   -d:          detailed events, L1 and LLC data cache
> +        -d -d:     more detailed events, dTLB and iTLB events
> +     -d -d -d:     very detailed events, adding prefetch events

Btw., something I noticed: it would be really nice if 'perf stat -h -d' printed 
the most detailed documentation available for the option. Right now it prints:

 triton:~/tip/tools/perf> perf stat -h -d

  Usage: perf stat [<options>] [<command>]

    -d, --detailed        detailed run - start a lot of events

The only way to get to the most detailed documentation is the not very obvious 
path of:

 perf help stat

and then searching for '-d'.

Ideally there should only be a single place to document options - but I guess 
unifying the in-code option descriptions and more verbose description in the man 
pages is very non-trivial ...

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 03/15] perf session: Simplify tool stubs
  2016-03-08  7:45   ` Adrian Hunter
@ 2016-03-08  9:17     ` Ingo Molnar
  0 siblings, 0 replies; 25+ messages in thread
From: Ingo Molnar @ 2016-03-08  9:17 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Arnaldo Carvalho de Melo, linux-kernel, Jiri Olsa,
	Stephane Eranian, Arnaldo Carvalho de Melo


* Adrian Hunter <adrian.hunter@intel.com> wrote:

> On 07/03/16 21:44, Arnaldo Carvalho de Melo wrote:
> > From: Adrian Hunter <ajhunter@gmail.com>
> 
> Very sorry, but I just noticed that this and patch 5 (perf jit: Move clockid
> validation) have the wrong email address for the "From" and "Signed-off-by".
>  It should be adrian.hunter@intel.com.  Please consider changing it or
> dropping these patches for now.

Ok, I've changed these two patches and have applied the whole series from email. 
(we are getting close to the merge window)

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH] perf tool: Build jitdump only on supported archs
  2016-03-07 19:44 [GIT PULL 00/15] perf/core improvements and fixes Arnaldo Carvalho de Melo
                   ` (14 preceding siblings ...)
  2016-03-07 19:44 ` [PATCH 15/15] perf report: Use hierarchy hpp list on gtk Arnaldo Carvalho de Melo
@ 2016-03-10  9:32 ` Jiri Olsa
  2016-03-10 15:57   ` Arnaldo Carvalho de Melo
  15 siblings, 1 reply; 25+ messages in thread
From: Jiri Olsa @ 2016-03-10  9:32 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, linux-kernel, Adrian Hunter, Alexander Shishkin,
	Andi Kleen, Borislav Petkov, Colin Ian King, David Ahern,
	Davidlohr Bueso, He Kuang, Mel Gorman, Namhyung Kim,
	Peter Zijlstra, Stephane Eranian, Steven Rostedt, Wang Nan,
	Arnaldo Carvalho de Melo

On Mon, Mar 07, 2016 at 04:44:36PM -0300, Arnaldo Carvalho de Melo wrote:

SNIP

> ----------------------------------------------------------------
> Adrian Hunter (5):
>       perf inject: Hit all DSOs for AUX data in JIT and other cases
>       perf session: Simplify tool stubs
>       perf jit: Let jit_process() return errors
>       perf jit: Move clockid validation
>       perf tools: Use 64-bit shifts with (TSC) time conversion

hi,
perf build failed for me on s390x because of jitdump feature.

Attached patch tries to enable the build only on supported archs.

I haven't followed this feature too much, so I might have missed
something.. anyway it builds on s390x now ;-) I took the list of
archs from util/genelf.h

thanks,
jirka


---
Make jitdump being built only on architectures
defined in util/genelf.h file.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/arch/arm/Makefile     |  1 +
 tools/perf/arch/arm64/Makefile   |  1 +
 tools/perf/arch/powerpc/Makefile |  1 +
 tools/perf/arch/x86/Makefile     |  1 +
 tools/perf/builtin-inject.c      | 12 +++++++-----
 tools/perf/config/Makefile       |  7 +++++++
 tools/perf/util/Build            |  2 +-
 7 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/tools/perf/arch/arm/Makefile b/tools/perf/arch/arm/Makefile
index 7fbca175099e..18b13518d8d8 100644
--- a/tools/perf/arch/arm/Makefile
+++ b/tools/perf/arch/arm/Makefile
@@ -1,3 +1,4 @@
 ifndef NO_DWARF
 PERF_HAVE_DWARF_REGS := 1
 endif
+PERF_HAVE_JITDUMP := 1
diff --git a/tools/perf/arch/arm64/Makefile b/tools/perf/arch/arm64/Makefile
index 7fbca175099e..18b13518d8d8 100644
--- a/tools/perf/arch/arm64/Makefile
+++ b/tools/perf/arch/arm64/Makefile
@@ -1,3 +1,4 @@
 ifndef NO_DWARF
 PERF_HAVE_DWARF_REGS := 1
 endif
+PERF_HAVE_JITDUMP := 1
diff --git a/tools/perf/arch/powerpc/Makefile b/tools/perf/arch/powerpc/Makefile
index 9f9cea3478fd..56e05f126ad8 100644
--- a/tools/perf/arch/powerpc/Makefile
+++ b/tools/perf/arch/powerpc/Makefile
@@ -3,3 +3,4 @@ PERF_HAVE_DWARF_REGS := 1
 endif
 
 HAVE_KVM_STAT_SUPPORT := 1
+PERF_HAVE_JITDUMP := 1
diff --git a/tools/perf/arch/x86/Makefile b/tools/perf/arch/x86/Makefile
index 09ba923debe8..269af2143735 100644
--- a/tools/perf/arch/x86/Makefile
+++ b/tools/perf/arch/x86/Makefile
@@ -3,3 +3,4 @@ PERF_HAVE_DWARF_REGS := 1
 endif
 HAVE_KVM_STAT_SUPPORT := 1
 PERF_HAVE_ARCH_REGS_QUERY_REGISTER_OFFSET := 1
+PERF_HAVE_JITDUMP := 1
diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index e219ed458d97..6eb17e24c4cc 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -73,7 +73,7 @@ static int perf_event__repipe_oe_synth(struct perf_tool *tool,
 	return perf_event__repipe_synth(tool, event);
 }
 
-#if defined(HAVE_LIBELF_SUPPORT) && defined(HAVE_DWARF_SUPPORT)
+#ifdef HAVE_JITDUMP
 static int perf_event__drop_oe(struct perf_tool *tool __maybe_unused,
 			       union perf_event *event __maybe_unused,
 			       struct ordered_events *oe __maybe_unused)
@@ -245,7 +245,7 @@ static int perf_event__repipe_mmap(struct perf_tool *tool,
 	return err;
 }
 
-#if defined(HAVE_LIBELF_SUPPORT) && defined(HAVE_DWARF_SUPPORT)
+#if defined(HAVE_JITDUMP)
 static int perf_event__jit_repipe_mmap(struct perf_tool *tool,
 				       union perf_event *event,
 				       struct perf_sample *sample,
@@ -283,7 +283,7 @@ static int perf_event__repipe_mmap2(struct perf_tool *tool,
 	return err;
 }
 
-#if defined(HAVE_LIBELF_SUPPORT) && defined(HAVE_DWARF_SUPPORT)
+#if defined(HAVE_JITDUMP)
 static int perf_event__jit_repipe_mmap2(struct perf_tool *tool,
 					union perf_event *event,
 					struct perf_sample *sample,
@@ -778,7 +778,9 @@ int cmd_inject(int argc, const char **argv, const char *prefix __maybe_unused)
 		OPT_BOOLEAN('s', "sched-stat", &inject.sched_stat,
 			    "Merge sched-stat and sched-switch for getting events "
 			    "where and how long tasks slept"),
+#ifdef HAVE_JITDUMP
 		OPT_BOOLEAN('j', "jit", &inject.jit_mode, "merge jitdump files into perf.data file"),
+#endif
 		OPT_INCR('v', "verbose", &verbose,
 			 "be more verbose (show build ids, etc)"),
 		OPT_STRING(0, "kallsyms", &symbol_conf.kallsyms_name, "file",
@@ -795,7 +797,7 @@ int cmd_inject(int argc, const char **argv, const char *prefix __maybe_unused)
 		"perf inject [<options>]",
 		NULL
 	};
-#if !defined(HAVE_LIBELF_SUPPORT) || !defined(HAVE_DWARF_SUPPORT)
+#ifndef HAVE_JITDUMP
 	set_option_nobuild(options, 'j', "jit", "NO_LIBELF=1", true);
 #endif
 	argc = parse_options(argc, argv, options, inject_usage, 0);
@@ -833,7 +835,7 @@ int cmd_inject(int argc, const char **argv, const char *prefix __maybe_unused)
 		inject.tool.ordered_events = true;
 		inject.tool.ordering_requires_timestamps = true;
 	}
-#if defined(HAVE_LIBELF_SUPPORT) && defined(HAVE_DWARF_SUPPORT)
+#if defined(HAVE_JITDUMP)
 	if (inject.jit_mode) {
 		inject.tool.mmap2	   = perf_event__jit_repipe_mmap2;
 		inject.tool.mmap	   = perf_event__jit_repipe_mmap;
diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index f7aeaf303f5a..eca6a912e8c2 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -328,6 +328,13 @@ ifndef NO_LIBELF
   endif # NO_LIBBPF
 endif # NO_LIBELF
 
+ifdef PERF_HAVE_JITDUMP
+  ifndef NO_DWARF
+    $(call detected,CONFIG_JITDUMP)
+    CFLAGS += -DHAVE_JITDUMP
+  endif
+endif
+
 ifeq ($(ARCH),powerpc)
   ifndef NO_DWARF
     CFLAGS += -DHAVE_SKIP_CALLCHAIN_IDX
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index f130ce240158..eea25e2424e9 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -108,7 +108,7 @@ libperf-$(CONFIG_ZLIB) += zlib.o
 libperf-$(CONFIG_LZMA) += lzma.o
 libperf-y += demangle-java.o
 
-ifdef CONFIG_DWARF
+ifdef CONFIG_JITDUMP
 libperf-$(CONFIG_LIBELF) += jitdump.o
 libperf-$(CONFIG_LIBELF) += genelf.o
 libperf-$(CONFIG_LIBELF) += genelf_debug.o
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH] perf tool: Build jitdump only on supported archs
  2016-03-10  9:32 ` [PATCH] perf tool: Build jitdump only on supported archs Jiri Olsa
@ 2016-03-10 15:57   ` Arnaldo Carvalho de Melo
  2016-03-10 16:04     ` Jiri Olsa
  0 siblings, 1 reply; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-03-10 15:57 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, linux-kernel,
	Adrian Hunter, Alexander Shishkin, Andi Kleen, Borislav Petkov,
	Colin Ian King, David Ahern, Davidlohr Bueso, He Kuang,
	Mel Gorman, Namhyung Kim, Peter Zijlstra, Stephane Eranian,
	Steven Rostedt, Wang Nan

Em Thu, Mar 10, 2016 at 10:32:29AM +0100, Jiri Olsa escreveu:
> On Mon, Mar 07, 2016 at 04:44:36PM -0300, Arnaldo Carvalho de Melo wrote:
> 
> SNIP
> 
> > ----------------------------------------------------------------
> > Adrian Hunter (5):
> >       perf inject: Hit all DSOs for AUX data in JIT and other cases
> >       perf session: Simplify tool stubs
> >       perf jit: Let jit_process() return errors
> >       perf jit: Move clockid validation
> >       perf tools: Use 64-bit shifts with (TSC) time conversion
> 
> hi,
> perf build failed for me on s390x because of jitdump feature.
> 
> Attached patch tries to enable the build only on supported archs.
> 
> I haven't followed this feature too much, so I might have missed
> something.. anyway it builds on s390x now ;-) I took the list of
> archs from util/genelf.h

Hi Jiri,

	This may be related to this cset, still not in tip/perf/core:

  commit 46dad054a19297af65c417c97cb920aa5bdf7e8c
  Author: Arnaldo Carvalho de Melo <acme@redhat.com>
  Date:   Mon Mar 7 18:48:45 2016 -0300

    perf jitdump: DWARF is also needed

Can you check if without this one perf builds on s390? 

This patch is to make it build on ubuntu without libdw, i.e. it requires
both libdw and libelf, the way I fixed it may not be the best and
probably we need to fold these two patches before sending to Ingo, since
your patch essentially rewrites my previous patch :-)

Can you please check that?

- Arnaldo

 
> thanks,
> jirka
> 
> 
> ---
> Make jitdump being built only on architectures
> defined in util/genelf.h file.
> 
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> ---
>  tools/perf/arch/arm/Makefile     |  1 +
>  tools/perf/arch/arm64/Makefile   |  1 +
>  tools/perf/arch/powerpc/Makefile |  1 +
>  tools/perf/arch/x86/Makefile     |  1 +
>  tools/perf/builtin-inject.c      | 12 +++++++-----
>  tools/perf/config/Makefile       |  7 +++++++
>  tools/perf/util/Build            |  2 +-
>  7 files changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/arch/arm/Makefile b/tools/perf/arch/arm/Makefile
> index 7fbca175099e..18b13518d8d8 100644
> --- a/tools/perf/arch/arm/Makefile
> +++ b/tools/perf/arch/arm/Makefile
> @@ -1,3 +1,4 @@
>  ifndef NO_DWARF
>  PERF_HAVE_DWARF_REGS := 1
>  endif
> +PERF_HAVE_JITDUMP := 1
> diff --git a/tools/perf/arch/arm64/Makefile b/tools/perf/arch/arm64/Makefile
> index 7fbca175099e..18b13518d8d8 100644
> --- a/tools/perf/arch/arm64/Makefile
> +++ b/tools/perf/arch/arm64/Makefile
> @@ -1,3 +1,4 @@
>  ifndef NO_DWARF
>  PERF_HAVE_DWARF_REGS := 1
>  endif
> +PERF_HAVE_JITDUMP := 1
> diff --git a/tools/perf/arch/powerpc/Makefile b/tools/perf/arch/powerpc/Makefile
> index 9f9cea3478fd..56e05f126ad8 100644
> --- a/tools/perf/arch/powerpc/Makefile
> +++ b/tools/perf/arch/powerpc/Makefile
> @@ -3,3 +3,4 @@ PERF_HAVE_DWARF_REGS := 1
>  endif
>  
>  HAVE_KVM_STAT_SUPPORT := 1
> +PERF_HAVE_JITDUMP := 1
> diff --git a/tools/perf/arch/x86/Makefile b/tools/perf/arch/x86/Makefile
> index 09ba923debe8..269af2143735 100644
> --- a/tools/perf/arch/x86/Makefile
> +++ b/tools/perf/arch/x86/Makefile
> @@ -3,3 +3,4 @@ PERF_HAVE_DWARF_REGS := 1
>  endif
>  HAVE_KVM_STAT_SUPPORT := 1
>  PERF_HAVE_ARCH_REGS_QUERY_REGISTER_OFFSET := 1
> +PERF_HAVE_JITDUMP := 1
> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index e219ed458d97..6eb17e24c4cc 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
> @@ -73,7 +73,7 @@ static int perf_event__repipe_oe_synth(struct perf_tool *tool,
>  	return perf_event__repipe_synth(tool, event);
>  }
>  
> -#if defined(HAVE_LIBELF_SUPPORT) && defined(HAVE_DWARF_SUPPORT)
> +#ifdef HAVE_JITDUMP
>  static int perf_event__drop_oe(struct perf_tool *tool __maybe_unused,
>  			       union perf_event *event __maybe_unused,
>  			       struct ordered_events *oe __maybe_unused)
> @@ -245,7 +245,7 @@ static int perf_event__repipe_mmap(struct perf_tool *tool,
>  	return err;
>  }
>  
> -#if defined(HAVE_LIBELF_SUPPORT) && defined(HAVE_DWARF_SUPPORT)
> +#if defined(HAVE_JITDUMP)
>  static int perf_event__jit_repipe_mmap(struct perf_tool *tool,
>  				       union perf_event *event,
>  				       struct perf_sample *sample,
> @@ -283,7 +283,7 @@ static int perf_event__repipe_mmap2(struct perf_tool *tool,
>  	return err;
>  }
>  
> -#if defined(HAVE_LIBELF_SUPPORT) && defined(HAVE_DWARF_SUPPORT)
> +#if defined(HAVE_JITDUMP)
>  static int perf_event__jit_repipe_mmap2(struct perf_tool *tool,
>  					union perf_event *event,
>  					struct perf_sample *sample,
> @@ -778,7 +778,9 @@ int cmd_inject(int argc, const char **argv, const char *prefix __maybe_unused)
>  		OPT_BOOLEAN('s', "sched-stat", &inject.sched_stat,
>  			    "Merge sched-stat and sched-switch for getting events "
>  			    "where and how long tasks slept"),
> +#ifdef HAVE_JITDUMP
>  		OPT_BOOLEAN('j', "jit", &inject.jit_mode, "merge jitdump files into perf.data file"),
> +#endif
>  		OPT_INCR('v', "verbose", &verbose,
>  			 "be more verbose (show build ids, etc)"),
>  		OPT_STRING(0, "kallsyms", &symbol_conf.kallsyms_name, "file",
> @@ -795,7 +797,7 @@ int cmd_inject(int argc, const char **argv, const char *prefix __maybe_unused)
>  		"perf inject [<options>]",
>  		NULL
>  	};
> -#if !defined(HAVE_LIBELF_SUPPORT) || !defined(HAVE_DWARF_SUPPORT)
> +#ifndef HAVE_JITDUMP
>  	set_option_nobuild(options, 'j', "jit", "NO_LIBELF=1", true);
>  #endif
>  	argc = parse_options(argc, argv, options, inject_usage, 0);
> @@ -833,7 +835,7 @@ int cmd_inject(int argc, const char **argv, const char *prefix __maybe_unused)
>  		inject.tool.ordered_events = true;
>  		inject.tool.ordering_requires_timestamps = true;
>  	}
> -#if defined(HAVE_LIBELF_SUPPORT) && defined(HAVE_DWARF_SUPPORT)
> +#if defined(HAVE_JITDUMP)
>  	if (inject.jit_mode) {
>  		inject.tool.mmap2	   = perf_event__jit_repipe_mmap2;
>  		inject.tool.mmap	   = perf_event__jit_repipe_mmap;
> diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
> index f7aeaf303f5a..eca6a912e8c2 100644
> --- a/tools/perf/config/Makefile
> +++ b/tools/perf/config/Makefile
> @@ -328,6 +328,13 @@ ifndef NO_LIBELF
>    endif # NO_LIBBPF
>  endif # NO_LIBELF
>  
> +ifdef PERF_HAVE_JITDUMP
> +  ifndef NO_DWARF
> +    $(call detected,CONFIG_JITDUMP)
> +    CFLAGS += -DHAVE_JITDUMP
> +  endif
> +endif
> +
>  ifeq ($(ARCH),powerpc)
>    ifndef NO_DWARF
>      CFLAGS += -DHAVE_SKIP_CALLCHAIN_IDX
> diff --git a/tools/perf/util/Build b/tools/perf/util/Build
> index f130ce240158..eea25e2424e9 100644
> --- a/tools/perf/util/Build
> +++ b/tools/perf/util/Build
> @@ -108,7 +108,7 @@ libperf-$(CONFIG_ZLIB) += zlib.o
>  libperf-$(CONFIG_LZMA) += lzma.o
>  libperf-y += demangle-java.o
>  
> -ifdef CONFIG_DWARF
> +ifdef CONFIG_JITDUMP
>  libperf-$(CONFIG_LIBELF) += jitdump.o
>  libperf-$(CONFIG_LIBELF) += genelf.o
>  libperf-$(CONFIG_LIBELF) += genelf_debug.o
> -- 
> 2.4.3

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] perf tool: Build jitdump only on supported archs
  2016-03-10 15:57   ` Arnaldo Carvalho de Melo
@ 2016-03-10 16:04     ` Jiri Olsa
  2016-03-10 16:13       ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 25+ messages in thread
From: Jiri Olsa @ 2016-03-10 16:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, linux-kernel,
	Adrian Hunter, Alexander Shishkin, Andi Kleen, Borislav Petkov,
	Colin Ian King, David Ahern, Davidlohr Bueso, He Kuang,
	Mel Gorman, Namhyung Kim, Peter Zijlstra, Stephane Eranian,
	Steven Rostedt, Wang Nan

On Thu, Mar 10, 2016 at 12:57:00PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Mar 10, 2016 at 10:32:29AM +0100, Jiri Olsa escreveu:
> > On Mon, Mar 07, 2016 at 04:44:36PM -0300, Arnaldo Carvalho de Melo wrote:
> > 
> > SNIP
> > 
> > > ----------------------------------------------------------------
> > > Adrian Hunter (5):
> > >       perf inject: Hit all DSOs for AUX data in JIT and other cases
> > >       perf session: Simplify tool stubs
> > >       perf jit: Let jit_process() return errors
> > >       perf jit: Move clockid validation
> > >       perf tools: Use 64-bit shifts with (TSC) time conversion
> > 
> > hi,
> > perf build failed for me on s390x because of jitdump feature.
> > 
> > Attached patch tries to enable the build only on supported archs.
> > 
> > I haven't followed this feature too much, so I might have missed
> > something.. anyway it builds on s390x now ;-) I took the list of
> > archs from util/genelf.h
> 
> Hi Jiri,
> 
> 	This may be related to this cset, still not in tip/perf/core:
> 
>   commit 46dad054a19297af65c417c97cb920aa5bdf7e8c
>   Author: Arnaldo Carvalho de Melo <acme@redhat.com>
>   Date:   Mon Mar 7 18:48:45 2016 -0300
> 
>     perf jitdump: DWARF is also needed
> 
> Can you check if without this one perf builds on s390? 
> 
> This patch is to make it build on ubuntu without libdw, i.e. it requires
> both libdw and libelf, the way I fixed it may not be the best and
> probably we need to fold these two patches before sending to Ingo, since
> your patch essentially rewrites my previous patch :-)
> 
> Can you please check that?

well there's following line in util/genelf.h:

#else
#error "unsupported architecture"
#endif

which makes the build fail on unssuported arch,
even without your patch:

  CC       util/jitdump.o
In file included from util/jitdump.c:23:
util/genelf.h:41:2: error: #error "unsupported architecture"
util/genelf.h:44:5: error: "GEN_ELF_CLASS" is not defined
mv: cannot stat `util/.jitdump.o.tmp': No such file or directory
make[3]: *** [util/jitdump.o] Error 1


jirka

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH] perf tool: Build jitdump only on supported archs
  2016-03-10 16:04     ` Jiri Olsa
@ 2016-03-10 16:13       ` Arnaldo Carvalho de Melo
  2016-03-10 16:41         ` [PATCHv2] " Jiri Olsa
  0 siblings, 1 reply; 25+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-03-10 16:13 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, linux-kernel,
	Adrian Hunter, Alexander Shishkin, Andi Kleen, Borislav Petkov,
	Colin Ian King, David Ahern, Davidlohr Bueso, He Kuang,
	Mel Gorman, Namhyung Kim, Peter Zijlstra, Stephane Eranian,
	Steven Rostedt, Wang Nan

Em Thu, Mar 10, 2016 at 05:04:36PM +0100, Jiri Olsa escreveu:
> On Thu, Mar 10, 2016 at 12:57:00PM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Thu, Mar 10, 2016 at 10:32:29AM +0100, Jiri Olsa escreveu:
> > > On Mon, Mar 07, 2016 at 04:44:36PM -0300, Arnaldo Carvalho de Melo wrote:
> > > 
> > > SNIP
> > > 
> > > > ----------------------------------------------------------------
> > > > Adrian Hunter (5):
> > > >       perf inject: Hit all DSOs for AUX data in JIT and other cases
> > > >       perf session: Simplify tool stubs
> > > >       perf jit: Let jit_process() return errors
> > > >       perf jit: Move clockid validation
> > > >       perf tools: Use 64-bit shifts with (TSC) time conversion
> > > 
> > > hi,
> > > perf build failed for me on s390x because of jitdump feature.
> > > 
> > > Attached patch tries to enable the build only on supported archs.
> > > 
> > > I haven't followed this feature too much, so I might have missed
> > > something.. anyway it builds on s390x now ;-) I took the list of
> > > archs from util/genelf.h
> > 
> > Hi Jiri,
> > 
> > 	This may be related to this cset, still not in tip/perf/core:
> > 
> >   commit 46dad054a19297af65c417c97cb920aa5bdf7e8c
> >   Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> >   Date:   Mon Mar 7 18:48:45 2016 -0300
> > 
> >     perf jitdump: DWARF is also needed
> > 
> > Can you check if without this one perf builds on s390? 
> > 
> > This patch is to make it build on ubuntu without libdw, i.e. it requires
> > both libdw and libelf, the way I fixed it may not be the best and
> > probably we need to fold these two patches before sending to Ingo, since
> > your patch essentially rewrites my previous patch :-)
> > 
> > Can you please check that?
> 
> well there's following line in util/genelf.h:
> 
> #else
> #error "unsupported architecture"
> #endif
> 
> which makes the build fail on unssuported arch,
> even without your patch:

Ok, so I'll just apply your patch, thanks for clarifying.

- Arnaldo
 
>   CC       util/jitdump.o
> In file included from util/jitdump.c:23:
> util/genelf.h:41:2: error: #error "unsupported architecture"
> util/genelf.h:44:5: error: "GEN_ELF_CLASS" is not defined
> mv: cannot stat `util/.jitdump.o.tmp': No such file or directory
> make[3]: *** [util/jitdump.o] Error 1
> 
> 
> jirka

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCHv2] perf tool: Build jitdump only on supported archs
  2016-03-10 16:13       ` Arnaldo Carvalho de Melo
@ 2016-03-10 16:41         ` Jiri Olsa
  2016-03-11  8:47           ` [tip:perf/core] perf jitdump: Build " tip-bot for Jiri Olsa
  0 siblings, 1 reply; 25+ messages in thread
From: Jiri Olsa @ 2016-03-10 16:41 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, linux-kernel,
	Adrian Hunter, Alexander Shishkin, Andi Kleen, Borislav Petkov,
	Colin Ian King, David Ahern, Davidlohr Bueso, He Kuang,
	Mel Gorman, Namhyung Kim, Peter Zijlstra, Stephane Eranian,
	Steven Rostedt, Wang Nan

On Thu, Mar 10, 2016 at 01:13:00PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Mar 10, 2016 at 05:04:36PM +0100, Jiri Olsa escreveu:
> > On Thu, Mar 10, 2016 at 12:57:00PM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Thu, Mar 10, 2016 at 10:32:29AM +0100, Jiri Olsa escreveu:
> > > > On Mon, Mar 07, 2016 at 04:44:36PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > 
> > > > SNIP
> > > > 
> > > > > ----------------------------------------------------------------
> > > > > Adrian Hunter (5):
> > > > >       perf inject: Hit all DSOs for AUX data in JIT and other cases
> > > > >       perf session: Simplify tool stubs
> > > > >       perf jit: Let jit_process() return errors
> > > > >       perf jit: Move clockid validation
> > > > >       perf tools: Use 64-bit shifts with (TSC) time conversion
> > > > 
> > > > hi,
> > > > perf build failed for me on s390x because of jitdump feature.
> > > > 
> > > > Attached patch tries to enable the build only on supported archs.
> > > > 
> > > > I haven't followed this feature too much, so I might have missed
> > > > something.. anyway it builds on s390x now ;-) I took the list of
> > > > archs from util/genelf.h
> > > 
> > > Hi Jiri,
> > > 
> > > 	This may be related to this cset, still not in tip/perf/core:
> > > 
> > >   commit 46dad054a19297af65c417c97cb920aa5bdf7e8c
> > >   Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> > >   Date:   Mon Mar 7 18:48:45 2016 -0300
> > > 
> > >     perf jitdump: DWARF is also needed
> > > 
> > > Can you check if without this one perf builds on s390? 
> > > 
> > > This patch is to make it build on ubuntu without libdw, i.e. it requires
> > > both libdw and libelf, the way I fixed it may not be the best and
> > > probably we need to fold these two patches before sending to Ingo, since
> > > your patch essentially rewrites my previous patch :-)
> > > 
> > > Can you please check that?
> > 
> > well there's following line in util/genelf.h:
> > 
> > #else
> > #error "unsupported architecture"
> > #endif
> > 
> > which makes the build fail on unssuported arch,
> > even without your patch:
> 
> Ok, so I'll just apply your patch, thanks for clarifying.
> 

oops, just consolidated #if defined into #ifdef, v2 attached

thanks,
jirka


---
Make jitdump being built only on architectures
defined in util/genelf.h file.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 tools/perf/arch/arm/Makefile     |  1 +
 tools/perf/arch/arm64/Makefile   |  1 +
 tools/perf/arch/powerpc/Makefile |  1 +
 tools/perf/arch/x86/Makefile     |  1 +
 tools/perf/builtin-inject.c      | 12 +++++++-----
 tools/perf/config/Makefile       |  7 +++++++
 tools/perf/util/Build            |  2 +-
 7 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/tools/perf/arch/arm/Makefile b/tools/perf/arch/arm/Makefile
index 7fbca175099e..18b13518d8d8 100644
--- a/tools/perf/arch/arm/Makefile
+++ b/tools/perf/arch/arm/Makefile
@@ -1,3 +1,4 @@
 ifndef NO_DWARF
 PERF_HAVE_DWARF_REGS := 1
 endif
+PERF_HAVE_JITDUMP := 1
diff --git a/tools/perf/arch/arm64/Makefile b/tools/perf/arch/arm64/Makefile
index 7fbca175099e..18b13518d8d8 100644
--- a/tools/perf/arch/arm64/Makefile
+++ b/tools/perf/arch/arm64/Makefile
@@ -1,3 +1,4 @@
 ifndef NO_DWARF
 PERF_HAVE_DWARF_REGS := 1
 endif
+PERF_HAVE_JITDUMP := 1
diff --git a/tools/perf/arch/powerpc/Makefile b/tools/perf/arch/powerpc/Makefile
index 9f9cea3478fd..56e05f126ad8 100644
--- a/tools/perf/arch/powerpc/Makefile
+++ b/tools/perf/arch/powerpc/Makefile
@@ -3,3 +3,4 @@ PERF_HAVE_DWARF_REGS := 1
 endif
 
 HAVE_KVM_STAT_SUPPORT := 1
+PERF_HAVE_JITDUMP := 1
diff --git a/tools/perf/arch/x86/Makefile b/tools/perf/arch/x86/Makefile
index 09ba923debe8..269af2143735 100644
--- a/tools/perf/arch/x86/Makefile
+++ b/tools/perf/arch/x86/Makefile
@@ -3,3 +3,4 @@ PERF_HAVE_DWARF_REGS := 1
 endif
 HAVE_KVM_STAT_SUPPORT := 1
 PERF_HAVE_ARCH_REGS_QUERY_REGISTER_OFFSET := 1
+PERF_HAVE_JITDUMP := 1
diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index e219ed458d97..7fa68663ed72 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -73,7 +73,7 @@ static int perf_event__repipe_oe_synth(struct perf_tool *tool,
 	return perf_event__repipe_synth(tool, event);
 }
 
-#if defined(HAVE_LIBELF_SUPPORT) && defined(HAVE_DWARF_SUPPORT)
+#ifdef HAVE_JITDUMP
 static int perf_event__drop_oe(struct perf_tool *tool __maybe_unused,
 			       union perf_event *event __maybe_unused,
 			       struct ordered_events *oe __maybe_unused)
@@ -245,7 +245,7 @@ static int perf_event__repipe_mmap(struct perf_tool *tool,
 	return err;
 }
 
-#if defined(HAVE_LIBELF_SUPPORT) && defined(HAVE_DWARF_SUPPORT)
+#ifdef HAVE_JITDUMP
 static int perf_event__jit_repipe_mmap(struct perf_tool *tool,
 				       union perf_event *event,
 				       struct perf_sample *sample,
@@ -283,7 +283,7 @@ static int perf_event__repipe_mmap2(struct perf_tool *tool,
 	return err;
 }
 
-#if defined(HAVE_LIBELF_SUPPORT) && defined(HAVE_DWARF_SUPPORT)
+#ifdef HAVE_JITDUMP
 static int perf_event__jit_repipe_mmap2(struct perf_tool *tool,
 					union perf_event *event,
 					struct perf_sample *sample,
@@ -778,7 +778,9 @@ int cmd_inject(int argc, const char **argv, const char *prefix __maybe_unused)
 		OPT_BOOLEAN('s', "sched-stat", &inject.sched_stat,
 			    "Merge sched-stat and sched-switch for getting events "
 			    "where and how long tasks slept"),
+#ifdef HAVE_JITDUMP
 		OPT_BOOLEAN('j', "jit", &inject.jit_mode, "merge jitdump files into perf.data file"),
+#endif
 		OPT_INCR('v', "verbose", &verbose,
 			 "be more verbose (show build ids, etc)"),
 		OPT_STRING(0, "kallsyms", &symbol_conf.kallsyms_name, "file",
@@ -795,7 +797,7 @@ int cmd_inject(int argc, const char **argv, const char *prefix __maybe_unused)
 		"perf inject [<options>]",
 		NULL
 	};
-#if !defined(HAVE_LIBELF_SUPPORT) || !defined(HAVE_DWARF_SUPPORT)
+#ifndef HAVE_JITDUMP
 	set_option_nobuild(options, 'j', "jit", "NO_LIBELF=1", true);
 #endif
 	argc = parse_options(argc, argv, options, inject_usage, 0);
@@ -833,7 +835,7 @@ int cmd_inject(int argc, const char **argv, const char *prefix __maybe_unused)
 		inject.tool.ordered_events = true;
 		inject.tool.ordering_requires_timestamps = true;
 	}
-#if defined(HAVE_LIBELF_SUPPORT) && defined(HAVE_DWARF_SUPPORT)
+#ifdef HAVE_JITDUMP
 	if (inject.jit_mode) {
 		inject.tool.mmap2	   = perf_event__jit_repipe_mmap2;
 		inject.tool.mmap	   = perf_event__jit_repipe_mmap;
diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index f7aeaf303f5a..eca6a912e8c2 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -328,6 +328,13 @@ ifndef NO_LIBELF
   endif # NO_LIBBPF
 endif # NO_LIBELF
 
+ifdef PERF_HAVE_JITDUMP
+  ifndef NO_DWARF
+    $(call detected,CONFIG_JITDUMP)
+    CFLAGS += -DHAVE_JITDUMP
+  endif
+endif
+
 ifeq ($(ARCH),powerpc)
   ifndef NO_DWARF
     CFLAGS += -DHAVE_SKIP_CALLCHAIN_IDX
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index f130ce240158..eea25e2424e9 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -108,7 +108,7 @@ libperf-$(CONFIG_ZLIB) += zlib.o
 libperf-$(CONFIG_LZMA) += lzma.o
 libperf-y += demangle-java.o
 
-ifdef CONFIG_DWARF
+ifdef CONFIG_JITDUMP
 libperf-$(CONFIG_LIBELF) += jitdump.o
 libperf-$(CONFIG_LIBELF) += genelf.o
 libperf-$(CONFIG_LIBELF) += genelf_debug.o
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [tip:perf/core] perf jitdump: Build only on supported archs
  2016-03-10 16:41         ` [PATCHv2] " Jiri Olsa
@ 2016-03-11  8:47           ` tip-bot for Jiri Olsa
  0 siblings, 0 replies; 25+ messages in thread
From: tip-bot for Jiri Olsa @ 2016-03-11  8:47 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tglx, alexander.shishkin, acme, peterz, adrian.hunter, bp,
	hekuang, dbueso, mingo, hpa, linux-kernel, rostedt, andi, jolsa,
	jolsa, eranian, wangnan0, colin.king, mgorman, namhyung, dsahern

Commit-ID:  e12b202f8fb9b62a3997cad8e93401f85293390c
Gitweb:     http://git.kernel.org/tip/e12b202f8fb9b62a3997cad8e93401f85293390c
Author:     Jiri Olsa <jolsa@redhat.com>
AuthorDate: Thu, 10 Mar 2016 17:41:13 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 10 Mar 2016 16:33:19 -0300

perf jitdump: Build only on supported archs

Build jitdump only on architectures defined in util/genelf.h file, to avoid
breaking the build on such arches.

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Borislav Petkov <bp@suse.de>
Cc: Colin Ian King <colin.king@canonical.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Davidlohr Bueso <dbueso@suse.com>
Cc: He Kuang <hekuang@huawei.com>
Cc: Mel Gorman <mgorman@suse.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/20160310164113.GA11357@krava.redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/arch/arm/Makefile     |  1 +
 tools/perf/arch/arm64/Makefile   |  1 +
 tools/perf/arch/powerpc/Makefile |  1 +
 tools/perf/arch/x86/Makefile     |  1 +
 tools/perf/builtin-inject.c      | 12 +++++++-----
 tools/perf/config/Makefile       |  7 +++++++
 tools/perf/util/Build            |  2 +-
 7 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/tools/perf/arch/arm/Makefile b/tools/perf/arch/arm/Makefile
index 7fbca17..18b1351 100644
--- a/tools/perf/arch/arm/Makefile
+++ b/tools/perf/arch/arm/Makefile
@@ -1,3 +1,4 @@
 ifndef NO_DWARF
 PERF_HAVE_DWARF_REGS := 1
 endif
+PERF_HAVE_JITDUMP := 1
diff --git a/tools/perf/arch/arm64/Makefile b/tools/perf/arch/arm64/Makefile
index 7fbca17..18b1351 100644
--- a/tools/perf/arch/arm64/Makefile
+++ b/tools/perf/arch/arm64/Makefile
@@ -1,3 +1,4 @@
 ifndef NO_DWARF
 PERF_HAVE_DWARF_REGS := 1
 endif
+PERF_HAVE_JITDUMP := 1
diff --git a/tools/perf/arch/powerpc/Makefile b/tools/perf/arch/powerpc/Makefile
index 9f9cea3..56e05f1 100644
--- a/tools/perf/arch/powerpc/Makefile
+++ b/tools/perf/arch/powerpc/Makefile
@@ -3,3 +3,4 @@ PERF_HAVE_DWARF_REGS := 1
 endif
 
 HAVE_KVM_STAT_SUPPORT := 1
+PERF_HAVE_JITDUMP := 1
diff --git a/tools/perf/arch/x86/Makefile b/tools/perf/arch/x86/Makefile
index 09ba923..269af21 100644
--- a/tools/perf/arch/x86/Makefile
+++ b/tools/perf/arch/x86/Makefile
@@ -3,3 +3,4 @@ PERF_HAVE_DWARF_REGS := 1
 endif
 HAVE_KVM_STAT_SUPPORT := 1
 PERF_HAVE_ARCH_REGS_QUERY_REGISTER_OFFSET := 1
+PERF_HAVE_JITDUMP := 1
diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index e219ed4..7fa6866 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -73,7 +73,7 @@ static int perf_event__repipe_oe_synth(struct perf_tool *tool,
 	return perf_event__repipe_synth(tool, event);
 }
 
-#if defined(HAVE_LIBELF_SUPPORT) && defined(HAVE_DWARF_SUPPORT)
+#ifdef HAVE_JITDUMP
 static int perf_event__drop_oe(struct perf_tool *tool __maybe_unused,
 			       union perf_event *event __maybe_unused,
 			       struct ordered_events *oe __maybe_unused)
@@ -245,7 +245,7 @@ static int perf_event__repipe_mmap(struct perf_tool *tool,
 	return err;
 }
 
-#if defined(HAVE_LIBELF_SUPPORT) && defined(HAVE_DWARF_SUPPORT)
+#ifdef HAVE_JITDUMP
 static int perf_event__jit_repipe_mmap(struct perf_tool *tool,
 				       union perf_event *event,
 				       struct perf_sample *sample,
@@ -283,7 +283,7 @@ static int perf_event__repipe_mmap2(struct perf_tool *tool,
 	return err;
 }
 
-#if defined(HAVE_LIBELF_SUPPORT) && defined(HAVE_DWARF_SUPPORT)
+#ifdef HAVE_JITDUMP
 static int perf_event__jit_repipe_mmap2(struct perf_tool *tool,
 					union perf_event *event,
 					struct perf_sample *sample,
@@ -778,7 +778,9 @@ int cmd_inject(int argc, const char **argv, const char *prefix __maybe_unused)
 		OPT_BOOLEAN('s', "sched-stat", &inject.sched_stat,
 			    "Merge sched-stat and sched-switch for getting events "
 			    "where and how long tasks slept"),
+#ifdef HAVE_JITDUMP
 		OPT_BOOLEAN('j', "jit", &inject.jit_mode, "merge jitdump files into perf.data file"),
+#endif
 		OPT_INCR('v', "verbose", &verbose,
 			 "be more verbose (show build ids, etc)"),
 		OPT_STRING(0, "kallsyms", &symbol_conf.kallsyms_name, "file",
@@ -795,7 +797,7 @@ int cmd_inject(int argc, const char **argv, const char *prefix __maybe_unused)
 		"perf inject [<options>]",
 		NULL
 	};
-#if !defined(HAVE_LIBELF_SUPPORT) || !defined(HAVE_DWARF_SUPPORT)
+#ifndef HAVE_JITDUMP
 	set_option_nobuild(options, 'j', "jit", "NO_LIBELF=1", true);
 #endif
 	argc = parse_options(argc, argv, options, inject_usage, 0);
@@ -833,7 +835,7 @@ int cmd_inject(int argc, const char **argv, const char *prefix __maybe_unused)
 		inject.tool.ordered_events = true;
 		inject.tool.ordering_requires_timestamps = true;
 	}
-#if defined(HAVE_LIBELF_SUPPORT) && defined(HAVE_DWARF_SUPPORT)
+#ifdef HAVE_JITDUMP
 	if (inject.jit_mode) {
 		inject.tool.mmap2	   = perf_event__jit_repipe_mmap2;
 		inject.tool.mmap	   = perf_event__jit_repipe_mmap;
diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index f7aeaf3..eca6a91 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -328,6 +328,13 @@ ifndef NO_LIBELF
   endif # NO_LIBBPF
 endif # NO_LIBELF
 
+ifdef PERF_HAVE_JITDUMP
+  ifndef NO_DWARF
+    $(call detected,CONFIG_JITDUMP)
+    CFLAGS += -DHAVE_JITDUMP
+  endif
+endif
+
 ifeq ($(ARCH),powerpc)
   ifndef NO_DWARF
     CFLAGS += -DHAVE_SKIP_CALLCHAIN_IDX
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index f130ce2..eea25e2 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -108,7 +108,7 @@ libperf-$(CONFIG_ZLIB) += zlib.o
 libperf-$(CONFIG_LZMA) += lzma.o
 libperf-y += demangle-java.o
 
-ifdef CONFIG_DWARF
+ifdef CONFIG_JITDUMP
 libperf-$(CONFIG_LIBELF) += jitdump.o
 libperf-$(CONFIG_LIBELF) += genelf.o
 libperf-$(CONFIG_LIBELF) += genelf_debug.o

^ permalink raw reply related	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2016-03-11  8:48 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-07 19:44 [GIT PULL 00/15] perf/core improvements and fixes Arnaldo Carvalho de Melo
2016-03-07 19:44 ` [PATCH 01/15] perf tools: Explicitly declare inc_group_count as a void function Arnaldo Carvalho de Melo
2016-03-07 19:44 ` [PATCH 02/15] perf inject: Hit all DSOs for AUX data in JIT and other cases Arnaldo Carvalho de Melo
2016-03-07 19:44 ` [PATCH 03/15] perf session: Simplify tool stubs Arnaldo Carvalho de Melo
2016-03-08  7:45   ` Adrian Hunter
2016-03-08  9:17     ` Ingo Molnar
2016-03-07 19:44 ` [PATCH 04/15] perf jit: Let jit_process() return errors Arnaldo Carvalho de Melo
2016-03-07 19:44 ` [PATCH 05/15] perf jit: Move clockid validation Arnaldo Carvalho de Melo
2016-03-07 19:44 ` [PATCH 06/15] perf tools: Use 64-bit shifts with (TSC) time conversion Arnaldo Carvalho de Melo
2016-03-07 19:44 ` [PATCH 07/15] perf hists: Add level field to struct perf_hpp_fmt Arnaldo Carvalho de Melo
2016-03-07 19:44 ` [PATCH 08/15] perf stat: Document --detailed option Arnaldo Carvalho de Melo
2016-03-08  9:10   ` Ingo Molnar
2016-03-07 19:44 ` [PATCH 09/15] perf hists: Introduce perf_hpp__setup_hists_formats() Arnaldo Carvalho de Melo
2016-03-07 19:44 ` [PATCH 10/15] perf hists: Use own hpp_list for hierarchy mode Arnaldo Carvalho de Melo
2016-03-07 19:44 ` [PATCH 11/15] perf hists: Support multiple sort keys in a hierarchy level Arnaldo Carvalho de Melo
2016-03-07 19:44 ` [PATCH 12/15] perf hists: Fix indent for multiple hierarchy sort key Arnaldo Carvalho de Melo
2016-03-07 19:44 ` [PATCH 13/15] perf report: Use hierarchy hpp list on stdio Arnaldo Carvalho de Melo
2016-03-07 19:44 ` [PATCH 14/15] perf hists browser: Use hierarchy hpp list Arnaldo Carvalho de Melo
2016-03-07 19:44 ` [PATCH 15/15] perf report: Use hierarchy hpp list on gtk Arnaldo Carvalho de Melo
2016-03-10  9:32 ` [PATCH] perf tool: Build jitdump only on supported archs Jiri Olsa
2016-03-10 15:57   ` Arnaldo Carvalho de Melo
2016-03-10 16:04     ` Jiri Olsa
2016-03-10 16:13       ` Arnaldo Carvalho de Melo
2016-03-10 16:41         ` [PATCHv2] " Jiri Olsa
2016-03-11  8:47           ` [tip:perf/core] perf jitdump: Build " tip-bot for Jiri Olsa

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).