From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Ingo Molnar <mingo@kernel.org>
Cc: linux-kernel@vger.kernel.org,
Arnaldo Carvalho de Melo <acme@redhat.com>,
Adrian Hunter <adrian.hunter@intel.com>,
David Ahern <dsahern@gmail.com>, Jiri Olsa <jolsa@redhat.com>,
Namhyung Kim <namhyung@kernel.org>
Subject: [PATCH 15/15] perf tools: Reference count struct dso
Date: Mon, 8 Jun 2015 11:17:16 -0300 [thread overview]
Message-ID: <1433773036-11525-16-git-send-email-acme@kernel.org> (raw)
In-Reply-To: <1433773036-11525-1-git-send-email-acme@kernel.org>
From: Arnaldo Carvalho de Melo <acme@redhat.com>
This has a different model than the 'thread' and 'map' struct lifetimes:
there is not a definitive "don't use this DSO anymore" event, i.e. we may
get many 'struct map' holding references to the '/usr/lib64/libc-2.20.so'
DSO but then at some point some DSO may have no references but we still
don't want to straight away release its resources, because "soon" we may
get a new 'struct map' that needs it and we want to reuse its symtab or
other resources.
So we need some way to garbage collect it when crossing some memory
usage threshold, which is left for anoter patch, for now it is
sufficient to release it when calling dsos__exit(), i.e. when deleting
the whole list as part of deleting the 'struct machine' containing it,
which will leave only referenced objects being used.
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: http://lkml.kernel.org/n/tip-majzgz07cm90t2tejrjy4clf@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/tests/dso-data.c | 4 ++--
tools/perf/tests/hists_common.c | 6 +++++-
tools/perf/util/dso.c | 37 ++++++++++++++++++++++++++++++++++++-
tools/perf/util/dso.h | 14 +++++++++++++-
| 1 +
tools/perf/util/machine.c | 15 +++++++++++----
tools/perf/util/map.c | 11 +++++++++--
tools/perf/util/probe-finder.c | 2 +-
tools/perf/util/symbol-elf.c | 2 +-
tools/perf/util/symbol.c | 2 +-
tools/perf/util/vdso.c | 1 +
11 files changed, 81 insertions(+), 14 deletions(-)
diff --git a/tools/perf/tests/dso-data.c b/tools/perf/tests/dso-data.c
index 3e41c61bd861..a218aeaf56a0 100644
--- a/tools/perf/tests/dso-data.c
+++ b/tools/perf/tests/dso-data.c
@@ -166,7 +166,7 @@ int test__dso_data(void)
free(buf);
}
- dso__delete(dso);
+ dso__put(dso);
unlink(file);
return 0;
}
@@ -226,7 +226,7 @@ static void dsos__delete(int cnt)
struct dso *dso = dsos[i];
unlink(dso->name);
- dso__delete(dso);
+ dso__put(dso);
}
free(dsos);
diff --git a/tools/perf/tests/hists_common.c b/tools/perf/tests/hists_common.c
index 915f60af6a0e..ce80b274b097 100644
--- a/tools/perf/tests/hists_common.c
+++ b/tools/perf/tests/hists_common.c
@@ -134,11 +134,15 @@ struct machine *setup_fake_machine(struct machines *machines)
sym = symbol__new(fsym->start, fsym->length,
STB_GLOBAL, fsym->name);
- if (sym == NULL)
+ if (sym == NULL) {
+ dso__put(dso);
goto out;
+ }
symbols__insert(&dso->symbols[MAP__FUNCTION], sym);
}
+
+ dso__put(dso);
}
return machine;
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index ff0204ac4321..7c0c08386a1d 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -1049,6 +1049,7 @@ struct dso *dso__new(const char *name)
INIT_LIST_HEAD(&dso->node);
INIT_LIST_HEAD(&dso->data.open_entry);
pthread_mutex_init(&dso->lock, NULL);
+ atomic_set(&dso->refcnt, 1);
}
return dso;
@@ -1083,6 +1084,19 @@ void dso__delete(struct dso *dso)
free(dso);
}
+struct dso *dso__get(struct dso *dso)
+{
+ if (dso)
+ atomic_inc(&dso->refcnt);
+ return dso;
+}
+
+void dso__put(struct dso *dso)
+{
+ if (dso && atomic_dec_and_test(&dso->refcnt))
+ dso__delete(dso);
+}
+
void dso__set_build_id(struct dso *dso, void *build_id)
{
memcpy(dso->build_id, build_id, sizeof(dso->build_id));
@@ -1153,6 +1167,27 @@ void __dsos__add(struct dsos *dsos, struct dso *dso)
{
list_add_tail(&dso->node, &dsos->head);
__dso__findlink_by_longname(&dsos->root, dso, NULL);
+ /*
+ * It is now in the linked list, grab a reference, then garbage collect
+ * this when needing memory, by looking at LRU dso instances in the
+ * list with atomic_read(&dso->refcnt) == 1, i.e. no references
+ * anywhere besides the one for the list, do, under a lock for the
+ * list: remove it from the list, then a dso__put(), that probably will
+ * be the last and will then call dso__delete(), end of life.
+ *
+ * That, or at the end of the 'struct machine' lifetime, when all
+ * 'struct dso' instances will be removed from the list, in
+ * dsos__exit(), if they have no other reference from some other data
+ * structure.
+ *
+ * E.g.: after processing a 'perf.data' file and storing references
+ * to objects instantiated while processing events, we will have
+ * references to the 'thread', 'map', 'dso' structs all from 'struct
+ * hist_entry' instances, but we may not need anything not referenced,
+ * so we might as well call machines__exit()/machines__delete() and
+ * garbage collect it.
+ */
+ dso__get(dso);
}
void dsos__add(struct dsos *dsos, struct dso *dso)
@@ -1206,7 +1241,7 @@ struct dso *dsos__findnew(struct dsos *dsos, const char *name)
{
struct dso *dso;
pthread_rwlock_wrlock(&dsos->lock);
- dso = __dsos__findnew(dsos, name);
+ dso = dso__get(__dsos__findnew(dsos, name));
pthread_rwlock_unlock(&dsos->lock);
return dso;
}
diff --git a/tools/perf/util/dso.h b/tools/perf/util/dso.h
index c16ab5d849c3..2fe98bb0e95b 100644
--- a/tools/perf/util/dso.h
+++ b/tools/perf/util/dso.h
@@ -1,6 +1,7 @@
#ifndef __PERF_DSO
#define __PERF_DSO
+#include <linux/atomic.h>
#include <linux/types.h>
#include <linux/rbtree.h>
#include <stdbool.h>
@@ -179,7 +180,7 @@ struct dso {
void *priv;
u64 db_id;
};
-
+ atomic_t refcnt;
char name[0];
};
@@ -206,6 +207,17 @@ void dso__set_long_name(struct dso *dso, const char *name, bool name_allocated);
int dso__name_len(const struct dso *dso);
+struct dso *dso__get(struct dso *dso);
+void dso__put(struct dso *dso);
+
+static inline void __dso__zput(struct dso **dso)
+{
+ dso__put(*dso);
+ *dso = NULL;
+}
+
+#define dso__zput(dso) __dso__zput(&dso)
+
bool dso__loaded(const struct dso *dso, enum map_type type);
bool dso__sorted_by_name(const struct dso *dso, enum map_type type);
--git a/tools/perf/util/header.c b/tools/perf/util/header.c
index ac5aaaeed7ff..21a77e7a171e 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -1277,6 +1277,7 @@ static int __event_process_build_id(struct build_id_event *bev,
sbuild_id);
pr_debug("build id event received for %s: %s\n",
dso->long_name, sbuild_id);
+ dso__put(dso);
}
err = 0;
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 0cf56d6f073a..132e35765101 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -82,7 +82,7 @@ out_delete:
return NULL;
}
-static void dsos__exit(struct dsos *dsos)
+static void dsos__purge(struct dsos *dsos)
{
struct dso *pos, *n;
@@ -90,12 +90,16 @@ static void dsos__exit(struct dsos *dsos)
list_for_each_entry_safe(pos, n, &dsos->head, node) {
RB_CLEAR_NODE(&pos->rb_node);
- list_del(&pos->node);
- dso__delete(pos);
+ list_del_init(&pos->node);
+ dso__put(pos);
}
pthread_rwlock_unlock(&dsos->lock);
+}
+static void dsos__exit(struct dsos *dsos)
+{
+ dsos__purge(dsos);
pthread_rwlock_destroy(&dsos->lock);
}
@@ -524,6 +528,7 @@ static struct dso *machine__findnew_module_dso(struct machine *machine,
dso__set_long_name(dso, strdup(filename), true);
}
+ dso__get(dso);
out_unlock:
pthread_rwlock_unlock(&machine->dsos.lock);
return dso;
@@ -1205,8 +1210,10 @@ static int machine__process_kernel_mmap_event(struct machine *machine,
goto out_problem;
kernel->kernel = kernel_type;
- if (__machine__create_kernel_maps(machine, kernel) < 0)
+ if (__machine__create_kernel_maps(machine, kernel) < 0) {
+ dso__put(kernel);
goto out_problem;
+ }
if (strstr(kernel->long_name, "vmlinux"))
dso__set_short_name(kernel, "[kernel.vmlinux]", false);
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 365011c233a6..1241ab989cf5 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -132,7 +132,7 @@ void map__init(struct map *map, enum map_type type,
map->end = end;
map->pgoff = pgoff;
map->reloc = 0;
- map->dso = dso;
+ map->dso = dso__get(dso);
map->map_ip = map__map_ip;
map->unmap_ip = map__unmap_ip;
RB_CLEAR_NODE(&map->rb_node);
@@ -198,6 +198,7 @@ struct map *map__new(struct machine *machine, u64 start, u64 len,
if (type != MAP__FUNCTION)
dso__set_loaded(dso, map->type);
}
+ dso__put(dso);
}
return map;
out_delete:
@@ -224,9 +225,15 @@ struct map *map__new2(u64 start, struct dso *dso, enum map_type type)
return map;
}
-void map__delete(struct map *map)
+static void map__exit(struct map *map)
{
BUG_ON(!RB_EMPTY_NODE(&map->rb_node));
+ dso__zput(map->dso);
+}
+
+void map__delete(struct map *map)
+{
+ map__exit(map);
free(map);
}
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index c50da392e256..2da65a710893 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -130,7 +130,7 @@ struct debuginfo *debuginfo__new(const char *path)
continue;
dinfo = __debuginfo__new(buf);
}
- dso__delete(dso);
+ dso__put(dso);
out:
/* if failed to open all distro debuginfo, open given binary */
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index a93ba85509b2..65f7e389ae09 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -1016,7 +1016,7 @@ int dso__load_sym(struct dso *dso, struct map *map,
curr_map = map__new2(start, curr_dso,
map->type);
if (curr_map == NULL) {
- dso__delete(curr_dso);
+ dso__put(curr_dso);
goto out_elf_end;
}
if (adjust_kernel_syms) {
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index eaee5d32d39d..504f2d73b7ee 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -786,7 +786,7 @@ static int dso__split_kallsyms(struct dso *dso, struct map *map, u64 delta,
curr_map = map__new2(pos->start, ndso, map->type);
if (curr_map == NULL) {
- dso__delete(ndso);
+ dso__put(ndso);
return -1;
}
diff --git a/tools/perf/util/vdso.c b/tools/perf/util/vdso.c
index c646c74c34f8..4b89118f158d 100644
--- a/tools/perf/util/vdso.c
+++ b/tools/perf/util/vdso.c
@@ -314,6 +314,7 @@ struct dso *machine__findnew_vdso(struct machine *machine,
}
out_unlock:
+ dso__get(dso);
pthread_rwlock_unlock(&machine->dsos.lock);
return dso;
}
--
2.1.0
next prev parent reply other threads:[~2015-06-08 14:20 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-08 14:17 [GIT PULL 00/15] perf/core improvements and fixes Arnaldo Carvalho de Melo
2015-06-08 14:17 ` [PATCH 01/15] perf stat: Add id into perf_stat struct Arnaldo Carvalho de Melo
2015-06-08 14:17 ` [PATCH 02/15] perf stat: Replace transaction event possition check with id check Arnaldo Carvalho de Melo
2015-06-08 14:17 ` [PATCH 03/15] perf stat: Remove setup_events function Arnaldo Carvalho de Melo
2015-06-08 14:17 ` [PATCH 04/15] perf stat: Remove transaction_run from shadow update/print code Arnaldo Carvalho de Melo
2015-06-08 14:17 ` [PATCH 05/15] perf stat: Introduce reset_shadow_stats function Arnaldo Carvalho de Melo
2015-06-08 14:17 ` [PATCH 06/15] perf stat: Introduce print_shadow_stats function Arnaldo Carvalho de Melo
2015-06-08 14:17 ` [PATCH 07/15] perf stat: Add output file argument to " Arnaldo Carvalho de Melo
2015-06-08 14:17 ` [PATCH 08/15] perf stat: Add aggr_mode " Arnaldo Carvalho de Melo
2015-06-08 14:17 ` [PATCH 09/15] perf stat: Move shadow stat counters into separate object Arnaldo Carvalho de Melo
2015-06-08 14:17 ` [PATCH 10/15] tools lib traceevent: Export dynamic symbols used by traceevent plugins Arnaldo Carvalho de Melo
2015-06-08 14:17 ` [PATCH 11/15] tools lib traceevent: Ignore libtrace-dynamic-list file Arnaldo Carvalho de Melo
2015-06-08 14:17 ` [PATCH 12/15] perf record: Fix perf.data size in no-buildid mode Arnaldo Carvalho de Melo
2015-06-08 14:17 ` [PATCH 13/15] perf machine: Fix up some more method names Arnaldo Carvalho de Melo
2015-06-08 14:17 ` [PATCH 14/15] perf tools: Protect accesses the dso rbtrees/lists with a rw lock Arnaldo Carvalho de Melo
2015-06-08 14:17 ` Arnaldo Carvalho de Melo [this message]
2015-06-09 9:47 ` [GIT PULL 00/15] perf/core improvements and fixes Ingo Molnar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1433773036-11525-16-git-send-email-acme@kernel.org \
--to=acme@kernel.org \
--cc=acme@redhat.com \
--cc=adrian.hunter@intel.com \
--cc=dsahern@gmail.com \
--cc=jolsa@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=namhyung@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.