perf tests: Use function to add missing maps lock
authorIan Rogers <irogers@google.com>
Thu, 7 Dec 2023 01:16:39 +0000 (17:16 -0800)
committerArnaldo Carvalho de Melo <acme@redhat.com>
Tue, 19 Dec 2023 00:35:01 +0000 (21:35 -0300)
Switch loop macro maps__for_each_entry to maps__for_each_map function
that takes a callback. The function holds the maps lock, which should
be held during iteration.

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: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Cc: Changbin Du <changbin.du@huawei.com>
Cc: Colin Ian King <colin.i.king@gmail.com>
Cc: Dmitrii Dolgov <9erthalion6@gmail.com>
Cc: German Gomez <german.gomez@arm.com>
Cc: Guilherme Amadio <amadio@gentoo.org>
Cc: Huacai Chen <chenhuacai@kernel.org>
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: Kajol Jain <kjain@linux.ibm.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Leo Yan <leo.yan@linaro.org>
Cc: Li Dong <lidong@vivo.com>
Cc: Liam Howlett <liam.howlett@oracle.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Miguel Ojeda <ojeda@kernel.org>
Cc: Ming Wang <wangming01@loongson.cn>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Nick Terrell <terrelln@fb.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ravi Bangoria <ravi.bangoria@amd.com>
Cc: Sandipan Das <sandipan.das@amd.com>
Cc: Sean Christopherson <seanjc@google.com>
Cc: Steinar H. Gunderson <sesse@google.com>
Cc: Vincent Whitchurch <vincent.whitchurch@axis.com>
Cc: Wenyu Liu <liuwenyu7@huawei.com>
Cc: Yang Jihong <yangjihong1@huawei.com>
Link: https://lore.kernel.org/r/20231207011722.1220634-6-irogers@google.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
tools/perf/tests/maps.c
tools/perf/tests/vmlinux-kallsyms.c

index 5bb1123a91a7ccf0f38a021581bcd194c36bec1c..bb3fbfe5a73e2302155fe40a953102e15f0dd103 100644 (file)
@@ -14,44 +14,59 @@ struct map_def {
        u64 end;
 };
 
+struct check_maps_cb_args {
+       struct map_def *merged;
+       unsigned int i;
+};
+
+static int check_maps_cb(struct map *map, void *data)
+{
+       struct check_maps_cb_args *args = data;
+       struct map_def *merged = &args->merged[args->i];
+
+       if (map__start(map) != merged->start ||
+           map__end(map) != merged->end ||
+           strcmp(map__dso(map)->name, merged->name) ||
+           refcount_read(map__refcnt(map)) != 1) {
+               return 1;
+       }
+       args->i++;
+       return 0;
+}
+
+static int failed_cb(struct map *map, void *data __maybe_unused)
+{
+       pr_debug("\tstart: %" PRIu64 " end: %" PRIu64 " name: '%s' refcnt: %d\n",
+               map__start(map),
+               map__end(map),
+               map__dso(map)->name,
+               refcount_read(map__refcnt(map)));
+
+       return 0;
+}
+
 static int check_maps(struct map_def *merged, unsigned int size, struct maps *maps)
 {
-       struct map_rb_node *rb_node;
-       unsigned int i = 0;
        bool failed = false;
 
        if (maps__nr_maps(maps) != size) {
                pr_debug("Expected %d maps, got %d", size, maps__nr_maps(maps));
                failed = true;
        } else {
-               maps__for_each_entry(maps, rb_node) {
-                       struct map *map = rb_node->map;
-
-                       if (map__start(map) != merged[i].start ||
-                           map__end(map) != merged[i].end ||
-                           strcmp(map__dso(map)->name, merged[i].name) ||
-                           refcount_read(map__refcnt(map)) != 1) {
-                               failed = true;
-                       }
-                       i++;
-               }
+               struct check_maps_cb_args args = {
+                       .merged = merged,
+                       .i = 0,
+               };
+               failed = maps__for_each_map(maps, check_maps_cb, &args);
        }
        if (failed) {
                pr_debug("Expected:\n");
-               for (i = 0; i < size; i++) {
+               for (unsigned int i = 0; i < size; i++) {
                        pr_debug("\tstart: %" PRIu64 " end: %" PRIu64 " name: '%s' refcnt: 1\n",
                                merged[i].start, merged[i].end, merged[i].name);
                }
                pr_debug("Got:\n");
-               maps__for_each_entry(maps, rb_node) {
-                       struct map *map = rb_node->map;
-
-                       pr_debug("\tstart: %" PRIu64 " end: %" PRIu64 " name: '%s' refcnt: %d\n",
-                               map__start(map),
-                               map__end(map),
-                               map__dso(map)->name,
-                               refcount_read(map__refcnt(map)));
-               }
+               maps__for_each_map(maps, failed_cb, NULL);
        }
        return failed ? TEST_FAIL : TEST_OK;
 }
index 1078a93b01aa018f3be1932dee7f3f694ad51828..822f893e67d5f6643f5f1794801b87630f64fca1 100644 (file)
@@ -112,18 +112,92 @@ static bool is_ignored_symbol(const char *name, char type)
        return false;
 }
 
+struct test__vmlinux_matches_kallsyms_cb_args {
+       struct machine kallsyms;
+       struct map *vmlinux_map;
+       bool header_printed;
+};
+
+static int test__vmlinux_matches_kallsyms_cb1(struct map *map, void *data)
+{
+       struct test__vmlinux_matches_kallsyms_cb_args *args = data;
+       struct dso *dso = map__dso(map);
+       /*
+        * If it is the kernel, kallsyms is always "[kernel.kallsyms]", while
+        * the kernel will have the path for the vmlinux file being used, so use
+        * the short name, less descriptive but the same ("[kernel]" in both
+        * cases.
+        */
+       struct map *pair = maps__find_by_name(args->kallsyms.kmaps,
+                                       (dso->kernel ? dso->short_name : dso->name));
+
+       if (pair)
+               map__set_priv(pair, 1);
+       else {
+               if (!args->header_printed) {
+                       pr_info("WARN: Maps only in vmlinux:\n");
+                       args->header_printed = true;
+               }
+               map__fprintf(map, stderr);
+       }
+       return 0;
+}
+
+static int test__vmlinux_matches_kallsyms_cb2(struct map *map, void *data)
+{
+       struct test__vmlinux_matches_kallsyms_cb_args *args = data;
+       struct map *pair;
+       u64 mem_start = map__unmap_ip(args->vmlinux_map, map__start(map));
+       u64 mem_end = map__unmap_ip(args->vmlinux_map, map__end(map));
+
+       pair = maps__find(args->kallsyms.kmaps, mem_start);
+       if (pair == NULL || map__priv(pair))
+               return 0;
+
+       if (map__start(pair) == mem_start) {
+               struct dso *dso = map__dso(map);
+
+               if (!args->header_printed) {
+                       pr_info("WARN: Maps in vmlinux with a different name in kallsyms:\n");
+                       args->header_printed = true;
+               }
+
+               pr_info("WARN: %" PRIx64 "-%" PRIx64 " %" PRIx64 " %s in kallsyms as",
+                       map__start(map), map__end(map), map__pgoff(map), dso->name);
+               if (mem_end != map__end(pair))
+                       pr_info(":\nWARN: *%" PRIx64 "-%" PRIx64 " %" PRIx64,
+                               map__start(pair), map__end(pair), map__pgoff(pair));
+               pr_info(" %s\n", dso->name);
+               map__set_priv(pair, 1);
+       }
+       return 0;
+}
+
+static int test__vmlinux_matches_kallsyms_cb3(struct map *map, void *data)
+{
+       struct test__vmlinux_matches_kallsyms_cb_args *args = data;
+
+       if (!map__priv(map)) {
+               if (!args->header_printed) {
+                       pr_info("WARN: Maps only in kallsyms:\n");
+                       args->header_printed = true;
+               }
+               map__fprintf(map, stderr);
+       }
+       return 0;
+}
+
 static int test__vmlinux_matches_kallsyms(struct test_suite *test __maybe_unused,
                                        int subtest __maybe_unused)
 {
        int err = TEST_FAIL;
        struct rb_node *nd;
        struct symbol *sym;
-       struct map *kallsyms_map, *vmlinux_map;
-       struct map_rb_node *rb_node;
-       struct machine kallsyms, vmlinux;
+       struct map *kallsyms_map;
+       struct machine vmlinux;
        struct maps *maps;
        u64 mem_start, mem_end;
-       bool header_printed;
+       struct test__vmlinux_matches_kallsyms_cb_args args;
 
        /*
         * Step 1:
@@ -131,7 +205,7 @@ static int test__vmlinux_matches_kallsyms(struct test_suite *test __maybe_unused
         * Init the machines that will hold kernel, modules obtained from
         * both vmlinux + .ko files and from /proc/kallsyms split by modules.
         */
-       machine__init(&kallsyms, "", HOST_KERNEL_ID);
+       machine__init(&args.kallsyms, "", HOST_KERNEL_ID);
        machine__init(&vmlinux, "", HOST_KERNEL_ID);
 
        maps = machine__kernel_maps(&vmlinux);
@@ -143,7 +217,7 @@ static int test__vmlinux_matches_kallsyms(struct test_suite *test __maybe_unused
         * load /proc/kallsyms. Also create the modules maps from /proc/modules
         * and find the .ko files that match them in /lib/modules/`uname -r`/.
         */
-       if (machine__create_kernel_maps(&kallsyms) < 0) {
+       if (machine__create_kernel_maps(&args.kallsyms) < 0) {
                pr_debug("machine__create_kernel_maps failed");
                err = TEST_SKIP;
                goto out;
@@ -160,7 +234,7 @@ static int test__vmlinux_matches_kallsyms(struct test_suite *test __maybe_unused
         * be compacted against the list of modules found in the "vmlinux"
         * code and with the one got from /proc/modules from the "kallsyms" code.
         */
-       if (machine__load_kallsyms(&kallsyms, "/proc/kallsyms") <= 0) {
+       if (machine__load_kallsyms(&args.kallsyms, "/proc/kallsyms") <= 0) {
                pr_debug("machine__load_kallsyms failed");
                err = TEST_SKIP;
                goto out;
@@ -174,7 +248,7 @@ static int test__vmlinux_matches_kallsyms(struct test_suite *test __maybe_unused
         * to see if the running kernel was relocated by checking if it has the
         * same value in the vmlinux file we load.
         */
-       kallsyms_map = machine__kernel_map(&kallsyms);
+       kallsyms_map = machine__kernel_map(&args.kallsyms);
 
        /*
         * Step 5:
@@ -186,7 +260,7 @@ static int test__vmlinux_matches_kallsyms(struct test_suite *test __maybe_unused
                goto out;
        }
 
-       vmlinux_map = machine__kernel_map(&vmlinux);
+       args.vmlinux_map = machine__kernel_map(&vmlinux);
 
        /*
         * Step 6:
@@ -213,7 +287,7 @@ static int test__vmlinux_matches_kallsyms(struct test_suite *test __maybe_unused
         * in the kallsyms dso. For the ones that are in both, check its names and
         * end addresses too.
         */
-       map__for_each_symbol(vmlinux_map, sym, nd) {
+       map__for_each_symbol(args.vmlinux_map, sym, nd) {
                struct symbol *pair, *first_pair;
 
                sym  = rb_entry(nd, struct symbol, rb_node);
@@ -221,10 +295,10 @@ static int test__vmlinux_matches_kallsyms(struct test_suite *test __maybe_unused
                if (sym->start == sym->end)
                        continue;
 
-               mem_start = map__unmap_ip(vmlinux_map, sym->start);
-               mem_end = map__unmap_ip(vmlinux_map, sym->end);
+               mem_start = map__unmap_ip(args.vmlinux_map, sym->start);
+               mem_end = map__unmap_ip(args.vmlinux_map, sym->end);
 
-               first_pair = machine__find_kernel_symbol(&kallsyms, mem_start, NULL);
+               first_pair = machine__find_kernel_symbol(&args.kallsyms, mem_start, NULL);
                pair = first_pair;
 
                if (pair && UM(pair->start) == mem_start) {
@@ -253,7 +327,8 @@ next_pair:
                                 */
                                continue;
                        } else {
-                               pair = machine__find_kernel_symbol_by_name(&kallsyms, sym->name, NULL);
+                               pair = machine__find_kernel_symbol_by_name(&args.kallsyms,
+                                                                          sym->name, NULL);
                                if (pair) {
                                        if (UM(pair->start) == mem_start)
                                                goto next_pair;
@@ -267,7 +342,7 @@ next_pair:
 
                                continue;
                        }
-               } else if (mem_start == map__end(kallsyms.vmlinux_map)) {
+               } else if (mem_start == map__end(args.kallsyms.vmlinux_map)) {
                        /*
                         * Ignore aliases to _etext, i.e. to the end of the kernel text area,
                         * such as __indirect_thunk_end.
@@ -289,78 +364,18 @@ next_pair:
        if (verbose <= 0)
                goto out;
 
-       header_printed = false;
-
-       maps__for_each_entry(maps, rb_node) {
-               struct map *map = rb_node->map;
-               struct dso *dso = map__dso(map);
-               /*
-                * If it is the kernel, kallsyms is always "[kernel.kallsyms]", while
-                * the kernel will have the path for the vmlinux file being used,
-                * so use the short name, less descriptive but the same ("[kernel]" in
-                * both cases.
-                */
-               struct map *pair = maps__find_by_name(kallsyms.kmaps, (dso->kernel ?
-                                                               dso->short_name :
-                                                               dso->name));
-               if (pair) {
-                       map__set_priv(pair, 1);
-               } else {
-                       if (!header_printed) {
-                               pr_info("WARN: Maps only in vmlinux:\n");
-                               header_printed = true;
-                       }
-                       map__fprintf(map, stderr);
-               }
-       }
-
-       header_printed = false;
-
-       maps__for_each_entry(maps, rb_node) {
-               struct map *pair, *map = rb_node->map;
-
-               mem_start = map__unmap_ip(vmlinux_map, map__start(map));
-               mem_end = map__unmap_ip(vmlinux_map, map__end(map));
+       args.header_printed = false;
+       maps__for_each_map(maps, test__vmlinux_matches_kallsyms_cb1, &args);
 
-               pair = maps__find(kallsyms.kmaps, mem_start);
-               if (pair == NULL || map__priv(pair))
-                       continue;
-
-               if (map__start(pair) == mem_start) {
-                       struct dso *dso = map__dso(map);
-
-                       if (!header_printed) {
-                               pr_info("WARN: Maps in vmlinux with a different name in kallsyms:\n");
-                               header_printed = true;
-                       }
-
-                       pr_info("WARN: %" PRIx64 "-%" PRIx64 " %" PRIx64 " %s in kallsyms as",
-                               map__start(map), map__end(map), map__pgoff(map), dso->name);
-                       if (mem_end != map__end(pair))
-                               pr_info(":\nWARN: *%" PRIx64 "-%" PRIx64 " %" PRIx64,
-                                       map__start(pair), map__end(pair), map__pgoff(pair));
-                       pr_info(" %s\n", dso->name);
-                       map__set_priv(pair, 1);
-               }
-       }
-
-       header_printed = false;
-
-       maps = machine__kernel_maps(&kallsyms);
+       args.header_printed = false;
+       maps__for_each_map(maps, test__vmlinux_matches_kallsyms_cb2, &args);
 
-       maps__for_each_entry(maps, rb_node) {
-               struct map *map = rb_node->map;
+       args.header_printed = false;
+       maps = machine__kernel_maps(&args.kallsyms);
+       maps__for_each_map(maps, test__vmlinux_matches_kallsyms_cb3, &args);
 
-               if (!map__priv(map)) {
-                       if (!header_printed) {
-                               pr_info("WARN: Maps only in kallsyms:\n");
-                               header_printed = true;
-                       }
-                       map__fprintf(map, stderr);
-               }
-       }
 out:
-       machine__exit(&kallsyms);
+       machine__exit(&args.kallsyms);
        machine__exit(&vmlinux);
        return err;
 }