Re: [PATCH v5 04/13] perf evlist: Maintain evlist->all_cpus

Jiri Olsa
2019-11-11T13:31:45Z

On Thu, Nov 07, 2019 at 10:16:37AM -0800, Andi Kleen wrote:

SNIP

> diff --git a/tools/perf/lib/cpumap.c b/tools/perf/lib/cpumap.c
> index d81656b4635e..b9f573438b93 100644
> --- a/tools/perf/lib/cpumap.c
> +++ b/tools/perf/lib/cpumap.c
> @@ -286,3 +286,46 @@ int perf_cpu_map__max(struct perf_cpu_map *map)
>
> return max;
> }
> +
> +struct perf_cpu_map *perf_cpu_map__merge(struct perf_cpu_map *orig,
> + struct perf_cpu_map *other)
> +{
> + int *tmp_cpus;
> + int tmp_len;
> + int i, j, k;
> + struct perf_cpu_map *merged;
> +
> + if (!orig && !other)
> + return NULL;
> + if (!orig) {
> + perf_cpu_map__get(other);
> + return other;
> + }
> + if (!other)
> + return orig;

so we bump refcnt for other but not for orig?

could you please put to the comment expected results
wrt refcnt values for above cases?

> + if (orig->nr == other->nr &&
> + !memcmp(orig->map, other->map, orig->nr * sizeof(int)))
> + return orig;
> + tmp_len = orig->nr + other->nr;
> + tmp_cpus = malloc(tmp_len * sizeof(int));
> + if (!tmp_cpus)
> + return NULL;
> + i = j = k = 0;
> + while (i < orig->nr && j < other->nr) {
> + if (orig->map[i] <= other->map[j]) {
> + if (orig->map[i] == other->map[j])
> + j++;
> + tmp_cpus[k++] = orig->map[i++];
> + } else
> + tmp_cpus[k++] = other->map[j++];
> + }
> + while (i < orig->nr)
> + tmp_cpus[k++] = orig->map[i++];
> + while (j < other->nr)
> + tmp_cpus[k++] = other->map[j++];
> + assert(k <= tmp_len);
> + merged = cpu_map__trim_new(k, tmp_cpus);
> + free(tmp_cpus);
> + perf_cpu_map__put(orig);
> + return merged;

please use some comments and blank lines to separate
the code above, it's too much.. ;-)

SNIP

> LIBPERF_API int perf_cpu_map__cpu(const struct perf_cpu_map *cpus, int idx);
> LIBPERF_API int perf_cpu_map__nr(const struct perf_cpu_map *cpus);
> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> index 8b286e9b7549..5fa37cf7f283 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
> @@ -259,6 +259,11 @@ static struct test generic_tests[] = {
> .desc = "Print cpu map",
> .func = test__cpu_map_print,
> },
> + {
> + .desc = "Merge cpu map",
> + .func = test__cpu_map_merge,
> + },

awesome ,thanks

jirka