Messages in this thread | | | From | Ian Rogers <> | Date | Fri, 26 May 2023 23:05:25 -0700 | Subject | Re: [PATCH v4 03/35] perf cpumap: Add equal function |
| |
On Fri, May 26, 2023 at 6:40 PM Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > Em Fri, May 26, 2023 at 10:32:12PM -0300, Arnaldo Carvalho de Melo escreveu: > > Em Fri, May 26, 2023 at 02:53:38PM -0700, Ian Rogers escreveu: > > > Equality is a useful property to compare after merging and > > > intersecting maps. > > > > > > Signed-off-by: Ian Rogers <irogers@google.com> > > > Reviewed-by: Kan Liang <kan.liang@linux.intel.com> > > > --- > > > tools/lib/perf/cpumap.c | 21 ++++++++++++++++ > > > tools/lib/perf/include/perf/cpumap.h | 2 ++ > > > tools/perf/tests/cpumap.c | 37 ++++++++++++++++++++++++++++ > > > 3 files changed, 60 insertions(+) > > > > > > diff --git a/tools/lib/perf/cpumap.c b/tools/lib/perf/cpumap.c > > > index d4f3a1a12522..48595a3ad69c 100644 > > > --- a/tools/lib/perf/cpumap.c > > > +++ b/tools/lib/perf/cpumap.c > > > @@ -321,6 +321,27 @@ bool perf_cpu_map__has(const struct perf_cpu_map *cpus, struct perf_cpu cpu) > > > return perf_cpu_map__idx(cpus, cpu) != -1; > > > } > > > > > > +bool perf_cpu_map__equal(const struct perf_cpu_map *lhs, const struct perf_cpu_map *rhs) > > > +{ > > > + int nr; > > > + > > > + if (lhs == rhs) > > > + return true; > > > + > > > + if (!lhs || !rhs) > > > + return false; > > > + > > > + nr = perf_cpu_map__nr(lhs); > > > + if (nr != perf_cpu_map__nr(rhs)) > > > + return false; > > > + > > > + for (int idx = 0; idx < nr; idx++) { > > > + if (RC_CHK_ACCESS(lhs)->map[idx].cpu != RC_CHK_ACCESS(rhs)->map[idx].cpu) > > > + return false; > > > > Don't we have an accessor to avoid this RC_CHK_ACCESS()-> access? > > In the following patch you use it: > > +bool perf_cpu_map__has_any_cpu(const struct perf_cpu_map *map) > +{ > + return map && perf_cpu_map__cpu(map, 0).cpu == -1; > +} > > But it does extra checks you did already: > > struct perf_cpu perf_cpu_map__cpu(const struct perf_cpu_map *cpus, int idx) > { > struct perf_cpu result = { > .cpu = -1 > }; > > if (cpus && idx < RC_CHK_ACCESS(cpus)->nr) > return RC_CHK_ACCESS(cpus)->map[idx]; > > return result; > } > > Usually we have: > > struct perf_cpu __perf_cpu_map__cpu(const struct perf_cpu_map *cpus, int idx) > { > return RC_CHK_ACCESS(cpus)->map[idx]; > } > > struct perf_cpu perf_cpu_map__cpu(const struct perf_cpu_map *cpus, int idx) > { > struct perf_cpu result = { > .cpu = -1 > }; > > if (cpus && idx < __perf_cpu_map__nr(cpus)) > return __perf_cpu_map__cpu(cpus, idx); > > return result; > } > > Then you would have: > > bool perf_cpu_map__equal(const struct perf_cpu_map *lhs, const struct perf_cpu_map *rhs) > { > int nr; > > if (lhs == rhs) > return true; > > if (!lhs || !rhs) > return false; > > nr = __perf_cpu_map__nr(lhs); // no need to check lhs again for NULL > if (nr != __perf_cpu_map__nr(rhs)) // ditto for rhs > return false; > > for (int idx = 0; idx < nr; idx++) { > if (__perf_cpu_map__cpu(lhs, idx)->cpu != __perf_cpu_map__cpu(rhs, idx)->cpu) > return false; >
Thanks, I'll update for v5. Fwiw, on intersect, I keep forgetting to mention that this was sent previously as: https://lore.kernel.org/lkml/20220408035616.1356953-4-irogers@google.com/
Ian
> > > + } > > > + return true; > > > +} > > > + > > > struct perf_cpu perf_cpu_map__max(const struct perf_cpu_map *map) > > > { > > > struct perf_cpu result = { > > > diff --git a/tools/lib/perf/include/perf/cpumap.h b/tools/lib/perf/include/perf/cpumap.h > > > index 0466c4216fbb..d0ae9552f8e2 100644 > > > --- a/tools/lib/perf/include/perf/cpumap.h > > > +++ b/tools/lib/perf/include/perf/cpumap.h > > > @@ -28,6 +28,8 @@ LIBPERF_API int perf_cpu_map__nr(const struct perf_cpu_map *cpus); > > > LIBPERF_API bool perf_cpu_map__empty(const struct perf_cpu_map *map); > > > LIBPERF_API struct perf_cpu perf_cpu_map__max(const struct perf_cpu_map *map); > > > LIBPERF_API bool perf_cpu_map__has(const struct perf_cpu_map *map, struct perf_cpu cpu); > > > +LIBPERF_API bool perf_cpu_map__equal(const struct perf_cpu_map *lhs, > > > + const struct perf_cpu_map *rhs); > > > > > > #define perf_cpu_map__for_each_cpu(cpu, idx, cpus) \ > > > for ((idx) = 0, (cpu) = perf_cpu_map__cpu(cpus, idx); \ > > > diff --git a/tools/perf/tests/cpumap.c b/tools/perf/tests/cpumap.c > > > index 83805690c209..7730fc2ab40b 100644 > > > --- a/tools/perf/tests/cpumap.c > > > +++ b/tools/perf/tests/cpumap.c > > > @@ -211,11 +211,48 @@ static int test__cpu_map_intersect(struct test_suite *test __maybe_unused, > > > return ret; > > > } > > > > > > +static int test__cpu_map_equal(struct test_suite *test __maybe_unused, int subtest __maybe_unused) > > > +{ > > > + struct perf_cpu_map *any = perf_cpu_map__dummy_new(); > > > + struct perf_cpu_map *one = perf_cpu_map__new("1"); > > > + struct perf_cpu_map *two = perf_cpu_map__new("2"); > > > + struct perf_cpu_map *empty = perf_cpu_map__intersect(one, two); > > > + struct perf_cpu_map *pair = perf_cpu_map__new("1-2"); > > > + struct perf_cpu_map *tmp; > > > + struct perf_cpu_map *maps[] = {empty, any, one, two, pair}; > > > + > > > + for (size_t i = 0; i < ARRAY_SIZE(maps); i++) { > > > + /* Maps equal themself. */ > > > + TEST_ASSERT_VAL("equal", perf_cpu_map__equal(maps[i], maps[i])); > > > + for (size_t j = 0; j < ARRAY_SIZE(maps); j++) { > > > + /* Maps dont't equal each other. */ > > > + if (i == j) > > > + continue; > > > + TEST_ASSERT_VAL("not equal", !perf_cpu_map__equal(maps[i], maps[j])); > > > + } > > > + } > > > + > > > + /* 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); > > > + > > > + tmp = perf_cpu_map__intersect(pair, one); > > > + TEST_ASSERT_VAL("one", perf_cpu_map__equal(one, tmp)); > > > + perf_cpu_map__put(tmp); > > > + > > > + for (size_t i = 0; i < ARRAY_SIZE(maps); i++) > > > + perf_cpu_map__put(maps[i]); > > > + > > > + return TEST_OK; > > > +} > > > + > > > static struct test_case tests__cpu_map[] = { > > > TEST_CASE("Synthesize cpu map", cpu_map_synthesize), > > > TEST_CASE("Print cpu map", cpu_map_print), > > > TEST_CASE("Merge cpu map", cpu_map_merge), > > > TEST_CASE("Intersect cpu map", cpu_map_intersect), > > > + TEST_CASE("Equal cpu map", cpu_map_equal), > > > { .name = NULL, } > > > }; > > > > > > -- > > > 2.41.0.rc0.172.g3f132b7071-goog > > > > > > > -- > > > > - Arnaldo > > -- > > - Arnaldo
| |