Re: [PATCHv3 5/5] perf tools: Add perf data cache feature

From: Arnaldo Carvalho de Melo
Date: Tue Feb 16 2016 - 10:29:46 EST

Next message: Dave Hansen: "Re: [PATCHv2 04/28] mm: make remove_migration_ptes() beyond mm/migration.c"
Previous message: Andrew Lunn: "Re: [PATCH v3] phy: marvell: Fix and unify reg-init behavior"
In reply to: Jiri Olsa: "[PATCHv3 5/5] perf tools: Add perf data cache feature"
Next in thread: Arnaldo Carvalho de Melo: "Re: [PATCHv3 5/5] perf tools: Add perf data cache feature"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Em Tue, Feb 16, 2016 at 04:01:43PM +0100, Jiri Olsa escreveu:
> On Tue, Feb 16, 2016 at 11:22:19PM +0900, Namhyung Kim wrote:
>
> SNIP
>
> >
> > [SNIP]
> > > +static int build_caches(struct cache_level caches[], u32 size, u32 *cntp)
> > > +{
> > > + u32 i, cnt = 0;
> > > + long ncpus;
> > > + u32 nr, cpu;
> > > + u16 level;
> > > +
> > > + ncpus = sysconf(_SC_NPROCESSORS_CONF);
> > > + if (ncpus < 0)
> > > + return -1;
> > > +
> > > + nr = (u32)(ncpus & UINT_MAX);
> > > +
> > > + for (cpu = 0; cpu < nr; cpu++) {
> > > + for (level = 0; level < 10; level++) {
> > > + struct cache_level c;
> > > + int err;
> > > +
> > > + err = cache_level__read(&c, cpu, level);
> > > + if (err < 0)
> > > + return err;
> > > +
> > > + if (err == 1)
> > > + break;
> > > +
> > > + for (i = 0; i < cnt; i++) {
> > > + if (cache_level__cmp(&c, &caches[i]))
> > > + break;
> > > + }
> > > +
> > > + if (i == cnt)
> > > + caches[cnt++] = c;
> >
> > else
> > cache_level__free(&c);
> > ???
>
> ouch right.. v3 attached
>
> thanks,
> jirka
>
>
> ---
> Storing CPU cache details under perf data. It's stored
> as new HEADER_CACHE feature and it's displayed under
> header info with -I option:
>
> $ perf report --header-only -I
> ...
> # CPU cache info:
> # L1 Data 32K [0-1]
> # L1 Instruction 32K [0-1]
> # L1 Data 32K [2-3]
> # L1 Instruction 32K [2-3]
> # L2 Unified 256K [0-1]
> # L2 Unified 256K [2-3]
> # L3 Unified 4096K [0-3]
> ...
>
> All distinct caches are stored/displayed.
>
> Link: http://lkml.kernel.org/n/tip-byxl1gwto8z9d5hyozprtaty@xxxxxxxxxxxxxx
> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> ---
> tools/perf/util/env.c | 13 +++
> tools/perf/util/env.h | 15 +++
> tools/perf/util/header.c | 267 +++++++++++++++++++++++++++++++++++++++++++++++
> tools/perf/util/header.h | 1 +
> 4 files changed, 296 insertions(+)
>
> diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
> index 7dd5939dea2e..02a6970f2495 100644
> --- a/tools/perf/util/env.c
> +++ b/tools/perf/util/env.c
> @@ -6,6 +6,8 @@ struct perf_env perf_env;
>
> void perf_env__exit(struct perf_env *env)
> {
> + int i;
> +
> zfree(&env->hostname);
> zfree(&env->os_release);
> zfree(&env->version);
> @@ -19,6 +21,10 @@ void perf_env__exit(struct perf_env *env)
> zfree(&env->numa_nodes);
> zfree(&env->pmu_mappings);
> zfree(&env->cpu);
> +
> + for (i = 0; i < env->caches_cnt; i++)
> + cache_level__free(&env->caches[i]);
> + zfree(&env->caches);
> }
>
> int perf_env__set_cmdline(struct perf_env *env, int argc, const char *argv[])
> @@ -75,3 +81,10 @@ int perf_env__read_cpu_topology_map(struct perf_env *env)
> env->nr_cpus_avail = nr_cpus;
> return 0;
> }
> +
> +void cache_level__free(struct cache_level *cache)
> +{
> + free(cache->type);
> + free(cache->map);
> + free(cache->size);
> +}
> diff --git a/tools/perf/util/env.h b/tools/perf/util/env.h
> index 0132b9557c02..9963499820e7 100644
> --- a/tools/perf/util/env.h
> +++ b/tools/perf/util/env.h
> @@ -1,11 +1,23 @@
> #ifndef __PERF_ENV_H
> #define __PERF_ENV_H
>
> +#include <linux/types.h>
> +
> struct cpu_topology_map {
> int socket_id;
> int core_id;
> };
>
> +struct cache_level {
> + u32 level;
> + u32 line_size;
> + u32 sets;
> + u32 ways;
> + char *type;
> + char *size;
> + char *map;
> +};
> +
> struct perf_env {
> char *hostname;
> char *os_release;
> @@ -31,6 +43,8 @@ struct perf_env {
> char *numa_nodes;
> char *pmu_mappings;
> struct cpu_topology_map *cpu;
> + struct cache_level *caches;
> + int caches_cnt;
> };
>
> extern struct perf_env perf_env;
> @@ -41,4 +55,5 @@ 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);
>
> +void cache_level__free(struct cache_level *cache);
> #endif /* __PERF_ENV_H */
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index f50b7235ecb6..f5803fa3dc63 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -23,6 +23,8 @@
> #include "strbuf.h"
> #include "build-id.h"
> #include "data.h"
> +#include <api/fs/fs.h>
> +#include "asm/bug.h"
>
> /*
> * magic2 = "PERFILE2"
> @@ -868,6 +870,199 @@ static int write_auxtrace(int fd, struct perf_header *h,
> return err;
> }
>
> +static int cache_level__sort(const void *a, const void *b)
> +{
> + struct cache_level *cache_a = (struct cache_level *) a;
> + struct cache_level *cache_b = (struct cache_level *) b;
> +
> + return cache_a->level - cache_b->level;
> +}
> +
> +static bool cache_level__cmp(struct cache_level *a, struct cache_level *b)
> +{
> + if (a->level != b->level)
> + return false;
> +
> + if (a->line_size != b->line_size)
> + return false;
> +
> + if (a->sets != b->sets)
> + return false;
> +
> + if (a->ways != b->ways)
> + return false;
> +
> + if (strcmp(a->type, b->type))
> + return false;
> +
> + if (strcmp(a->size, b->size))
> + return false;
> +
> + if (strcmp(a->map, b->map))
> + return false;
> +
> + return true;
> +}
> +
> +static int cache_level__read(struct cache_level *cache, u32 cpu, u16 level)
> +{
> + char path[PATH_MAX], file[PATH_MAX];
> + struct stat st;
> + size_t len;
> +
> + scnprintf(path, PATH_MAX, "devices/system/cpu/cpu%d/cache/index%d/", cpu, level);
> + scnprintf(file, PATH_MAX, "%s/%s", sysfs__mountpoint(), path);
> +
> + if (stat(file, &st))
> + return 1;
> +
> + scnprintf(file, PATH_MAX, "%s/level", path);
> + if (sysfs__read_int(file, (int *) &cache->level))
> + return -1;
> +
> + scnprintf(file, PATH_MAX, "%s/coherency_line_size", path);
> + if (sysfs__read_int(file, (int *) &cache->line_size))
> + return -1;
> +
> + scnprintf(file, PATH_MAX, "%s/number_of_sets", path);
> + if (sysfs__read_int(file, (int *) &cache->sets))
> + return -1;
> +
> + scnprintf(file, PATH_MAX, "%s/ways_of_associativity", path);
> + if (sysfs__read_int(file, (int *) &cache->ways))
> + return -1;
> +
> + scnprintf(file, PATH_MAX, "%s/type", path);
> + if (sysfs__read_str(file, &cache->type, &len))
> + return -1;
> +
> + cache->type[len] = 0;
> + cache->type = rtrim(cache->type);
> +
> + scnprintf(file, PATH_MAX, "%s/size", path);
> + if (sysfs__read_str(file, &cache->size, &len)) {
> + free(cache->type);
> + return -1;
> + }
> +
> + cache->size[len] = 0;
> + cache->size = rtrim(cache->size);
> +
> + scnprintf(file, PATH_MAX, "%s/shared_cpu_list", path);
> + if (sysfs__read_str(file, &cache->map, &len)) {
> + free(cache->map);
> + free(cache->type);
> + return -1;
> + }
> +
> + cache->map[len] = 0;
> + cache->map = rtrim(cache->map);
> + return 0;
> +}
> +
> +static void cache_level__fprintf(FILE *out, struct cache_level *c)
> +{
> + fprintf(out, "L%d %-15s %8s [%s]\n", c->level, c->type, c->size, c->map);
> +}
> +
> +static int build_caches(struct cache_level caches[], u32 size, u32 *cntp)
> +{
> + u32 i, cnt = 0;
> + long ncpus;
> + u32 nr, cpu;
> + u16 level;
> +
> + ncpus = sysconf(_SC_NPROCESSORS_CONF);
> + if (ncpus < 0)
> + return -1;
> +
> + nr = (u32)(ncpus & UINT_MAX);
> +
> + for (cpu = 0; cpu < nr; cpu++) {
> + for (level = 0; level < 10; level++) {
> + struct cache_level c;
> + int err;
> +
> + err = cache_level__read(&c, cpu, level);
> + if (err < 0)
> + return err;
> +
> + if (err == 1)
> + break;
> +
> + for (i = 0; i < cnt; i++) {
> + if (cache_level__cmp(&c, &caches[i]))
> + break;
> + }
> +
> + if (i == cnt)
> + caches[cnt++] = c;
> + else
> + cache_level__free(&c);
> +
> + if (WARN_ONCE(cnt == size, "way too many cpu caches.."))
> + goto out;
> + }
> + }
> + out:
> + *cntp = cnt;
> + return 0;
> +}
> +
> +#define MAX_CACHES 2000
> +
> +static int write_cache(int fd, struct perf_header *h __maybe_unused,
> + struct perf_evlist *evlist __maybe_unused)
> +{
> + struct cache_level caches[MAX_CACHES];
> + u32 cnt = 0, i, version = 1;
> + int ret;
> +
> + ret = build_caches(caches, MAX_CACHES, &cnt);
> + if (ret)
> + goto out;
> +
> + qsort(&caches, cnt, sizeof(struct cache_level), cache_level__sort);
> +
> + ret = do_write(fd, &version, sizeof(u32));
> + if (ret < 0)
> + goto out;
> +
> + ret = do_write(fd, &cnt, sizeof(u32));
> + if (ret < 0)
> + goto out;
> +
> + for (i = 0; i < cnt; i++) {
> + struct cache_level *c = &caches[i];
> +
> + #define _W(v) \
> + ret = do_write(fd, &c->v, sizeof(u32)); \
> + if (ret < 0) \
> + goto out;
> +
> + _W(level)
> + _W(line_size)
> + _W(sets)
> + _W(ways)
> + #undef _W
> +
> + #define _W(v) \
> + ret = do_write_string(fd, (const char *) c->v); \
> + if (ret < 0) \
> + goto out;
> +
> + _W(type)
> + _W(size)
> + _W(map)
> + #undef _W
> + }
> +
> +out:
> + for (i = 0; i < cnt; i++)
> + cache_level__free(&caches[i]);
> + return ret;
> +}
> +
> static int write_stat(int fd __maybe_unused,
> struct perf_header *h __maybe_unused,
> struct perf_evlist *evlist __maybe_unused)
> @@ -1172,6 +1367,18 @@ static void print_stat(struct perf_header *ph __maybe_unused,
> fprintf(fp, "# contains stat data\n");
> }
>
> +static void print_cache(struct perf_header *ph __maybe_unused,
> + int fd __maybe_unused, FILE *fp __maybe_unused)
> +{
> + int i;
> +
> + fprintf(fp, "# CPU cache info:\n");
> + for (i = 0; i < ph->env.caches_cnt; i++) {
> + fprintf(fp, "# ");
> + cache_level__fprintf(fp, &ph->env.caches[i]);
> + }
> +}
> +
> static void print_pmu_mappings(struct perf_header *ph, int fd __maybe_unused,
> FILE *fp)
> {
> @@ -1920,6 +2127,65 @@ static int process_auxtrace(struct perf_file_section *section,
> return err;
> }
>
> +static int process_cache(struct perf_file_section *section __maybe_unused,
> + struct perf_header *ph __maybe_unused, int fd __maybe_unused,
> + void *data __maybe_unused)
> +{
> + struct cache_level *caches;
> + u32 cnt, i, version;
> +
> + if (readn(fd, &version, sizeof(version)) != sizeof(version))
> + return -1;
> +
> + if (ph->needs_swap)
> + version = bswap_32(version);
> +
> + if (version != 1)
> + return -1;
> +
> + if (readn(fd, &cnt, sizeof(cnt)) != sizeof(cnt))
> + return -1;
> +
> + if (ph->needs_swap)
> + cnt = bswap_32(cnt);
> +
> + caches = zalloc(sizeof(struct cache_level) * cnt);
> + if (!caches)
> + return -1;
> +
> + for (i = 0; i < cnt; i++) {
> + struct cache_level c;
> +
> + #define _R(v) \
> + if (readn(fd, &c.v, sizeof(u32)) != sizeof(u32))\
> + return -1; \

So in this case that 'caches' variable leaks?

> + if (ph->needs_swap) \
> + c.v = bswap_32(c.v); \
> +
> + _R(level)
> + _R(line_size)
> + _R(sets)
> + _R(ways)
> + #undef _R
> +
> + #define _R(v) \
> + c.v = do_read_string(fd, ph); \
> + if (!c.v) \

Ditto.

> + return -1;
> +
> + _R(type)
> + _R(size)
> + _R(map)
> + #undef _R
> +
> + caches[i] = c;
> + }

Because it is only here that you assign it to something outside this
function.

Also, as discussed on IRC, please s/cache_level/cpu_cache_level/g, as
per Ingo's comment.

- Arnaldo

> + ph->env.caches = caches;
> + ph->env.caches_cnt = cnt;
> + return 0;
> +}
> +
> struct feature_ops {
> int (*write)(int fd, struct perf_header *h, struct perf_evlist *evlist);
> void (*print)(struct perf_header *h, int fd, FILE *fp);
> @@ -1962,6 +2228,7 @@ static const struct feature_ops feat_ops[HEADER_LAST_FEATURE] = {
> FEAT_OPP(HEADER_GROUP_DESC, group_desc),
> FEAT_OPP(HEADER_AUXTRACE, auxtrace),
> FEAT_OPA(HEADER_STAT, stat),
> + FEAT_OPF(HEADER_CACHE, cache),
> };
>
> struct header_print_data {
> diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
> index cff9892452ee..3d87ca823c0a 100644
> --- a/tools/perf/util/header.h
> +++ b/tools/perf/util/header.h
> @@ -32,6 +32,7 @@ enum {
> HEADER_GROUP_DESC,
> HEADER_AUXTRACE,
> HEADER_STAT,
> + HEADER_CACHE,
> HEADER_LAST_FEATURE,
> HEADER_FEAT_BITS = 256,
> };
> --
> 2.4.3

Next message: Dave Hansen: "Re: [PATCHv2 04/28] mm: make remove_migration_ptes() beyond mm/migration.c"
Previous message: Andrew Lunn: "Re: [PATCH v3] phy: marvell: Fix and unify reg-init behavior"
In reply to: Jiri Olsa: "[PATCHv3 5/5] perf tools: Add perf data cache feature"
Next in thread: Arnaldo Carvalho de Melo: "Re: [PATCHv3 5/5] perf tools: Add perf data cache feature"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]