libperf cpumap: Refactor perf_cpu_map__merge()
authorLeo Yan <leo.yan@arm.com>
Thu, 7 Nov 2024 12:53:06 +0000 (12:53 +0000)
committerArnaldo Carvalho de Melo <acme@redhat.com>
Mon, 9 Dec 2024 20:52:41 +0000 (17:52 -0300)
The perf_cpu_map__merge() function has two arguments, 'orig' and
'other'.  The function definition might cause confusion as it could give
the impression that the CPU maps in the two arguments are copied into a
new allocated structure, which is then returned as the result.

The purpose of the function is to merge the CPU map 'other' into the CPU
map 'orig'.  This commit changes the 'orig' argument to a pointer to
pointer, so the new result will be updated into 'orig'.

The return value is changed to an int type, as an error number or 0 for
success.

Update callers and tests for the new function definition.

Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Leo Yan <leo.yan@arm.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Ian Rogers <irogers@google.com>
Cc: James Clark <james.clark@linaro.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: https://lore.kernel.org/r/20241107125308.41226-2-leo.yan@arm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
tools/lib/perf/cpumap.c
tools/lib/perf/evlist.c
tools/lib/perf/include/perf/cpumap.h
tools/perf/tests/cpumap.c
tools/perf/util/mem-events.c

index cae799ad44e1362059e45635853ed3321713ae8e..a36e90d381428b000c2a0d31c9b3658c8b65dd11 100644 (file)
@@ -1,4 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0-only
+#include <errno.h>
 #include <perf/cpumap.h>
 #include <stdlib.h>
 #include <linux/refcount.h>
@@ -436,46 +437,49 @@ bool perf_cpu_map__is_subset(const struct perf_cpu_map *a, const struct perf_cpu
 }
 
 /*
- * Merge two cpumaps
+ * Merge two cpumaps.
  *
- * orig either gets freed and replaced with a new map, or reused
- * with no reference count change (similar to "realloc")
- * other has its reference count increased.
+ * If 'other' is subset of '*orig', '*orig' keeps itself with no reference count
+ * change (similar to "realloc").
+ *
+ * If '*orig' is subset of 'other', '*orig' reuses 'other' with its reference
+ * count increased.
+ *
+ * Otherwise, '*orig' gets freed and replaced with a new map.
  */
-
-struct perf_cpu_map *perf_cpu_map__merge(struct perf_cpu_map *orig,
-                                        struct perf_cpu_map *other)
+int perf_cpu_map__merge(struct perf_cpu_map **orig, struct perf_cpu_map *other)
 {
        struct perf_cpu *tmp_cpus;
        int tmp_len;
        int i, j, k;
        struct perf_cpu_map *merged;
 
-       if (perf_cpu_map__is_subset(orig, other))
-               return orig;
-       if (perf_cpu_map__is_subset(other, orig)) {
-               perf_cpu_map__put(orig);
-               return perf_cpu_map__get(other);
+       if (perf_cpu_map__is_subset(*orig, other))
+               return 0;
+       if (perf_cpu_map__is_subset(other, *orig)) {
+               perf_cpu_map__put(*orig);
+               *orig = perf_cpu_map__get(other);
+               return 0;
        }
 
-       tmp_len = __perf_cpu_map__nr(orig) + __perf_cpu_map__nr(other);
+       tmp_len = __perf_cpu_map__nr(*orig) + __perf_cpu_map__nr(other);
        tmp_cpus = malloc(tmp_len * sizeof(struct perf_cpu));
        if (!tmp_cpus)
-               return NULL;
+               return -ENOMEM;
 
        /* Standard merge algorithm from wikipedia */
        i = j = k = 0;
-       while (i < __perf_cpu_map__nr(orig) && j < __perf_cpu_map__nr(other)) {
-               if (__perf_cpu_map__cpu(orig, i).cpu <= __perf_cpu_map__cpu(other, j).cpu) {
-                       if (__perf_cpu_map__cpu(orig, i).cpu == __perf_cpu_map__cpu(other, j).cpu)
+       while (i < __perf_cpu_map__nr(*orig) && j < __perf_cpu_map__nr(other)) {
+               if (__perf_cpu_map__cpu(*orig, i).cpu <= __perf_cpu_map__cpu(other, j).cpu) {
+                       if (__perf_cpu_map__cpu(*orig, i).cpu == __perf_cpu_map__cpu(other, j).cpu)
                                j++;
-                       tmp_cpus[k++] = __perf_cpu_map__cpu(orig, i++);
+                       tmp_cpus[k++] = __perf_cpu_map__cpu(*orig, i++);
                } else
                        tmp_cpus[k++] = __perf_cpu_map__cpu(other, j++);
        }
 
-       while (i < __perf_cpu_map__nr(orig))
-               tmp_cpus[k++] = __perf_cpu_map__cpu(orig, i++);
+       while (i < __perf_cpu_map__nr(*orig))
+               tmp_cpus[k++] = __perf_cpu_map__cpu(*orig, i++);
 
        while (j < __perf_cpu_map__nr(other))
                tmp_cpus[k++] = __perf_cpu_map__cpu(other, j++);
@@ -483,8 +487,9 @@ struct perf_cpu_map *perf_cpu_map__merge(struct perf_cpu_map *orig,
 
        merged = cpu_map__trim_new(k, tmp_cpus);
        free(tmp_cpus);
-       perf_cpu_map__put(orig);
-       return merged;
+       perf_cpu_map__put(*orig);
+       *orig = merged;
+       return 0;
 }
 
 struct perf_cpu_map *perf_cpu_map__intersect(struct perf_cpu_map *orig,
index c6d67fc9e57ef0554675bb29bdfeaf5c116fd1df..94b7369f3efe2ec5d6b780f563aef07df3d3aa9d 100644 (file)
@@ -75,7 +75,7 @@ static void __perf_evlist__propagate_maps(struct perf_evlist *evlist,
                evsel->threads = perf_thread_map__get(evlist->threads);
        }
 
-       evlist->all_cpus = perf_cpu_map__merge(evlist->all_cpus, evsel->cpus);
+       perf_cpu_map__merge(&evlist->all_cpus, evsel->cpus);
 }
 
 static void perf_evlist__propagate_maps(struct perf_evlist *evlist)
index 90457d17fb2f51d0aa0363f3a2ef55f11e743bf1..c83bfb2c36ff9bd0950ff9626368f1024ef82b44 100644 (file)
@@ -39,8 +39,8 @@ LIBPERF_API struct perf_cpu_map *perf_cpu_map__new_online_cpus(void);
 LIBPERF_API struct perf_cpu_map *perf_cpu_map__new(const char *cpu_list);
 LIBPERF_API struct perf_cpu_map *perf_cpu_map__read(FILE *file);
 LIBPERF_API struct perf_cpu_map *perf_cpu_map__get(struct perf_cpu_map *map);
-LIBPERF_API struct perf_cpu_map *perf_cpu_map__merge(struct perf_cpu_map *orig,
-                                                    struct perf_cpu_map *other);
+LIBPERF_API int perf_cpu_map__merge(struct perf_cpu_map **orig,
+                                   struct perf_cpu_map *other);
 LIBPERF_API struct perf_cpu_map *perf_cpu_map__intersect(struct perf_cpu_map *orig,
                                                         struct perf_cpu_map *other);
 LIBPERF_API void perf_cpu_map__put(struct perf_cpu_map *map);
index 2f0168b2a5a9ecce22a9c76b1cf90d1f7b82fa39..7f189d57232f59101a435adefcc7afdd3402d891 100644 (file)
@@ -160,14 +160,14 @@ static int test__cpu_map_merge(struct test_suite *test __maybe_unused, int subte
 {
        struct perf_cpu_map *a = perf_cpu_map__new("4,2,1");
        struct perf_cpu_map *b = perf_cpu_map__new("4,5,7");
-       struct perf_cpu_map *c = perf_cpu_map__merge(a, b);
        char buf[100];
 
-       TEST_ASSERT_VAL("failed to merge map: bad nr", perf_cpu_map__nr(c) == 5);
-       cpu_map__snprint(c, buf, sizeof(buf));
+       perf_cpu_map__merge(&a, b);
+       TEST_ASSERT_VAL("failed to merge map: bad nr", perf_cpu_map__nr(a) == 5);
+       cpu_map__snprint(a, buf, sizeof(buf));
        TEST_ASSERT_VAL("failed to merge map: bad result", !strcmp(buf, "1-2,4-5,7"));
        perf_cpu_map__put(b);
-       perf_cpu_map__put(c);
+       perf_cpu_map__put(a);
        return 0;
 }
 
@@ -233,9 +233,8 @@ static int test__cpu_map_equal(struct test_suite *test __maybe_unused, int subte
        }
 
        /* Maps equal made maps. */
-       tmp = perf_cpu_map__merge(perf_cpu_map__get(one), two);
-       TEST_ASSERT_VAL("pair", perf_cpu_map__equal(pair, tmp));
-       perf_cpu_map__put(tmp);
+       perf_cpu_map__merge(&two, one);
+       TEST_ASSERT_VAL("pair", perf_cpu_map__equal(pair, two));
 
        tmp = perf_cpu_map__intersect(pair, one);
        TEST_ASSERT_VAL("one", perf_cpu_map__equal(one, tmp));
index bf5090f5220bbdc760b31cee04a97f77a9ce4469..3692e988c86e5d90306dd5bce58525c613abc093 100644 (file)
@@ -258,6 +258,7 @@ int perf_mem_events__record_args(const char **rec_argv, int *argv_nr)
        const char *s;
        char *copy;
        struct perf_cpu_map *cpu_map = NULL;
+       int ret;
 
        while ((pmu = perf_pmus__scan_mem(pmu)) != NULL) {
                for (int j = 0; j < PERF_MEM_EVENTS__MAX; j++) {
@@ -283,7 +284,9 @@ int perf_mem_events__record_args(const char **rec_argv, int *argv_nr)
                        rec_argv[i++] = "-e";
                        rec_argv[i++] = copy;
 
-                       cpu_map = perf_cpu_map__merge(cpu_map, pmu->cpus);
+                       ret = perf_cpu_map__merge(&cpu_map, pmu->cpus);
+                       if (ret < 0)
+                               return ret;
                }
        }