Messages in this thread | | | Date | Thu, 30 Jun 2016 18:20:09 -0300 | From | Arnaldo Carvalho de Melo <> | Subject | Re: [PATCH 5/5] perf tools: Transform nodes string info to struct |
| |
Em Tue, Jun 28, 2016 at 01:29:05PM +0200, Jiri Olsa escreveu: > Storing NUMA info within struct numa_node instead > of strings. This way it's usable in future patches. > > Also it turned out it's slightly less code involved > than using strings. > > Link: http://lkml.kernel.org/n/tip-ka37sax3gfaxwvytfxi0ycy1@git.kernel.org > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > --- > tools/perf/util/env.c | 5 +++- > tools/perf/util/env.h | 12 ++++++-- > tools/perf/util/header.c | 78 +++++++++++++++++------------------------------- > 3 files changed, 41 insertions(+), 54 deletions(-) > > diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c > index 49a11d9d8b8f..153985977475 100644 > --- a/tools/perf/util/env.c > +++ b/tools/perf/util/env.c > @@ -18,10 +18,13 @@ void perf_env__exit(struct perf_env *env) > zfree(&env->cmdline_argv); > zfree(&env->sibling_cores); > zfree(&env->sibling_threads); > - zfree(&env->numa_nodes); > zfree(&env->pmu_mappings); > zfree(&env->cpu); > > + for (i = 0; i < env->numa_nodes_cnt; i++) > + cpu_map__put(env->numa_nodes[i].map); > + zfree(&env->numa_nodes); > + > for (i = 0; i < env->caches_cnt; i++) > cpu_cache_level__free(&env->caches[i]); > zfree(&env->caches); > diff --git a/tools/perf/util/env.h b/tools/perf/util/env.h > index 56cffb60a0b4..027d0f4e6bff 100644 > --- a/tools/perf/util/env.h > +++ b/tools/perf/util/env.h > @@ -2,6 +2,7 @@ > #define __PERF_ENV_H > > #include <linux/types.h> > +#include "cpumap.h" > > struct cpu_topology_map { > int socket_id; > @@ -18,6 +19,13 @@ struct cpu_cache_level { > char *map; > }; > > +struct numa_node { > + u32 node; > + u64 mem_total; > + u64 mem_free; > + struct cpu_map *map; > +}; > + > struct perf_env { > char *hostname; > char *os_release; > @@ -33,18 +41,18 @@ struct perf_env { > int nr_cmdline; > int nr_sibling_cores; > int nr_sibling_threads; > - int nr_numa_nodes;
why rename it from nr_numa_nodes to numa_nodes_cnt? Seems gratuitous and potentially introduces up to three 4 byte holes into 'struct perf_env' :-\
Applied the other patches in this series,
Thanks,
- Arnaldo
> int nr_pmu_mappings; > int nr_groups; > char *cmdline; > const char **cmdline_argv; > char *sibling_cores; > char *sibling_threads; > - char *numa_nodes; > char *pmu_mappings; > struct cpu_topology_map *cpu; > struct cpu_cache_level *caches; > int caches_cnt; > + struct numa_node *numa_nodes; > + int numa_nodes_cnt; > }; > > extern struct perf_env perf_env; > diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c > index c5cd2698281f..af5f275462b3 100644 > --- a/tools/perf/util/header.c > +++ b/tools/perf/util/header.c > @@ -1306,42 +1306,19 @@ static void print_total_mem(struct perf_header *ph, int fd __maybe_unused, > static void print_numa_topology(struct perf_header *ph, int fd __maybe_unused, > FILE *fp) > { > - u32 nr, c, i; > - char *str, *tmp; > - uint64_t mem_total, mem_free; > - > - /* nr nodes */ > - nr = ph->env.nr_numa_nodes; > - str = ph->env.numa_nodes; > - > - for (i = 0; i < nr; i++) { > - /* node number */ > - c = strtoul(str, &tmp, 0); > - if (*tmp != ':') > - goto error; > - > - str = tmp + 1; > - mem_total = strtoull(str, &tmp, 0); > - if (*tmp != ':') > - goto error; > + int i; > + struct numa_node *n; > > - str = tmp + 1; > - mem_free = strtoull(str, &tmp, 0); > - if (*tmp != ':') > - goto error; > + for (i = 0; i < ph->env.numa_nodes_cnt; i++) { > + n = &ph->env.numa_nodes[i]; > > fprintf(fp, "# node%u meminfo : total = %"PRIu64" kB," > " free = %"PRIu64" kB\n", > - c, mem_total, mem_free); > + n->node, n->mem_total, n->mem_free); > > - str = tmp + 1; > - fprintf(fp, "# node%u cpu list : %s\n", c, str); > - > - str += strlen(str) + 1; > + fprintf(fp, "# node%u cpu list : ", n->node); > + cpu_map__fprintf(n->map, fp); > } > - return; > -error: > - fprintf(fp, "# numa topology : not available\n"); > } > > static void print_cpuid(struct perf_header *ph, int fd __maybe_unused, FILE *fp) > @@ -1906,11 +1883,10 @@ static int process_numa_topology(struct perf_file_section *section __maybe_unuse > struct perf_header *ph, int fd, > void *data __maybe_unused) > { > + struct numa_node *nodes, *n; > ssize_t ret; > - u32 nr, node, i; > + u32 nr, i; > char *str; > - uint64_t mem_total, mem_free; > - struct strbuf sb; > > /* nr nodes */ > ret = readn(fd, &nr, sizeof(nr)); > @@ -1920,48 +1896,48 @@ static int process_numa_topology(struct perf_file_section *section __maybe_unuse > if (ph->needs_swap) > nr = bswap_32(nr); > > - ph->env.nr_numa_nodes = nr; > - if (strbuf_init(&sb, 256) < 0) > - return -1; > + ph->env.numa_nodes_cnt = nr; > + nodes = zalloc(sizeof(*nodes) * nr); > + if (!nodes) > + return -ENOMEM; > > for (i = 0; i < nr; i++) { > + n = &nodes[i]; > + > /* node number */ > - ret = readn(fd, &node, sizeof(node)); > - if (ret != sizeof(node)) > + ret = readn(fd, &n->node, sizeof(u32)); > + if (ret != sizeof(n->node)) > goto error; > > - ret = readn(fd, &mem_total, sizeof(u64)); > + ret = readn(fd, &n->mem_total, sizeof(u64)); > if (ret != sizeof(u64)) > goto error; > > - ret = readn(fd, &mem_free, sizeof(u64)); > + ret = readn(fd, &n->mem_free, sizeof(u64)); > if (ret != sizeof(u64)) > goto error; > > if (ph->needs_swap) { > - node = bswap_32(node); > - mem_total = bswap_64(mem_total); > - mem_free = bswap_64(mem_free); > + n->node = bswap_32(n->node); > + n->mem_total = bswap_64(n->mem_total); > + n->mem_free = bswap_64(n->mem_free); > } > > - if (strbuf_addf(&sb, "%u:%"PRIu64":%"PRIu64":", > - node, mem_total, mem_free) < 0) > - goto error; > - > str = do_read_string(fd, ph); > if (!str) > goto error; > > - /* include a NULL character at the end */ > - if (strbuf_add(&sb, str, strlen(str) + 1) < 0) > + n->map = cpu_map__new(str); > + if (!n->map) > goto error; > + > free(str); > } > - ph->env.numa_nodes = strbuf_detach(&sb, NULL); > + ph->env.numa_nodes = nodes; > return 0; > > error: > - strbuf_release(&sb); > + free(nodes); > return -1; > } > > -- > 2.4.11
| |