perf maps: Modify maps_by_name to hold a reference to a map

To make it clearer about the ownership of a reference count split the
by-name case from the regular start-address sorted tree. Put the
reference count when maps_by_name is freed, which requires moving a
decrement to nr_maps in maps__remove. Add two missing map puts in
maps__fixup_overlappings in the event maps__insert fails.

Signed-off-by: Ian Rogers <irogers@google.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Alexey Bayduraev <alexey.v.bayduraev@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Darren Hart <dvhart@infradead.org>
Cc: Davidlohr Bueso <dave@stgolabs.net>
Cc: Dmitriy Vyukov <dvyukov@google.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: German Gomez <german.gomez@arm.com>
Cc: Hao Luo <haoluo@google.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: James Clark <james.clark@arm.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: John Garry <john.g.garry@oracle.com>
Cc: Kajol Jain <kjain@linux.ibm.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Leo Yan <leo.yan@linaro.org>
Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Miaoqian Lin <linmq006@gmail.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Riccardo Mancini <rickyman7@gmail.com>
Cc: Shunsuke Nakamura <nakamura.shun@fujitsu.com>
Cc: Song Liu <song@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Stephen Brennan <stephen.s.brennan@oracle.com>
Cc: Steven Rostedt (VMware) <rostedt@goodmis.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Thomas Richter <tmricht@linux.ibm.com>
Cc: Yury Norov <yury.norov@gmail.com>
Link: https://lore.kernel.org/r/20230404205954.2245628-2-irogers@google.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/util/maps.c b/tools/perf/util/maps.c
index 0eee27e..5afed53 100644 (file)
--- a/tools/perf/util/maps.c
+++ b/tools/perf/util/maps.c
@@ -26,6 +26,9 @@ static void __maps__free_maps_by_name(struct maps *maps)
        /*
         * Free everything to try to do it from the rbtree in the next search
         */
+       for (unsigned int i = 0; i < maps__nr_maps(maps); i++)
+               map__put(maps__maps_by_name(maps)[i]);
+
        zfree(&maps->maps_by_name);
        maps->nr_maps_allocated = 0;
 }
@@ -42,7 +45,7 @@ static int __maps__insert(struct maps *maps, struct map *map)
                return -ENOMEM;
 
        RB_CLEAR_NODE(&new_rb_node->rb_node);
-       new_rb_node->map = map;
+       new_rb_node->map = map__get(map);
 
        while (*p != NULL) {
                parent = *p;
@@ -55,7 +58,6 @@ static int __maps__insert(struct maps *maps, struct map *map)
 
        rb_link_node(&new_rb_node->rb_node, parent, p);
        rb_insert_color(&new_rb_node->rb_node, maps__entries(maps));
-       map__get(map);
        return 0;
 }
 
@@ -100,7 +102,7 @@ int maps__insert(struct maps *maps, struct map *map)
                        maps->maps_by_name = maps_by_name;
                        maps->nr_maps_allocated = nr_allocate;
                }
-               maps__maps_by_name(maps)[maps__nr_maps(maps) - 1] = map;
+               maps__maps_by_name(maps)[maps__nr_maps(maps) - 1] = map__get(map);
                __maps__sort_by_name(maps);
        }
  out:
@@ -126,9 +128,9 @@ void maps__remove(struct maps *maps, struct map *map)
        rb_node = maps__find_node(maps, map);
        assert(rb_node->map == map);
        __maps__remove(maps, rb_node);
-       --maps->nr_maps;
        if (maps__maps_by_name(maps))
                __maps__free_maps_by_name(maps);
+       --maps->nr_maps;
        up_write(maps__lock(maps));
 }
 
@@ -136,6 +138,9 @@ static void __maps__purge(struct maps *maps)
 {
        struct map_rb_node *pos, *next;
 
+       if (maps__maps_by_name(maps))
+               __maps__free_maps_by_name(maps);
+
        maps__for_each_entry_safe(maps, pos, next) {
                rb_erase_init(&pos->rb_node,  maps__entries(maps));
                map__put(pos->map);
@@ -293,7 +298,7 @@ int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp)
        }
 
        next = first;
-       while (next) {
+       while (next && !err) {
                struct map_rb_node *pos = rb_entry(next, struct map_rb_node, rb_node);
                next = rb_next(&pos->rb_node);
 
@@ -331,8 +336,10 @@ int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp)
 
                        before->end = map__start(map);
                        err = __maps__insert(maps, before);
-                       if (err)
+                       if (err) {
+                               map__put(before);
                                goto put_map;
+                       }
 
                        if (verbose >= 2 && !use_browser)
                                map__fprintf(before, fp);
@@ -352,22 +359,17 @@ int maps__fixup_overlappings(struct maps *maps, struct map *map, FILE *fp)
                        assert(map__map_ip(pos->map, map__end(map)) ==
                                map__map_ip(after, map__end(map)));
                        err = __maps__insert(maps, after);
-                       if (err)
+                       if (err) {
+                               map__put(after);
                                goto put_map;
-
+                       }
                        if (verbose >= 2 && !use_browser)
                                map__fprintf(after, fp);
                        map__put(after);
                }
 put_map:
                map__put(pos->map);
-
-               if (err)
-                       goto out;
        }
-
-       err = 0;
-out:
        up_write(maps__lock(maps));
        return err;
 }

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 7282119..91ebf93 100644 (file)
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -2053,10 +2053,23 @@ out:
 
 static int map__strcmp(const void *a, const void *b)
 {
-       const struct dso *dso_a = map__dso(*(const struct map **)a);
-       const struct dso *dso_b = map__dso(*(const struct map **)b);
+       const struct map *map_a = *(const struct map **)a;
+       const struct map *map_b = *(const struct map **)b;
+       const struct dso *dso_a = map__dso(map_a);
+       const struct dso *dso_b = map__dso(map_b);
+       int ret = strcmp(dso_a->short_name, dso_b->short_name);
 
-       return strcmp(dso_a->short_name, dso_b->short_name);
+       if (ret == 0 && map_a != map_b) {
+               /*
+                * Ensure distinct but name equal maps have an order in part to
+                * aid reference counting.
+                */
+               ret = (int)map__start(map_a) - (int)map__start(map_b);
+               if (ret == 0)
+                       ret = (int)((intptr_t)map_a - (intptr_t)map_b);
+       }
+
+       return ret;
 }
 
 static int map__strcmp_name(const void *name, const void *b)
@@ -2088,7 +2101,7 @@ static int map__groups__sort_by_name_from_rbtree(struct maps *maps)
        maps->nr_maps_allocated = maps__nr_maps(maps);
 
        maps__for_each_entry(maps, rb_node)
-               maps_by_name[i++] = rb_node->map;
+               maps_by_name[i++] = map__get(rb_node->map);
 
        __maps__sort_by_name(maps);