linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [GIT PULL 0/6] perf/core fixes
@ 2012-05-20  2:16 Arnaldo Carvalho de Melo
  2012-05-20  2:16 ` [PATCH 1/6] perf target: Rename functions to avoid double negation Arnaldo Carvalho de Melo
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-05-20  2:16 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Corey Ashford,
	David Ahern, Frederic Weisbecker, Jiri Olsa, Linus Torvalds,
	Namhyung Kim, Namhyung Kim, Paul Mackerras, Peter Zijlstra,
	Robert Richter, Stephane Eranian, arnaldo.melo,
	Arnaldo Carvalho de Melo

Hi Ingo,

	This also includes a merge with tip/perf/urgent, please consider
pulling,

- Arnaldo

The following changes since commit 978da300c7a65494692b329a6a4cbf364afc37c5:

  perf/x86/ibs: Fix undefined reference to `get_ibs_caps' (2012-05-14 14:31:35 +0200)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux perf/core

for you to fetch changes up to 5e1c81d98a5621007824b49dde556fead5ff9c6c:

  perf evsel: Create events initially disabled -- again (2012-05-18 16:02:42 -0300)

----------------------------------------------------------------
Fixes for perf/core:

. Rename some perf_target methods to avoid double negation, from Namhyung Kim.

. Revert change to use per task events with inheritance, from Namhyung Kim.

. Events should start disabled till children starts running, from David Ahern.

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

----------------------------------------------------------------
Arnaldo Carvalho de Melo (1):
      Merge remote-tracking branch 'tip/perf/urgent' into perf/core

David Ahern (1):
      perf evsel: Create events initially disabled -- again

Jiri Olsa (2):
      perf hists: Fix callchain ip printf format
      perf tools: Split term type into value type and term type

Namhyung Kim (3):
      perf target: Rename functions to avoid double negation
      Revert 'perf evlist: Fix creation of cpu map'
      perf target: Add uses_mmap field

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

* [PATCH 1/6] perf target: Rename functions to avoid double negation
  2012-05-20  2:16 [GIT PULL 0/6] perf/core fixes Arnaldo Carvalho de Melo
@ 2012-05-20  2:16 ` Arnaldo Carvalho de Melo
  2012-05-20  2:16 ` [PATCH 2/6] Revert 'perf evlist: Fix creation of cpu map' Arnaldo Carvalho de Melo
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-05-20  2:16 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Namhyung Kim, Ingo Molnar, Namhyung Kim,
	Paul Mackerras, Peter Zijlstra, Arnaldo Carvalho de Melo

From: Namhyung Kim <namhyung.kim@lge.com>

Rename perf_target__no_{cpu,task} to perf_target__has_{cpu,task} because
it's more intuitive and easy to parse (for human beings) when used with
negation.

The names are came out from David Ahern.  It is intended to be a
mechanical substitution without any functional change.

The perf_target__none remains unchanged since I couldn't find a right
name and it is hardly used with negation.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
Suggested-by: David Ahern <dsahern@gmail.com>
Suggested-by: Ingo Molnar <mingo@kernel.org>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1337161549-9870-1-git-send-email-namhyung.kim@lge.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-stat.c |   14 +++++++-------
 tools/perf/builtin-top.c  |    2 +-
 tools/perf/util/evlist.c  |    2 +-
 tools/perf/util/evsel.c   |    2 +-
 tools/perf/util/target.h  |   10 +++++-----
 5 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index d060568..0f4b51a 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -292,10 +292,10 @@ static int create_perf_stat_counter(struct perf_evsel *evsel,
 
 	attr->inherit = !no_inherit;
 
-	if (!perf_target__no_cpu(&target))
+	if (perf_target__has_cpu(&target))
 		return perf_evsel__open_per_cpu(evsel, evsel_list->cpus,
 						group, group_fd);
-	if (perf_target__no_task(&target) && (!group || evsel == first)) {
+	if (!perf_target__has_task(&target) && (!group || evsel == first)) {
 		attr->disabled = 1;
 		attr->enable_on_exec = 1;
 	}
@@ -972,7 +972,7 @@ static void print_stat(int argc, const char **argv)
 	if (!csv_output) {
 		fprintf(output, "\n");
 		fprintf(output, " Performance counter stats for ");
-		if (perf_target__no_task(&target)) {
+		if (!perf_target__has_task(&target)) {
 			fprintf(output, "\'%s", argv[0]);
 			for (i = 1; i < argc; i++)
 				fprintf(output, " %s", argv[i]);
@@ -1194,13 +1194,13 @@ int cmd_stat(int argc, const char **argv, const char *prefix __used)
 	} else if (big_num_opt == 0) /* User passed --no-big-num */
 		big_num = false;
 
-	if (!argc && perf_target__no_task(&target))
+	if (!argc && !perf_target__has_task(&target))
 		usage_with_options(stat_usage, options);
 	if (run_count <= 0)
 		usage_with_options(stat_usage, options);
 
 	/* no_aggr, cgroup are for system-wide only */
-	if ((no_aggr || nr_cgroups) && perf_target__no_cpu(&target)) {
+	if ((no_aggr || nr_cgroups) && !perf_target__has_cpu(&target)) {
 		fprintf(stderr, "both cgroup and no-aggregation "
 			"modes only available in system-wide mode\n");
 
@@ -1213,9 +1213,9 @@ int cmd_stat(int argc, const char **argv, const char *prefix __used)
 	perf_target__validate(&target);
 
 	if (perf_evlist__create_maps(evsel_list, &target) < 0) {
-		if (!perf_target__no_task(&target))
+		if (perf_target__has_task(&target))
 			pr_err("Problems finding threads of monitor\n");
-		if (!perf_target__no_cpu(&target))
+		if (perf_target__has_cpu(&target))
 			perror("failed to parse CPUs map");
 
 		usage_with_options(stat_usage, options);
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 4eb6171..553560a 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1020,7 +1020,7 @@ static int __cmd_top(struct perf_top *top)
 	if (ret)
 		goto out_delete;
 
-	if (!perf_target__no_task(&top->target))
+	if (perf_target__has_task(&top->target))
 		perf_event__synthesize_thread_map(&top->tool, top->evlist->threads,
 						  perf_event__process,
 						  &top->session->host_machine);
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 1201daf..80bef28 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -609,7 +609,7 @@ int perf_evlist__create_maps(struct perf_evlist *evlist,
 	if (evlist->threads == NULL)
 		return -1;
 
-	if (!perf_target__no_cpu(target))
+	if (perf_target__has_cpu(target))
 		evlist->cpus = cpu_map__new(target->cpu_list);
 	else
 		evlist->cpus = cpu_map__dummy_new();
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 21eaab2..d7a2b4b 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -115,7 +115,7 @@ void perf_evsel__config(struct perf_evsel *evsel, struct perf_record_opts *opts,
 
 	if (!opts->sample_id_all_missing &&
 	    (opts->sample_time || !opts->no_inherit ||
-	     !perf_target__no_cpu(&opts->target)))
+	     perf_target__has_cpu(&opts->target)))
 		attr->sample_type	|= PERF_SAMPLE_TIME;
 
 	if (opts->raw_samples) {
diff --git a/tools/perf/util/target.h b/tools/perf/util/target.h
index 127cff3..c43f632 100644
--- a/tools/perf/util/target.h
+++ b/tools/perf/util/target.h
@@ -46,19 +46,19 @@ enum perf_target_errno perf_target__parse_uid(struct perf_target *target);
 int perf_target__strerror(struct perf_target *target, int errnum, char *buf,
 			  size_t buflen);
 
-static inline bool perf_target__no_task(struct perf_target *target)
+static inline bool perf_target__has_task(struct perf_target *target)
 {
-	return !target->pid && !target->tid && !target->uid_str;
+	return target->tid || target->pid || target->uid_str;
 }
 
-static inline bool perf_target__no_cpu(struct perf_target *target)
+static inline bool perf_target__has_cpu(struct perf_target *target)
 {
-	return !target->system_wide && !target->cpu_list;
+	return target->system_wide || target->cpu_list;
 }
 
 static inline bool perf_target__none(struct perf_target *target)
 {
-	return perf_target__no_task(target) && perf_target__no_cpu(target);
+	return !perf_target__has_task(target) && !perf_target__has_cpu(target);
 }
 
 #endif /* _PERF_TARGET_H */
-- 
1.7.9.2.358.g22243


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

* [PATCH 2/6] Revert 'perf evlist: Fix creation of cpu map'
  2012-05-20  2:16 [GIT PULL 0/6] perf/core fixes Arnaldo Carvalho de Melo
  2012-05-20  2:16 ` [PATCH 1/6] perf target: Rename functions to avoid double negation Arnaldo Carvalho de Melo
@ 2012-05-20  2:16 ` Arnaldo Carvalho de Melo
  2012-05-20  2:16 ` [PATCH 3/6] perf target: Add uses_mmap field Arnaldo Carvalho de Melo
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-05-20  2:16 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Namhyung Kim, Ingo Molnar, Namhyung Kim,
	Paul Mackerras, Peter Zijlstra, Arnaldo Carvalho de Melo

From: Namhyung Kim <namhyung.kim@lge.com>

The commit 55261f46702c ("perf evlist: Fix creation of cpu map") changed
to create a per-task event when no cpu target is specified. However it
caused a problem since perf-task do not allow event inheritance due to
scalability issues so that the result will contain samples only from
parent, not from its children.

So we should use perf-task-per-cpu events anyway to get the right
result. Revert it.

Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Analysed-by: Ingo Molnar <mingo@kernel.org>
Acked-and-tested-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1337161549-9870-2-git-send-email-namhyung.kim@lge.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/evlist.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 80bef28..87889f3 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -609,10 +609,10 @@ int perf_evlist__create_maps(struct perf_evlist *evlist,
 	if (evlist->threads == NULL)
 		return -1;
 
-	if (perf_target__has_cpu(target))
-		evlist->cpus = cpu_map__new(target->cpu_list);
-	else
+	if (perf_target__has_task(target))
 		evlist->cpus = cpu_map__dummy_new();
+	else
+		evlist->cpus = cpu_map__new(target->cpu_list);
 
 	if (evlist->cpus == NULL)
 		goto out_delete_threads;
-- 
1.7.9.2.358.g22243


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

* [PATCH 3/6] perf target: Add uses_mmap field
  2012-05-20  2:16 [GIT PULL 0/6] perf/core fixes Arnaldo Carvalho de Melo
  2012-05-20  2:16 ` [PATCH 1/6] perf target: Rename functions to avoid double negation Arnaldo Carvalho de Melo
  2012-05-20  2:16 ` [PATCH 2/6] Revert 'perf evlist: Fix creation of cpu map' Arnaldo Carvalho de Melo
@ 2012-05-20  2:16 ` Arnaldo Carvalho de Melo
  2012-05-20  2:16 ` [PATCH 4/6] perf hists: Fix callchain ip printf format Arnaldo Carvalho de Melo
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-05-20  2:16 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Namhyung Kim, Ingo Molnar, Namhyung Kim,
	Paul Mackerras, Peter Zijlstra, Arnaldo Carvalho de Melo

From: Namhyung Kim <namhyung.kim@lge.com>

If perf doesn't mmap on event (like perf stat), it should not create
per-task-per-cpu events. So just use a dummy cpu map to create a
per-task event for this case.

Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1337161549-9870-3-git-send-email-namhyung.kim@lge.com
[ committer note: renamed .need_mmap to .uses_mmap ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-record.c |    3 +++
 tools/perf/builtin-test.c   |    1 +
 tools/perf/builtin-top.c    |    3 +++
 tools/perf/util/evlist.c    |    2 ++
 tools/perf/util/target.h    |    1 +
 5 files changed, 10 insertions(+)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index d19058a..8a3dfac 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -754,6 +754,9 @@ static struct perf_record record = {
 		.user_freq	     = UINT_MAX,
 		.user_interval	     = ULLONG_MAX,
 		.freq		     = 1000,
+		.target		     = {
+			.uses_mmap   = true,
+		},
 	},
 	.write_mode = WRITE_FORCE,
 	.file_new   = true,
diff --git a/tools/perf/builtin-test.c b/tools/perf/builtin-test.c
index 9d9abbb..4eaa665 100644
--- a/tools/perf/builtin-test.c
+++ b/tools/perf/builtin-test.c
@@ -1167,6 +1167,7 @@ static int test__PERF_RECORD(void)
 	struct perf_record_opts opts = {
 		.target = {
 			.uid = UINT_MAX,
+			.uses_mmap = true,
 		},
 		.no_delay   = true,
 		.freq	    = 10,
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 553560a..3e981a7 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1162,6 +1162,9 @@ int cmd_top(int argc, const char **argv, const char *prefix __used)
 		.freq		     = 1000, /* 1 KHz */
 		.mmap_pages	     = 128,
 		.sym_pcnt_filter     = 5,
+		.target		     = {
+			.uses_mmap   = true,
+		},
 	};
 	char callchain_default_opt[] = "fractal,0.5,callee";
 	const struct option options[] = {
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 87889f3..4ac5f5a 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -611,6 +611,8 @@ int perf_evlist__create_maps(struct perf_evlist *evlist,
 
 	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();
 	else
 		evlist->cpus = cpu_map__new(target->cpu_list);
 
diff --git a/tools/perf/util/target.h b/tools/perf/util/target.h
index c43f632..a4be857 100644
--- a/tools/perf/util/target.h
+++ b/tools/perf/util/target.h
@@ -11,6 +11,7 @@ struct perf_target {
 	const char   *uid_str;
 	uid_t	     uid;
 	bool	     system_wide;
+	bool	     uses_mmap;
 };
 
 enum perf_target_errno {
-- 
1.7.9.2.358.g22243


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

* [PATCH 4/6] perf hists: Fix callchain ip printf format
  2012-05-20  2:16 [GIT PULL 0/6] perf/core fixes Arnaldo Carvalho de Melo
                   ` (2 preceding siblings ...)
  2012-05-20  2:16 ` [PATCH 3/6] perf target: Add uses_mmap field Arnaldo Carvalho de Melo
@ 2012-05-20  2:16 ` Arnaldo Carvalho de Melo
  2012-05-20  2:16 ` [PATCH 5/6] perf tools: Split term type into value type and term type Arnaldo Carvalho de Melo
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-05-20  2:16 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Jiri Olsa, Corey Ashford, David Ahern,
	Frederic Weisbecker, Ingo Molnar, Paul Mackerras, Peter Zijlstra,
	Arnaldo Carvalho de Melo

From: Jiri Olsa <jolsa@redhat.com>

The callchain address is stored as u64. Current code uses following
format string to display callchain address:

  "%p\n", (void *)(long)chain->ip

This way we lose upper 32 bits if we report 64 bit addresses in 32 bit
environment. Fixing this to always display whole 64 bits.

Note, running following to test perf endianity handling:
test 1)
  - origin system:
    # perf record -a -- sleep 10 (any perf record will do)
    # perf report > report.origin
    # perf archive perf.data

  - copy the perf.data, report.origin and perf.data.tar.bz2
    to a target system and run:
    # tar xjvf perf.data.tar.bz2 -C ~/.debug
    # perf report > report.target
    # diff -u report.origin report.target

  - the diff should produce no output
    (besides some white space stuff and possibly different
     date/TZ output)

test 2)
  - origin system:
    # perf record -ag -fo /tmp/perf.data -- sleep 1
  - mount origin system root to the target system on /mnt/origin
  - target system:
    # perf script --symfs /mnt/origin -I -i /mnt/origin/tmp/perf.data \
     --kallsyms /mnt/origin/proc/kallsyms
  - complete perf.data header is displayed

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1337151548-2396-8-git-send-email-jolsa@redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/hist.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 9f6d630..1293b5e 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -599,7 +599,7 @@ static size_t ipchain__fprintf_graph(FILE *fp, struct callchain_list *chain,
 	if (chain->ms.sym)
 		ret += fprintf(fp, "%s\n", chain->ms.sym->name);
 	else
-		ret += fprintf(fp, "%p\n", (void *)(long)chain->ip);
+		ret += fprintf(fp, "0x%0" PRIx64 "\n", chain->ip);
 
 	return ret;
 }
-- 
1.7.9.2.358.g22243


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

* [PATCH 5/6] perf tools: Split term type into value type and term type
  2012-05-20  2:16 [GIT PULL 0/6] perf/core fixes Arnaldo Carvalho de Melo
                   ` (3 preceding siblings ...)
  2012-05-20  2:16 ` [PATCH 4/6] perf hists: Fix callchain ip printf format Arnaldo Carvalho de Melo
@ 2012-05-20  2:16 ` Arnaldo Carvalho de Melo
  2012-05-20  2:16 ` [PATCH 6/6] perf evsel: Create events initially disabled -- again Arnaldo Carvalho de Melo
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-05-20  2:16 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Jiri Olsa, Corey Ashford, Frederic Weisbecker,
	Ingo Molnar, Paul Mackerras, Peter Zijlstra, Robert Richter,
	Stephane Eranian, Arnaldo Carvalho de Melo

From: Jiri Olsa <jolsa@redhat.com>

Introducing type_val and type_term for term instead of a single type
value. Currently the term type marked out the value type as well.

With this change we can have future string term values being specified
by user and translated into proper number along the processing.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Robert Richter <robert.richter@amd.com>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1335371102-11358-2-git-send-email-jolsa@redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/parse-events.c |   45 +++++++++++++++++++-------
 tools/perf/util/parse-events.h |   21 ++++++------
 tools/perf/util/parse-events.y |   16 ++++-----
 tools/perf/util/pmu.c          |   70 ++++++++++++++++++++++++----------------
 4 files changed, 96 insertions(+), 56 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 5b3a0ef..c7fc18a 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -593,17 +593,27 @@ int parse_events_add_breakpoint(struct list_head *list, int *idx,
 static int config_term(struct perf_event_attr *attr,
 		       struct parse_events__term *term)
 {
-	switch (term->type) {
+#define CHECK_TYPE_VAL(type)					\
+do {								\
+	if (PARSE_EVENTS__TERM_TYPE_ ## type != term->type_val)	\
+		return -EINVAL;					\
+} while (0)
+
+	switch (term->type_term) {
 	case PARSE_EVENTS__TERM_TYPE_CONFIG:
+		CHECK_TYPE_VAL(NUM);
 		attr->config = term->val.num;
 		break;
 	case PARSE_EVENTS__TERM_TYPE_CONFIG1:
+		CHECK_TYPE_VAL(NUM);
 		attr->config1 = term->val.num;
 		break;
 	case PARSE_EVENTS__TERM_TYPE_CONFIG2:
+		CHECK_TYPE_VAL(NUM);
 		attr->config2 = term->val.num;
 		break;
 	case PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD:
+		CHECK_TYPE_VAL(NUM);
 		attr->sample_period = term->val.num;
 		break;
 	case PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE:
@@ -615,7 +625,9 @@ static int config_term(struct perf_event_attr *attr,
 	default:
 		return -EINVAL;
 	}
+
 	return 0;
+#undef CHECK_TYPE_VAL
 }
 
 static int config_attr(struct perf_event_attr *attr,
@@ -1015,11 +1027,12 @@ void print_events(const char *event_glob)
 
 int parse_events__is_hardcoded_term(struct parse_events__term *term)
 {
-	return term->type <= PARSE_EVENTS__TERM_TYPE_HARDCODED_MAX;
+	return term->type_term != PARSE_EVENTS__TERM_TYPE_USER;
 }
 
-int parse_events__new_term(struct parse_events__term **_term, int type,
-			   char *config, char *str, long num)
+static int new_term(struct parse_events__term **_term, int type_val,
+		    int type_term, char *config,
+		    char *str, long num)
 {
 	struct parse_events__term *term;
 
@@ -1028,15 +1041,11 @@ int parse_events__new_term(struct parse_events__term **_term, int type,
 		return -ENOMEM;
 
 	INIT_LIST_HEAD(&term->list);
-	term->type = type;
+	term->type_val  = type_val;
+	term->type_term = type_term;
 	term->config = config;
 
-	switch (type) {
-	case PARSE_EVENTS__TERM_TYPE_CONFIG:
-	case PARSE_EVENTS__TERM_TYPE_CONFIG1:
-	case PARSE_EVENTS__TERM_TYPE_CONFIG2:
-	case PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD:
-	case PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE:
+	switch (type_val) {
 	case PARSE_EVENTS__TERM_TYPE_NUM:
 		term->val.num = num;
 		break;
@@ -1051,6 +1060,20 @@ int parse_events__new_term(struct parse_events__term **_term, int type,
 	return 0;
 }
 
+int parse_events__term_num(struct parse_events__term **term,
+			   int type_term, char *config, long num)
+{
+	return new_term(term, PARSE_EVENTS__TERM_TYPE_NUM, type_term,
+			config, NULL, num);
+}
+
+int parse_events__term_str(struct parse_events__term **term,
+			   int type_term, char *config, char *str)
+{
+	return new_term(term, PARSE_EVENTS__TERM_TYPE_STR, type_term,
+			config, str, 0);
+}
+
 void parse_events__free_terms(struct list_head *terms)
 {
 	struct parse_events__term *term, *h;
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index 5cb0028..3fddd61 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -36,16 +36,17 @@ extern int parse_filter(const struct option *opt, const char *str, int unset);
 #define EVENTS_HELP_MAX (128*1024)
 
 enum {
+	PARSE_EVENTS__TERM_TYPE_NUM,
+	PARSE_EVENTS__TERM_TYPE_STR,
+};
+
+enum {
+	PARSE_EVENTS__TERM_TYPE_USER,
 	PARSE_EVENTS__TERM_TYPE_CONFIG,
 	PARSE_EVENTS__TERM_TYPE_CONFIG1,
 	PARSE_EVENTS__TERM_TYPE_CONFIG2,
 	PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD,
 	PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE,
-	PARSE_EVENTS__TERM_TYPE_NUM,
-	PARSE_EVENTS__TERM_TYPE_STR,
-
-	PARSE_EVENTS__TERM_TYPE_HARDCODED_MAX =
-		PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE,
 };
 
 struct parse_events__term {
@@ -54,14 +55,16 @@ struct parse_events__term {
 		char *str;
 		long  num;
 	} val;
-	int type;
-
+	int type_val;
+	int type_term;
 	struct list_head list;
 };
 
 int parse_events__is_hardcoded_term(struct parse_events__term *term);
-int parse_events__new_term(struct parse_events__term **term, int type,
-			   char *config, char *str, long num);
+int parse_events__term_num(struct parse_events__term **_term,
+			   int type_term, char *config, long num);
+int parse_events__term_str(struct parse_events__term **_term,
+			   int type_term, char *config, char *str);
 void parse_events__free_terms(struct list_head *terms);
 int parse_events_modifier(struct list_head *list __used, char *str __used);
 int parse_events_add_tracepoint(struct list_head *list, int *idx,
diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
index d9637da..936913e 100644
--- a/tools/perf/util/parse-events.y
+++ b/tools/perf/util/parse-events.y
@@ -176,8 +176,8 @@ PE_NAME '=' PE_NAME
 {
 	struct parse_events__term *term;
 
-	ABORT_ON(parse_events__new_term(&term, PARSE_EVENTS__TERM_TYPE_STR,
-		 $1, $3, 0));
+	ABORT_ON(parse_events__term_str(&term, PARSE_EVENTS__TERM_TYPE_USER,
+					$1, $3));
 	$$ = term;
 }
 |
@@ -185,8 +185,8 @@ PE_NAME '=' PE_VALUE
 {
 	struct parse_events__term *term;
 
-	ABORT_ON(parse_events__new_term(&term, PARSE_EVENTS__TERM_TYPE_NUM,
-		 $1, NULL, $3));
+	ABORT_ON(parse_events__term_num(&term, PARSE_EVENTS__TERM_TYPE_USER,
+					$1, $3));
 	$$ = term;
 }
 |
@@ -194,8 +194,8 @@ PE_NAME
 {
 	struct parse_events__term *term;
 
-	ABORT_ON(parse_events__new_term(&term, PARSE_EVENTS__TERM_TYPE_NUM,
-		 $1, NULL, 1));
+	ABORT_ON(parse_events__term_num(&term, PARSE_EVENTS__TERM_TYPE_USER,
+					$1, 1));
 	$$ = term;
 }
 |
@@ -203,7 +203,7 @@ PE_TERM '=' PE_VALUE
 {
 	struct parse_events__term *term;
 
-	ABORT_ON(parse_events__new_term(&term, $1, NULL, NULL, $3));
+	ABORT_ON(parse_events__term_num(&term, $1, NULL, $3));
 	$$ = term;
 }
 |
@@ -211,7 +211,7 @@ PE_TERM
 {
 	struct parse_events__term *term;
 
-	ABORT_ON(parse_events__new_term(&term, $1, NULL, NULL, 1));
+	ABORT_ON(parse_events__term_num(&term, $1, NULL, 1));
 	$$ = term;
 }
 
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index cb08a11..8ee219b 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -225,7 +225,7 @@ static int pmu_config_term(struct list_head *formats,
 	if (parse_events__is_hardcoded_term(term))
 		return 0;
 
-	if (term->type != PARSE_EVENTS__TERM_TYPE_NUM)
+	if (term->type_val != PARSE_EVENTS__TERM_TYPE_NUM)
 		return -EINVAL;
 
 	format = pmu_find_format(formats, term->config);
@@ -246,6 +246,11 @@ static int pmu_config_term(struct list_head *formats,
 		return -EINVAL;
 	}
 
+	/*
+	 * XXX If we ever decide to go with string values for
+	 * non-hardcoded terms, here's the place to translate
+	 * them into value.
+	 */
 	*vp |= pmu_format_value(format->bits, term->val.num);
 	return 0;
 }
@@ -324,49 +329,58 @@ static struct test_format {
 /* Simulated users input. */
 static struct parse_events__term test_terms[] = {
 	{
-		.config  = (char *) "krava01",
-		.val.num = 15,
-		.type    = PARSE_EVENTS__TERM_TYPE_NUM,
+		.config    = (char *) "krava01",
+		.val.num   = 15,
+		.type_val  = PARSE_EVENTS__TERM_TYPE_NUM,
+		.type_term = PARSE_EVENTS__TERM_TYPE_USER,
 	},
 	{
-		.config  = (char *) "krava02",
-		.val.num = 170,
-		.type    = PARSE_EVENTS__TERM_TYPE_NUM,
+		.config    = (char *) "krava02",
+		.val.num   = 170,
+		.type_val  = PARSE_EVENTS__TERM_TYPE_NUM,
+		.type_term = PARSE_EVENTS__TERM_TYPE_USER,
 	},
 	{
-		.config  = (char *) "krava03",
-		.val.num = 1,
-		.type    = PARSE_EVENTS__TERM_TYPE_NUM,
+		.config    = (char *) "krava03",
+		.val.num   = 1,
+		.type_val  = PARSE_EVENTS__TERM_TYPE_NUM,
+		.type_term = PARSE_EVENTS__TERM_TYPE_USER,
 	},
 	{
-		.config  = (char *) "krava11",
-		.val.num = 27,
-		.type    = PARSE_EVENTS__TERM_TYPE_NUM,
+		.config    = (char *) "krava11",
+		.val.num   = 27,
+		.type_val  = PARSE_EVENTS__TERM_TYPE_NUM,
+		.type_term = PARSE_EVENTS__TERM_TYPE_USER,
 	},
 	{
-		.config  = (char *) "krava12",
-		.val.num = 1,
-		.type    = PARSE_EVENTS__TERM_TYPE_NUM,
+		.config    = (char *) "krava12",
+		.val.num   = 1,
+		.type_val  = PARSE_EVENTS__TERM_TYPE_NUM,
+		.type_term = PARSE_EVENTS__TERM_TYPE_USER,
 	},
 	{
-		.config  = (char *) "krava13",
-		.val.num = 2,
-		.type    = PARSE_EVENTS__TERM_TYPE_NUM,
+		.config    = (char *) "krava13",
+		.val.num   = 2,
+		.type_val  = PARSE_EVENTS__TERM_TYPE_NUM,
+		.type_term = PARSE_EVENTS__TERM_TYPE_USER,
 	},
 	{
-		.config  = (char *) "krava21",
-		.val.num = 119,
-		.type    = PARSE_EVENTS__TERM_TYPE_NUM,
+		.config    = (char *) "krava21",
+		.val.num   = 119,
+		.type_val  = PARSE_EVENTS__TERM_TYPE_NUM,
+		.type_term = PARSE_EVENTS__TERM_TYPE_USER,
 	},
 	{
-		.config  = (char *) "krava22",
-		.val.num = 11,
-		.type    = PARSE_EVENTS__TERM_TYPE_NUM,
+		.config    = (char *) "krava22",
+		.val.num   = 11,
+		.type_val  = PARSE_EVENTS__TERM_TYPE_NUM,
+		.type_term = PARSE_EVENTS__TERM_TYPE_USER,
 	},
 	{
-		.config  = (char *) "krava23",
-		.val.num = 2,
-		.type    = PARSE_EVENTS__TERM_TYPE_NUM,
+		.config    = (char *) "krava23",
+		.val.num   = 2,
+		.type_val  = PARSE_EVENTS__TERM_TYPE_NUM,
+		.type_term = PARSE_EVENTS__TERM_TYPE_USER,
 	},
 };
 #define TERMS_CNT (sizeof(test_terms) / sizeof(struct parse_events__term))
-- 
1.7.9.2.358.g22243


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

* [PATCH 6/6] perf evsel: Create events initially disabled -- again
  2012-05-20  2:16 [GIT PULL 0/6] perf/core fixes Arnaldo Carvalho de Melo
                   ` (4 preceding siblings ...)
  2012-05-20  2:16 ` [PATCH 5/6] perf tools: Split term type into value type and term type Arnaldo Carvalho de Melo
@ 2012-05-20  2:16 ` Arnaldo Carvalho de Melo
  2012-05-20  2:55 ` [GIT PULL 0/6] perf/core fixes David Ahern
  2012-05-21  7:19 ` Ingo Molnar
  7 siblings, 0 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-05-20  2:16 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, David Ahern, Arnaldo Carvalho de Melo

From: David Ahern <dsahern@gmail.com>

764e16a changed perf-record to create events disabled by default and
enable them once perf initializations are done. This setting was dropped
by 0f82ebc. Now perf events are once again generated during perf's
initialization phase (e.g., generating maps).

As an example, perf opens a lot of files at startup. Unpatched:

perf record -e syscalls:sys_enter_open -ga -fo /tmp/perf.data -- sleep 1
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.087 MB /tmp/perf.data (~3798 samples) ]

Using perf-script to look at the samples shows the perf command generating
563 of the 566 total events.

Patched:

perf record -e syscalls:sys_enter_open -ga -fo /tmp/perf.data -- sleep 1
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.028 MB /tmp/perf.data (~1206 samples) ]

Using perf-script to look at the samples does not show perf command.

Signed-off-by: David Ahern <dsahern@gmail.com>
Link: http://lkml.kernel.org/r/1336968088-11531-1-git-send-email-dsahern@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/evsel.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index d7a2b4b..f4f427c 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -70,6 +70,7 @@ void perf_evsel__config(struct perf_evsel *evsel, struct perf_record_opts *opts,
 	struct perf_event_attr *attr = &evsel->attr;
 	int track = !evsel->idx; /* only the first counter needs these */
 
+	attr->disabled = 1;
 	attr->sample_id_all = opts->sample_id_all_missing ? 0 : 1;
 	attr->inherit	    = !opts->no_inherit;
 	attr->read_format   = PERF_FORMAT_TOTAL_TIME_ENABLED |
@@ -138,7 +139,6 @@ void perf_evsel__config(struct perf_evsel *evsel, struct perf_record_opts *opts,
 
 	if (perf_target__none(&opts->target) &&
 	    (!opts->group || evsel == first)) {
-		attr->disabled = 1;
 		attr->enable_on_exec = 1;
 	}
 }
-- 
1.7.9.2.358.g22243


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

* Re: [GIT PULL 0/6] perf/core fixes
  2012-05-20  2:16 [GIT PULL 0/6] perf/core fixes Arnaldo Carvalho de Melo
                   ` (5 preceding siblings ...)
  2012-05-20  2:16 ` [PATCH 6/6] perf evsel: Create events initially disabled -- again Arnaldo Carvalho de Melo
@ 2012-05-20  2:55 ` David Ahern
  2012-05-21  7:19 ` Ingo Molnar
  7 siblings, 0 replies; 9+ messages in thread
From: David Ahern @ 2012-05-20  2:55 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, linux-kernel, Arnaldo Carvalho de Melo,
	Corey Ashford, Frederic Weisbecker, Jiri Olsa, Linus Torvalds,
	Namhyung Kim, Namhyung Kim, Paul Mackerras, Peter Zijlstra,
	Robert Richter, Stephane Eranian, arnaldo.melo,
	Arnaldo Carvalho de Melo

On 5/19/12 8:16 PM, Arnaldo Carvalho de Melo wrote:

> Arnaldo Carvalho de Melo (1):
>        Merge remote-tracking branch 'tip/perf/urgent' into perf/core
>
> David Ahern (1):
>        perf evsel: Create events initially disabled -- again
>
> Jiri Olsa (2):
>        perf hists: Fix callchain ip printf format
>        perf tools: Split term type into value type and term type
>
> Namhyung Kim (3):
>        perf target: Rename functions to avoid double negation
>        Revert 'perf evlist: Fix creation of cpu map'
>        perf target: Add uses_mmap field

What about Stephane's pipe mode patch set? The first 3 and the last one 
of the 5 are good fixes to pick up.

David

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

* Re: [GIT PULL 0/6] perf/core fixes
  2012-05-20  2:16 [GIT PULL 0/6] perf/core fixes Arnaldo Carvalho de Melo
                   ` (6 preceding siblings ...)
  2012-05-20  2:55 ` [GIT PULL 0/6] perf/core fixes David Ahern
@ 2012-05-21  7:19 ` Ingo Molnar
  7 siblings, 0 replies; 9+ messages in thread
From: Ingo Molnar @ 2012-05-21  7:19 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Corey Ashford, David Ahern, Frederic Weisbecker,
	Jiri Olsa, Linus Torvalds, Namhyung Kim, Namhyung Kim,
	Paul Mackerras, Peter Zijlstra, Robert Richter, Stephane Eranian,
	arnaldo.melo, Arnaldo Carvalho de Melo


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

> Hi Ingo,
> 
> 	This also includes a merge with tip/perf/urgent, please consider
> pulling,
> 
> - Arnaldo
> 
> The following changes since commit 978da300c7a65494692b329a6a4cbf364afc37c5:
> 
>   perf/x86/ibs: Fix undefined reference to `get_ibs_caps' (2012-05-14 14:31:35 +0200)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux perf/core
> 
> for you to fetch changes up to 5e1c81d98a5621007824b49dde556fead5ff9c6c:
> 
>   perf evsel: Create events initially disabled -- again (2012-05-18 16:02:42 -0300)
> 
> ----------------------------------------------------------------
> Fixes for perf/core:
> 
> . Rename some perf_target methods to avoid double negation, from Namhyung Kim.
> 
> . Revert change to use per task events with inheritance, from Namhyung Kim.
> 
> . Events should start disabled till children starts running, from David Ahern.
> 
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> ----------------------------------------------------------------
> Arnaldo Carvalho de Melo (1):
>       Merge remote-tracking branch 'tip/perf/urgent' into perf/core
> 
> David Ahern (1):
>       perf evsel: Create events initially disabled -- again
> 
> Jiri Olsa (2):
>       perf hists: Fix callchain ip printf format
>       perf tools: Split term type into value type and term type
> 
> Namhyung Kim (3):
>       perf target: Rename functions to avoid double negation
>       Revert 'perf evlist: Fix creation of cpu map'
>       perf target: Add uses_mmap field

Pulled, thanks a lot Arnaldo!

This should also fix the regression Linus reported, AFAICS.

Thanks,

	Ingo

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

end of thread, other threads:[~2012-05-21  7:19 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-20  2:16 [GIT PULL 0/6] perf/core fixes Arnaldo Carvalho de Melo
2012-05-20  2:16 ` [PATCH 1/6] perf target: Rename functions to avoid double negation Arnaldo Carvalho de Melo
2012-05-20  2:16 ` [PATCH 2/6] Revert 'perf evlist: Fix creation of cpu map' Arnaldo Carvalho de Melo
2012-05-20  2:16 ` [PATCH 3/6] perf target: Add uses_mmap field Arnaldo Carvalho de Melo
2012-05-20  2:16 ` [PATCH 4/6] perf hists: Fix callchain ip printf format Arnaldo Carvalho de Melo
2012-05-20  2:16 ` [PATCH 5/6] perf tools: Split term type into value type and term type Arnaldo Carvalho de Melo
2012-05-20  2:16 ` [PATCH 6/6] perf evsel: Create events initially disabled -- again Arnaldo Carvalho de Melo
2012-05-20  2:55 ` [GIT PULL 0/6] perf/core fixes David Ahern
2012-05-21  7:19 ` Ingo Molnar

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).