From: Peter Zijlstra <peterz@infradead.org>
To: Namhyung Kim <namhyung@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>,
Linus Torvalds <torvalds@linux-foundation.org>,
Arnaldo Carvalho de Melo <acme@infradead.org>,
linux-kernel@vger.kernel.org,
Arjan van de Ven <arjan@infradead.org>,
David Ahern <dsahern@gmail.com>,
Frederic Weisbecker <fweisbec@gmail.com>,
Mike Galbraith <efault@gmx.de>, Paul Mackerras <paulus@samba.org>,
Stephane Eranian <eranian@google.com>,
arnaldo.melo@gmail.com,
Arnaldo Carvalho de Melo <acme@redhat.com>
Subject: Re: [GIT PULL 0/8] Annotation weekly ponies delivery
Date: Tue, 15 May 2012 19:07:13 +0200 [thread overview]
Message-ID: <1337101633.27694.92.camel@twins> (raw)
In-Reply-To: <1337093080.1493.22.camel@leonhard>
On Tue, 2012-05-15 at 23:44 +0900, Namhyung Kim wrote:
> Got it. So it means that we do need to create an event for each cpu
> in order to profile a task (and its children), right? (Originally, I
> thought it's a bug :-p)
Almost, to profile a task you can get away with a per-task event, but to
add the 'and its children', you need per-task-per-cpu.
This is because while one task will only ever run on one cpu at the time
(spare scheduler bugs :-), multiple tasks can run on multiple cpus. So
if you have one event all output will need to go to the one buffer
associated with it, causing multiple cpus to write to the one buffer.
This creates resource contention to the point that a 64-cpu machine
would appear deadlocked (the original perf had it this way and made some
people very unhappy).
> If so, yes, the commit 55261f46702c ("perf evlist: Fix creation of
> cpu map") should be reverted like below (note that target->cpu_list
> check no longer needed since perf_target__validate() will handle
> those cases). If it looks ok to you guys, I'll send a formal patch
> with name changes (to avoid the double negation suggested by Ingo):
>
>
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 1201daf71719..f6979ba391d1 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
> if (evlist->threads == NULL)
> return -1;
>
> - if (!perf_target__no_cpu(target))
> - evlist->cpus = cpu_map__new(target->cpu_list);
> - else
> + if (!perf_target__no_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;
This does indeed make it work again:
pre patch:
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.005 MB perf.data (~212 samples) ]
post patch:
[ perf record: Woken up 209 times to write data ]
[ perf record: Captured and wrote 54.931 MB perf.data (~2399974 samples) ]
So please do send as a proper patch, and you can add:
Acked-and-tested-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
next prev parent reply other threads:[~2012-05-15 17:07 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-05-12 19:52 [GIT PULL 0/8] Annotation weekly ponies delivery Arnaldo Carvalho de Melo
2012-05-12 19:53 ` [PATCH 1/8] perf annotate: Use raw form for register indirect call instructions Arnaldo Carvalho de Melo
2012-05-12 19:53 ` [PATCH 2/8] perf annotate: Resolve symbols using objdump comment Arnaldo Carvalho de Melo
2012-05-12 19:53 ` [PATCH 3/8] perf annotate: Resolve symbols using objdump comment for single op ins Arnaldo Carvalho de Melo
2012-05-12 19:53 ` [PATCH 4/8] perf annotate: Augment lock instruction output Arnaldo Carvalho de Melo
2012-05-12 19:53 ` [PATCH 5/8] perf annotate: Introduce ->free() method in ins_ops Arnaldo Carvalho de Melo
2012-05-12 19:53 ` [PATCH 6/8] perf annotate browser: Count the numbers of jump sources to a target Arnaldo Carvalho de Melo
2012-05-12 19:53 ` [PATCH 7/8] perf annotate browser: Show 'jumpy' functions Arnaldo Carvalho de Melo
2012-05-12 19:53 ` [PATCH 8/8] perf annotate browser: Add key bindings help window Arnaldo Carvalho de Melo
2012-05-12 20:40 ` [GIT PULL 0/8] Annotation weekly ponies delivery Linus Torvalds
2012-05-12 21:25 ` Arnaldo Carvalho de Melo
2012-05-12 22:29 ` Linus Torvalds
2012-05-14 7:05 ` Ingo Molnar
2012-05-14 11:13 ` Peter Zijlstra
2012-05-14 11:55 ` Ingo Molnar
2012-05-14 15:06 ` Namhyung Kim
2012-05-15 4:48 ` Namhyung Kim
2012-05-15 10:32 ` Peter Zijlstra
2012-05-15 14:44 ` Namhyung Kim
2012-05-15 17:07 ` Peter Zijlstra [this message]
2012-05-16 5:57 ` Namhyung Kim
2012-05-16 8:19 ` Namhyung Kim
2012-05-17 0:13 ` Namhyung Kim
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1337101633.27694.92.camel@twins \
--to=peterz@infradead.org \
--cc=acme@infradead.org \
--cc=acme@redhat.com \
--cc=arjan@infradead.org \
--cc=arnaldo.melo@gmail.com \
--cc=dsahern@gmail.com \
--cc=efault@gmx.de \
--cc=eranian@google.com \
--cc=fweisbec@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=namhyung@gmail.com \
--cc=paulus@samba.org \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox