2015-08-28 20:59:43

by Liang, Kan

[permalink] [raw]
Subject: [PATCH V2 1/2] perf,tools: store cpu socket_id and core_id in perf.date

From: Kan Liang <[email protected]>

This patch stores cpu socket_id and core_id in perf.date, and read them
to perf_env in header process.

Signed-off-by: Kan Liang <[email protected]>
---

Changes since V1:
- Store core_id and socket_id in perf.date

tools/perf/util/header.c | 97 +++++++++++++++++++++++++++++++++++++++++++++--
tools/perf/util/header.h | 6 +++
tools/perf/util/session.c | 1 +
3 files changed, 100 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 4181454..482749f 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -439,14 +439,42 @@ static int write_cmdline(int fd, struct perf_header *h __maybe_unused,
"/sys/devices/system/cpu/cpu%d/topology/core_siblings_list"
#define THRD_SIB_FMT \
"/sys/devices/system/cpu/cpu%d/topology/thread_siblings_list"
+#define CORE_ID_FMT \
+ "/sys/devices/system/cpu/cpu%d/topology/core_id"
+#define PHY_PKG_ID_FMT \
+ "/sys/devices/system/cpu/cpu%d/topology/physical_package_id"

struct cpu_topo {
+ u32 cpu_nr;
u32 core_sib;
u32 thread_sib;
char **core_siblings;
char **thread_siblings;
+ int *core_id;
+ int *phy_pkg_id;
};

+static int read_id(const char *path, int cpu)
+{
+ FILE *fp;
+ char filename[MAXPATHLEN];
+ char *buf = NULL;
+ size_t len = 0;
+ int ret = -1;
+
+ sprintf(filename, path, cpu);
+ fp = fopen(filename, "r");
+ if (fp == NULL)
+ return ret;
+
+ if (getline(&buf, &len, fp) > 0)
+ ret = atoi(buf);
+
+ fclose(fp);
+ free(buf);
+ return ret;
+}
+
static int build_cpu_topo(struct cpu_topo *tp, int cpu)
{
FILE *fp;
@@ -507,6 +535,9 @@ try_threads:
}
ret = 0;
done:
+ tp->core_id[cpu] = read_id(CORE_ID_FMT, cpu);
+ tp->phy_pkg_id[cpu] = read_id(PHY_PKG_ID_FMT, cpu);
+
if(fp)
fclose(fp);
free(buf);
@@ -534,7 +565,7 @@ static struct cpu_topo *build_cpu_topology(void)
struct cpu_topo *tp;
void *addr;
u32 nr, i;
- size_t sz;
+ size_t sz, sz_id;
long ncpus;
int ret = -1;

@@ -545,17 +576,22 @@ 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);
+ addr = calloc(1, sizeof(*tp) + 2 * sz + 2 * sz_id);
if (!addr)
return NULL;

tp = addr;
-
+ tp->cpu_nr = nr;
addr += sizeof(*tp);
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);
@@ -598,6 +634,15 @@ static int write_cpu_topology(int fd, struct perf_header *h __maybe_unused,
if (ret < 0)
break;
}
+
+ for (i = 0; i < tp->cpu_nr; i++) {
+ ret = do_write(fd, &tp->core_id[i], sizeof(int));
+ if (ret < 0)
+ return ret;
+ ret = do_write(fd, &tp->phy_pkg_id[i], sizeof(int));
+ if (ret < 0)
+ return ret;
+ }
done:
free_cpu_topo(tp);
return ret;
@@ -938,6 +983,7 @@ static void print_cpu_topology(struct perf_header *ph, int fd __maybe_unused,
{
int nr, i;
char *str;
+ int cpu_nr = ph->env.nr_cpus_online;

nr = ph->env.nr_sibling_cores;
str = ph->env.sibling_cores;
@@ -954,6 +1000,10 @@ static void print_cpu_topology(struct perf_header *ph, int fd __maybe_unused,
fprintf(fp, "# sibling threads : %s\n", str);
str += strlen(str) + 1;
}
+
+ for (i = 0; i < cpu_nr; i++)
+ fprintf(fp, "# CPU %d: Core ID %d, Socket ID %d\n", i,
+ ph->env.cpu[i].core_id, ph->env.cpu[i].socket_id);
}

static void free_event_desc(struct perf_evsel *events)
@@ -1590,10 +1640,15 @@ static int process_cpu_topology(struct perf_file_section *section __maybe_unused
u32 nr, i;
char *str;
struct strbuf sb;
+ int cpu_nr = ph->env.nr_cpus_online;
+
+ ph->env.cpu = calloc(cpu_nr, sizeof(*ph->env.cpu));
+ if (!ph->env.cpu)
+ return -1;

ret = readn(fd, &nr, sizeof(nr));
if (ret != sizeof(nr))
- return -1;
+ goto free_cpu;

if (ph->needs_swap)
nr = bswap_32(nr);
@@ -1631,10 +1686,44 @@ static int process_cpu_topology(struct perf_file_section *section __maybe_unused
free(str);
}
ph->env.sibling_threads = strbuf_detach(&sb, NULL);
+
+ for (i = 0; i < (u32)cpu_nr; i++) {
+ ret = readn(fd, &nr, sizeof(nr));
+ if (ret != sizeof(nr))
+ goto free_cpu;
+
+ if (ph->needs_swap)
+ nr = bswap_32(nr);
+
+ if (nr > (u32)cpu_nr) {
+ pr_debug("core_id number is too big."
+ "You may need to upgrade the perf tool.\n");
+ goto free_cpu;
+ }
+ ph->env.cpu[i].core_id = nr;
+
+ ret = readn(fd, &nr, sizeof(nr));
+ if (ret != sizeof(nr))
+ goto free_cpu;
+
+ if (ph->needs_swap)
+ nr = bswap_32(nr);
+
+ if (nr > (u32)cpu_nr) {
+ pr_debug("socket_id number is too big."
+ "You may need to upgrade the perf tool.\n");
+ goto free_cpu;
+ }
+
+ ph->env.cpu[i].socket_id = nr;
+ }
+
return 0;

error:
strbuf_release(&sb);
+free_cpu:
+ free(ph->env.cpu);
return -1;
}

diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
index 396e496..975d803 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -66,6 +66,11 @@ struct perf_header;
int perf_file_header__read(struct perf_file_header *header,
struct perf_header *ph, int fd);

+struct cpu_topology_map {
+ int socket_id;
+ int core_id;
+};
+
struct perf_env {
char *hostname;
char *os_release;
@@ -89,6 +94,7 @@ struct perf_env {
char *sibling_threads;
char *numa_nodes;
char *pmu_mappings;
+ struct cpu_topology_map *cpu;
};

struct perf_header {
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 8a4537e..61669be 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -185,6 +185,7 @@ static void perf_session_env__exit(struct perf_env *env)
zfree(&env->sibling_threads);
zfree(&env->numa_nodes);
zfree(&env->pmu_mappings);
+ zfree(&env->cpu);
}

void perf_session__delete(struct perf_session *session)
--
1.8.3.1


2015-08-28 20:59:56

by Liang, Kan

[permalink] [raw]
Subject: [PATCH V2 2/2] perf,test: test cpu topology

From: Jiri Olsa <[email protected]>

This patch test cpu core_id and socket_id which are stored in perf_env.

Signed-off-by: Jiri Olsa <[email protected]>
Signed-off-by: Kan Liang <[email protected]>
---

Changes since jirka's original version
- Use pr_debug to replace fprintf
- Add date_size to avoid warning
- Introduce cpu_map, and compare core_id and socket_id
between cpu_map and perf_env

tools/perf/tests/Build | 1 +
tools/perf/tests/builtin-test.c | 4 ++
tools/perf/tests/tests.h | 1 +
tools/perf/tests/topology.c | 85 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 91 insertions(+)
create mode 100644 tools/perf/tests/topology.c

diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
index c1518bd..208bbdf 100644
--- a/tools/perf/tests/Build
+++ b/tools/perf/tests/Build
@@ -33,6 +33,7 @@ perf-y += parse-no-sample-id-all.o
perf-y += kmod-path.o
perf-y += thread-map.o
perf-y += llvm.o
+perf-y += topology.o

perf-$(CONFIG_X86) += perf-time-to-tsc.o

diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 136cd93..6650f26 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -179,6 +179,10 @@ static struct test {
.func = test__llvm,
},
{
+ .desc = "Test topology in session",
+ .func = test_session_topology,
+ },
+ {
.func = NULL,
},
};
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index bf113a2..95654d7 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -63,6 +63,7 @@ int test__fdarray__add(void);
int test__kmod_path__parse(void);
int test__thread_map(void);
int test__llvm(void);
+int test_session_topology(void);

#if defined(__x86_64__) || defined(__i386__) || defined(__arm__) || defined(__aarch64__)
#ifdef HAVE_DWARF_UNWIND_SUPPORT
diff --git a/tools/perf/tests/topology.c b/tools/perf/tests/topology.c
new file mode 100644
index 0000000..15ae0f3
--- /dev/null
+++ b/tools/perf/tests/topology.c
@@ -0,0 +1,85 @@
+#include <string.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include "tests.h"
+#include "util.h"
+#include "session.h"
+#include "evlist.h"
+#include "debug.h"
+
+#define TEMPL "/tmp/perf-test-XXXXXX"
+#define DATA_SIZE 10
+
+static int get_temp(char *path)
+{
+ int fd;
+
+ strcpy(path, TEMPL);
+
+ fd = mkstemp(path);
+ if (fd < 0) {
+ perror("mkstemp failed");
+ return -1;
+ }
+
+ close(fd);
+ return 0;
+}
+
+int test_session_topology(void)
+{
+ struct perf_session *session;
+ char path[PATH_MAX];
+ struct cpu_map *map;
+ struct perf_data_file file = {
+ .path = path,
+ .mode = PERF_DATA_MODE_WRITE,
+ };
+ int i;
+
+ TEST_ASSERT_VAL("can't get templ file", !get_temp(path));
+
+ pr_debug("krava %s\n", path);
+
+ session = perf_session__new(&file, false, NULL);
+ TEST_ASSERT_VAL("can't get session", session);
+
+ session->evlist = perf_evlist__new_default();
+ TEST_ASSERT_VAL("can't get evlist", session->evlist);
+
+ perf_header__set_feat(&session->header, HEADER_CPU_TOPOLOGY);
+ perf_header__set_feat(&session->header, HEADER_NRCPUS);
+
+ session->header.data_size += DATA_SIZE;
+
+ TEST_ASSERT_VAL("failed to write header",
+ !perf_session__write_header(session, session->evlist, file.fd, true));
+
+ perf_session__delete(session);
+
+ map = cpu_map__new(NULL);
+ TEST_ASSERT_VAL("failed to get system cpumap", !(map == NULL));
+
+ file.mode = PERF_DATA_MODE_READ;
+ session = perf_session__new(&file, false, NULL);
+ TEST_ASSERT_VAL("can't get session", session);
+
+ for (i = 0; i < session->header.env.nr_cpus_online; i++) {
+ pr_debug("CPU %d, core %d, socket %d\n", i,
+ session->header.env.cpu[i].core_id,
+ session->header.env.cpu[i].socket_id);
+ }
+
+ for (i = 0; i < map->nr; i++) {
+ TEST_ASSERT_VAL("Core ID doesn't match",
+ (session->header.env.cpu[map->map[i]].core_id == (cpu_map__get_core(map, i) & 0xffff)));
+
+ TEST_ASSERT_VAL("Socket ID doesn't match",
+ (session->header.env.cpu[map->map[i]].socket_id == cpu_map__get_socket(map, i)));
+ }
+
+ perf_session__delete(session);
+ cpu_map__put(map);
+
+ return 0;
+}
--
1.8.3.1

2015-08-29 10:47:21

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH V2 1/2] perf,tools: store cpu socket_id and core_id in perf.date

On Fri, Aug 28, 2015 at 09:43:38AM -0400, Kan Liang wrote:
> From: Kan Liang <[email protected]>
>
> This patch stores cpu socket_id and core_id in perf.date, and read them
> to perf_env in header process.
>
> Signed-off-by: Kan Liang <[email protected]>
> ---
>
> Changes since V1:
> - Store core_id and socket_id in perf.date
>
> tools/perf/util/header.c | 97 +++++++++++++++++++++++++++++++++++++++++++++--
> tools/perf/util/header.h | 6 +++
> tools/perf/util/session.c | 1 +
> 3 files changed, 100 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index 4181454..482749f 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -439,14 +439,42 @@ static int write_cmdline(int fd, struct perf_header *h __maybe_unused,
> "/sys/devices/system/cpu/cpu%d/topology/core_siblings_list"
> #define THRD_SIB_FMT \
> "/sys/devices/system/cpu/cpu%d/topology/thread_siblings_list"
> +#define CORE_ID_FMT \
> + "/sys/devices/system/cpu/cpu%d/topology/core_id"
> +#define PHY_PKG_ID_FMT \
> + "/sys/devices/system/cpu/cpu%d/topology/physical_package_id"

hardcoded /sys

>
> struct cpu_topo {
> + u32 cpu_nr;
> u32 core_sib;
> u32 thread_sib;
> char **core_siblings;
> char **thread_siblings;
> + int *core_id;
> + int *phy_pkg_id;
> };
>
> +static int read_id(const char *path, int cpu)
> +{
> + FILE *fp;
> + char filename[MAXPATHLEN];
> + char *buf = NULL;
> + size_t len = 0;
> + int ret = -1;
> +
> + sprintf(filename, path, cpu);
> + fp = fopen(filename, "r");
> + if (fp == NULL)
> + return ret;
> +
> + if (getline(&buf, &len, fp) > 0)
> + ret = atoi(buf);
> +
> + fclose(fp);
> + free(buf);
> + return ret;
> +}
> +
> static int build_cpu_topo(struct cpu_topo *tp, int cpu)
> {
> FILE *fp;
> @@ -507,6 +535,9 @@ try_threads:
> }
> ret = 0;
> done:
> + tp->core_id[cpu] = read_id(CORE_ID_FMT, cpu);
> + tp->phy_pkg_id[cpu] = read_id(PHY_PKG_ID_FMT, cpu);

hu,m dont we read this already somehow in cpumap.c ?

> +
> if(fp)
> fclose(fp);
> free(buf);
> @@ -534,7 +565,7 @@ static struct cpu_topo *build_cpu_topology(void)
> struct cpu_topo *tp;
> void *addr;
> u32 nr, i;
> - size_t sz;
> + size_t sz, sz_id;
> long ncpus;
> int ret = -1;
>

SNIP

> @@ -1631,10 +1686,44 @@ static int process_cpu_topology(struct perf_file_section *section __maybe_unused
> free(str);
> }
> ph->env.sibling_threads = strbuf_detach(&sb, NULL);
> +
> + for (i = 0; i < (u32)cpu_nr; i++) {
> + ret = readn(fd, &nr, sizeof(nr));
> + if (ret != sizeof(nr))
> + goto free_cpu;
> +
> + if (ph->needs_swap)
> + nr = bswap_32(nr);
> +
> + if (nr > (u32)cpu_nr) {
> + pr_debug("core_id number is too big."
> + "You may need to upgrade the perf tool.\n");
> + goto free_cpu;
> + }
> + ph->env.cpu[i].core_id = nr;
> +
> + ret = readn(fd, &nr, sizeof(nr));
> + if (ret != sizeof(nr))
> + goto free_cpu;
> +
> + if (ph->needs_swap)
> + nr = bswap_32(nr);
> +
> + if (nr > (u32)cpu_nr) {
> + pr_debug("socket_id number is too big."
> + "You may need to upgrade the perf tool.\n");
> + goto free_cpu;

we should keep some degree of backward compatibility,
you changed the CPU_TOPOLOGY feature.. are we still able
read older perf.data? can old perf read new perf.data?

this needs to be explained in some comment, or if it's
breakage we need separate feature or event


jirka

2015-08-29 10:47:55

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH V2 2/2] perf,test: test cpu topology

On Fri, Aug 28, 2015 at 09:43:39AM -0400, Kan Liang wrote:

SNIP

> + TEST_ASSERT_VAL("failed to get system cpumap", !(map == NULL));
> +
> + file.mode = PERF_DATA_MODE_READ;
> + session = perf_session__new(&file, false, NULL);
> + TEST_ASSERT_VAL("can't get session", session);
> +
> + for (i = 0; i < session->header.env.nr_cpus_online; i++) {
> + pr_debug("CPU %d, core %d, socket %d\n", i,
> + session->header.env.cpu[i].core_id,
> + session->header.env.cpu[i].socket_id);
> + }
> +
> + for (i = 0; i < map->nr; i++) {
> + TEST_ASSERT_VAL("Core ID doesn't match",
> + (session->header.env.cpu[map->map[i]].core_id == (cpu_map__get_core(map, i) & 0xffff)));
> +
> + TEST_ASSERT_VAL("Socket ID doesn't match",
> + (session->header.env.cpu[map->map[i]].socket_id == cpu_map__get_socket(map, i)));
> + }
> +
> + perf_session__delete(session);
> + cpu_map__put(map);

unlink(path);

> +
> + return 0;
> +}
> --
> 1.8.3.1
>

2015-08-31 17:19:20

by Liang, Kan

[permalink] [raw]
Subject: RE: [PATCH V2 1/2] perf,tools: store cpu socket_id and core_id in perf.date



> On Fri, Aug 28, 2015 at 09:43:38AM -0400, Kan Liang wrote:
> > From: Kan Liang <[email protected]>
> >
> > This patch stores cpu socket_id and core_id in perf.date, and read
> > them to perf_env in header process.
> >
> > Signed-off-by: Kan Liang <[email protected]>
> > ---
> >
> > Changes since V1:
> > - Store core_id and socket_id in perf.date
> >
> > tools/perf/util/header.c | 97
> > +++++++++++++++++++++++++++++++++++++++++++++--
> > tools/perf/util/header.h | 6 +++
> > tools/perf/util/session.c | 1 +
> > 3 files changed, 100 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c index
> > 4181454..482749f 100644
> > --- a/tools/perf/util/header.c
> > +++ b/tools/perf/util/header.c
> > @@ -439,14 +439,42 @@ static int write_cmdline(int fd, struct
> perf_header *h __maybe_unused,
> > "/sys/devices/system/cpu/cpu%d/topology/core_siblings_list"
> > #define THRD_SIB_FMT \
> > "/sys/devices/system/cpu/cpu%d/topology/thread_siblings_list"
> > +#define CORE_ID_FMT \
> > + "/sys/devices/system/cpu/cpu%d/topology/core_id"
> > +#define PHY_PKG_ID_FMT \
> > + "/sys/devices/system/cpu/cpu%d/topology/physical_package_id"
>
> hardcoded /sys
>
> >
> > struct cpu_topo {
> > + u32 cpu_nr;
> > u32 core_sib;
> > u32 thread_sib;
> > char **core_siblings;
> > char **thread_siblings;
> > + int *core_id;
> > + int *phy_pkg_id;
> > };
> >
> > +static int read_id(const char *path, int cpu) {
> > + FILE *fp;
> > + char filename[MAXPATHLEN];
> > + char *buf = NULL;
> > + size_t len = 0;
> > + int ret = -1;
> > +
> > + sprintf(filename, path, cpu);
> > + fp = fopen(filename, "r");
> > + if (fp == NULL)
> > + return ret;
> > +
> > + if (getline(&buf, &len, fp) > 0)
> > + ret = atoi(buf);
> > +
> > + fclose(fp);
> > + free(buf);
> > + return ret;
> > +}
> > +
> > static int build_cpu_topo(struct cpu_topo *tp, int cpu) {
> > FILE *fp;
> > @@ -507,6 +535,9 @@ try_threads:
> > }
> > ret = 0;
> > done:
> > + tp->core_id[cpu] = read_id(CORE_ID_FMT, cpu);
> > + tp->phy_pkg_id[cpu] = read_id(PHY_PKG_ID_FMT, cpu);
>
> hu,m dont we read this already somehow in cpumap.c ?

Yes, but we need to modify the functions in cpumap.c a little bit.
I will write a new patch for it in next version.

>
> > +
> > if(fp)
> > fclose(fp);
> > free(buf);
> > @@ -534,7 +565,7 @@ static struct cpu_topo *build_cpu_topology(void)
> > struct cpu_topo *tp;
> > void *addr;
> > u32 nr, i;
> > - size_t sz;
> > + size_t sz, sz_id;
> > long ncpus;
> > int ret = -1;
> >
>
> SNIP
>
> > @@ -1631,10 +1686,44 @@ static int process_cpu_topology(struct
> perf_file_section *section __maybe_unused
> > free(str);
> > }
> > ph->env.sibling_threads = strbuf_detach(&sb, NULL);
> > +
> > + for (i = 0; i < (u32)cpu_nr; i++) {
> > + ret = readn(fd, &nr, sizeof(nr));
> > + if (ret != sizeof(nr))
> > + goto free_cpu;
> > +
> > + if (ph->needs_swap)
> > + nr = bswap_32(nr);
> > +
> > + if (nr > (u32)cpu_nr) {
> > + pr_debug("core_id number is too big."
> > + "You may need to upgrade the perf
> tool.\n");
> > + goto free_cpu;
> > + }
> > + ph->env.cpu[i].core_id = nr;
> > +
> > + ret = readn(fd, &nr, sizeof(nr));
> > + if (ret != sizeof(nr))
> > + goto free_cpu;
> > +
> > + if (ph->needs_swap)
> > + nr = bswap_32(nr);
> > +
> > + if (nr > (u32)cpu_nr) {
> > + pr_debug("socket_id number is too big."
> > + "You may need to upgrade the perf
> tool.\n");
> > + goto free_cpu;
>
> we should keep some degree of backward compatibility, you changed the
> CPU_TOPOLOGY feature.. are we still able read older perf.data? can old
> perf read new perf.data?
>
The old perf can definitely read new perf.data, because the new codes are
added at the end of the section. It can ignore the extra data.
The new perf can read part of the old perf.data. It's because it will error
out when core_id and socket_id are not detected. So the following sections
will not be read. But we have message for them to let them upgrade
the perf tool. It should be enough.


> this needs to be explained in some comment, or if it's breakage we need
> separate feature or event
>
OK. I will describe it in the changelog.

Thanks,
Kan