linux-kernel - Re: [GIT PULL 0/8] Annotation weekly ponies delivery

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 15 May 2012 19:07:13 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	Namhyung Kim <namhyung@...il.com>
Cc:	Ingo Molnar <mingo@...nel.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Arnaldo Carvalho de Melo <acme@...radead.org>,
	linux-kernel@...r.kernel.org,
	Arjan van de Ven <arjan@...radead.org>,
	David Ahern <dsahern@...il.com>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Mike Galbraith <efault@....de>,
	Paul Mackerras <paulus@...ba.org>,
	Stephane Eranian <eranian@...gle.com>, arnaldo.melo@...il.com,
	Arnaldo Carvalho de Melo <acme@...hat.com>
Subject: Re: [GIT PULL 0/8] Annotation weekly ponies delivery

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