Messages in this thread | | | Date | Thu, 29 Jan 2015 09:34:45 -0300 | From | Arnaldo Carvalho de Melo <> | Subject | Re: [PATCH 27/42] perf tools: Protect dso symbol loading using a mutex |
| |
Em Thu, Jan 29, 2015 at 05:07:08PM +0900, Namhyung Kim escreveu: > When multi-thread support for perf report is enabled, it's possible to > access a dso concurrently. Add a new pthread_mutex to protect it from > concurrent dso__load(). > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > --- > tools/perf/util/dso.c | 2 ++ > tools/perf/util/dso.h | 1 + > tools/perf/util/symbol.c | 34 ++++++++++++++++++++++++---------- > 3 files changed, 27 insertions(+), 10 deletions(-) > > diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c > index 45be944d450a..3da75816b8f8 100644 > --- a/tools/perf/util/dso.c > +++ b/tools/perf/util/dso.c > @@ -888,6 +888,7 @@ struct dso *dso__new(const char *name) > RB_CLEAR_NODE(&dso->rb_node); > INIT_LIST_HEAD(&dso->node); > INIT_LIST_HEAD(&dso->data.open_entry); > + pthread_mutex_init(&dso->lock, NULL); > } > > return dso; > @@ -917,6 +918,7 @@ void dso__delete(struct dso *dso) > dso_cache__free(&dso->data.cache); > dso__free_a2l(dso); > zfree(&dso->symsrc_filename); > + pthread_mutex_destroy(&dso->lock); > free(dso); > } > > diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h > index 3782c82c6e44..ac753594a469 100644 > --- a/tools/perf/util/dso.h > +++ b/tools/perf/util/dso.h > @@ -102,6 +102,7 @@ struct dsos { > }; > > struct dso { > + pthread_mutex_t lock; > struct list_head node; > struct rb_node rb_node; /* rbtree node sorted by long name */ > struct rb_root symbols[MAP__NR_TYPES]; > diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c > index a69066865a55..714e20c99354 100644 > --- a/tools/perf/util/symbol.c > +++ b/tools/perf/util/symbol.c > @@ -1357,12 +1357,22 @@ int dso__load(struct dso *dso, struct map *map, symbol_filter_t filter) > struct symsrc *syms_ss = NULL, *runtime_ss = NULL; > bool kmod; > > - dso__set_loaded(dso, map->type); > + pthread_mutex_lock(&dso->lock); > + > + /* check again under the dso->lock */
Again? Where was it first checked? Perhaps we should lock there, so that we don't have to do two checks, one unlocked, the other locked?
> + if (dso__loaded(dso, map->type)) { > + ret = 1; > + goto out; > + } > +
- Arnaldo
| |