perf dsos: Switch more loops to dsos__for_each_dso()
authorIan Rogers <irogers@google.com>
Wed, 10 Apr 2024 06:42:07 +0000 (23:42 -0700)
committerArnaldo Carvalho de Melo <acme@redhat.com>
Fri, 12 Apr 2024 15:04:13 +0000 (12:04 -0300)
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@google.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Anne Macedo <retpolanne@posteo.net>
Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Cc: Ben Gainey <ben.gainey@arm.com>
Cc: Changbin Du <changbin.du@huawei.com>
Cc: Chengen Du <chengen.du@canonical.com>
Cc: Colin Ian King <colin.i.king@gmail.com>
Cc: Ilkka Koskinen <ilkka@os.amperecomputing.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: James Clark <james.clark@arm.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: K Prateek Nayak <kprateek.nayak@amd.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Leo Yan <leo.yan@linux.dev>
Cc: Li Dong <lidong@vivo.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Markus Elfring <Markus.Elfring@web.de>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paran Lee <p4ranlee@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ravi Bangoria <ravi.bangoria@amd.com>
Cc: Song Liu <song@kernel.org>
Cc: Sun Haiyong <sunhaiyong@loongson.cn>
Cc: Thomas Richter <tmricht@linux.ibm.com>
Cc: Yang Jihong <yangjihong1@huawei.com>
Cc: Yanteng Si <siyanteng@loongson.cn>
Cc: Yicong Yang <yangyicong@hisilicon.com>
Cc: zhaimingbing <zhaimingbing@cmss.chinamobile.com>
Link: https://lore.kernel.org/r/20240410064214.2755936-6-irogers@google.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
tools/perf/util/build-id.c
tools/perf/util/dsos.c
tools/perf/util/dsos.h
tools/perf/util/machine.c

index a6d3c253f19fa14c9f2e92c15807aa68fd0ac034..864bc26b6b461066afbff3d1a81bf6454749161f 100644 (file)
@@ -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)
index f816927a21ffbf777868356565adc85c9c05d0f7..b7fbfb877ae3650ff8d3bcda5b786701199f8c57 100644 (file)
@@ -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 @@ out_unlock:
        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;
 }
index 317a263f0e37f8136e1c98095e09d96881ee11fc..50bd5152347552362a1bd93f6692481c40cbf534 100644 (file)
@@ -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);
index e3deef38405c373a746f02cd4dd70835b1788fd6..c5c895131bca90b8391ef648b02bf0229f2a34b5 100644 (file)
@@ -853,11 +853,11 @@ out:
 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,
@@ -3232,5 +3232,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);
 }