Re: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method

["Wangnan (F)" <wangnan0@huawei.com>]

On 2015/9/10 21:12, Arnaldo Carvalho de Melo wrote:

[SNIP]

> +
> +int perf_env__read_cpu_topology_map(struct perf_env *env)
> +{
> +	int cpu, nr_cpus;
> +
> +	if (env->cpu != NULL)
> +		return 0;
> +
> +	if (env->nr_cpus_avail == 0)
> +		env->nr_cpus_avail = sysconf(_SC_NPROCESSORS_CONF);
> +
> +	nr_cpus = env->nr_cpus_avail;
> +	if (nr_cpus == -1)
> +		return -EINVAL;
> +
> +	env->cpu = calloc(nr_cpus, sizeof(env->cpu[0]));
> +	if (env->cpu == NULL)
> +		return -ENOMEM;
> +
> +	for (cpu = 0; cpu < nr_cpus; ++cpu) {
> +		env->cpu[cpu].core_id	= cpu_map__get_core_id(cpu);
> +		env->cpu[cpu].socket_id	= cpu_map__get_socket_id(cpu);
> +	}

Shouldn't we check the failure of these two functions?

At this point perf_env__read_cpu_topology_map and build_cpu_topology are 
doing
similar things. build_cpu_topology() reads 
/sys/xxxxx/cpu%d/topology/{core,thread}_siblings_list,
perf_env__read_cpu_topology_map() reads /sys/xxxxx/cpu%d/topology/core_id,
but build_cpu_topology() returns error if any read failed, but
perf_env__read_cpu_topology_map() fills core_id and socket_id with -1 if 
read fail.

I tried to offline a core between build_cpu_topology() and 
perf_env__read_cpu_topology_map(),
and perf report say:

  # perf report -v --header-only -I
  build id event received for [kernel.kallsyms]: (...)
  core_id number is too big.You may need to upgrade the perf tool. <-- 
*see this warning*
  # ========
  # captured on: Sun Feb 15 15:01:05 2009
  # hostname : localhost
  ...
  # sibling cores   : ...
  # sibling threads : 7
  # Core ID and Socket ID information is not available
  # pmu mappings: not available
  # ========

Thank you.

> +
> +	env->nr_cpus_avail = nr_cpus;
> +	return 0;
> +}
> diff --git a/tools/perf/util/env.h b/tools/perf/util/env.h
> index 70124d9a1624..c4e36323d91e 100644
> --- a/tools/perf/util/env.h
> +++ b/tools/perf/util/env.h
> @@ -38,4 +38,6 @@ void perf_env__exit(struct perf_env *env);
>   
>   int perf_env__set_cmdline(struct perf_env *env, int argc, const char *argv[]);
>   
> +int perf_env__read_cpu_topology_map(struct perf_env *env);
> +
>   #endif /* __PERF_ENV_H */
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index 151b8310ac70..d6437465f70f 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -415,8 +415,6 @@ struct cpu_topo {
>   	u32 thread_sib;
>   	char **core_siblings;
>   	char **thread_siblings;
> -	int *core_id;
> -	int *phy_pkg_id;
>   };
>   
>   static int build_cpu_topo(struct cpu_topo *tp, int cpu)
> @@ -479,9 +477,6 @@ try_threads:
>   	}
>   	ret = 0;
>   done:
> -	tp->core_id[cpu] = cpu_map__get_core_id(cpu);
> -	tp->phy_pkg_id[cpu] = cpu_map__get_socket_id(cpu);
> -
>   	if(fp)
>   		fclose(fp);
>   	free(buf);
> @@ -509,7 +504,7 @@ static struct cpu_topo *build_cpu_topology(void)
>   	struct cpu_topo *tp;
>   	void *addr;
>   	u32 nr, i;
> -	size_t sz, sz_id;
> +	size_t sz;
>   	long ncpus;
>   	int ret = -1;
>   
> @@ -520,9 +515,8 @@ static struct cpu_topo *build_cpu_topology(void)
>   	nr = (u32)(ncpus & UINT_MAX);
>   
>   	sz = nr * sizeof(char *);
> -	sz_id = nr * sizeof(int);
>   
> -	addr = calloc(1, sizeof(*tp) + 2 * sz + 2 * sz_id);
> +	addr = calloc(1, sizeof(*tp) + 2 * sz);
>   	if (!addr)
>   		return NULL;
>   
> @@ -532,10 +526,6 @@ static struct cpu_topo *build_cpu_topology(void)
>   	tp->core_siblings = addr;
>   	addr += sz;
>   	tp->thread_siblings = addr;
> -	addr += sz;
> -	tp->core_id = addr;
> -	addr += sz_id;
> -	tp->phy_pkg_id = addr;
>   
>   	for (i = 0; i < nr; i++) {
>   		ret = build_cpu_topo(tp, i);
> @@ -554,7 +544,7 @@ static int write_cpu_topology(int fd, struct perf_header *h __maybe_unused,
>   {
>   	struct cpu_topo *tp;
>   	u32 i;
> -	int ret;
> +	int ret, j;
>   
>   	tp = build_cpu_topology();
>   	if (!tp)
> @@ -579,11 +569,17 @@ static int write_cpu_topology(int fd, struct perf_header *h __maybe_unused,
>   			break;
>   	}
>   
> -	for (i = 0; i < tp->cpu_nr; i++) {
> -		ret = do_write(fd, &tp->core_id[i], sizeof(int));
> +	ret = perf_env__read_cpu_topology_map(&perf_env);
> +	if (ret < 0)
> +		goto done;
> +
> +	for (j = 0; j < perf_env.nr_cpus_avail; j++) {
> +		ret = do_write(fd, &perf_env.cpu[j].core_id,
> +			       sizeof(perf_env.cpu[j].core_id));
>   		if (ret < 0)
>   			return ret;
> -		ret = do_write(fd, &tp->phy_pkg_id[i], sizeof(int));
> +		ret = do_write(fd, &perf_env.cpu[j].socket_id,
> +			       sizeof(perf_env.cpu[j].socket_id));
>   		if (ret < 0)
>   			return ret;
>   	}