Messages in this thread Patch in this message | | | Date | Tue, 14 Oct 2014 15:03:53 -0300 | From | Arnaldo Carvalho de Melo <> | 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@hp.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);
| |