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

[Arnaldo Carvalho de Melo]

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@xxxxxx>

> > > 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/