| 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
| ||
|
Date: Fri, 29 Jul 2016 20:56:32 +0800
From: Bryton Lee <brytonlee01@...il.com>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
Cc: Ingo Molnar <mingo@...nel.org>, linux-kernel@...r.kernel.org,
Namhyung Kim <namhyung@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>,
Andi Kleen <andi@...stfloor.org>,
David Ahern <dsahern@...il.com>,
Frederic Weisbecker <fweisbec@...il.com>,
Jiri Olsa <jolsa@...hat.com>,
Peter Zijlstra <a.p.zijlstra@...llo.nl>,
Stephane Eranian <eranian@...gle.com>,
Arnaldo Carvalho de Melo <acme@...hat.com>
Subject: Re: [PATCH 13/18] perf symbols: Protect dso cache tree using dso->lock
Sorry for disturb you! maybe my question isn't a good question, but I
do want to know what situation made dwarf callchain unwind process
concurrently, can you guys give a example to elaborate on that. thank
you very much!
On Mon, May 18, 2015 at 11:50 PM, Arnaldo Carvalho de Melo
<acme@...nel.org> wrote:
> From: Namhyung Kim <namhyung@...nel.org>
>
> The dso cache is accessed during dwarf callchain unwind and it might be
> processed concurrently. Protect it under dso->lock.
>
> Note that it doesn't protect dso_cache__find(). I think it's safe to
> access to the cache tree without the lock since we don't delete nodes.
>
> It it missed an existing node due to rotation, it'll find it during
> dso_cache__insert() anyway.
>
> Signed-off-by: Namhyung Kim <namhyung@...nel.org>
> Cc: Adrian Hunter <adrian.hunter@...el.com>
> Cc: Andi Kleen <andi@...stfloor.org>
> Cc: David Ahern <dsahern@...il.com>
> Cc: Frederic Weisbecker <fweisbec@...il.com>
> Cc: Jiri Olsa <jolsa@...hat.com>
> Cc: Peter Zijlstra <a.p.zijlstra@...llo.nl>
> Cc: Stephane Eranian <eranian@...gle.com>
> Link: http://lkml.kernel.org/r/1431909055-21442-27-git-send-email-namhyung@kernel.org
> Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.com>
> ---
> tools/perf/util/dso.c | 34 +++++++++++++++++++++++++++-------
> 1 file changed, 27 insertions(+), 7 deletions(-)
>
> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index 482d6024ef13..666e1db44541 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -495,10 +495,12 @@ bool dso__data_status_seen(struct dso *dso, enum dso_data_status_seen by)
> }
>
> static void
> -dso_cache__free(struct rb_root *root)
> +dso_cache__free(struct dso *dso)
> {
> + struct rb_root *root = &dso->data.cache;
> struct rb_node *next = rb_first(root);
>
> + pthread_mutex_lock(&dso->lock);
> while (next) {
> struct dso_cache *cache;
>
> @@ -507,10 +509,12 @@ dso_cache__free(struct rb_root *root)
> rb_erase(&cache->rb_node, root);
> free(cache);
> }
> + pthread_mutex_unlock(&dso->lock);
> }
>
> -static struct dso_cache *dso_cache__find(const struct rb_root *root, u64 offset)
> +static struct dso_cache *dso_cache__find(struct dso *dso, u64 offset)
> {
> + const struct rb_root *root = &dso->data.cache;
> struct rb_node * const *p = &root->rb_node;
> const struct rb_node *parent = NULL;
> struct dso_cache *cache;
> @@ -529,17 +533,20 @@ static struct dso_cache *dso_cache__find(const struct rb_root *root, u64 offset)
> else
> return cache;
> }
> +
> return NULL;
> }
>
> -static void
> -dso_cache__insert(struct rb_root *root, struct dso_cache *new)
> +static struct dso_cache *
> +dso_cache__insert(struct dso *dso, struct dso_cache *new)
> {
> + struct rb_root *root = &dso->data.cache;
> struct rb_node **p = &root->rb_node;
> struct rb_node *parent = NULL;
> struct dso_cache *cache;
> u64 offset = new->offset;
>
> + pthread_mutex_lock(&dso->lock);
> while (*p != NULL) {
> u64 end;
>
> @@ -551,10 +558,17 @@ dso_cache__insert(struct rb_root *root, struct dso_cache *new)
> p = &(*p)->rb_left;
> else if (offset >= end)
> p = &(*p)->rb_right;
> + else
> + goto out;
> }
>
> rb_link_node(&new->rb_node, parent, p);
> rb_insert_color(&new->rb_node, root);
> +
> + cache = NULL;
> +out:
> + pthread_mutex_unlock(&dso->lock);
> + return cache;
> }
>
> static ssize_t
> @@ -572,6 +586,7 @@ static ssize_t
> dso_cache__read(struct dso *dso, u64 offset, u8 *data, ssize_t size)
> {
> struct dso_cache *cache;
> + struct dso_cache *old;
> ssize_t ret;
>
> do {
> @@ -591,7 +606,12 @@ dso_cache__read(struct dso *dso, u64 offset, u8 *data, ssize_t size)
>
> cache->offset = cache_offset;
> cache->size = ret;
> - dso_cache__insert(&dso->data.cache, cache);
> + old = dso_cache__insert(dso, cache);
> + if (old) {
> + /* we lose the race */
> + free(cache);
> + cache = old;
> + }
>
> ret = dso_cache__memcpy(cache, offset, data, size);
>
> @@ -608,7 +628,7 @@ static ssize_t dso_cache_read(struct dso *dso, u64 offset,
> {
> struct dso_cache *cache;
>
> - cache = dso_cache__find(&dso->data.cache, offset);
> + cache = dso_cache__find(dso, offset);
> if (cache)
> return dso_cache__memcpy(cache, offset, data, size);
> else
> @@ -964,7 +984,7 @@ void dso__delete(struct dso *dso)
>
> dso__data_close(dso);
> auxtrace_cache__free(dso->auxtrace_cache);
> - dso_cache__free(&dso->data.cache);
> + dso_cache__free(dso);
> dso__free_a2l(dso);
> zfree(&dso->symsrc_filename);
> pthread_mutex_destroy(&dso->lock);
> --
> 2.1.0
>
> --
> 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/
--
Best Regards
Bryton.Lee
Powered by blists - more mailing lists