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

[Liang, Kan]

> 
> Em Wed, Sep 09, 2015 at 09:41:18PM +0000, Liang, Kan escreveu:
> > > -	for (i = 0; i < tp->cpu_nr; i++) {
> > > -		ret = do_write(fd, &tp->core_id[i], sizeof(int));
> > > +	perf_env__read_cpu_topology_map(&perf_env);
> > > +
> >
> > I think we need to handle error here.
> 
> Ok, adding a test, updated patch below. While doing that, noticed that the
> in the thread_sib case the "ret = do_something()" doesn't makes the
> whole function fail, will fix later.
> 

Thanks.
Except patch 11 & 13 (for reverting), the rest of patches are good to me.

Thanks,
Kan

> - Arnaldo
> 
> commit c98989f4a47a6fd3f3f8bd937b96faaced8b203e
> Author: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> Date:   Wed Sep 9 10:37:01 2015 -0300
> 
>     perf env: Introduce read_cpu_topology_map() method
> 
>     Out of the code to write the cpu topology map in the perf.data file
>     header.
> 
>     Now if one needs the CPU topology map for the running machine, one
> needs
>     to call perf_env__read_cpu_topology_map(perf_env) and the info will
> be
>     stored in perf_env.cpu.
> 
>     For now we're using a global perf_env variable, that will have its
>     contents freed after we run a builtin.
> 
>     v2: Check perf_env__read_cpu_topology_map() return in
>         write_cpu_topology() (Kan Liang)
> 
>     Cc: Adrian Hunter <adrian.hunter@xxxxxxxxx>
>     Cc: Borislav Petkov <bp@xxxxxxx>
>     Cc: David Ahern <dsahern@xxxxxxxxx>
>     Cc: Frederic Weisbecker <fweisbec@xxxxxxxxx>
>     Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
>     Cc: Kan Liang <kan.liang@xxxxxxxxx>
>     Cc: Namhyung Kim <namhyung@xxxxxxxxxx>
>     Cc: Stephane Eranian <eranian@xxxxxxxxxx>
>     Cc: Wang Nan <wangnan0@xxxxxxxxxx>
>     Link: http://lkml.kernel.org/r/1441828225-667-5-git-send-email-
> acme@xxxxxxxxxx
>     Signed-off-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> 
> diff --git a/tools/perf/perf.c b/tools/perf/perf.c index
> f2fc019b3671..1fded922bcc8 100644
> --- a/tools/perf/perf.c
> +++ b/tools/perf/perf.c
> @@ -8,6 +8,7 @@
>   */
>  #include "builtin.h"
> 
> +#include "util/env.h"
>  #include "util/exec_cmd.h"
>  #include "util/cache.h"
>  #include "util/quote.h"
> @@ -369,6 +370,7 @@ static int run_builtin(struct cmd_struct *p, int argc,
> const char **argv)
> 
>  	status = p->fn(argc, argv, prefix);
>  	exit_browser(status);
> +	perf_env__exit(&perf_env);
> 
>  	if (status)
>  		return status & 0xff;
> diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c index
> ca1e33a2203e..6af4f7c36820 100644
> --- a/tools/perf/util/env.c
> +++ b/tools/perf/util/env.c
> @@ -1,3 +1,4 @@
> +#include "cpumap.h"
>  #include "env.h"
>  #include "util.h"
> 
> @@ -56,3 +57,30 @@ out_free:
>  out_enomem:
>  	return -ENOMEM;
>  }
> +
> +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);
> +	}
> +
> +	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;
>  	}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/