linux-kernel - Re: [PATCH 6/8] perf symbols: Improve DSO long names lookup speed with rbtree

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Tue, 14 Oct 2014 15:03:53 -0300
From:	Arnaldo Carvalho de Melo <acme@...nel.org>
To:	Jiri Olsa <jolsa@...hat.com>
Cc:	Ingo Molnar <mingo@...nel.org>, linux-kernel@...r.kernel.org,
	Waiman Long <Waiman.Long@...com>,
	Adrian Hunter <adrian.hunter@...el.com>,
	Don Zickus <dzickus@...hat.com>,
	Douglas Hatch <doug.hatch@...com>,
	Ingo Molnar <mingo@...hat.com>, Jiri Olsa <jolsa@...nel.org>,
	Namhyung Kim <namhyung@...nel.org>,
	Paul Mackerras <paulus@...ba.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Scott J Norton <scott.norton@...com>
Subject: Re: [PATCH 6/8] perf symbols: Improve DSO long names lookup speed
 with rbtree

Em Tue, Oct 14, 2014 at 02:34:03PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Oct 14, 2014 at 11:09:58AM +0200, Jiri Olsa escreveu:
> > On Wed, Oct 01, 2014 at 04:50:41PM -0300, Arnaldo Carvalho de Melo wrote:
> > > From: Waiman Long <Waiman.Long@...com>

> > > With workload that spawns and destroys many threads and processes, it
> > > was found that perf-mem could took a long time to post-process the perf
> > > data after the target workload had completed its operation.

> > > The performance bottleneck was found to be the lookup and insertion of
> > > the new DSO structures (thousands of them in this case).

> > this change segfaults (below) some tests, but only if I compiled
> > without DEBUG when I revert this commit, I can no longer reproduce..
 
> Reproduced, looking at it... 

Fixed, this happens because we end up using a struct machines on the
stack, and then machines__init() was not initializing the newly
introduced rb_root, just the existing list_head.

When we introduced struct dsos, to group the two ways to store dsos,
i.e. the linked list and the rbtree, we didn't turned the initialization
done in machines__init(machines->host) -> machine__init() ->
INIT_LIST_HEAD into a dsos__init() to keep on initializing the list_head
but _as well_ initializing the rb_root, oops. All worked because outside
perf-test we probably zalloc the whole thing which ends up initializing
it in to NULL.

So the problem looks contained to 'perf test' that uses it on stack,
etc.

Will add your Reported-by, but if you're quick, I can give you a
Tested-by too ;-)

- Arnaldo

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index b7d477fbda02..34fc7c8672e4 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -13,12 +13,18 @@
 #include <symbol/kallsyms.h>
 #include "unwind.h"
 
+static void dsos__init(struct dsos *dsos)
+{
+	INIT_LIST_HEAD(&dsos->head);
+	dsos->root = RB_ROOT;
+}
+
 int machine__init(struct machine *machine, const char *root_dir, pid_t pid)
 {
 	map_groups__init(&machine->kmaps);
 	RB_CLEAR_NODE(&machine->rb_node);
-	INIT_LIST_HEAD(&machine->user_dsos.head);
-	INIT_LIST_HEAD(&machine->kernel_dsos.head);
+	dsos__init(&machine->user_dsos);
+	dsos__init(&machine->kernel_dsos);
 
 	machine->threads = RB_ROOT;
 	INIT_LIST_HEAD(&machine->dead_threads);
--
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/