Re: [PATCH 02/22] perf stat: Add aggr creators that are passed a cpu.

[John Garry]

On 08/12/2021 02:45, Ian Rogers wrote:
> The cpu_map and index can get confused. Add variants of the cpu_map__get
> routines that are passed a cpu. Make the existing cpu_map__get routines
> use the new functions with a view to remove them when no longer used.
>
> Signed-off-by: Ian Rogers<irogers@xxxxxxxxxx>
> ---
>   tools/perf/util/cpumap.c | 79 +++++++++++++++++++++++-----------------
>   tools/perf/util/cpumap.h |  6 ++-
>   2 files changed, 51 insertions(+), 34 deletions(-)
>
> diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
> index 87d3eca9b872..49fba2c53822 100644
> --- a/tools/perf/util/cpumap.c
> +++ b/tools/perf/util/cpumap.c
> @@ -128,21 +128,23 @@ int cpu_map__get_socket_id(int cpu)
>   	return ret ?: value;
>   }
>   
> -struct aggr_cpu_id cpu_map__get_socket(struct perf_cpu_map *map, int idx,
> -					void *data __maybe_unused)
> +struct aggr_cpu_id cpu_map__get_socket_aggr_by_cpu(int cpu, void *data __maybe_unused)
>   {
> -	int cpu;
>   	struct aggr_cpu_id id = cpu_map__empty_aggr_cpu_id();
>   
> -	if (idx > map->nr)
> -		return id;
> -
> -	cpu = map->map[idx];
> -
>   	id.socket = cpu_map__get_socket_id(cpu);
>   	return id;
>   }
>   
> +struct aggr_cpu_id cpu_map__get_socket(struct perf_cpu_map *map, int idx,

This is code added in this patch - so is "idx" a cpu map index? that's 
what the commit message implies.

regardless of this - you add code here and then remove it later in the 
series. Can you arrange the series such that any code added in the 
series is not removed (later in that series)? That's a general practice 
we adhere to.

Thanks,
John