| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20160630212009.GG5324@kernel.org>
Date: Thu, 30 Jun 2016 18:20:09 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Jiri Olsa <jolsa@...nel.org>
Cc: lkml <linux-kernel@...r.kernel.org>,
David Ahern <dsahern@...il.com>,
Ingo Molnar <mingo@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
Peter Zijlstra <a.p.zijlstra@...llo.nl>
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@...nel.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
Powered by blists - more mailing lists