Messages in this thread | | | From | "Liang, Kan" <> | Subject | RE: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method | Date | Thu, 10 Sep 2015 20:00:54 +0000 |
| |
> > 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@redhat.com> > 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@intel.com> > Cc: Borislav Petkov <bp@suse.de> > Cc: David Ahern <dsahern@gmail.com> > Cc: Frederic Weisbecker <fweisbec@gmail.com> > Cc: Jiri Olsa <jolsa@redhat.com> > Cc: Kan Liang <kan.liang@intel.com> > Cc: Namhyung Kim <namhyung@kernel.org> > Cc: Stephane Eranian <eranian@google.com> > Cc: Wang Nan <wangnan0@huawei.com> > Link: http://lkml.kernel.org/r/1441828225-667-5-git-send-email- > acme@kernel.org > Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> > > 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; > }
| |