Messages in this thread | | | Date | Tue, 4 Jan 2022 15:24:19 +0100 | From | Jiri Olsa <> | Subject | Re: [PATCH v3 00/48] Refactor perf cpumap |
| |
On Wed, Dec 29, 2021 at 11:19:41PM -0800, Ian Rogers wrote: > Perf cpu map has various functions where a cpumap and index are passed > in order to load the cpu. A problem with this is that the wrong index > may be passed for the cpumap, causing problems like aggregation on the > wrong CPU: > https://lore.kernel.org/lkml/20211204023409.969668-1-irogers@google.com/ > > This patch set refactors the cpu map API, reducing it and explicitly > passing the cpu (rather than the pair) to functions that need > it. Comments are added at the same time. Changes modify the same > file/function more than once as refactoring and fixes are broken apart > for the sake of bisection. > > v2. Incorproates fixes suggested Jiri Olsa, rewrites the evlist CPU > iterator in part in a way suggested by Riccardo Mancini. The new > fixes start at patch 23. The final change was suggested by John > Garry to make the CPUs have their own struct wrapper. > > v3. Incorporates fixes suggested by Namhyung Kim. > > Ian Rogers (48):
you doubled the amount of patches from v1? ;-)
I had small comments for the first 22 patches and would be ok with them merged.. will try to go through the rest soon
thanks, jirka
> libperf: Add comments to perf_cpu_map. > perf stat: Add aggr creators that are passed a cpu. > perf stat: Correct aggregation CPU map > perf stat: Switch aggregation to use for_each loop > perf stat: Switch to cpu version of cpu_map__get > perf cpumap: Switch cpu_map__build_map to cpu function > perf cpumap: Remove map+index get_socket > perf cpumap: Remove map+index get_die > perf cpumap: Remove map+index get_core > perf cpumap: Remove map+index get_node > perf cpumap: Add comments to aggr_cpu_id > perf cpumap: Remove unused cpu_map__socket > perf cpumap: Simplify equal function name. > perf cpumap: Rename empty functions. > perf cpumap: Document cpu__get_node and remove redundant function > perf cpumap: Remove map from function names that don't use a map. > perf cpumap: Remove cpu_map__cpu, use libperf function. > perf cpumap: Refactor cpu_map__build_map > perf cpumap: Rename cpu_map__get_X_aggr_by_cpu functions > perf cpumap: Move 'has' function to libperf > perf cpumap: Add some comments to cpu_aggr_map > perf cpumap: Trim the cpu_aggr_map > perf stat: Fix memory leak in check_per_pkg > perf cpumap: Add CPU to aggr_cpu_id > perf stat-display: Avoid use of core for CPU. > perf evsel: Derive CPUs and threads in alloc_counts > libperf: Switch cpu to more accurate cpu_map_idx > libperf: Use cpu not index for evsel mmap > perf counts: Switch name cpu to cpu_map_idx > perf stat: Rename aggr_data cpu to imply it's an index > perf stat: Use perf_cpu_map__for_each_cpu > perf script: Use for each cpu to aid readability > libperf: Allow NULL in perf_cpu_map__idx > perf evlist: Refactor evlist__for_each_cpu. > perf evsel: Pass cpu not cpu map index to synthesize > perf stat: Correct variable name for read counter > perf evsel: Rename CPU around get_group_fd > perf evsel: Reduce scope of evsel__ignore_missing_thread > perf evsel: Rename variable cpu to index > perf test: Use perf_cpu_map__for_each_cpu > perf stat: Correct check_per_pkg cpu > perf stat: Swap variable name cpu to index > libperf: Sync evsel documentation > perf bpf: Rename cpu to cpu_map_idx > perf c2c: Use more intention revealing iterator > perf script: Fix flipped index and cpu > perf stat: Correct first_shadow_cpu to return index > perf cpumap: Give CPUs their own type. > > tools/lib/perf/Documentation/libperf.txt | 11 +- > tools/lib/perf/cpumap.c | 131 +++-- > tools/lib/perf/evlist.c | 4 +- > tools/lib/perf/evsel.c | 92 ++-- > tools/lib/perf/include/internal/cpumap.h | 18 +- > tools/lib/perf/include/internal/evlist.h | 3 +- > tools/lib/perf/include/internal/evsel.h | 4 +- > tools/lib/perf/include/internal/mmap.h | 5 +- > tools/lib/perf/include/perf/cpumap.h | 8 +- > tools/lib/perf/include/perf/evsel.h | 10 +- > tools/lib/perf/libperf.map | 1 + > tools/lib/perf/mmap.c | 2 +- > tools/perf/arch/arm/util/cs-etm.c | 16 +- > tools/perf/bench/epoll-ctl.c | 2 +- > tools/perf/bench/epoll-wait.c | 2 +- > tools/perf/bench/futex-hash.c | 2 +- > tools/perf/bench/futex-lock-pi.c | 2 +- > tools/perf/bench/futex-requeue.c | 2 +- > tools/perf/bench/futex-wake-parallel.c | 2 +- > tools/perf/bench/futex-wake.c | 2 +- > tools/perf/builtin-c2c.c | 15 +- > tools/perf/builtin-ftrace.c | 2 +- > tools/perf/builtin-kmem.c | 2 +- > tools/perf/builtin-record.c | 2 +- > tools/perf/builtin-sched.c | 71 +-- > tools/perf/builtin-script.c | 10 +- > tools/perf/builtin-stat.c | 516 +++++++++--------- > tools/perf/tests/attr.c | 6 +- > tools/perf/tests/bitmap.c | 2 +- > tools/perf/tests/cpumap.c | 6 +- > tools/perf/tests/event_update.c | 6 +- > tools/perf/tests/mem2node.c | 2 +- > tools/perf/tests/mmap-basic.c | 4 +- > tools/perf/tests/openat-syscall-all-cpus.c | 39 +- > tools/perf/tests/stat.c | 3 +- > tools/perf/tests/topology.c | 43 +- > tools/perf/util/affinity.c | 2 +- > tools/perf/util/auxtrace.c | 12 +- > tools/perf/util/auxtrace.h | 5 +- > tools/perf/util/bpf_counter.c | 16 +- > tools/perf/util/bpf_counter.h | 4 +- > tools/perf/util/counts.c | 8 +- > tools/perf/util/counts.h | 14 +- > tools/perf/util/cpumap.c | 253 ++++----- > tools/perf/util/cpumap.h | 116 ++-- > tools/perf/util/cputopo.c | 6 +- > tools/perf/util/env.c | 29 +- > tools/perf/util/env.h | 3 +- > tools/perf/util/evlist.c | 148 ++--- > tools/perf/util/evlist.h | 50 +- > tools/perf/util/evsel.c | 143 ++--- > tools/perf/util/evsel.h | 27 +- > tools/perf/util/expr.c | 2 +- > tools/perf/util/header.c | 6 +- > tools/perf/util/mmap.c | 19 +- > tools/perf/util/mmap.h | 3 +- > tools/perf/util/perf_api_probe.c | 15 +- > tools/perf/util/python.c | 4 +- > tools/perf/util/record.c | 11 +- > .../scripting-engines/trace-event-python.c | 6 +- > tools/perf/util/session.c | 10 +- > tools/perf/util/stat-display.c | 138 ++--- > tools/perf/util/stat-shadow.c | 308 +++++------ > tools/perf/util/stat.c | 47 +- > tools/perf/util/stat.h | 9 +- > tools/perf/util/svghelper.c | 6 +- > tools/perf/util/synthetic-events.c | 12 +- > tools/perf/util/synthetic-events.h | 3 +- > tools/perf/util/util.h | 5 +- > 69 files changed, 1333 insertions(+), 1155 deletions(-) > > -- > 2.34.1.448.ga2b2bfdf31-goog >
| |