All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] perf target: Rename functions to avoid double negation
@ 2012-05-16  9:45 Namhyung Kim
  2012-05-16  9:45 ` [PATCH 2/3] Revert 'perf evlist: Fix creation of cpu map' Namhyung Kim
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Namhyung Kim @ 2012-05-16  9:45 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML

Rename perf_target__no_{cpu,task} to perf_target__has_{cpu,task}
because it's more intuitive and easy to parse (for human-being)
when used with negation. The names are came out with 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.

Suggested-by: Ingo Molnar <mingo@kernel.org>
Suggested-by: David Ahern <dsahern@gmail.com>
Signed-off-by: Namhyung Kim <namhyung.kim@lge.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 d0605689bad9..0f4b51ae4be7 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 4eb6171e143b..553560a8b1be 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 1201daf71719..80bef281eee0 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 21eaab240396..d7a2b4b9801d 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 127cff3f8ced..c43f632955fa 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.10.1


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

* [PATCH 2/3] Revert 'perf evlist: Fix creation of cpu map'
  2012-05-16  9:45 [PATCH 1/3] perf target: Rename functions to avoid double negation Namhyung Kim
@ 2012-05-16  9:45 ` Namhyung Kim
  2012-05-16 15:26   ` Arnaldo Carvalho de Melo
  2012-05-21  7:38   ` [tip:perf/core] " tip-bot for Namhyung Kim
  2012-05-16  9:45 ` [PATCH 3/3] perf target: Add need_mmap field Namhyung Kim
  2012-05-21  7:37 ` [tip:perf/core] perf target: Rename functions to avoid double negation tip-bot for Namhyung Kim
  2 siblings, 2 replies; 12+ messages in thread
From: Namhyung Kim @ 2012-05-16  9:45 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML

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>
---
 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 80bef281eee0..87889f325678 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.10.1


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

* [PATCH 3/3] perf target: Add need_mmap field
  2012-05-16  9:45 [PATCH 1/3] perf target: Rename functions to avoid double negation Namhyung Kim
  2012-05-16  9:45 ` [PATCH 2/3] Revert 'perf evlist: Fix creation of cpu map' Namhyung Kim
@ 2012-05-16  9:45 ` Namhyung Kim
  2012-05-16 15:28   ` Arnaldo Carvalho de Melo
  2012-05-21  7:39   ` [tip:perf/core] perf target: Add uses_mmap field tip-bot for Namhyung Kim
  2012-05-21  7:37 ` [tip:perf/core] perf target: Rename functions to avoid double negation tip-bot for Namhyung Kim
  2 siblings, 2 replies; 12+ messages in thread
From: Namhyung Kim @ 2012-05-16  9:45 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML

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>
---
 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 d19058a7b84c..185e59a4715d 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		     = {
+			.need_mmap   = true,
+		},
 	},
 	.write_mode = WRITE_FORCE,
 	.file_new   = true,
diff --git a/tools/perf/builtin-test.c b/tools/perf/builtin-test.c
index 9d9abbbe23be..cd326f2b5488 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,
+			.need_mmap = true,
 		},
 		.no_delay   = true,
 		.freq	    = 10,
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 553560a8b1be..ebcd15883ab8 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		     = {
+			.need_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 87889f325678..ba43c3a4046c 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->need_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 c43f632955fa..ffa247d3ede3 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	     need_mmap;
 };
 
 enum perf_target_errno {
-- 
1.7.10.1


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

* Re: [PATCH 2/3] Revert 'perf evlist: Fix creation of cpu map'
  2012-05-16  9:45 ` [PATCH 2/3] Revert 'perf evlist: Fix creation of cpu map' Namhyung Kim
@ 2012-05-16 15:26   ` Arnaldo Carvalho de Melo
  2012-05-17  8:57     ` Namhyung Kim
  2012-05-21  7:38   ` [tip:perf/core] " tip-bot for Namhyung Kim
  1 sibling, 1 reply; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-05-16 15:26 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML

Em Wed, May 16, 2012 at 06:45:48PM +0900, Namhyung Kim escreveu:
> 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>

I'm applying it, but while trying to figure out if the logic was right I
tried:

perf top -C 0 -u acme

To check what is that this user is doing on that CPU, and its not
possible :-\

UID switch overriding CPU!

- Arnaldo

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

* Re: [PATCH 3/3] perf target: Add need_mmap field
  2012-05-16  9:45 ` [PATCH 3/3] perf target: Add need_mmap field Namhyung Kim
@ 2012-05-16 15:28   ` Arnaldo Carvalho de Melo
  2012-05-16 15:32     ` David Ahern
  2012-05-21  7:39   ` [tip:perf/core] perf target: Add uses_mmap field tip-bot for Namhyung Kim
  1 sibling, 1 reply; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-05-16 15:28 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML

Em Wed, May 16, 2012 at 06:45:49PM +0900, Namhyung Kim escreveu:
> 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.

Humm, perhaps target.no_mmap is better? I.e. just perf-stat would set
it.

- Arnaldo

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

* Re: [PATCH 3/3] perf target: Add need_mmap field
  2012-05-16 15:28   ` Arnaldo Carvalho de Melo
@ 2012-05-16 15:32     ` David Ahern
  2012-05-16 15:50       ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 12+ messages in thread
From: David Ahern @ 2012-05-16 15:32 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Namhyung Kim, Peter Zijlstra, Paul Mackerras, Ingo Molnar,
	Namhyung Kim, LKML

On 5/16/12 9:28 AM, Arnaldo Carvalho de Melo wrote:
> Em Wed, May 16, 2012 at 06:45:49PM +0900, Namhyung Kim escreveu:
>> 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.
>
> Humm, perhaps target.no_mmap is better? I.e. just perf-stat would set
> it.
>

That gets back to the potential double negation Ingo frowns upon (!no_mmap).

David

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

* Re: [PATCH 3/3] perf target: Add need_mmap field
  2012-05-16 15:32     ` David Ahern
@ 2012-05-16 15:50       ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 12+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-05-16 15:50 UTC (permalink / raw)
  To: David Ahern
  Cc: Namhyung Kim, Peter Zijlstra, Paul Mackerras, Ingo Molnar,
	Namhyung Kim, LKML

Em Wed, May 16, 2012 at 09:32:37AM -0600, David Ahern escreveu:
> On 5/16/12 9:28 AM, Arnaldo Carvalho de Melo wrote:
> >Em Wed, May 16, 2012 at 06:45:49PM +0900, Namhyung Kim escreveu:
> >>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.
> >
> >Humm, perhaps target.no_mmap is better? I.e. just perf-stat would set
> >it.
> >
> 
> That gets back to the potential double negation Ingo frowns upon (!no_mmap).

Ok, but this really doesn't feel like a good name :-\ Short of ideas
now tho.

- Arnaldo

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

* Re: [PATCH 2/3] Revert 'perf evlist: Fix creation of cpu map'
  2012-05-16 15:26   ` Arnaldo Carvalho de Melo
@ 2012-05-17  8:57     ` Namhyung Kim
  2012-05-17  9:03       ` Namhyung Kim
  0 siblings, 1 reply; 12+ messages in thread
From: Namhyung Kim @ 2012-05-17  8:57 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, LKML

Hi,

On Wed, 16 May 2012 12:26:19 -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, May 16, 2012 at 06:45:48PM +0900, Namhyung Kim escreveu:
>> 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>
>
> I'm applying it, but while trying to figure out if the logic was right I
> tried:
>
> perf top -C 0 -u acme
>
> To check what is that this user is doing on that CPU, and its not
> possible :-\
>
> UID switch overriding CPU!
>

Maybe I need to rethink about it since I wasn't aware of the
per-task-per-cpu events at that time.

AFAIK the uid switch is basically a same thing as pid/tid switch, so
your complain should be extended to them too. And I think we can remove
the check from perf_target__validate(). But before that something like
below is needed also IMHO:


diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index ebcd15883ab8..8e3cf429dd18 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -900,6 +900,9 @@ static void perf_top__start_counters(struct perf_top *top)
                        attr->read_format |= PERF_FORMAT_ID;
                }
 
+               if (perf_target__has_cpu(&top->target))
+                       attr->sample_type |= PERF_SAMPLE_CPU;
+
                if (symbol_conf.use_callchain)
                        attr->sample_type |= PERF_SAMPLE_CALLCHAIN;
 
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index d7a2b4b9801d..d26b8fe0abd1 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -107,7 +107,7 @@ void perf_evsel__config(struct perf_evsel *evsel, struct perf_record_opts *opts,
        if (opts->call_graph)
                attr->sample_type       |= PERF_SAMPLE_CALLCHAIN;
 
-       if (opts->target.system_wide)
+       if (perf_target__has_cpu(&opts->target))
                attr->sample_type       |= PERF_SAMPLE_CPU;
 
        if (opts->period)

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

* Re: [PATCH 2/3] Revert 'perf evlist: Fix creation of cpu map'
  2012-05-17  8:57     ` Namhyung Kim
@ 2012-05-17  9:03       ` Namhyung Kim
  0 siblings, 0 replies; 12+ messages in thread
From: Namhyung Kim @ 2012-05-17  9:03 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, LKML

Hi,

On Thu, 17 May 2012 17:57:16 +0900, Namhyung Kim wrote:
> On Wed, 16 May 2012 12:26:19 -0300, Arnaldo Carvalho de Melo wrote:
>> I'm applying it, but while trying to figure out if the logic was right I
>> tried:
>>
>> perf top -C 0 -u acme
>>
>> To check what is that this user is doing on that CPU, and its not
>> possible :-\
>>
>> UID switch overriding CPU!
>>
>
> Maybe I need to rethink about it since I wasn't aware of the
> per-task-per-cpu events at that time.
>
> AFAIK the uid switch is basically a same thing as pid/tid switch, so
> your complain should be extended to them too. And I think we can remove
> the check from perf_target__validate(). But before that something like
> below is needed also IMHO:
>

After applying the patch below and removing the check, I got:

namhyung@sejong:perf$ sudo ./perf top -C0 -u namhyung -s cpu,dso,sym --stdio


   PerfTop:       5 irqs/sec  kernel:100.0%  exact:  0.0% [1000Hz cycles],  (uid: namhyung, CPU: 0)
-------------------------------------------------------------------------------------------------------------------------------

    91.67%  1  [kernel]  [k] dequeue_entity          
     7.25%  5  [kernel]  [k] schedule_hrtimeout_range
     1.01%  1  [kernel]  [k] __perf_event_task_sched 
     0.06%  5  [kernel]  [k] native_write_msr_safe   
     0.01%  1  [kernel]  [k] native_write_msr_safe   
Failed to open /tmp/perf-1340.map, continuing without symbols


So definitely it needs more work...

Thanks,
Namhyung


>
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index ebcd15883ab8..8e3cf429dd18 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -900,6 +900,9 @@ static void perf_top__start_counters(struct perf_top *top)
>                         attr->read_format |= PERF_FORMAT_ID;
>                 }
>  
> +               if (perf_target__has_cpu(&top->target))
> +                       attr->sample_type |= PERF_SAMPLE_CPU;
> +
>                 if (symbol_conf.use_callchain)
>                         attr->sample_type |= PERF_SAMPLE_CALLCHAIN;
>  
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index d7a2b4b9801d..d26b8fe0abd1 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -107,7 +107,7 @@ void perf_evsel__config(struct perf_evsel *evsel, struct perf_record_opts *opts,
>         if (opts->call_graph)
>                 attr->sample_type       |= PERF_SAMPLE_CALLCHAIN;
>  
> -       if (opts->target.system_wide)
> +       if (perf_target__has_cpu(&opts->target))
>                 attr->sample_type       |= PERF_SAMPLE_CPU;
>  
>         if (opts->period)

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

* [tip:perf/core] perf target: Rename functions to avoid double negation
  2012-05-16  9:45 [PATCH 1/3] perf target: Rename functions to avoid double negation Namhyung Kim
  2012-05-16  9:45 ` [PATCH 2/3] Revert 'perf evlist: Fix creation of cpu map' Namhyung Kim
  2012-05-16  9:45 ` [PATCH 3/3] perf target: Add need_mmap field Namhyung Kim
@ 2012-05-21  7:37 ` tip-bot for Namhyung Kim
  2 siblings, 0 replies; 12+ messages in thread
From: tip-bot for Namhyung Kim @ 2012-05-21  7:37 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, mingo, hpa, mingo, a.p.zijlstra,
	namhyung.kim, namhyung, dsahern, tglx

Commit-ID:  aa22dd4990e38700b1855555aa0def5215859abb
Gitweb:     http://git.kernel.org/tip/aa22dd4990e38700b1855555aa0def5215859abb
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Wed, 16 May 2012 18:45:47 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 16 May 2012 12:09:34 -0300

perf target: Rename functions to avoid double negation

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 */

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

* [tip:perf/core] Revert 'perf evlist: Fix creation of cpu map'
  2012-05-16  9:45 ` [PATCH 2/3] Revert 'perf evlist: Fix creation of cpu map' Namhyung Kim
  2012-05-16 15:26   ` Arnaldo Carvalho de Melo
@ 2012-05-21  7:38   ` tip-bot for Namhyung Kim
  1 sibling, 0 replies; 12+ messages in thread
From: tip-bot for Namhyung Kim @ 2012-05-21  7:38 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, mingo, hpa, mingo, a.p.zijlstra,
	torvalds, namhyung.kim, namhyung, tglx

Commit-ID:  879d77d0cbe20ad1d6099a1e16f03b72c0649828
Gitweb:     http://git.kernel.org/tip/879d77d0cbe20ad1d6099a1e16f03b72c0649828
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Wed, 16 May 2012 18:45:48 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 16 May 2012 12:12:05 -0300

Revert 'perf evlist: Fix creation of cpu map'

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 files 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;

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

* [tip:perf/core] perf target: Add uses_mmap field
  2012-05-16  9:45 ` [PATCH 3/3] perf target: Add need_mmap field Namhyung Kim
  2012-05-16 15:28   ` Arnaldo Carvalho de Melo
@ 2012-05-21  7:39   ` tip-bot for Namhyung Kim
  1 sibling, 0 replies; 12+ messages in thread
From: tip-bot for Namhyung Kim @ 2012-05-21  7:39 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, linux-kernel, paulus, mingo, hpa, mingo, a.p.zijlstra,
	namhyung.kim, namhyung, tglx

Commit-ID:  d1cb9fce92c41454bd594fb0920575fc63301878
Gitweb:     http://git.kernel.org/tip/d1cb9fce92c41454bd594fb0920575fc63301878
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Wed, 16 May 2012 18:45:49 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 17 May 2012 12:32:54 -0300

perf target: Add uses_mmap field

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(+), 0 deletions(-)

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 {

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

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

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-16  9:45 [PATCH 1/3] perf target: Rename functions to avoid double negation Namhyung Kim
2012-05-16  9:45 ` [PATCH 2/3] Revert 'perf evlist: Fix creation of cpu map' Namhyung Kim
2012-05-16 15:26   ` Arnaldo Carvalho de Melo
2012-05-17  8:57     ` Namhyung Kim
2012-05-17  9:03       ` Namhyung Kim
2012-05-21  7:38   ` [tip:perf/core] " tip-bot for Namhyung Kim
2012-05-16  9:45 ` [PATCH 3/3] perf target: Add need_mmap field Namhyung Kim
2012-05-16 15:28   ` Arnaldo Carvalho de Melo
2012-05-16 15:32     ` David Ahern
2012-05-16 15:50       ` Arnaldo Carvalho de Melo
2012-05-21  7:39   ` [tip:perf/core] perf target: Add uses_mmap field tip-bot for Namhyung Kim
2012-05-21  7:37 ` [tip:perf/core] perf target: Rename functions to avoid double negation tip-bot for Namhyung Kim

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.