[PATCH v2 06/13] perf dsos: Switch more loops to dsos__for_each_dso

From: Ian Rogers
Date: Thu Mar 21 2024 - 12:05:02 EST

Next message: Ian Rogers: "[PATCH v2 07/13] perf dsos: Switch backing storage to array from rbtree/list"
Previous message: Ian Rogers: "[PATCH v2 05/13] perf dso: Move dso functions out of dsos"
In reply to: Ian Rogers: "[PATCH v2 05/13] perf dso: Move dso functions out of dsos"
Next in thread: Ian Rogers: "[PATCH v2 07/13] perf dsos: Switch backing storage to array from rbtree/list"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Switch loops within dsos.c, add a version that isn't locked. Switch
some unlocked loops to hold the read lock.

Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx>
---
tools/perf/util/build-id.c | 2 +-
tools/perf/util/dsos.c | 258 ++++++++++++++++++++++++-------------
tools/perf/util/dsos.h | 8 +-
tools/perf/util/machine.c | 8 +-
4 files changed, 174 insertions(+), 102 deletions(-)

diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index a6d3c253f19f..864bc26b6b46 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -964,7 +964,7 @@ int perf_session__cache_build_ids(struct perf_session *session)

static bool machine__read_build_ids(struct machine *machine, bool with_hits)
{
- return __dsos__read_build_ids(&machine->dsos, with_hits);
+ return dsos__read_build_ids(&machine->dsos, with_hits);
}

bool perf_session__read_build_ids(struct perf_session *session, bool with_hits)
diff --git a/tools/perf/util/dsos.c b/tools/perf/util/dsos.c
index f816927a21ff..b7fbfb877ae3 100644
--- a/tools/perf/util/dsos.c
+++ b/tools/perf/util/dsos.c
@@ -41,38 +41,65 @@ void dsos__exit(struct dsos *dsos)
exit_rwsem(&dsos->lock);
}

-bool __dsos__read_build_ids(struct dsos *dsos, bool with_hits)
+
+static int __dsos__for_each_dso(struct dsos *dsos,
+ int (*cb)(struct dso *dso, void *data),
+ void *data)
+{
+ struct dso *dso;
+
+ list_for_each_entry(dso, &dsos->head, node) {
+ int err;
+
+ err = cb(dso, data);
+ if (err)
+ return err;
+ }
+ return 0;
+}
+
+struct dsos__read_build_ids_cb_args {
+ bool with_hits;
+ bool have_build_id;
+};
+
+static int dsos__read_build_ids_cb(struct dso *dso, void *data)
{
- struct list_head *head = &dsos->head;
- bool have_build_id = false;
- struct dso *pos;
+ struct dsos__read_build_ids_cb_args *args = data;
struct nscookie nsc;

- list_for_each_entry(pos, head, node) {
- if (with_hits && !pos->hit && !dso__is_vdso(pos))
- continue;
- if (pos->has_build_id) {
- have_build_id = true;
- continue;
- }
- nsinfo__mountns_enter(pos->nsinfo, &nsc);
- if (filename__read_build_id(pos->long_name, &pos->bid) > 0) {
- have_build_id = true;
- pos->has_build_id = true;
- } else if (errno == ENOENT && pos->nsinfo) {
- char *new_name = dso__filename_with_chroot(pos, pos->long_name);
-
- if (new_name && filename__read_build_id(new_name,
- &pos->bid) > 0) {
- have_build_id = true;
- pos->has_build_id = true;
- }
- free(new_name);
+ if (args->with_hits && !dso->hit && !dso__is_vdso(dso))
+ return 0;
+ if (dso->has_build_id) {
+ args->have_build_id = true;
+ return 0;
+ }
+ nsinfo__mountns_enter(dso->nsinfo, &nsc);
+ if (filename__read_build_id(dso->long_name, &dso->bid) > 0) {
+ args->have_build_id = true;
+ dso->has_build_id = true;
+ } else if (errno == ENOENT && dso->nsinfo) {
+ char *new_name = dso__filename_with_chroot(dso, dso->long_name);
+
+ if (new_name && filename__read_build_id(new_name, &dso->bid) > 0) {
+ args->have_build_id = true;
+ dso->has_build_id = true;
}
- nsinfo__mountns_exit(&nsc);
+ free(new_name);
}
+ nsinfo__mountns_exit(&nsc);
+ return 0;
+}

- return have_build_id;
+bool dsos__read_build_ids(struct dsos *dsos, bool with_hits)
+{
+ struct dsos__read_build_ids_cb_args args = {
+ .with_hits = with_hits,
+ .have_build_id = false,
+ };
+
+ dsos__for_each_dso(dsos, dsos__read_build_ids_cb, &args);
+ return args.have_build_id;
}

static int __dso__cmp_long_name(const char *long_name, struct dso_id *id, struct dso *b)
@@ -105,6 +132,7 @@ struct dso *__dsos__findnew_link_by_longname_id(struct rb_root *root, struct dso

if (!name)
name = dso->long_name;
+
/*
* Find node with the matching name
*/
@@ -185,17 +213,40 @@ static struct dso *__dsos__findnew_by_longname_id(struct rb_root *root, const ch
return __dsos__findnew_link_by_longname_id(root, NULL, name, id);
}

+struct dsos__find_id_cb_args {
+ const char *name;
+ struct dso_id *id;
+ struct dso *res;
+};
+
+static int dsos__find_id_cb(struct dso *dso, void *data)
+{
+ struct dsos__find_id_cb_args *args = data;
+
+ if (__dso__cmp_short_name(args->name, args->id, dso) == 0) {
+ args->res = dso__get(dso);
+ return 1;
+ }
+ return 0;
+
+}
+
static struct dso *__dsos__find_id(struct dsos *dsos, const char *name, struct dso_id *id, bool cmp_short)
{
- struct dso *pos;
+ struct dso *res;

if (cmp_short) {
- list_for_each_entry(pos, &dsos->head, node)
- if (__dso__cmp_short_name(name, id, pos) == 0)
- return dso__get(pos);
- return NULL;
+ struct dsos__find_id_cb_args args = {
+ .name = name,
+ .id = id,
+ .res = NULL,
+ };
+
+ __dsos__for_each_dso(dsos, dsos__find_id_cb, &args);
+ return args.res;
}
- return __dsos__findnew_by_longname_id(&dsos->root, name, id);
+ res = __dsos__findnew_by_longname_id(&dsos->root, name, id);
+ return res;
}

struct dso *dsos__find(struct dsos *dsos, const char *name, bool cmp_short)
@@ -275,48 +326,74 @@ struct dso *dsos__findnew_id(struct dsos *dsos, const char *name, struct dso_id
return dso;
}

-size_t __dsos__fprintf_buildid(struct dsos *dsos, FILE *fp,
- bool (skip)(struct dso *dso, int parm), int parm)
-{
- struct list_head *head = &dsos->head;
- struct dso *pos;
- size_t ret = 0;
+struct dsos__fprintf_buildid_cb_args {
+ FILE *fp;
+ bool (*skip)(struct dso *dso, int parm);
+ int parm;
+ size_t ret;
+};

- list_for_each_entry(pos, head, node) {
- char sbuild_id[SBUILD_ID_SIZE];
+static int dsos__fprintf_buildid_cb(struct dso *dso, void *data)
+{
+ struct dsos__fprintf_buildid_cb_args *args = data;
+ char sbuild_id[SBUILD_ID_SIZE];

- if (skip && skip(pos, parm))
- continue;
- build_id__sprintf(&pos->bid, sbuild_id);
- ret += fprintf(fp, "%-40s %s\n", sbuild_id, pos->long_name);
- }
- return ret;
+ if (args->skip && args->skip(dso, args->parm))
+ return 0;
+ build_id__sprintf(&dso->bid, sbuild_id);
+ args->ret += fprintf(args->fp, "%-40s %s\n", sbuild_id, dso->long_name);
+ return 0;
}

-size_t __dsos__fprintf(struct dsos *dsos, FILE *fp)
+size_t dsos__fprintf_buildid(struct dsos *dsos, FILE *fp,
+ bool (*skip)(struct dso *dso, int parm), int parm)
{
- struct list_head *head = &dsos->head;
- struct dso *pos;
- size_t ret = 0;
+ struct dsos__fprintf_buildid_cb_args args = {
+ .fp = fp,
+ .skip = skip,
+ .parm = parm,
+ .ret = 0,
+ };
+
+ dsos__for_each_dso(dsos, dsos__fprintf_buildid_cb, &args);
+ return args.ret;
+}

- list_for_each_entry(pos, head, node) {
- ret += dso__fprintf(pos, fp);
- }
+struct dsos__fprintf_cb_args {
+ FILE *fp;
+ size_t ret;
+};

- return ret;
+static int dsos__fprintf_cb(struct dso *dso, void *data)
+{
+ struct dsos__fprintf_cb_args *args = data;
+
+ args->ret += dso__fprintf(dso, args->fp);
+ return 0;
}

-int __dsos__hit_all(struct dsos *dsos)
+size_t dsos__fprintf(struct dsos *dsos, FILE *fp)
{
- struct list_head *head = &dsos->head;
- struct dso *pos;
+ struct dsos__fprintf_cb_args args = {
+ .fp = fp,
+ .ret = 0,
+ };

- list_for_each_entry(pos, head, node)
- pos->hit = true;
+ dsos__for_each_dso(dsos, dsos__fprintf_cb, &args);
+ return args.ret;
+}

+static int dsos__hit_all_cb(struct dso *dso, void *data __maybe_unused)
+{
+ dso->hit = true;
return 0;
}

+int dsos__hit_all(struct dsos *dsos)
+{
+ return dsos__for_each_dso(dsos, dsos__hit_all_cb, NULL);
+}
+
struct dso *dsos__findnew_module_dso(struct dsos *dsos,
struct machine *machine,
struct kmod_path *m,
@@ -342,49 +419,44 @@ struct dso *dsos__findnew_module_dso(struct dsos *dsos,
return dso;
}

-struct dso *dsos__find_kernel_dso(struct dsos *dsos)
+static int dsos__find_kernel_dso_cb(struct dso *dso, void *data)
{
- struct dso *dso, *res = NULL;
+ struct dso **res = data;
+ /*
+ * The cpumode passed to is_kernel_module is not the cpumode of *this*
+ * event. If we insist on passing correct cpumode to is_kernel_module,
+ * we should record the cpumode when we adding this dso to the linked
+ * list.
+ *
+ * However we don't really need passing correct cpumode. We know the
+ * correct cpumode must be kernel mode (if not, we should not link it
+ * onto kernel_dsos list).
+ *
+ * Therefore, we pass PERF_RECORD_MISC_CPUMODE_UNKNOWN.
+ * is_kernel_module() treats it as a kernel cpumode.
+ */
+ if (!dso->kernel ||
+ is_kernel_module(dso->long_name, PERF_RECORD_MISC_CPUMODE_UNKNOWN))
+ return 0;

- down_read(&dsos->lock);
- list_for_each_entry(dso, &dsos->head, node) {
- /*
- * The cpumode passed to is_kernel_module is not the cpumode of
- * *this* event. If we insist on passing correct cpumode to
- * is_kernel_module, we should record the cpumode when we adding
- * this dso to the linked list.
- *
- * However we don't really need passing correct cpumode. We
- * know the correct cpumode must be kernel mode (if not, we
- * should not link it onto kernel_dsos list).
- *
- * Therefore, we pass PERF_RECORD_MISC_CPUMODE_UNKNOWN.
- * is_kernel_module() treats it as a kernel cpumode.
- */
- if (!dso->kernel ||
- is_kernel_module(dso->long_name,
- PERF_RECORD_MISC_CPUMODE_UNKNOWN))
- continue;
+ *res = dso__get(dso);
+ return 1;
+}

- res = dso__get(dso);
- break;
- }
- up_read(&dsos->lock);
+struct dso *dsos__find_kernel_dso(struct dsos *dsos)
+{
+ struct dso *res = NULL;
+
+ dsos__for_each_dso(dsos, dsos__find_kernel_dso_cb, &res);
return res;
}

int dsos__for_each_dso(struct dsos *dsos, int (*cb)(struct dso *dso, void *data), void *data)
{
- struct dso *dso;
+ int err;

down_read(&dsos->lock);
- list_for_each_entry(dso, &dsos->head, node) {
- int err;
-
- err = cb(dso, data);
- if (err)
- return err;
- }
+ err = __dsos__for_each_dso(dsos, cb, data);
up_read(&dsos->lock);
- return 0;
+ return err;
}
diff --git a/tools/perf/util/dsos.h b/tools/perf/util/dsos.h
index 317a263f0e37..50bd51523475 100644
--- a/tools/perf/util/dsos.h
+++ b/tools/perf/util/dsos.h
@@ -33,16 +33,16 @@ struct dso *dsos__find(struct dsos *dsos, const char *name, bool cmp_short);

struct dso *dsos__findnew_id(struct dsos *dsos, const char *name, struct dso_id *id);

-bool __dsos__read_build_ids(struct dsos *dsos, bool with_hits);
+bool dsos__read_build_ids(struct dsos *dsos, bool with_hits);

struct dso *__dsos__findnew_link_by_longname_id(struct rb_root *root, struct dso *dso,
const char *name, struct dso_id *id);

-size_t __dsos__fprintf_buildid(struct dsos *dsos, FILE *fp,
+size_t dsos__fprintf_buildid(struct dsos *dsos, FILE *fp,
bool (skip)(struct dso *dso, int parm), int parm);
-size_t __dsos__fprintf(struct dsos *dsos, FILE *fp);
+size_t dsos__fprintf(struct dsos *dsos, FILE *fp);

-int __dsos__hit_all(struct dsos *dsos);
+int dsos__hit_all(struct dsos *dsos);

struct dso *dsos__findnew_module_dso(struct dsos *dsos, struct machine *machine,
struct kmod_path *m, const char *filename);
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index bd153bc2c8da..97a0c21ba2a5 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -853,11 +853,11 @@ static struct map *machine__addnew_module_map(struct machine *machine, u64 start
size_t machines__fprintf_dsos(struct machines *machines, FILE *fp)
{
struct rb_node *nd;
- size_t ret = __dsos__fprintf(&machines->host.dsos, fp);
+ size_t ret = dsos__fprintf(&machines->host.dsos, fp);

for (nd = rb_first_cached(&machines->guests); nd; nd = rb_next(nd)) {
struct machine *pos = rb_entry(nd, struct machine, rb_node);
- ret += __dsos__fprintf(&pos->dsos, fp);
+ ret += dsos__fprintf(&pos->dsos, fp);
}

return ret;
@@ -866,7 +866,7 @@ size_t machines__fprintf_dsos(struct machines *machines, FILE *fp)
size_t machine__fprintf_dsos_buildid(struct machine *m, FILE *fp,
bool (skip)(struct dso *dso, int parm), int parm)
{
- return __dsos__fprintf_buildid(&m->dsos, fp, skip, parm);
+ return dsos__fprintf_buildid(&m->dsos, fp, skip, parm);
}

size_t machines__fprintf_dsos_buildid(struct machines *machines, FILE *fp,
@@ -3208,5 +3208,5 @@ bool machine__is_lock_function(struct machine *machine, u64 addr)

int machine__hit_all_dsos(struct machine *machine)
{
- return __dsos__hit_all(&machine->dsos);
+ return dsos__hit_all(&machine->dsos);
}
--
2.44.0.396.g6e790dbe36-goog

Next message: Ian Rogers: "[PATCH v2 07/13] perf dsos: Switch backing storage to array from rbtree/list"
Previous message: Ian Rogers: "[PATCH v2 05/13] perf dso: Move dso functions out of dsos"
In reply to: Ian Rogers: "[PATCH v2 05/13] perf dso: Move dso functions out of dsos"
Next in thread: Ian Rogers: "[PATCH v2 07/13] perf dsos: Switch backing storage to array from rbtree/list"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]