Messages in this thread | | | Date | Mon, 11 Nov 2019 14:31:37 +0100 | From | Jiri Olsa <> | Subject | Re: [PATCH v5 04/13] perf evlist: Maintain evlist->all_cpus |
| |
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
| |