[PATCH V3 00/11] perf tools: fixes and tweaks

[PATCH V3 00/11] perf tools: fixes and tweaks

[Adrian Hunter]

Hi

Here are some fixes and tweaks (version 3) for perf tools.

Changes in V3:
	perf evsel: Always use perf_evsel__set_sample_bit()
		New patch
	perf evsel: Add missing overflow check
		New patch
	perf test: Update "sample parsing" test for PERF_SAMPLE_TRANSACTION
		New patch
	perf evsel: Add missing PERF_SAMPLE_TRANSACTION
		New patch

	Patches dropped because they have been applied:
		perf tools: Fix non-debug build
		perf inject: Do not repipe attributes to a perf.data file

Changes in V2:

	perf tools: Fix non-debug build
		New patch
	perf evsel: Add a debug print if perf_event_open fails
		Unchanged
	perf script: Make perf_script a local variable
		Split from "perf script: Set up output options for in-stream attributes"
	perf script: Set up output options for in-stream attributes
		Split out "perf script: Make perf_script a local variable"
	perf inject: Do not repipe attributes to a perf.data file
		Unchanged
	perf tools: Fix 32-bit cross build
		Pass only EXTRA_CFLAGS
	perf tools: Fix libunwind build and feature detection for 32-bit build
		Add Jiri's Ack
	perf evlist: Add a debug print if event buffer mmap fails
		Add errno
	perf tools: Allow non-matching sample types
		Suppress compatible sample types for trace tool
	perf sched: Make struct perf_sched sched a local variable
		New patch
	perf sched: Fix optimized build time
		New patch
	perf tools: Do not accept parse_tag_value() overflow
		New patch
	perf tools: Validate that mmap_pages is not too big
		New patch

	Patches dropped because they have been applied:
		perf evsel: Add missing 'mmap2' from debug print
		perf record: Improve write_output error message
		perf evsel: Add missing decrement in id sample parsing
		perf session: Add missing sample flush for piped events
		perf session: Add missing members to perf_event__attr_swap()
		perf evlist: Fix 32-bit build error
		perf tools: Fix test_on_exit for 32-bit build
		perf tools: Fix bench/numa.c for 32-bit build
		perf tools: fix perf_evlist__mmap comments
		perf tools: factor out duplicated evlist mmap code
		perf script: print addr by default for BTS


Adrian Hunter (11):
      perf evsel: Add a debug print if perf_event_open fails
      perf script: Set up output options for in-stream attributes
      perf tools: Fix 32-bit cross build
      perf tools: Fix libunwind build and feature detection for 32-bit build
      perf evlist: Add a debug print if event buffer mmap fails
      perf record: Add an option to force per-cpu mmaps
      perf evsel: Always use perf_evsel__set_sample_bit()
      perf evsel: Add missing overflow check
      perf test: Update "sample parsing" test for PERF_SAMPLE_TRANSACTION
      perf evsel: Add missing PERF_SAMPLE_TRANSACTION
      perf tools: Allow non-matching sample types

 tools/perf/Documentation/perf-record.txt  |  6 +++
 tools/perf/Makefile.perf                  |  2 +-
 tools/perf/builtin-record.c               |  2 +
 tools/perf/builtin-script.c               | 64 +++++++++++++++++++++++--------
 tools/perf/builtin-trace.c                |  1 +
 tools/perf/config/Makefile                |  8 ++--
 tools/perf/config/feature-checks/Makefile |  6 +--
 tools/perf/perf.h                         |  1 +
 tools/perf/tests/sample-parsing.c         |  7 +++-
 tools/perf/util/event.h                   | 16 ++++++++
 tools/perf/util/evlist.c                  | 31 ++++++++++++++-
 tools/perf/util/evlist.h                  |  1 +
 tools/perf/util/evsel.c                   | 21 +++++++---
 tools/perf/util/record.c                  |  5 ++-
 tools/perf/util/target.h                  |  1 +
 15 files changed, 141 insertions(+), 31 deletions(-)


Regards
Adrian

[PATCH V3 01/11] perf evsel: Add a debug print if perf_event_open fails

[Adrian Hunter]

There is a debug print (at verbose level 2) for each
call to perf_event_open.  Add another debug print if
the call fails, and print the error number.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/evsel.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index ec0cc1e..e09c7e6 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1052,6 +1052,8 @@ retry_open:
 								     group_fd, flags);
 			if (FD(evsel, cpu, thread) < 0) {
 				err = -errno;
+				pr_debug2("perf_event_open failed, error %d\n",
+					  err);
 				goto try_fallback;
 			}
 			set_rlimit = NO_CHANGE;
-- 
1.7.11.7

[tip:perf/core] perf evsel: Add a debug print if perf_event_open fails

[tip-bot for Adrian Hunter]

Commit-ID:  f852fd621ca19f557f2e3d05900366be7c7afb83
Gitweb:     http://git.kernel.org/tip/f852fd621ca19f557f2e3d05900366be7c7afb83
Author:     Adrian Hunter <adrian.hunter@intel.com>
AuthorDate: Fri, 1 Nov 2013 15:51:29 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 4 Nov 2013 12:18:17 -0300

perf evsel: Add a debug print if perf_event_open fails

There is a debug print (at verbose level 2) for each call to
perf_event_open.  Add another debug print if the call fails, and print
the error number.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1383313899-15987-2-git-send-email-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/evsel.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 3a334f0..f0e65de 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1051,6 +1051,8 @@ retry_open:
 								     group_fd, flags);
 			if (FD(evsel, cpu, thread) < 0) {
 				err = -errno;
+				pr_debug2("perf_event_open failed, error %d\n",
+					  err);
 				goto try_fallback;
 			}
 			set_rlimit = NO_CHANGE;

[PATCH V3 02/11] perf script: Set up output options for in-stream attributes

[Adrian Hunter]

Attributes (struct perf_event_attr) are recorded
separately in the perf.data file.  perf script uses
them to set up output options.  However attributes
can also be in the event stream, for example when
the input is a pipe (i.e. live mode).  This patch
makes perf script process in-stream attributes in
the same way as on-file attributes.

Here is an example:

Before this patch:

$ perf record uname | perf script
Linux
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.015 MB (null) (~655 samples) ]
:4220  4220 [-01] 2933367.838906: cycles:

:4220  4220 [-01] 2933367.838910: cycles:

:4220  4220 [-01] 2933367.838912: cycles:

:4220  4220 [-01] 2933367.838914: cycles:

:4220  4220 [-01] 2933367.838916: cycles:

:4220  4220 [-01] 2933367.838918: cycles:

uname  4220 [-01] 2933367.838938: cycles:

uname  4220 [-01] 2933367.839207: cycles:

After this patch:

$ perf record uname | perf script
Linux
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.015 MB (null) (~655 samples) ]
           :4582  4582 2933425.707724: cycles:  ffffffff81043ffa native_write_msr_safe ([kernel.kallsyms])
           :4582  4582 2933425.707728: cycles:  ffffffff81043ffa native_write_msr_safe ([kernel.kallsyms])
           :4582  4582 2933425.707730: cycles:  ffffffff81043ffa native_write_msr_safe ([kernel.kallsyms])
           :4582  4582 2933425.707732: cycles:  ffffffff81043ffa native_write_msr_safe ([kernel.kallsyms])
           :4582  4582 2933425.707734: cycles:  ffffffff81043ffa native_write_msr_safe ([kernel.kallsyms])
           :4582  4582 2933425.707736: cycles:  ffffffff81309a24 memcpy ([kernel.kallsyms])
           uname  4582 2933425.707760: cycles:  ffffffff8109c1c7 enqueue_task_fair ([kernel.kallsyms])
           uname  4582 2933425.707978: cycles:  ffffffff81308457 clear_page_c ([kernel.kallsyms])

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/builtin-script.c | 64 +++++++++++++++++++++++++++++++++------------
 1 file changed, 48 insertions(+), 16 deletions(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 0ae88c2..471fe0e 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -229,6 +229,24 @@ static int perf_evsel__check_attr(struct perf_evsel *evsel,
 	return 0;
 }
 
+static void set_print_ip_opts(struct perf_event_attr *attr)
+{
+	unsigned int type = attr->type;
+
+	output[type].print_ip_opts = 0;
+	if (PRINT_FIELD(IP))
+		output[type].print_ip_opts |= PRINT_IP_OPT_IP;
+
+	if (PRINT_FIELD(SYM))
+		output[type].print_ip_opts |= PRINT_IP_OPT_SYM;
+
+	if (PRINT_FIELD(DSO))
+		output[type].print_ip_opts |= PRINT_IP_OPT_DSO;
+
+	if (PRINT_FIELD(SYMOFFSET))
+		output[type].print_ip_opts |= PRINT_IP_OPT_SYMOFFSET;
+}
+
 /*
  * verify all user requested events exist and the samples
  * have the expected data
@@ -237,7 +255,6 @@ static int perf_session__check_output_opt(struct perf_session *session)
 {
 	int j;
 	struct perf_evsel *evsel;
-	struct perf_event_attr *attr;
 
 	for (j = 0; j < PERF_TYPE_MAX; ++j) {
 		evsel = perf_session__find_first_evtype(session, j);
@@ -260,20 +277,7 @@ static int perf_session__check_output_opt(struct perf_session *session)
 		if (evsel == NULL)
 			continue;
 
-		attr = &evsel->attr;
-
-		output[j].print_ip_opts = 0;
-		if (PRINT_FIELD(IP))
-			output[j].print_ip_opts |= PRINT_IP_OPT_IP;
-
-		if (PRINT_FIELD(SYM))
-			output[j].print_ip_opts |= PRINT_IP_OPT_SYM;
-
-		if (PRINT_FIELD(DSO))
-			output[j].print_ip_opts |= PRINT_IP_OPT_DSO;
-
-		if (PRINT_FIELD(SYMOFFSET))
-			output[j].print_ip_opts |= PRINT_IP_OPT_SYMOFFSET;
+		set_print_ip_opts(&evsel->attr);
 	}
 
 	return 0;
@@ -547,6 +551,34 @@ struct perf_script {
 	struct perf_session	*session;
 };
 
+static int process_attr(struct perf_tool *tool, union perf_event *event,
+			struct perf_evlist **pevlist)
+{
+	struct perf_script *scr = container_of(tool, struct perf_script, tool);
+	struct perf_evlist *evlist;
+	struct perf_evsel *evsel, *pos;
+	int err;
+
+	err = perf_event__process_attr(tool, event, pevlist);
+	if (err)
+		return err;
+
+	evlist = *pevlist;
+	evsel = perf_evlist__last(*pevlist);
+
+	if (evsel->attr.type >= PERF_TYPE_MAX)
+		return 0;
+
+	list_for_each_entry(pos, &evlist->entries, node) {
+		if (pos->attr.type == evsel->attr.type && pos != evsel)
+			return 0;
+	}
+
+	set_print_ip_opts(&evsel->attr);
+
+	return perf_evsel__check_attr(evsel, scr->session);
+}
+
 static void sig_handler(int sig __maybe_unused)
 {
 	session_done = 1;
@@ -1272,7 +1304,7 @@ int cmd_script(int argc, const char **argv, const char *prefix __maybe_unused)
 			.comm		 = perf_event__process_comm,
 			.exit		 = perf_event__process_exit,
 			.fork		 = perf_event__process_fork,
-			.attr		 = perf_event__process_attr,
+			.attr		 = process_attr,
 			.tracing_data	 = perf_event__process_tracing_data,
 			.build_id	 = perf_event__process_build_id,
 			.ordered_samples = true,
-- 
1.7.11.7

[tip:perf/core] perf script: Set up output options for in-stream attributes

[tip-bot for Adrian Hunter]

Commit-ID:  7ea95727af571d592c9d6aa7627690d44b114a2d
Gitweb:     http://git.kernel.org/tip/7ea95727af571d592c9d6aa7627690d44b114a2d
Author:     Adrian Hunter <adrian.hunter@intel.com>
AuthorDate: Fri, 1 Nov 2013 15:51:30 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 4 Nov 2013 12:19:26 -0300

perf script: Set up output options for in-stream attributes

Attributes (struct perf_event_attr) are recorded separately in the
perf.data file.  perf script uses them to set up output options.
However attributes can also be in the event stream, for example when the
input is a pipe (i.e. live mode).  This patch makes perf script process
in-stream attributes in the same way as on-file attributes.

Here is an example:

Before this patch:

$ perf record uname | perf script
Linux
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.015 MB (null) (~655 samples) ]
:4220  4220 [-01] 2933367.838906: cycles:

:4220  4220 [-01] 2933367.838910: cycles:

:4220  4220 [-01] 2933367.838912: cycles:

:4220  4220 [-01] 2933367.838914: cycles:

:4220  4220 [-01] 2933367.838916: cycles:

:4220  4220 [-01] 2933367.838918: cycles:

uname  4220 [-01] 2933367.838938: cycles:

uname  4220 [-01] 2933367.839207: cycles:

After this patch:

$ perf record uname | perf script
Linux
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.015 MB (null) (~655 samples) ]
           :4582  4582 2933425.707724: cycles:  ffffffff81043ffa native_write_msr_safe ([kernel.kallsyms])
           :4582  4582 2933425.707728: cycles:  ffffffff81043ffa native_write_msr_safe ([kernel.kallsyms])
           :4582  4582 2933425.707730: cycles:  ffffffff81043ffa native_write_msr_safe ([kernel.kallsyms])
           :4582  4582 2933425.707732: cycles:  ffffffff81043ffa native_write_msr_safe ([kernel.kallsyms])
           :4582  4582 2933425.707734: cycles:  ffffffff81043ffa native_write_msr_safe ([kernel.kallsyms])
           :4582  4582 2933425.707736: cycles:  ffffffff81309a24 memcpy ([kernel.kallsyms])
           uname  4582 2933425.707760: cycles:  ffffffff8109c1c7 enqueue_task_fair ([kernel.kallsyms])
           uname  4582 2933425.707978: cycles:  ffffffff81308457 clear_page_c ([kernel.kallsyms])

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1383313899-15987-3-git-send-email-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-script.c | 64 +++++++++++++++++++++++++++++++++------------
 1 file changed, 48 insertions(+), 16 deletions(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index b866cc8..baf1798 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -229,6 +229,24 @@ static int perf_evsel__check_attr(struct perf_evsel *evsel,
 	return 0;
 }
 
+static void set_print_ip_opts(struct perf_event_attr *attr)
+{
+	unsigned int type = attr->type;
+
+	output[type].print_ip_opts = 0;
+	if (PRINT_FIELD(IP))
+		output[type].print_ip_opts |= PRINT_IP_OPT_IP;
+
+	if (PRINT_FIELD(SYM))
+		output[type].print_ip_opts |= PRINT_IP_OPT_SYM;
+
+	if (PRINT_FIELD(DSO))
+		output[type].print_ip_opts |= PRINT_IP_OPT_DSO;
+
+	if (PRINT_FIELD(SYMOFFSET))
+		output[type].print_ip_opts |= PRINT_IP_OPT_SYMOFFSET;
+}
+
 /*
  * verify all user requested events exist and the samples
  * have the expected data
@@ -237,7 +255,6 @@ static int perf_session__check_output_opt(struct perf_session *session)
 {
 	int j;
 	struct perf_evsel *evsel;
-	struct perf_event_attr *attr;
 
 	for (j = 0; j < PERF_TYPE_MAX; ++j) {
 		evsel = perf_session__find_first_evtype(session, j);
@@ -260,20 +277,7 @@ static int perf_session__check_output_opt(struct perf_session *session)
 		if (evsel == NULL)
 			continue;
 
-		attr = &evsel->attr;
-
-		output[j].print_ip_opts = 0;
-		if (PRINT_FIELD(IP))
-			output[j].print_ip_opts |= PRINT_IP_OPT_IP;
-
-		if (PRINT_FIELD(SYM))
-			output[j].print_ip_opts |= PRINT_IP_OPT_SYM;
-
-		if (PRINT_FIELD(DSO))
-			output[j].print_ip_opts |= PRINT_IP_OPT_DSO;
-
-		if (PRINT_FIELD(SYMOFFSET))
-			output[j].print_ip_opts |= PRINT_IP_OPT_SYMOFFSET;
+		set_print_ip_opts(&evsel->attr);
 	}
 
 	return 0;
@@ -547,6 +551,34 @@ struct perf_script {
 	struct perf_session	*session;
 };
 
+static int process_attr(struct perf_tool *tool, union perf_event *event,
+			struct perf_evlist **pevlist)
+{
+	struct perf_script *scr = container_of(tool, struct perf_script, tool);
+	struct perf_evlist *evlist;
+	struct perf_evsel *evsel, *pos;
+	int err;
+
+	err = perf_event__process_attr(tool, event, pevlist);
+	if (err)
+		return err;
+
+	evlist = *pevlist;
+	evsel = perf_evlist__last(*pevlist);
+
+	if (evsel->attr.type >= PERF_TYPE_MAX)
+		return 0;
+
+	list_for_each_entry(pos, &evlist->entries, node) {
+		if (pos->attr.type == evsel->attr.type && pos != evsel)
+			return 0;
+	}
+
+	set_print_ip_opts(&evsel->attr);
+
+	return perf_evsel__check_attr(evsel, scr->session);
+}
+
 static void sig_handler(int sig __maybe_unused)
 {
 	session_done = 1;
@@ -1272,7 +1304,7 @@ int cmd_script(int argc, const char **argv, const char *prefix __maybe_unused)
 			.comm		 = perf_event__process_comm,
 			.exit		 = perf_event__process_exit,
 			.fork		 = perf_event__process_fork,
-			.attr		 = perf_event__process_attr,
+			.attr		 = process_attr,
 			.tracing_data	 = perf_event__process_tracing_data,
 			.build_id	 = perf_event__process_build_id,
 			.ordered_samples = true,

[PATCH V3 03/11] perf tools: Fix 32-bit cross build

[Adrian Hunter]

Setting EXTRA_CFLAGS=-m32 did not work because it
was not passed around.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/Makefile.perf                  | 2 +-
 tools/perf/config/Makefile                | 4 ++--
 tools/perf/config/feature-checks/Makefile | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index bc7cfa1..f1cb389 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -709,7 +709,7 @@ $(LIB_FILE): $(LIB_OBJS)
 TE_SOURCES = $(wildcard $(TRACE_EVENT_DIR)*.[ch])
 
 $(LIBTRACEEVENT): $(TE_SOURCES)
-	$(QUIET_SUBDIR0)$(TRACE_EVENT_DIR) $(QUIET_SUBDIR1) O=$(OUTPUT) libtraceevent.a
+	$(QUIET_SUBDIR0)$(TRACE_EVENT_DIR) $(QUIET_SUBDIR1) O=$(OUTPUT) CFLAGS="-g -Wall $(EXTRA_CFLAGS)" libtraceevent.a
 
 $(LIBTRACEEVENT)-clean:
 	$(call QUIET_CLEAN, libtraceevent)
diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index 543aa95..2f1d7d7 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -96,7 +96,7 @@ endif
 
 feature_check = $(eval $(feature_check_code))
 define feature_check_code
-  feature-$(1) := $(shell $(MAKE) OUTPUT=$(OUTPUT_FEATURES) LDFLAGS=$(LDFLAGS) -C config/feature-checks test-$1 >/dev/null 2>/dev/null && echo 1 || echo 0)
+  feature-$(1) := $(shell $(MAKE) OUTPUT=$(OUTPUT_FEATURES) CFLAGS="$(EXTRA_CFLAGS)" LDFLAGS=$(LDFLAGS) -C config/feature-checks test-$1 >/dev/null 2>/dev/null && echo 1 || echo 0)
 endef
 
 feature_set = $(eval $(feature_set_code))
@@ -173,7 +173,7 @@ ifeq ($(feature-all), 1)
   #
   $(foreach feat,$(CORE_FEATURE_TESTS),$(call feature_set,$(feat)))
 else
-  $(shell $(MAKE) OUTPUT=$(OUTPUT_FEATURES) LDFLAGS=$(LDFLAGS) -i -j -C config/feature-checks $(CORE_FEATURE_TESTS) >/dev/null 2>&1)
+  $(shell $(MAKE) OUTPUT=$(OUTPUT_FEATURES) CFLAGS="$(EXTRA_CFLAGS)" LDFLAGS=$(LDFLAGS) -i -j -C config/feature-checks $(CORE_FEATURE_TESTS) >/dev/null 2>&1)
   $(foreach feat,$(CORE_FEATURE_TESTS),$(call feature_check,$(feat)))
 endif
 
diff --git a/tools/perf/config/feature-checks/Makefile b/tools/perf/config/feature-checks/Makefile
index 452b67c..353c00c 100644
--- a/tools/perf/config/feature-checks/Makefile
+++ b/tools/perf/config/feature-checks/Makefile
@@ -31,7 +31,7 @@ CC := $(CC) -MD
 
 all: $(FILES)
 
-BUILD = $(CC) $(LDFLAGS) -o $(OUTPUT)$@ $@.c
+BUILD = $(CC) $(CFLAGS) $(LDFLAGS) -o $(OUTPUT)$@ $@.c
 
 ###############################
 
-- 
1.7.11.7

[tip:perf/core] perf tools: Fix 32-bit cross build

[tip-bot for Adrian Hunter]

Commit-ID:  28e962b9d79f496f214d7fc8ffd1a3f2a67e9090
Gitweb:     http://git.kernel.org/tip/28e962b9d79f496f214d7fc8ffd1a3f2a67e9090
Author:     Adrian Hunter <adrian.hunter@intel.com>
AuthorDate: Fri, 1 Nov 2013 15:51:31 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 4 Nov 2013 12:20:45 -0300

perf tools: Fix 32-bit cross build

Setting EXTRA_CFLAGS=-m32 did not work because it was not passed around.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1383313899-15987-4-git-send-email-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Makefile.perf                  | 2 +-
 tools/perf/config/Makefile                | 4 ++--
 tools/perf/config/feature-checks/Makefile | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf
index cb52bdb..5b86390 100644
--- a/tools/perf/Makefile.perf
+++ b/tools/perf/Makefile.perf
@@ -711,7 +711,7 @@ $(LIB_FILE): $(LIB_OBJS)
 TE_SOURCES = $(wildcard $(TRACE_EVENT_DIR)*.[ch])
 
 $(LIBTRACEEVENT): $(TE_SOURCES)
-	$(QUIET_SUBDIR0)$(TRACE_EVENT_DIR) $(QUIET_SUBDIR1) O=$(OUTPUT) libtraceevent.a
+	$(QUIET_SUBDIR0)$(TRACE_EVENT_DIR) $(QUIET_SUBDIR1) O=$(OUTPUT) CFLAGS="-g -Wall $(EXTRA_CFLAGS)" libtraceevent.a
 
 $(LIBTRACEEVENT)-clean:
 	$(call QUIET_CLEAN, libtraceevent)
diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index 543aa95..2f1d7d7 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -96,7 +96,7 @@ endif
 
 feature_check = $(eval $(feature_check_code))
 define feature_check_code
-  feature-$(1) := $(shell $(MAKE) OUTPUT=$(OUTPUT_FEATURES) LDFLAGS=$(LDFLAGS) -C config/feature-checks test-$1 >/dev/null 2>/dev/null && echo 1 || echo 0)
+  feature-$(1) := $(shell $(MAKE) OUTPUT=$(OUTPUT_FEATURES) CFLAGS="$(EXTRA_CFLAGS)" LDFLAGS=$(LDFLAGS) -C config/feature-checks test-$1 >/dev/null 2>/dev/null && echo 1 || echo 0)
 endef
 
 feature_set = $(eval $(feature_set_code))
@@ -173,7 +173,7 @@ ifeq ($(feature-all), 1)
   #
   $(foreach feat,$(CORE_FEATURE_TESTS),$(call feature_set,$(feat)))
 else
-  $(shell $(MAKE) OUTPUT=$(OUTPUT_FEATURES) LDFLAGS=$(LDFLAGS) -i -j -C config/feature-checks $(CORE_FEATURE_TESTS) >/dev/null 2>&1)
+  $(shell $(MAKE) OUTPUT=$(OUTPUT_FEATURES) CFLAGS="$(EXTRA_CFLAGS)" LDFLAGS=$(LDFLAGS) -i -j -C config/feature-checks $(CORE_FEATURE_TESTS) >/dev/null 2>&1)
   $(foreach feat,$(CORE_FEATURE_TESTS),$(call feature_check,$(feat)))
 endif
 
diff --git a/tools/perf/config/feature-checks/Makefile b/tools/perf/config/feature-checks/Makefile
index 452b67c..353c00c 100644
--- a/tools/perf/config/feature-checks/Makefile
+++ b/tools/perf/config/feature-checks/Makefile
@@ -31,7 +31,7 @@ CC := $(CC) -MD
 
 all: $(FILES)
 
-BUILD = $(CC) $(LDFLAGS) -o $(OUTPUT)$@ $@.c
+BUILD = $(CC) $(CFLAGS) $(LDFLAGS) -o $(OUTPUT)$@ $@.c
 
 ###############################
 

[PATCH V3 04/11] perf tools: Fix libunwind build and feature detection for 32-bit build

[Adrian Hunter]

Use -lunwind-x86 instead of -lunwind-x86_64 for
32-bit build.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Acked-by: Jiri Olsa <jolsa@redhat.com>
---
 tools/perf/config/Makefile                | 6 ++++--
 tools/perf/config/feature-checks/Makefile | 4 ++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index 2f1d7d7..ffb5f55 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -25,9 +25,11 @@ ifeq ($(ARCH),x86_64)
     RAW_ARCH := x86_64
     CFLAGS += -DHAVE_ARCH_X86_64_SUPPORT
     ARCH_INCLUDE = ../../arch/x86/lib/memcpy_64.S ../../arch/x86/lib/memset_64.S
+    LIBUNWIND_LIBS = -lunwind -lunwind-x86_64
+  else
+    LIBUNWIND_LIBS = -lunwind -lunwind-x86
   endif
   NO_PERF_REGS := 0
-  LIBUNWIND_LIBS = -lunwind -lunwind-x86_64
 endif
 
 ifeq ($(NO_PERF_REGS),0)
@@ -96,7 +98,7 @@ endif
 
 feature_check = $(eval $(feature_check_code))
 define feature_check_code
-  feature-$(1) := $(shell $(MAKE) OUTPUT=$(OUTPUT_FEATURES) CFLAGS="$(EXTRA_CFLAGS)" LDFLAGS=$(LDFLAGS) -C config/feature-checks test-$1 >/dev/null 2>/dev/null && echo 1 || echo 0)
+  feature-$(1) := $(shell $(MAKE) OUTPUT=$(OUTPUT_FEATURES) CFLAGS="$(EXTRA_CFLAGS)" LDFLAGS=$(LDFLAGS) LIBUNWIND_LIBS="$(LIBUNWIND_LIBS)" -C config/feature-checks test-$1 >/dev/null 2>/dev/null && echo 1 || echo 0)
 endef
 
 feature_set = $(eval $(feature_set_code))
diff --git a/tools/perf/config/feature-checks/Makefile b/tools/perf/config/feature-checks/Makefile
index 353c00c..d37d58d 100644
--- a/tools/perf/config/feature-checks/Makefile
+++ b/tools/perf/config/feature-checks/Makefile
@@ -36,7 +36,7 @@ BUILD = $(CC) $(CFLAGS) $(LDFLAGS) -o $(OUTPUT)$@ $@.c
 ###############################
 
 test-all:
-	$(BUILD) -Werror -fstack-protector -fstack-protector-all -O2 -Werror -D_FORTIFY_SOURCE=2 -ldw -lelf -lnuma -lunwind -lunwind-x86_64 -lelf -laudit -I/usr/include/slang -lslang $(shell pkg-config --libs --cflags gtk+-2.0 2>/dev/null) $(FLAGS_PERL_EMBED) $(FLAGS_PYTHON_EMBED) -DPACKAGE='"perf"' -lbfd -ldl
+	$(BUILD) -Werror -fstack-protector -fstack-protector-all -O2 -Werror -D_FORTIFY_SOURCE=2 -ldw -lelf -lnuma $(LIBUNWIND_LIBS) -lelf -laudit -I/usr/include/slang -lslang $(shell pkg-config --libs --cflags gtk+-2.0 2>/dev/null) $(FLAGS_PERL_EMBED) $(FLAGS_PYTHON_EMBED) -DPACKAGE='"perf"' -lbfd -ldl
 
 test-hello:
 	$(BUILD)
@@ -72,7 +72,7 @@ test-libnuma:
 	$(BUILD) -lnuma
 
 test-libunwind:
-	$(BUILD) -lunwind -lunwind-x86_64 -lelf
+	$(BUILD) $(LIBUNWIND_LIBS) -lelf
 
 test-libaudit:
 	$(BUILD) -laudit
-- 
1.7.11.7

[tip:perf/core] perf tools: Fix libunwind build and feature detection for 32-bit build

[tip-bot for Adrian Hunter]

Commit-ID:  8a0c4c2843d3b72e23c3c12079b8d5c3ae99f3e3
Gitweb:     http://git.kernel.org/tip/8a0c4c2843d3b72e23c3c12079b8d5c3ae99f3e3
Author:     Adrian Hunter <adrian.hunter@intel.com>
AuthorDate: Fri, 1 Nov 2013 15:51:32 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 4 Nov 2013 12:21:18 -0300

perf tools: Fix libunwind build and feature detection for 32-bit build

Use -lunwind-x86 instead of -lunwind-x86_64 for 32-bit build.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Acked-by: Jiri Olsa <jolsa@redhat.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1383313899-15987-5-git-send-email-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/config/Makefile                | 6 ++++--
 tools/perf/config/feature-checks/Makefile | 4 ++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index 2f1d7d7..ffb5f55 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -25,9 +25,11 @@ ifeq ($(ARCH),x86_64)
     RAW_ARCH := x86_64
     CFLAGS += -DHAVE_ARCH_X86_64_SUPPORT
     ARCH_INCLUDE = ../../arch/x86/lib/memcpy_64.S ../../arch/x86/lib/memset_64.S
+    LIBUNWIND_LIBS = -lunwind -lunwind-x86_64
+  else
+    LIBUNWIND_LIBS = -lunwind -lunwind-x86
   endif
   NO_PERF_REGS := 0
-  LIBUNWIND_LIBS = -lunwind -lunwind-x86_64
 endif
 
 ifeq ($(NO_PERF_REGS),0)
@@ -96,7 +98,7 @@ endif
 
 feature_check = $(eval $(feature_check_code))
 define feature_check_code
-  feature-$(1) := $(shell $(MAKE) OUTPUT=$(OUTPUT_FEATURES) CFLAGS="$(EXTRA_CFLAGS)" LDFLAGS=$(LDFLAGS) -C config/feature-checks test-$1 >/dev/null 2>/dev/null && echo 1 || echo 0)
+  feature-$(1) := $(shell $(MAKE) OUTPUT=$(OUTPUT_FEATURES) CFLAGS="$(EXTRA_CFLAGS)" LDFLAGS=$(LDFLAGS) LIBUNWIND_LIBS="$(LIBUNWIND_LIBS)" -C config/feature-checks test-$1 >/dev/null 2>/dev/null && echo 1 || echo 0)
 endef
 
 feature_set = $(eval $(feature_set_code))
diff --git a/tools/perf/config/feature-checks/Makefile b/tools/perf/config/feature-checks/Makefile
index 353c00c..d37d58d 100644
--- a/tools/perf/config/feature-checks/Makefile
+++ b/tools/perf/config/feature-checks/Makefile
@@ -36,7 +36,7 @@ BUILD = $(CC) $(CFLAGS) $(LDFLAGS) -o $(OUTPUT)$@ $@.c
 ###############################
 
 test-all:
-	$(BUILD) -Werror -fstack-protector -fstack-protector-all -O2 -Werror -D_FORTIFY_SOURCE=2 -ldw -lelf -lnuma -lunwind -lunwind-x86_64 -lelf -laudit -I/usr/include/slang -lslang $(shell pkg-config --libs --cflags gtk+-2.0 2>/dev/null) $(FLAGS_PERL_EMBED) $(FLAGS_PYTHON_EMBED) -DPACKAGE='"perf"' -lbfd -ldl
+	$(BUILD) -Werror -fstack-protector -fstack-protector-all -O2 -Werror -D_FORTIFY_SOURCE=2 -ldw -lelf -lnuma $(LIBUNWIND_LIBS) -lelf -laudit -I/usr/include/slang -lslang $(shell pkg-config --libs --cflags gtk+-2.0 2>/dev/null) $(FLAGS_PERL_EMBED) $(FLAGS_PYTHON_EMBED) -DPACKAGE='"perf"' -lbfd -ldl
 
 test-hello:
 	$(BUILD)
@@ -72,7 +72,7 @@ test-libnuma:
 	$(BUILD) -lnuma
 
 test-libunwind:
-	$(BUILD) -lunwind -lunwind-x86_64 -lelf
+	$(BUILD) $(LIBUNWIND_LIBS) -lelf
 
 test-libaudit:
 	$(BUILD) -laudit

[PATCH V3 05/11] perf evlist: Add a debug print if event buffer mmap fails

[Adrian Hunter]

Add a debug print if mmap of the perf event
ring buffer fails.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/evlist.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 0582f67..1c173cc 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -607,6 +607,8 @@ static int __perf_evlist__mmap(struct perf_evlist *evlist,
 	evlist->mmap[idx].base = mmap(NULL, evlist->mmap_len, prot,
 				      MAP_SHARED, fd, 0);
 	if (evlist->mmap[idx].base == MAP_FAILED) {
+		pr_debug2("failed to mmap perf event ring buffer, error %d\n",
+			  errno);
 		evlist->mmap[idx].base = NULL;
 		return -1;
 	}
-- 
1.7.11.7

[tip:perf/core] perf evlist: Add a debug print if event buffer mmap fails

[tip-bot for Adrian Hunter]

Commit-ID:  026359658aecd348bc5c4a136a26f204b169103b
Gitweb:     http://git.kernel.org/tip/026359658aecd348bc5c4a136a26f204b169103b
Author:     Adrian Hunter <adrian.hunter@intel.com>
AuthorDate: Fri, 1 Nov 2013 15:51:33 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 4 Nov 2013 12:21:41 -0300

perf evlist: Add a debug print if event buffer mmap fails

Add a debug print if mmap of the perf event ring buffer fails.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1383313899-15987-6-git-send-email-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/evlist.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 0582f67..1c173cc 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -607,6 +607,8 @@ static int __perf_evlist__mmap(struct perf_evlist *evlist,
 	evlist->mmap[idx].base = mmap(NULL, evlist->mmap_len, prot,
 				      MAP_SHARED, fd, 0);
 	if (evlist->mmap[idx].base == MAP_FAILED) {
+		pr_debug2("failed to mmap perf event ring buffer, error %d\n",
+			  errno);
 		evlist->mmap[idx].base = NULL;
 		return -1;
 	}

[PATCH V3 06/11] perf record: Add an option to force per-cpu mmaps

[Adrian Hunter]

By default, when tasks are specified (i.e. -p, -t
or -u options) per-thread mmaps are created.  Add
an option to override that and force per-cpu mmaps.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/Documentation/perf-record.txt | 6 ++++++
 tools/perf/builtin-record.c              | 2 ++
 tools/perf/util/evlist.c                 | 4 +++-
 tools/perf/util/evsel.c                  | 4 ++--
 tools/perf/util/target.h                 | 1 +
 5 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index f10ab63..2ea6685 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -189,6 +189,12 @@ abort events and some memory events in precise mode on modern Intel CPUs.
 --transaction::
 Record transaction flags for transaction related events.
 
+--force-per-cpu::
+Force the use of per-cpu mmaps.  By default, when tasks are specified (i.e. -p,
+-t or -u options) per-thread mmaps are created.  This option overrides that and
+forces per-cpu mmaps.  A side-effect of that is that inheritance is
+automatically enabled.  Add the -i option also to disable inheritance.
+
 SEE ALSO
 --------
 linkperf:perf-stat[1], linkperf:perf-list[1]
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index ab8d15e..4c0657f 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -860,6 +860,8 @@ const struct option record_options[] = {
 		    "sample by weight (on special events only)"),
 	OPT_BOOLEAN(0, "transaction", &record.opts.sample_transaction,
 		    "sample transaction flags (special events only)"),
+	OPT_BOOLEAN(0, "force-per-cpu", &record.opts.target.force_per_cpu,
+		    "force the use of per-cpu mmaps"),
 	OPT_END()
 };
 
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 1c173cc..b3183d5 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -805,7 +805,9 @@ int perf_evlist__create_maps(struct perf_evlist *evlist,
 	if (evlist->threads == NULL)
 		return -1;
 
-	if (perf_target__has_task(target))
+	if (target->force_per_cpu)
+		evlist->cpus = cpu_map__new(target->cpu_list);
+	else if (perf_target__has_task(target))
 		evlist->cpus = cpu_map__dummy_new();
 	else if (!perf_target__has_cpu(target) && !target->uses_mmap)
 		evlist->cpus = cpu_map__dummy_new();
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index e09c7e6..f1839b0 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -645,7 +645,7 @@ void perf_evsel__config(struct perf_evsel *evsel,
 		}
 	}
 
-	if (perf_target__has_cpu(&opts->target))
+	if (perf_target__has_cpu(&opts->target) || opts->target.force_per_cpu)
 		perf_evsel__set_sample_bit(evsel, CPU);
 
 	if (opts->period)
@@ -653,7 +653,7 @@ void perf_evsel__config(struct perf_evsel *evsel,
 
 	if (!perf_missing_features.sample_id_all &&
 	    (opts->sample_time || !opts->no_inherit ||
-	     perf_target__has_cpu(&opts->target)))
+	     perf_target__has_cpu(&opts->target) || opts->target.force_per_cpu))
 		perf_evsel__set_sample_bit(evsel, TIME);
 
 	if (opts->raw_samples) {
diff --git a/tools/perf/util/target.h b/tools/perf/util/target.h
index a4be857..6d7efbd 100644
--- a/tools/perf/util/target.h
+++ b/tools/perf/util/target.h
@@ -12,6 +12,7 @@ struct perf_target {
 	uid_t	     uid;
 	bool	     system_wide;
 	bool	     uses_mmap;
+	bool	     force_per_cpu;
 };
 
 enum perf_target_errno {
-- 
1.7.11.7

Re: [PATCH V3 06/11] perf record: Add an option to force per-cpu mmaps

[Adrian Hunter]

On 04/11/13 17:29, Jiri Olsa wrote:
> On Fri, Nov 01, 2013 at 03:51:34PM +0200, Adrian Hunter wrote:
>> By default, when tasks are specified (i.e. -p, -t
>> or -u options) per-thread mmaps are created.  Add
>> an option to override that and force per-cpu mmaps.
>>
>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>> ---
>>  tools/perf/Documentation/perf-record.txt | 6 ++++++
>>  tools/perf/builtin-record.c              | 2 ++
>>  tools/perf/util/evlist.c                 | 4 +++-
>>  tools/perf/util/evsel.c                  | 4 ++--
>>  tools/perf/util/target.h                 | 1 +
>>  5 files changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
>> index f10ab63..2ea6685 100644
>> --- a/tools/perf/Documentation/perf-record.txt
>> +++ b/tools/perf/Documentation/perf-record.txt
>> @@ -189,6 +189,12 @@ abort events and some memory events in precise mode on modern Intel CPUs.
>>  --transaction::
>>  Record transaction flags for transaction related events.
>>  
>> +--force-per-cpu::
>> +Force the use of per-cpu mmaps.  By default, when tasks are specified (i.e. -p,
>> +-t or -u options) per-thread mmaps are created.  This option overrides that and
>> +forces per-cpu mmaps.  A side-effect of that is that inheritance is
>> +automatically enabled.  Add the -i option also to disable inheritance.
> 
> I recently sent out patch that actually force perf cpu mmaps for -p,-t,-u
> http://marc.info/?l=linux-kernel&m=138332119912433&w=2
> 
> Is there a reason why would you want to keep single
> mmap (in record command) and cut yourself from inherited
> events?

Not sure I understand the question.

perf supports having a single context for a thread.  That is a
feature.  You seem to be removing perf tools support for it.

I image a case where the user has hundreds of CPUs but just
wants to record one thread.  Currently -t does that. i.e.
one file descriptor and one mmap saving megabytes of memory.

Another advantage of per-thread mmaps is that you do not
need to sample time (nor cpu), because the events are recorded
in order.

I was adding a feature.  Users can choose per-cpu mmaps if
they want.

Re: [PATCH V3 06/11] perf record: Add an option to force per-cpu mmaps

[Jiri Olsa]

On Tue, Nov 05, 2013 at 10:28:38AM +0200, Adrian Hunter wrote:
> On 04/11/13 17:29, Jiri Olsa wrote:
> > On Fri, Nov 01, 2013 at 03:51:34PM +0200, Adrian Hunter wrote:
> >> By default, when tasks are specified (i.e. -p, -t
> >> or -u options) per-thread mmaps are created.  Add
> >> an option to override that and force per-cpu mmaps.
> >>
> >> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> >> ---
> >>  tools/perf/Documentation/perf-record.txt | 6 ++++++
> >>  tools/perf/builtin-record.c              | 2 ++
> >>  tools/perf/util/evlist.c                 | 4 +++-
> >>  tools/perf/util/evsel.c                  | 4 ++--
> >>  tools/perf/util/target.h                 | 1 +
> >>  5 files changed, 14 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
> >> index f10ab63..2ea6685 100644
> >> --- a/tools/perf/Documentation/perf-record.txt
> >> +++ b/tools/perf/Documentation/perf-record.txt
> >> @@ -189,6 +189,12 @@ abort events and some memory events in precise mode on modern Intel CPUs.
> >>  --transaction::
> >>  Record transaction flags for transaction related events.
> >>  
> >> +--force-per-cpu::
> >> +Force the use of per-cpu mmaps.  By default, when tasks are specified (i.e. -p,
> >> +-t or -u options) per-thread mmaps are created.  This option overrides that and
> >> +forces per-cpu mmaps.  A side-effect of that is that inheritance is
> >> +automatically enabled.  Add the -i option also to disable inheritance.
> > 
> > I recently sent out patch that actually force perf cpu mmaps for -p,-t,-u
> > http://marc.info/?l=linux-kernel&m=138332119912433&w=2
> > 
> > Is there a reason why would you want to keep single
> > mmap (in record command) and cut yourself from inherited
> > events?
> 
> Not sure I understand the question.
> 
> perf supports having a single context for a thread.  That is a
> feature.  You seem to be removing perf tools support for it.
> 
> I image a case where the user has hundreds of CPUs but just
> wants to record one thread.  Currently -t does that. i.e.
> one file descriptor and one mmap saving megabytes of memory.
> 
> Another advantage of per-thread mmaps is that you do not
> need to sample time (nor cpu), because the events are recorded
> in order.
> 
> I was adding a feature.  Users can choose per-cpu mmaps if
> they want.

right, I haven't considered the hundreds CPU machine.. I just
saw that it disables inherited events in my test ;-) maybe we
could mentioned that somewhere, because it's not clear

jirka

Re: [PATCH V3 06/11] perf record: Add an option to force per-cpu mmaps

[Adrian Hunter]

On 05/11/13 11:42, Jiri Olsa wrote:
> On Tue, Nov 05, 2013 at 10:28:38AM +0200, Adrian Hunter wrote:
>> On 04/11/13 17:29, Jiri Olsa wrote:
>>> On Fri, Nov 01, 2013 at 03:51:34PM +0200, Adrian Hunter wrote:
>>>> By default, when tasks are specified (i.e. -p, -t
>>>> or -u options) per-thread mmaps are created.  Add
>>>> an option to override that and force per-cpu mmaps.
>>>>
>>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>>>> ---
>>>>  tools/perf/Documentation/perf-record.txt | 6 ++++++
>>>>  tools/perf/builtin-record.c              | 2 ++
>>>>  tools/perf/util/evlist.c                 | 4 +++-
>>>>  tools/perf/util/evsel.c                  | 4 ++--
>>>>  tools/perf/util/target.h                 | 1 +
>>>>  5 files changed, 14 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
>>>> index f10ab63..2ea6685 100644
>>>> --- a/tools/perf/Documentation/perf-record.txt
>>>> +++ b/tools/perf/Documentation/perf-record.txt
>>>> @@ -189,6 +189,12 @@ abort events and some memory events in precise mode on modern Intel CPUs.
>>>>  --transaction::
>>>>  Record transaction flags for transaction related events.
>>>>  
>>>> +--force-per-cpu::
>>>> +Force the use of per-cpu mmaps.  By default, when tasks are specified (i.e. -p,
>>>> +-t or -u options) per-thread mmaps are created.  This option overrides that and
>>>> +forces per-cpu mmaps.  A side-effect of that is that inheritance is
>>>> +automatically enabled.  Add the -i option also to disable inheritance.
>>>
>>> I recently sent out patch that actually force perf cpu mmaps for -p,-t,-u
>>> http://marc.info/?l=linux-kernel&m=138332119912433&w=2
>>>
>>> Is there a reason why would you want to keep single
>>> mmap (in record command) and cut yourself from inherited
>>> events?
>>
>> Not sure I understand the question.
>>
>> perf supports having a single context for a thread.  That is a
>> feature.  You seem to be removing perf tools support for it.
>>
>> I image a case where the user has hundreds of CPUs but just
>> wants to record one thread.  Currently -t does that. i.e.
>> one file descriptor and one mmap saving megabytes of memory.
>>
>> Another advantage of per-thread mmaps is that you do not
>> need to sample time (nor cpu), because the events are recorded
>> in order.
>>
>> I was adding a feature.  Users can choose per-cpu mmaps if
>> they want.
> 
> right, I haven't considered the hundreds CPU machine.. I just
> saw that it disables inherited events in my test ;-) maybe we
> could mentioned that somewhere, because it's not clear

How about this:

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

By default, when tasks are specified (i.e. -p, -t
or -u options) per-thread mmaps are created.  Add
an option to override that and force per-cpu mmaps.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/Documentation/perf-record.txt | 15 +++++++++++++++
 tools/perf/builtin-record.c              |  2 ++
 tools/perf/util/evlist.c                 |  4 +++-
 tools/perf/util/evsel.c                  |  4 ++--
 tools/perf/util/target.h                 |  1 +
 5 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 052f7c4..5d5cbed 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -53,14 +53,20 @@ OPTIONS
 -p::
 --pid=::
 	Record events on existing process ID (comma separated list).
+	Note this causes the use of per-thread mmaps which prevents
+	inheritance. This can be overridden by --force-per-cpu.
 
 -t::
 --tid=::
         Record events on existing thread ID (comma separated list).
+	Note this causes the use of per-thread mmaps which prevents
+	inheritance. This can be overridden by --force-per-cpu.
 
 -u::
 --uid=::
         Record events in threads owned by uid. Name or number.
+	Note this causes the use of per-thread mmaps which prevents
+	inheritance. This can be overridden by --force-per-cpu.
 
 -r::
 --realtime=::
@@ -81,6 +87,9 @@ OPTIONS
 -i::
 --no-inherit::
 	Child tasks do not inherit counters.
+	Note that inheritance is not possible when events are recorded
+	with per-thread mmaps e.g. -p, -t or -u options.
+
 -F::
 --freq=::
 	Profile at this frequency.
@@ -201,6 +210,12 @@ abort events and some memory events in precise mode on modern Intel CPUs.
 --transaction::
 Record transaction flags for transaction related events.
 
+--force-per-cpu::
+Force the use of per-cpu mmaps.  By default, when tasks are specified (i.e. -p,
+-t or -u options) per-thread mmaps are created.  This option overrides that and
+forces per-cpu mmaps.  A side-effect of that is that inheritance is
+automatically enabled.  Add the -i option also to disable inheritance.
+
 SEE ALSO
 --------
 linkperf:perf-stat[1], linkperf:perf-list[1]
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 8b45fce..6b6f5d9 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -889,6 +889,8 @@ const struct option record_options[] = {
 		    "sample by weight (on special events only)"),
 	OPT_BOOLEAN(0, "transaction", &record.opts.sample_transaction,
 		    "sample transaction flags (special events only)"),
+	OPT_BOOLEAN(0, "force-per-cpu", &record.opts.target.force_per_cpu,
+		    "force the use of per-cpu mmaps"),
 	OPT_END()
 };
 
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 1c173cc..b3183d5 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -805,7 +805,9 @@ int perf_evlist__create_maps(struct perf_evlist *evlist,
 	if (evlist->threads == NULL)
 		return -1;
 
-	if (perf_target__has_task(target))
+	if (target->force_per_cpu)
+		evlist->cpus = cpu_map__new(target->cpu_list);
+	else if (perf_target__has_task(target))
 		evlist->cpus = cpu_map__dummy_new();
 	else if (!perf_target__has_cpu(target) && !target->uses_mmap)
 		evlist->cpus = cpu_map__dummy_new();
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 5280820..080cc55 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -645,7 +645,7 @@ void perf_evsel__config(struct perf_evsel *evsel,
 		}
 	}
 
-	if (perf_target__has_cpu(&opts->target))
+	if (perf_target__has_cpu(&opts->target) || opts->target.force_per_cpu)
 		perf_evsel__set_sample_bit(evsel, CPU);
 
 	if (opts->period)
@@ -653,7 +653,7 @@ void perf_evsel__config(struct perf_evsel *evsel,
 
 	if (!perf_missing_features.sample_id_all &&
 	    (opts->sample_time || !opts->no_inherit ||
-	     perf_target__has_cpu(&opts->target)))
+	     perf_target__has_cpu(&opts->target) || opts->target.force_per_cpu))
 		perf_evsel__set_sample_bit(evsel, TIME);
 
 	if (opts->raw_samples) {
diff --git a/tools/perf/util/target.h b/tools/perf/util/target.h
index a4be857..6d7efbd 100644
--- a/tools/perf/util/target.h
+++ b/tools/perf/util/target.h
@@ -12,6 +12,7 @@ struct perf_target {
 	uid_t	     uid;
 	bool	     system_wide;
 	bool	     uses_mmap;
+	bool	     force_per_cpu;
 };
 
 enum perf_target_errno {
-- 
1.7.11.7

Re: [PATCH V3 06/11] perf record: Add an option to force per-cpu mmaps

[Jiri Olsa]

On Tue, Nov 05, 2013 at 03:09:30PM +0200, Adrian Hunter wrote:

SNIP

> >> Another advantage of per-thread mmaps is that you do not
> >> need to sample time (nor cpu), because the events are recorded
> >> in order.
> >>
> >> I was adding a feature.  Users can choose per-cpu mmaps if
> >> they want.
> > 
> > right, I haven't considered the hundreds CPU machine.. I just
> > saw that it disables inherited events in my test ;-) maybe we
> > could mentioned that somewhere, because it's not clear
> 
> How about this:
> 
> From: Adrian Hunter <adrian.hunter@intel.com>
> 
> By default, when tasks are specified (i.e. -p, -t
> or -u options) per-thread mmaps are created.  Add
> an option to override that and force per-cpu mmaps.
> 
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>

seems ok, thanks

Acked-by: Jiri Olsa <jolsa@redhat.com>

jirka

Re: [PATCH V3 06/11] perf record: Add an option to force per-cpu mmaps

[Arnaldo Carvalho de Melo]

Em Tue, Nov 05, 2013 at 02:30:24PM +0100, Jiri Olsa escreveu:
> On Tue, Nov 05, 2013 at 03:09:30PM +0200, Adrian Hunter wrote:
> > > right, I haven't considered the hundreds CPU machine.. I just
> > > saw that it disables inherited events in my test ;-) maybe we
> > > could mentioned that somewhere, because it's not clear
> > 
> > How about this:
> > 
> > From: Adrian Hunter <adrian.hunter@intel.com>
> > 
> > By default, when tasks are specified (i.e. -p, -t
> > or -u options) per-thread mmaps are created.  Add
> > an option to override that and force per-cpu mmaps.
> > 
> > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> 
> seems ok, thanks

Much better, but may I ask that the explanation in the man pages be
present in the changeset commit as well? So that by just looking at 'git
log' we can more readily get the whole picture, i.e. scalability versus
wanting to have inherit.

- Arnaldo
 
> Acked-by: Jiri Olsa <jolsa@redhat.com>
> 
> jirka

Re: [PATCH V3 06/11] perf record: Add an option to force per-cpu mmaps

[Arnaldo Carvalho de Melo]

PeterZ,

	Can I have your Acked-by for this one? I guess now the goal is
achieved, no?

- Arnaldo

Em Tue, Nov 05, 2013 at 11:25:45AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Nov 05, 2013 at 02:30:24PM +0100, Jiri Olsa escreveu:
> > On Tue, Nov 05, 2013 at 03:09:30PM +0200, Adrian Hunter wrote:
> > > > right, I haven't considered the hundreds CPU machine.. I just
> > > > saw that it disables inherited events in my test ;-) maybe we
> > > > could mentioned that somewhere, because it's not clear
> > > 
> > > How about this:
> > > 
> > > From: Adrian Hunter <adrian.hunter@intel.com>
> > > 
> > > By default, when tasks are specified (i.e. -p, -t
> > > or -u options) per-thread mmaps are created.  Add
> > > an option to override that and force per-cpu mmaps.
> > > 
> > > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> > 
> > seems ok, thanks
> 
> Much better, but may I ask that the explanation in the man pages be
> present in the changeset commit as well? So that by just looking at 'git
> log' we can more readily get the whole picture, i.e. scalability versus
> wanting to have inherit.
> 
> - Arnaldo
>  
> > Acked-by: Jiri Olsa <jolsa@redhat.com>
> > 
> > jirka

Re: [PATCH V3 06/11] perf record: Add an option to force per-cpu mmaps

[Adrian Hunter]

On 05/11/13 19:31, Arnaldo Carvalho de Melo wrote:
> PeterZ,
>
> 	Can I have your Acked-by for this one? I guess now the goal is
> achieved, no?

Ping

>
> - Arnaldo
>
> Em Tue, Nov 05, 2013 at 11:25:45AM -0300, Arnaldo Carvalho de Melo escreveu:
>> Em Tue, Nov 05, 2013 at 02:30:24PM +0100, Jiri Olsa escreveu:
>>> On Tue, Nov 05, 2013 at 03:09:30PM +0200, Adrian Hunter wrote:
>>>>> right, I haven't considered the hundreds CPU machine.. I just
>>>>> saw that it disables inherited events in my test ;-) maybe we
>>>>> could mentioned that somewhere, because it's not clear
>>>> How about this:
>>>>
>>>> From: Adrian Hunter <adrian.hunter@intel.com>
>>>>
>>>> By default, when tasks are specified (i.e. -p, -t
>>>> or -u options) per-thread mmaps are created.  Add
>>>> an option to override that and force per-cpu mmaps.
>>>>
>>>> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
>>> seems ok, thanks
>> Much better, but may I ask that the explanation in the man pages be
>> present in the changeset commit as well? So that by just looking at 'git
>> log' we can more readily get the whole picture, i.e. scalability versus
>> wanting to have inherit.
>>
>> - Arnaldo
>>  
>>> Acked-by: Jiri Olsa <jolsa@redhat.com>
>>>
>>> jirka
>

Re: [PATCH V3 06/11] perf record: Add an option to force per-cpu mmaps

[Peter Zijlstra]

On Tue, Nov 05, 2013 at 02:31:52PM -0300, Arnaldo Carvalho de Melo wrote:
> PeterZ,
> 
> 	Can I have your Acked-by for this one? I guess now the goal is
> achieved, no?

So this option allows -t/-p/-u to create one buffer per cpu and attach
all the various thread/process/user tasks' their counters to that one
buffer?

As opposed to the current state where each such counter would have its
own buffer.

If this is what the patch does, then yes, although I would prefer a
slightly clearer Changelog.

Acked-by: Peter Zijlstra <peterz@infradead.org>

Re: [PATCH V3 06/11] perf record: Add an option to force per-cpu mmaps

[Arnaldo Carvalho de Melo]

Em Fri, Nov 08, 2013 at 09:40:01AM +0100, Peter Zijlstra escreveu:
> On Tue, Nov 05, 2013 at 02:31:52PM -0300, Arnaldo Carvalho de Melo wrote:
> > 	Can I have your Acked-by for this one? I guess now the goal is
> > achieved, no?
 
> So this option allows -t/-p/-u to create one buffer per cpu and attach
> all the various thread/process/user tasks' their counters to that one
> buffer?
 
> As opposed to the current state where each such counter would have its
> own buffer.
 
> If this is what the patch does, then yes, although I would prefer a
> slightly clearer Changelog.

/me too

I will re-read the patch to check that it matches what you think it does
and will update the changelog to make it clearer.
 
> Acked-by: Peter Zijlstra <peterz@infradead.org>

Thanks!

- Arnaldo

Re: [PATCH V3 06/11] perf record: Add an option to force per-cpu mmaps

[Ingo Molnar]

* Peter Zijlstra <peterz@infradead.org> wrote:

> On Tue, Nov 05, 2013 at 02:31:52PM -0300, Arnaldo Carvalho de Melo wrote:
> > PeterZ,
> > 
> > 	Can I have your Acked-by for this one? I guess now the goal is
> > achieved, no?
> 
> So this option allows -t/-p/-u to create one buffer per cpu and attach
> all the various thread/process/user tasks' their counters to that one
> buffer?
> 
> As opposed to the current state where each such counter would have its
> own buffer.
> 
> If this is what the patch does, then yes, although I would prefer a
> slightly clearer Changelog.
> 
> Acked-by: Peter Zijlstra <peterz@infradead.org>

Is there any reason why we wouldn't want to make this the default 
behavior?

That way we could also lose the somewhat suboptimal 'force' naming: 
there's nothing forced really, we simply switch to another ring-buffer 
setup ...

Thanks,

	Ingo

Re: [PATCH V3 06/11] perf record: Add an option to force per-cpu mmaps

[Sukadev Bhattiprolu]

Ingo Molnar [mingo@kernel.org] wrote:
| 
| * Peter Zijlstra <peterz@infradead.org> wrote:
| 
| > On Tue, Nov 05, 2013 at 02:31:52PM -0300, Arnaldo Carvalho de Melo wrote:
| > > PeterZ,
| > > 
| > > 	Can I have your Acked-by for this one? I guess now the goal is
| > > achieved, no?

Being able to profile children with the --pid is a big plus.

| > 
| > So this option allows -t/-p/-u to create one buffer per cpu and attach
| > all the various thread/process/user tasks' their counters to that one
| > buffer?
| > 
| > As opposed to the current state where each such counter would have its
| > own buffer.
| > 
| > If this is what the patch does, then yes, although I would prefer a
| > slightly clearer Changelog.
| > 
| > Acked-by: Peter Zijlstra <peterz@infradead.org>
| 
| Is there any reason why we wouldn't want to make this the default 
| behavior?
| 
| That way we could also lose the somewhat suboptimal 'force' naming: 
| there's nothing forced really, we simply switch to another ring-buffer 
| setup ...

It would be also good if the man page added a comment on when a user
would want one ring-buffer setup over the other.

If the main benefit is to have the children profiled when tasks are 
specified, how about changing the option to --inherit (-I) ?

Or for consistency with 'perf record <application>', have --pid profile 
children by default and let users specify --no-inherit with --pid if 
they don't want children profiled.

Sukadev

[tip:perf/urgent] perf record: Add an option to force per-cpu mmaps

[tip-bot for Adrian Hunter]

Commit-ID:  539e6bb71e350541105e67e3d6c31392d9da25ef
Gitweb:     http://git.kernel.org/tip/539e6bb71e350541105e67e3d6c31392d9da25ef
Author:     Adrian Hunter <adrian.hunter@intel.com>
AuthorDate: Fri, 1 Nov 2013 15:51:34 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 14 Nov 2013 16:10:27 -0300

perf record: Add an option to force per-cpu mmaps

By default, when tasks are specified (i.e. -p, -t or -u options)
per-thread mmaps are created.

Add an option to override that and force per-cpu mmaps.

Further comments by peterz:

So this option allows -t/-p/-u to create one buffer per cpu and attach
all the various thread/process/user tasks' their counters to that one
buffer?

As opposed to the current state where each such counter would have its
own buffer.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Tested-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Acked-by: Peter Zijlstra <peterz@infradead.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1383313899-15987-7-git-send-email-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Documentation/perf-record.txt | 6 ++++++
 tools/perf/builtin-record.c              | 2 ++
 tools/perf/util/evlist.c                 | 4 +++-
 tools/perf/util/evsel.c                  | 4 ++--
 tools/perf/util/target.h                 | 1 +
 5 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
index 052f7c4..43b42c4 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -201,6 +201,12 @@ abort events and some memory events in precise mode on modern Intel CPUs.
 --transaction::
 Record transaction flags for transaction related events.
 
+--force-per-cpu::
+Force the use of per-cpu mmaps.  By default, when tasks are specified (i.e. -p,
+-t or -u options) per-thread mmaps are created.  This option overrides that and
+forces per-cpu mmaps.  A side-effect of that is that inheritance is
+automatically enabled.  Add the -i option also to disable inheritance.
+
 SEE ALSO
 --------
 linkperf:perf-stat[1], linkperf:perf-list[1]
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 4d644fe..7c8020a 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -888,6 +888,8 @@ const struct option record_options[] = {
 		    "sample by weight (on special events only)"),
 	OPT_BOOLEAN(0, "transaction", &record.opts.sample_transaction,
 		    "sample transaction flags (special events only)"),
+	OPT_BOOLEAN(0, "force-per-cpu", &record.opts.target.force_per_cpu,
+		    "force the use of per-cpu mmaps"),
 	OPT_END()
 };
 
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 5ce2ace..bbc746a 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -819,7 +819,9 @@ int perf_evlist__create_maps(struct perf_evlist *evlist, struct target *target)
 	if (evlist->threads == NULL)
 		return -1;
 
-	if (target__has_task(target))
+	if (target->force_per_cpu)
+		evlist->cpus = cpu_map__new(target->cpu_list);
+	else if (target__has_task(target))
 		evlist->cpus = cpu_map__dummy_new();
 	else if (!target__has_cpu(target) && !target->uses_mmap)
 		evlist->cpus = cpu_map__dummy_new();
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 18f7c18..46dd4c2 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -645,7 +645,7 @@ void perf_evsel__config(struct perf_evsel *evsel,
 		}
 	}
 
-	if (target__has_cpu(&opts->target))
+	if (target__has_cpu(&opts->target) || opts->target.force_per_cpu)
 		perf_evsel__set_sample_bit(evsel, CPU);
 
 	if (opts->period)
@@ -653,7 +653,7 @@ void perf_evsel__config(struct perf_evsel *evsel,
 
 	if (!perf_missing_features.sample_id_all &&
 	    (opts->sample_time || !opts->no_inherit ||
-	     target__has_cpu(&opts->target)))
+	     target__has_cpu(&opts->target) || opts->target.force_per_cpu))
 		perf_evsel__set_sample_bit(evsel, TIME);
 
 	if (opts->raw_samples) {
diff --git a/tools/perf/util/target.h b/tools/perf/util/target.h
index 89bab71..2d0c506 100644
--- a/tools/perf/util/target.h
+++ b/tools/perf/util/target.h
@@ -12,6 +12,7 @@ struct target {
 	uid_t	     uid;
 	bool	     system_wide;
 	bool	     uses_mmap;
+	bool	     force_per_cpu;
 };
 
 enum target_errno {

[PATCH V3 07/11] perf evsel: Always use perf_evsel__set_sample_bit()

[Adrian Hunter]

Always use perf_evsel__set_sample_bit() rather than
just setting the bit.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/evsel.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index f1839b0..f0e263c 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -663,7 +663,7 @@ void perf_evsel__config(struct perf_evsel *evsel,
 	}
 
 	if (opts->sample_address)
-		attr->sample_type	|= PERF_SAMPLE_DATA_SRC;
+		perf_evsel__set_sample_bit(evsel, DATA_SRC);
 
 	if (opts->no_delay) {
 		attr->watermark = 0;
@@ -675,14 +675,14 @@ void perf_evsel__config(struct perf_evsel *evsel,
 	}
 
 	if (opts->sample_weight)
-		attr->sample_type	|= PERF_SAMPLE_WEIGHT;
+		perf_evsel__set_sample_bit(evsel, WEIGHT);
 
 	attr->mmap  = track;
 	attr->mmap2 = track && !perf_missing_features.mmap2;
 	attr->comm  = track;
 
 	if (opts->sample_transaction)
-		attr->sample_type	|= PERF_SAMPLE_TRANSACTION;
+		perf_evsel__set_sample_bit(evsel, TRANSACTION);
 
 	/*
 	 * XXX see the function comment above
-- 
1.7.11.7

[tip:perf/core] perf evsel: Always use perf_evsel__set_sample_bit ()

[tip-bot for Adrian Hunter]

Commit-ID:  1e7ed5ec54e3998bda6ea625599a0644404cb421
Gitweb:     http://git.kernel.org/tip/1e7ed5ec54e3998bda6ea625599a0644404cb421
Author:     Adrian Hunter <adrian.hunter@intel.com>
AuthorDate: Fri, 1 Nov 2013 15:51:35 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 4 Nov 2013 12:42:28 -0300

perf evsel: Always use perf_evsel__set_sample_bit()

Always use perf_evsel__set_sample_bit() rather than just setting the
bit.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1383313899-15987-8-git-send-email-adrian.hunter@intel.com
[ Cope with 3090ffb "perf: Disable PERF_RECORD_MMAP2 support" ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/evsel.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index f0e65de..47bbf03 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -663,7 +663,7 @@ void perf_evsel__config(struct perf_evsel *evsel,
 	}
 
 	if (opts->sample_address)
-		attr->sample_type	|= PERF_SAMPLE_DATA_SRC;
+		perf_evsel__set_sample_bit(evsel, DATA_SRC);
 
 	if (opts->no_delay) {
 		attr->watermark = 0;
@@ -675,13 +675,13 @@ void perf_evsel__config(struct perf_evsel *evsel,
 	}
 
 	if (opts->sample_weight)
-		attr->sample_type	|= PERF_SAMPLE_WEIGHT;
+		perf_evsel__set_sample_bit(evsel, WEIGHT);
 
 	attr->mmap  = track;
 	attr->comm  = track;
 
 	if (opts->sample_transaction)
-		attr->sample_type	|= PERF_SAMPLE_TRANSACTION;
+		perf_evsel__set_sample_bit(evsel, TRANSACTION);
 
 	/*
 	 * XXX see the function comment above

[PATCH V3 08/11] perf evsel: Add missing overflow check

[Adrian Hunter]

Add missing overflow check in
perf_evsel__parse_sample()

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/evsel.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index f0e263c..dc2b2dc 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1482,6 +1482,7 @@ int perf_evsel__parse_sample(struct perf_evsel *evsel, union perf_event *event,
 
 	data->transaction = 0;
 	if (type & PERF_SAMPLE_TRANSACTION) {
+		OVERFLOW_CHECK_u64(array);
 		data->transaction = *array;
 		array++;
 	}
-- 
1.7.11.7

[tip:perf/core] perf evsel: Add missing overflow check for TRANSACTION

[tip-bot for Adrian Hunter]

Commit-ID:  87b955247d71975460774435241be3aa05218a7b
Gitweb:     http://git.kernel.org/tip/87b955247d71975460774435241be3aa05218a7b
Author:     Adrian Hunter <adrian.hunter@intel.com>
AuthorDate: Fri, 1 Nov 2013 15:51:36 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 4 Nov 2013 12:44:01 -0300

perf evsel: Add missing overflow check for TRANSACTION

Add missing overflow check for PERF_SAMPLE_TRANSACTION in
perf_evsel__parse_sample().

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1383313899-15987-9-git-send-email-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/evsel.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 47bbf03..b121717 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1481,6 +1481,7 @@ int perf_evsel__parse_sample(struct perf_evsel *evsel, union perf_event *event,
 
 	data->transaction = 0;
 	if (type & PERF_SAMPLE_TRANSACTION) {
+		OVERFLOW_CHECK_u64(array);
 		data->transaction = *array;
 		array++;
 	}

[PATCH V3 09/11] perf test: Update "sample parsing" test for PERF_SAMPLE_TRANSACTION

[Adrian Hunter]

In fact the "sample parsing" test does not automatically
check new sample type bits - they must be added to the
comparison logic.  Doing that shows that the test fails
because the functions perf_event__synthesize_sample()
and perf_event__sample_event_size() have not been
updated with PERF_SAMPLE_TRANSACTION either.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/tests/sample-parsing.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/perf/tests/sample-parsing.c b/tools/perf/tests/sample-parsing.c
index 61c9da2..1b67720 100644
--- a/tools/perf/tests/sample-parsing.c
+++ b/tools/perf/tests/sample-parsing.c
@@ -121,6 +121,9 @@ static bool samples_same(const struct perf_sample *s1,
 	if (type & PERF_SAMPLE_DATA_SRC)
 		COMP(data_src);
 
+	if (type & PERF_SAMPLE_TRANSACTION)
+		COMP(transaction);
+
 	return true;
 }
 
@@ -165,6 +168,7 @@ static int do_test(u64 sample_type, u64 sample_regs_user, u64 read_format)
 		.cpu		= 110,
 		.raw_size	= sizeof(raw_data),
 		.data_src	= 111,
+		.transaction	= 112,
 		.raw_data	= (void *)raw_data,
 		.callchain	= &callchain.callchain,
 		.branch_stack	= &branch_stack.branch_stack,
@@ -273,7 +277,8 @@ int test__sample_parsing(void)
 
 	/*
 	 * Fail the test if it has not been updated when new sample format bits
-	 * were added.
+	 * were added.  Please actually update the test rather than just change
+	 * the condition below.
 	 */
 	if (PERF_SAMPLE_MAX > PERF_SAMPLE_TRANSACTION << 1) {
 		pr_debug("sample format has changed, some new PERF_SAMPLE_ bit was introduced - test needs updating\n");
-- 
1.7.11.7

[tip:perf/core] perf test: Update "sample parsing" test for PERF_SAMPLE_TRANSACTION

[tip-bot for Adrian Hunter]

Commit-ID:  091a4ef5a94d46d26a05f0c32d2f64800ed91306
Gitweb:     http://git.kernel.org/tip/091a4ef5a94d46d26a05f0c32d2f64800ed91306
Author:     Adrian Hunter <adrian.hunter@intel.com>
AuthorDate: Fri, 1 Nov 2013 15:51:37 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 4 Nov 2013 12:47:24 -0300

perf test: Update "sample parsing" test for PERF_SAMPLE_TRANSACTION

In fact the "sample parsing" test does not automatically check new
sample type bits - they must be added to the comparison logic.

Doing that shows that the test fails because the functions
perf_event__synthesize_sample() and perf_event__sample_event_size() have
not been updated with PERF_SAMPLE_TRANSACTION either.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1383313899-15987-10-git-send-email-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/tests/sample-parsing.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/perf/tests/sample-parsing.c b/tools/perf/tests/sample-parsing.c
index 61c9da2..1b67720 100644
--- a/tools/perf/tests/sample-parsing.c
+++ b/tools/perf/tests/sample-parsing.c
@@ -121,6 +121,9 @@ static bool samples_same(const struct perf_sample *s1,
 	if (type & PERF_SAMPLE_DATA_SRC)
 		COMP(data_src);
 
+	if (type & PERF_SAMPLE_TRANSACTION)
+		COMP(transaction);
+
 	return true;
 }
 
@@ -165,6 +168,7 @@ static int do_test(u64 sample_type, u64 sample_regs_user, u64 read_format)
 		.cpu		= 110,
 		.raw_size	= sizeof(raw_data),
 		.data_src	= 111,
+		.transaction	= 112,
 		.raw_data	= (void *)raw_data,
 		.callchain	= &callchain.callchain,
 		.branch_stack	= &branch_stack.branch_stack,
@@ -273,7 +277,8 @@ int test__sample_parsing(void)
 
 	/*
 	 * Fail the test if it has not been updated when new sample format bits
-	 * were added.
+	 * were added.  Please actually update the test rather than just change
+	 * the condition below.
 	 */
 	if (PERF_SAMPLE_MAX > PERF_SAMPLE_TRANSACTION << 1) {
 		pr_debug("sample format has changed, some new PERF_SAMPLE_ bit was introduced - test needs updating\n");

[PATCH V3 10/11] perf evsel: Add missing PERF_SAMPLE_TRANSACTION

[Adrian Hunter]

Add missing PERF_SAMPLE_TRANSACTION to
perf_event__synthesize_sample() and
perf_event__sample_event_size().

This makes the "sample parsing" test pass.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/evsel.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index dc2b2dc..7802bde 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1579,6 +1579,9 @@ size_t perf_event__sample_event_size(const struct perf_sample *sample, u64 type,
 	if (type & PERF_SAMPLE_DATA_SRC)
 		result += sizeof(u64);
 
+	if (type & PERF_SAMPLE_TRANSACTION)
+		result += sizeof(u64);
+
 	return result;
 }
 
@@ -1752,6 +1755,11 @@ int perf_event__synthesize_sample(union perf_event *event, u64 type,
 		array++;
 	}
 
+	if (type & PERF_SAMPLE_TRANSACTION) {
+		*array = sample->transaction;
+		array++;
+	}
+
 	return 0;
 }
 
-- 
1.7.11.7

[tip:perf/core] perf evsel: Synthesize PERF_SAMPLE_TRANSACTION

[tip-bot for Adrian Hunter]

Commit-ID:  42d88910c717ba21089251d0ca559abfef0df22d
Gitweb:     http://git.kernel.org/tip/42d88910c717ba21089251d0ca559abfef0df22d
Author:     Adrian Hunter <adrian.hunter@intel.com>
AuthorDate: Fri, 1 Nov 2013 15:51:38 +0200
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 4 Nov 2013 12:49:36 -0300

perf evsel: Synthesize PERF_SAMPLE_TRANSACTION

Add missing PERF_SAMPLE_TRANSACTION to perf_event__synthesize_sample()
and perf_event__sample_event_size().

This makes the "sample parsing" test pass.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1383313899-15987-11-git-send-email-adrian.hunter@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/evsel.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index b121717..5280820 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1578,6 +1578,9 @@ size_t perf_event__sample_event_size(const struct perf_sample *sample, u64 type,
 	if (type & PERF_SAMPLE_DATA_SRC)
 		result += sizeof(u64);
 
+	if (type & PERF_SAMPLE_TRANSACTION)
+		result += sizeof(u64);
+
 	return result;
 }
 
@@ -1751,6 +1754,11 @@ int perf_event__synthesize_sample(union perf_event *event, u64 type,
 		array++;
 	}
 
+	if (type & PERF_SAMPLE_TRANSACTION) {
+		*array = sample->transaction;
+		array++;
+	}
+
 	return 0;
 }
 

[PATCH V3 11/11] perf tools: Allow non-matching sample types

[Adrian Hunter]

For kernels that do not support PERF_SAMPLE_IDENTIFIER,
sample types need not be identical to determine
the sample id from the event.  Only the position
of the sample id needs to be the same.

Compatible sample types are ones in which the bits
defined by PERF_COMPAT_MASK are the same.
'perf_evlist__config()' forces sample types to be
compatible on that basis.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/builtin-trace.c |  1 +
 tools/perf/perf.h          |  1 +
 tools/perf/util/event.h    | 16 ++++++++++++++++
 tools/perf/util/evlist.c   | 25 +++++++++++++++++++++++++
 tools/perf/util/evlist.h   |  1 +
 tools/perf/util/record.c   |  5 ++++-
 6 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index dc3da65..08a34e3 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -2062,6 +2062,7 @@ int cmd_trace(int argc, const char **argv, const char *prefix __maybe_unused)
 			.user_interval = ULLONG_MAX,
 			.no_delay      = true,
 			.mmap_pages    = 1024,
+			.incompatible_sample_types = true,
 		},
 		.output = stdout,
 		.show_comm = true,
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index f61c230..aeecdf7 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -233,6 +233,7 @@ struct perf_record_opts {
 	u64	     user_interval;
 	u16	     stack_dump_size;
 	bool	     sample_transaction;
+	bool         incompatible_sample_types;
 };
 
 #endif
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index 752709c..ca0689c 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -78,6 +78,22 @@ struct throttle_event {
 /* perf sample has 16 bits size limit */
 #define PERF_SAMPLE_MAX_SIZE (1 << 16)
 
+/*
+ * Events have compatible sample types if the following bits all have the same
+ * value.  This is because the order of sample members is fixed.  For sample
+ * events the order is: PERF_SAMPLE_IP, PERF_SAMPLE_TID, PERF_SAMPLE_TIME,
+ * PERF_SAMPLE_ADDR, PERF_SAMPLE_ID.  For non-sample events the sample members
+ * are accessed in reverse order.  The order is: PERF_SAMPLE_ID,
+ * PERF_SAMPLE_STREAM_ID, PERF_SAMPLE_CPU.  PERF_SAMPLE_IDENTIFIER is added for
+ * completeness but it should not be used with PERF_SAMPLE_ID.  Sample types
+ * that include PERF_SAMPLE_IDENTIFIER are always compatible.
+ */
+#define PERF_COMPAT_MASK				\
+	(PERF_SAMPLE_IP   | PERF_SAMPLE_TID       |	\
+	 PERF_SAMPLE_TIME | PERF_SAMPLE_ADDR      |	\
+	 PERF_SAMPLE_ID   | PERF_SAMPLE_STREAM_ID |	\
+	 PERF_SAMPLE_CPU  | PERF_SAMPLE_IDENTIFIER)
+
 struct sample_event {
 	struct perf_event_header        header;
 	u64 array[];
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index b3183d5..c893113 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -87,6 +87,31 @@ static void perf_evlist__update_id_pos(struct perf_evlist *evlist)
 	perf_evlist__set_id_pos(evlist);
 }
 
+/**
+ * perf_evlist__make_sample_types_compatible - make sample types compatible.
+ * @evlist: selected event list
+ *
+ * Events with compatible sample types all have the same id_pos and is_pos.
+ * This can be achieved by matching the bits of PERF_COMPAT_MASK.
+ */
+void perf_evlist__make_sample_types_compatible(struct perf_evlist *evlist)
+{
+	struct perf_evsel *evsel;
+	u64 compat = 0;
+
+	list_for_each_entry(evsel, &evlist->entries, node)
+		compat |= evsel->attr.sample_type & PERF_COMPAT_MASK;
+
+	list_for_each_entry(evsel, &evlist->entries, node) {
+		evsel->attr.sample_type |= compat;
+		evsel->sample_size =
+			__perf_evsel__sample_size(evsel->attr.sample_type);
+		perf_evsel__calc_id_pos(evsel);
+	}
+
+	perf_evlist__set_id_pos(evlist);
+}
+
 static void perf_evlist__purge(struct perf_evlist *evlist)
 {
 	struct perf_evsel *pos, *n;
diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index 6e8acc9..d0d8a39 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -96,6 +96,7 @@ int perf_evlist__open(struct perf_evlist *evlist);
 void perf_evlist__close(struct perf_evlist *evlist);
 
 void perf_evlist__set_id_pos(struct perf_evlist *evlist);
+void perf_evlist__make_sample_types_compatible(struct perf_evlist *evlist);
 bool perf_can_sample_identifier(void);
 void perf_evlist__config(struct perf_evlist *evlist,
 			 struct perf_record_opts *opts);
diff --git a/tools/perf/util/record.c b/tools/perf/util/record.c
index 18d73aa..1eb1290 100644
--- a/tools/perf/util/record.c
+++ b/tools/perf/util/record.c
@@ -104,5 +104,8 @@ void perf_evlist__config(struct perf_evlist *evlist,
 			perf_evsel__set_sample_id(evsel, use_sample_identifier);
 	}
 
-	perf_evlist__set_id_pos(evlist);
+	if (use_sample_identifier || opts->incompatible_sample_types)
+		perf_evlist__set_id_pos(evlist);
+	else
+		perf_evlist__make_sample_types_compatible(evlist);
 }
-- 
1.7.11.7