Mailing List Archive: Linux: Kernel

[PATCH 2/3] Revert 'perf evlist: Fix creation of cpu map'

Index | Next | Previous | View Threaded

namhyung.kim at lge

May 16, 2012, 2:45 AM

Post #1 of 4 (38 views)
Permalink

[PATCH 2/3] 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 [at] linux-foundation>
Analysed-by: Ingo Molnar <mingo [at] kernel>
Acked-and-tested-by: Peter Zijlstra <a.p.zijlstra [at] chello>
Signed-off-by: Namhyung Kim <namhyung.kim [at] lge>
---
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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

acme at ghostprotocols

May 16, 2012, 8:26 AM

Post #2 of 4 (35 views)
Permalink

Re: [PATCH 2/3] Revert 'perf evlist: Fix creation of cpu map' [In reply to]

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 [at] linux-foundation>
> Analysed-by: Ingo Molnar <mingo [at] kernel>
> Acked-and-tested-by: Peter Zijlstra <a.p.zijlstra [at] chello>

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

namhyung.kim at lge

May 17, 2012, 1:57 AM

Post #3 of 4 (36 views)
Permalink

Re: [PATCH 2/3] Revert 'perf evlist: Fix creation of cpu map' [In reply to]

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 [at] linux-foundation>
>> Analysed-by: Ingo Molnar <mingo [at] kernel>
>> Acked-and-tested-by: Peter Zijlstra <a.p.zijlstra [at] chello>
>
> 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)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

namhyung.kim at lge

May 17, 2012, 2:03 AM

Post #4 of 4 (36 views)
Permalink

Re: [PATCH 2/3] Revert 'perf evlist: Fix creation of cpu map' [In reply to]

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 [at] sejon: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)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact Gossamer Threads