Re: [PATCH v2 2/3] perf: make build_cpu_topology skip offline/absent CPUs

[Jiri Olsa]

On Mon, Feb 13, 2017 at 04:34:35PM +0100, Jan Stancek wrote:

SNIP

> This patch makes build_cpu_topology() skip offline/absent CPUs,
> by checking their presence against cpu_map built from online CPUs.
> 
> Signed-off-by: Jan Stancek <jstancek@xxxxxxxxxx>
> ---
>  tools/perf/util/header.c | 21 +++++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> Changes in v2:
> - drop out label, use return NULL where possible
> 
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index d89c9c7ef4e5..4b0ea4e92e9d 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -503,24 +503,31 @@ static void free_cpu_topo(struct cpu_topo *tp)
>  
>  static struct cpu_topo *build_cpu_topology(void)
>  {
> -	struct cpu_topo *tp;
> +	struct cpu_topo *tp = NULL;
>  	void *addr;
>  	u32 nr, i;
>  	size_t sz;
>  	long ncpus;
> -	int ret = -1;
> +	int ret = 0;

hum, shoudn't we fail if we dont find any online cpu?

jirka

> +	struct cpu_map *map;
>  
>  	ncpus = sysconf(_SC_NPROCESSORS_CONF);
>  	if (ncpus < 0)
>  		return NULL;
>  
> +	/* build online CPU map */
> +	map = cpu_map__new(NULL);
> +	if (map == NULL) {
> +		pr_debug("failed to get system cpumap\n");
> +		return NULL;
> +	}
> +
>  	nr = (u32)(ncpus & UINT_MAX);
>  
>  	sz = nr * sizeof(char *);
> -
>  	addr = calloc(1, sizeof(*tp) + 2 * sz);
>  	if (!addr)
> -		return NULL;
> +		goto out_free;
>  
>  	tp = addr;
>  	tp->cpu_nr = nr;
> @@ -530,10 +537,16 @@ static struct cpu_topo *build_cpu_topology(void)
>  	tp->thread_siblings = addr;
>  
>  	for (i = 0; i < nr; i++) {
> +		if (!cpu_map__has(map, i))
> +			continue;
> +
>  		ret = build_cpu_topo(tp, i);
>  		if (ret < 0)
>  			break;
>  	}
> +
> +out_free:
> +	cpu_map__put(map);
>  	if (ret) {
>  		free_cpu_topo(tp);
>  		tp = NULL;
> -- 
> 1.8.3.1
>