Re: [PATCH 07/13 v4] perf tools: restrict visibility of functions

[James Clark <james.clark@arm.com>]

On 15/11/2020 23:17, Jiri Olsa wrote:
> On Fri, Nov 13, 2020 at 07:26:48PM +0200, James Clark wrote:
>> These cpu_aggr_map refcounting functions are only used in
>> builtin-stat.c so their visibilty can be reduced to just
>> that file.
>>
>> No functional changes.
>>
>> Signed-off-by: James Clark <james.clark@arm.com>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>> Cc: Jiri Olsa <jolsa@redhat.com>
>> Cc: Namhyung Kim <namhyung@kernel.org>
>> Cc: Thomas Richter <tmricht@linux.ibm.com>
>> Cc: John Garry <john.garry@huawei.com>
>> ---
>>  tools/perf/builtin-stat.c | 15 +++++++++++++++
>>  tools/perf/util/cpumap.c  | 15 ---------------
>>  tools/perf/util/cpumap.h  |  2 --
>>  3 files changed, 15 insertions(+), 17 deletions(-)
>>
>> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
>> index 7daac139f6cc..344e50651b55 100644
>> --- a/tools/perf/builtin-stat.c
>> +++ b/tools/perf/builtin-stat.c
>> @@ -1326,6 +1326,21 @@ static int perf_stat_init_aggr_mode(void)
>>  	return stat_config.cpus_aggr_map ? 0 : -ENOMEM;
>>  }
>>  
>> +static void cpu_aggr_map__delete(struct cpu_aggr_map *map)
>> +{
>> +	if (map) {
>> +		WARN_ONCE(refcount_read(&map->refcnt) != 0,
>> +			  "cpu_aggr_map refcnt unbalanced\n");
>> +		free(map);
>> +	}
>> +}
>> +
>> +static void cpu_aggr_map__put(struct cpu_aggr_map *map)
>> +{
>> +	if (map && refcount_dec_and_test(&map->refcnt))
>> +		cpu_aggr_map__delete(map);
>> +}
> 
> you could add them directly as static and skip this change

I think I was trying to avoid the compilation error from the static
functions not being used. But I moved the addition into the commit
where they are used in V5.

James

> 
> jirka
> 
>> +
>>  static void perf_stat__exit_aggr_mode(void)
>>  {
>>  	cpu_aggr_map__put(stat_config.aggr_map);
>> diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
>> index e831a18ec95e..e90270f0be57 100644
>> --- a/tools/perf/util/cpumap.c
>> +++ b/tools/perf/util/cpumap.c
>> @@ -112,21 +112,6 @@ struct cpu_aggr_map *cpu_aggr_map__empty_new(int nr)
>>  	return cpus;
>>  }
>>  
>> -void cpu_aggr_map__delete(struct cpu_aggr_map *map)
>> -{
>> -	if (map) {
>> -		WARN_ONCE(refcount_read(&map->refcnt) != 0,
>> -			  "cpu_aggr_map refcnt unbalanced\n");
>> -		free(map);
>> -	}
>> -}
>> -
>> -void cpu_aggr_map__put(struct cpu_aggr_map *map)
>> -{
>> -	if (map && refcount_dec_and_test(&map->refcnt))
>> -		cpu_aggr_map__delete(map);
>> -}
>> -
>>  static int cpu__get_topology_int(int cpu, const char *name, int *value)
>>  {
>>  	char path[PATH_MAX];
>> diff --git a/tools/perf/util/cpumap.h b/tools/perf/util/cpumap.h
>> index d82822ddcbce..b112069038be 100644
>> --- a/tools/perf/util/cpumap.h
>> +++ b/tools/perf/util/cpumap.h
>> @@ -21,8 +21,6 @@ struct perf_record_cpu_map_data;
>>  
>>  struct perf_cpu_map *perf_cpu_map__empty_new(int nr);
>>  struct cpu_aggr_map *cpu_aggr_map__empty_new(int nr);
>> -void cpu_aggr_map__delete(struct cpu_aggr_map *map);
>> -void cpu_aggr_map__put(struct cpu_aggr_map *map);
>>  
>>  struct perf_cpu_map *cpu_map__new_data(struct perf_record_cpu_map_data *data);
>>  size_t cpu_map__snprint(struct perf_cpu_map *map, char *buf, size_t size);
>> -- 
>> 2.28.0
>>
>