[PATCH V4 1/3] perf,tools: Separated functions to get core_id and socket_id

[PATCH V4 1/3] perf,tools: Separated functions to get core_id and socket_id

[Kan Liang]

From: Kan Liang <kan.liang@intel.com>

This patch moves the codes which read core_id and socket_id into
separated functions, and expose them.

Signed-off-by: Kan Liang <kan.liang@intel.com>
---
 tools/perf/util/cpumap.c | 51 +++++++++++++++++++++++++++++++-----------------
 tools/perf/util/cpumap.h |  2 ++
 2 files changed, 35 insertions(+), 18 deletions(-)

diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
index 3667e21..a05d76a 100644
--- a/tools/perf/util/cpumap.c
+++ b/tools/perf/util/cpumap.c
@@ -225,17 +225,12 @@ void cpu_map__put(struct cpu_map *map)
 		cpu_map__delete(map);
 }
 
-int cpu_map__get_socket(struct cpu_map *map, int idx)
+int cpu_map__get_socket_id(int cpu)
 {
 	FILE *fp;
 	const char *mnt;
 	char path[PATH_MAX];
-	int cpu, ret;
-
-	if (idx > map->nr)
-		return -1;
-
-	cpu = map->map[idx];
+	int socket_id, ret;
 
 	mnt = sysfs__mountpoint();
 	if (!mnt)
@@ -248,9 +243,22 @@ int cpu_map__get_socket(struct cpu_map *map, int idx)
 	fp = fopen(path, "r");
 	if (!fp)
 		return -1;
-	ret = fscanf(fp, "%d", &cpu);
+	ret = fscanf(fp, "%d", &socket_id);
 	fclose(fp);
-	return ret == 1 ? cpu : -1;
+
+	return ret == 1 ? socket_id : -1;
+}
+
+int cpu_map__get_socket(struct cpu_map *map, int idx)
+{
+	int cpu;
+
+	if (idx > map->nr)
+		return -1;
+
+	cpu = map->map[idx];
+
+	return cpu_map__get_socket_id(cpu);
 }
 
 static int cmp_ids(const void *a, const void *b)
@@ -289,17 +297,12 @@ static int cpu_map__build_map(struct cpu_map *cpus, struct cpu_map **res,
 	return 0;
 }
 
-int cpu_map__get_core(struct cpu_map *map, int idx)
+int cpu_map__get_core_id(int cpu)
 {
 	FILE *fp;
 	const char *mnt;
 	char path[PATH_MAX];
-	int cpu, ret, s;
-
-	if (idx > map->nr)
-		return -1;
-
-	cpu = map->map[idx];
+	int core_id, ret;
 
 	mnt = sysfs__mountpoint();
 	if (!mnt)
@@ -312,11 +315,23 @@ int cpu_map__get_core(struct cpu_map *map, int idx)
 	fp = fopen(path, "r");
 	if (!fp)
 		return -1;
-	ret = fscanf(fp, "%d", &cpu);
+	ret = fscanf(fp, "%d", &core_id);
 	fclose(fp);
-	if (ret != 1)
+
+	return ret == 1 ? core_id : -1;
+}
+
+int cpu_map__get_core(struct cpu_map *map, int idx)
+{
+	int cpu, s;
+
+	if (idx > map->nr)
 		return -1;
 
+	cpu = map->map[idx];
+
+	cpu = cpu_map__get_core_id(cpu);
+
 	s = cpu_map__get_socket(map, idx);
 	if (s == -1)
 		return -1;
diff --git a/tools/perf/util/cpumap.h b/tools/perf/util/cpumap.h
index 0af9cec..8982d53 100644
--- a/tools/perf/util/cpumap.h
+++ b/tools/perf/util/cpumap.h
@@ -18,7 +18,9 @@ struct cpu_map *cpu_map__new(const char *cpu_list);
 struct cpu_map *cpu_map__dummy_new(void);
 struct cpu_map *cpu_map__read(FILE *file);
 size_t cpu_map__fprintf(struct cpu_map *map, FILE *fp);
+int cpu_map__get_socket_id(int cpu);
 int cpu_map__get_socket(struct cpu_map *map, int idx);
+int cpu_map__get_core_id(int cpu);
 int cpu_map__get_core(struct cpu_map *map, int idx);
 int cpu_map__build_socket_map(struct cpu_map *cpus, struct cpu_map **sockp);
 int cpu_map__build_core_map(struct cpu_map *cpus, struct cpu_map **corep);
-- 
1.8.3.1

[PATCH V4 2/3] perf,tools: store cpu socket_id and core_id in perf.date

[Kan Liang]

From: Kan Liang <kan.liang@intel.com>

This patch stores cpu socket_id and core_id in perf.date, and read them
to perf_env in header process.
The changes modify the CPU_TOPOLOGY section, but it still keeps backward
compatibility. The patch checks the section size before reading core id
and socket id. It never read data cross the section.
The old perf without this patch can also correctly read the perf.data
from the new perf with this patch. Because the new codes are added at
the end of the cpu_topology section. The old perf can ignore the extra
data.

Examples:

1. New perf with this patch read perf.data from old perf without the
patch.

 $ perf_new report -i perf_old.data --header-only -I
 ......
 # sibling threads : 33
 # sibling threads : 34
 # sibling threads : 35
 # Core ID and Socket ID information is not available
 # node0 meminfo  : total = 32823872 kB, free = 29315548 kB
 # node0 cpu list : 0-17,36-53
 ......

2. Old perf without the patch read perf.data from new perf with the
patch.
 $ perf_old report -i perf_new.data --header-only -I
 ......
 # sibling threads : 33
 # sibling threads : 34
 # sibling threads : 35
 # node0 meminfo  : total = 32823872 kB, free = 29190932 kB
 # node0 cpu list : 0-17,36-53
 ......

3. new perf read the new perf.data

 $ perf_new report -i perf_new.data --header-only -I
 ......
 # sibling threads : 33
 # sibling threads : 34
 # sibling threads : 35
 # CPU 0: Core ID 0, Socket ID 0
 # CPU 1: Core ID 1, Socket ID 0
 ......
 # CPU 61: Core ID 10, Socket ID 1
 # CPU 62: Core ID 11, Socket ID 1
 # CPU 63: Core ID 16, Socket ID 1
 # node0 meminfo  : total = 32823872 kB, free = 29190932 kB
 # node0 cpu list : 0-17,36-53

Signed-off-by: Kan Liang <kan.liang@intel.com>
---
Changes since V1:
 - Store core_id and socket_id in perf.date

Changes since V2:
 - Use funcitons in cpumap.c to get core_id and socket_id
 - Modify changelog

Changes since V3:
 - Improve backward compatibility by checking section size
 - Modify changelog and add some examples

 tools/perf/util/header.c  | 94 ++++++++++++++++++++++++++++++++++++++++++++---
 tools/perf/util/header.h  |  6 +++
 tools/perf/util/session.c |  1 +
 3 files changed, 96 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 4181454..8fd7b7d 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -88,6 +88,9 @@ int write_padded(int fd, const void *bf, size_t count, size_t count_aligned)
 	return err;
 }
 
+#define string_size(str)						\
+	(PERF_ALIGN((strlen(str) + 1), NAME_ALIGN) + sizeof(u32))
+
 static int do_write_string(int fd, const char *str)
 {
 	u32 len, olen;
@@ -441,10 +444,13 @@ static int write_cmdline(int fd, struct perf_header *h __maybe_unused,
 	"/sys/devices/system/cpu/cpu%d/topology/thread_siblings_list"
 
 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 build_cpu_topo(struct cpu_topo *tp, int cpu)
@@ -507,6 +513,9 @@ 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);
@@ -534,7 +543,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 +554,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 +612,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 +961,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 +978,13 @@ static void print_cpu_topology(struct perf_header *ph, int fd __maybe_unused,
 		fprintf(fp, "# sibling threads : %s\n", str);
 		str += strlen(str) + 1;
 	}
+
+	if (ph->env.cpu != NULL) {
+		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);
+	} else
+		fprintf(fp, "# Core ID and Socket ID information is not available\n");
 }
 
 static void free_event_desc(struct perf_evsel *events)
@@ -1582,7 +1613,7 @@ error:
 	return -1;
 }
 
-static int process_cpu_topology(struct perf_file_section *section __maybe_unused,
+static int process_cpu_topology(struct perf_file_section *section,
 				struct perf_header *ph, int fd,
 				void *data __maybe_unused)
 {
@@ -1590,15 +1621,22 @@ 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;
+	u64 size = 0;
+
+	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);
 
 	ph->env.nr_sibling_cores = nr;
+	size += sizeof(u32);
 	strbuf_init(&sb, 128);
 
 	for (i = 0; i < nr; i++) {
@@ -1608,6 +1646,7 @@ static int process_cpu_topology(struct perf_file_section *section __maybe_unused
 
 		/* include a NULL character at the end */
 		strbuf_add(&sb, str, strlen(str) + 1);
+		size += string_size(str);
 		free(str);
 	}
 	ph->env.sibling_cores = strbuf_detach(&sb, NULL);
@@ -1620,6 +1659,7 @@ static int process_cpu_topology(struct perf_file_section *section __maybe_unused
 		nr = bswap_32(nr);
 
 	ph->env.nr_sibling_threads = nr;
+	size += sizeof(u32);
 
 	for (i = 0; i < nr; i++) {
 		str = do_read_string(fd, ph);
@@ -1628,13 +1668,57 @@ static int process_cpu_topology(struct perf_file_section *section __maybe_unused
 
 		/* include a NULL character at the end */
 		strbuf_add(&sb, str, strlen(str) + 1);
+		size += string_size(str);
 		free(str);
 	}
 	ph->env.sibling_threads = strbuf_detach(&sb, NULL);
+
+	/*
+	 * The header may be from old perf,
+	 * which doesn't include core id and socket id information.
+	 */
+	if (section->size <= size) {
+		zfree(&ph->env.cpu);
+		return 0;
+	}
+
+	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:
+	zfree(&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

[PATCH V4 3/3] perf,test: test cpu topology

[Kan Liang]

From: Jiri Olsa <jolsa@kernel.org>

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

Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Signed-off-by: Kan Liang <kan.liang@intel.com>
---

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

Changes since V2:
 - unlink(path)

 tools/perf/tests/Build          | 1 +
 tools/perf/tests/builtin-test.c | 4 ++++
 tools/perf/tests/tests.h        | 1 +
 3 files changed, 6 insertions(+)

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
-- 
1.8.3.1

Re: [PATCH V4 2/3] perf,tools: store cpu socket_id and core_id in perf.date

[Arnaldo Carvalho de Melo]

Em Tue, Sep 01, 2015 at 09:58:12AM -0400, Kan Liang escreveu:
> 
> Changes since V3:
>  - Improve backward compatibility by checking section size
>  - Modify changelog and add some examples

This one works in my testing, no crashes, looks ok, Jiri, can I get your
Acked-by or Tested-by for V3?

- Arnaldo

Re: [PATCH V4 3/3] perf,test: test cpu topology

[Arnaldo Carvalho de Melo]

Em Tue, Sep 01, 2015 at 09:58:13AM -0400, Kan Liang escreveu:
> From: Jiri Olsa <jolsa@kernel.org>
> 
> This patch test cpu core_id and socket_id which are stored in perf_env.
> 
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> Signed-off-by: Kan Liang <kan.liang@intel.com>
> ---
> 
> 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

Humm, two questions:

Since you changed it that much, wouldn't be better to give you the
authorship while still giving credit to Jiri for the original version?
Something like:

Based-on-a-patch-by: Jiri Olsa

[acme@zoo linux]$ git log | grep Based-on-patch-by: | wc -l
61
[acme@zoo linux]$ git log | grep Based-on-a-patch-by: | wc -l
4
[acme@zoo linux]$ 

But then you need to resubmit this anyway, as you forgot to do the:

 git add tools/perf/tests/topology.c

:-)

- Arnaldo
 
> Changes since V2:
>  - unlink(path)
> 
>  tools/perf/tests/Build          | 1 +
>  tools/perf/tests/builtin-test.c | 4 ++++
>  tools/perf/tests/tests.h        | 1 +
>  3 files changed, 6 insertions(+)
> 
> 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
> -- 
> 1.8.3.1

RE: [PATCH V4 3/3] perf,test: test cpu topology

[Liang, Kan]

> 
> Em Tue, Sep 01, 2015 at 09:58:13AM -0400, Kan Liang escreveu:
> > From: Jiri Olsa <jolsa@kernel.org>
> >
> > This patch test cpu core_id and socket_id which are stored in perf_env.
> >
> > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > Signed-off-by: Kan Liang <kan.liang@intel.com>
> > ---
> >
> > 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
> 
> Humm, two questions:
> 
> Since you changed it that much, wouldn't be better to give you the
> authorship while still giving credit to Jiri for the original version?
> Something like:
> 
> Based-on-a-patch-by: Jiri Olsa

Jiri, are you OK with that?

> 
> [acme@zoo linux]$ git log | grep Based-on-patch-by: | wc -l
> 61
> [acme@zoo linux]$ git log | grep Based-on-a-patch-by: | wc -l
> 4
> [acme@zoo linux]$
> 
> But then you need to resubmit this anyway, as you forgot to do the:
> 
>  git add tools/perf/tests/topology.c

Ah... My bad. :(

I will resubmit the patch then.

Thanks,
Kan

> 
> :-)
> 
> - Arnaldo
> 
> > Changes since V2:
> >  - unlink(path)
> >
> >  tools/perf/tests/Build          | 1 +
> >  tools/perf/tests/builtin-test.c | 4 ++++
> >  tools/perf/tests/tests.h        | 1 +
> >  3 files changed, 6 insertions(+)
> >
> > 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
> > --
> > 1.8.3.1

Re: [PATCH V4 3/3] perf,test: test cpu topology

[Jiri Olsa]

On Tue, Sep 01, 2015 at 11:25:10PM +0000, Liang, Kan wrote:
> 
> 
> > 
> > Em Tue, Sep 01, 2015 at 09:58:13AM -0400, Kan Liang escreveu:
> > > From: Jiri Olsa <jolsa@kernel.org>
> > >
> > > This patch test cpu core_id and socket_id which are stored in perf_env.
> > >
> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> > > Signed-off-by: Kan Liang <kan.liang@intel.com>
> > > ---
> > >
> > > 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
> > 
> > Humm, two questions:
> > 
> > Since you changed it that much, wouldn't be better to give you the
> > authorship while still giving credit to Jiri for the original version?
> > Something like:
> > 
> > Based-on-a-patch-by: Jiri Olsa
> 
> Jiri, are you OK with that?

yep

> 
> > 
> > [acme@zoo linux]$ git log | grep Based-on-patch-by: | wc -l
> > 61
> > [acme@zoo linux]$ git log | grep Based-on-a-patch-by: | wc -l
> > 4
> > [acme@zoo linux]$
> > 
> > But then you need to resubmit this anyway, as you forgot to do the:
> > 
> >  git add tools/perf/tests/topology.c
> 
> Ah... My bad. :(
> 
> I will resubmit the patch then.

I'll review new version

jirka

[tip:perf/core] perf cpumap: Factor out functions to get core_id and socket_id

[tip-bot for Kan Liang]

Commit-ID:  193b6bd339ccb30c861a307a915d4532f443e0fb
Gitweb:     http://git.kernel.org/tip/193b6bd339ccb30c861a307a915d4532f443e0fb
Author:     Kan Liang <kan.liang@intel.com>
AuthorDate: Tue, 1 Sep 2015 09:58:11 -0400
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 2 Sep 2015 16:30:47 -0300

perf cpumap: Factor out functions to get core_id and socket_id

This patch moves the code which reads core_id and socket_id into
separate functions.

Signed-off-by: Kan Liang <kan.liang@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Link: http://lkml.kernel.org/r/1441115893-22006-1-git-send-email-kan.liang@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/cpumap.c | 51 +++++++++++++++++++++++++++++++-----------------
 tools/perf/util/cpumap.h |  2 ++
 2 files changed, 35 insertions(+), 18 deletions(-)

diff --git a/tools/perf/util/cpumap.c b/tools/perf/util/cpumap.c
index 3667e21..a05d76a 100644
--- a/tools/perf/util/cpumap.c
+++ b/tools/perf/util/cpumap.c
@@ -225,17 +225,12 @@ void cpu_map__put(struct cpu_map *map)
 		cpu_map__delete(map);
 }
 
-int cpu_map__get_socket(struct cpu_map *map, int idx)
+int cpu_map__get_socket_id(int cpu)
 {
 	FILE *fp;
 	const char *mnt;
 	char path[PATH_MAX];
-	int cpu, ret;
-
-	if (idx > map->nr)
-		return -1;
-
-	cpu = map->map[idx];
+	int socket_id, ret;
 
 	mnt = sysfs__mountpoint();
 	if (!mnt)
@@ -248,9 +243,22 @@ int cpu_map__get_socket(struct cpu_map *map, int idx)
 	fp = fopen(path, "r");
 	if (!fp)
 		return -1;
-	ret = fscanf(fp, "%d", &cpu);
+	ret = fscanf(fp, "%d", &socket_id);
 	fclose(fp);
-	return ret == 1 ? cpu : -1;
+
+	return ret == 1 ? socket_id : -1;
+}
+
+int cpu_map__get_socket(struct cpu_map *map, int idx)
+{
+	int cpu;
+
+	if (idx > map->nr)
+		return -1;
+
+	cpu = map->map[idx];
+
+	return cpu_map__get_socket_id(cpu);
 }
 
 static int cmp_ids(const void *a, const void *b)
@@ -289,17 +297,12 @@ static int cpu_map__build_map(struct cpu_map *cpus, struct cpu_map **res,
 	return 0;
 }
 
-int cpu_map__get_core(struct cpu_map *map, int idx)
+int cpu_map__get_core_id(int cpu)
 {
 	FILE *fp;
 	const char *mnt;
 	char path[PATH_MAX];
-	int cpu, ret, s;
-
-	if (idx > map->nr)
-		return -1;
-
-	cpu = map->map[idx];
+	int core_id, ret;
 
 	mnt = sysfs__mountpoint();
 	if (!mnt)
@@ -312,11 +315,23 @@ int cpu_map__get_core(struct cpu_map *map, int idx)
 	fp = fopen(path, "r");
 	if (!fp)
 		return -1;
-	ret = fscanf(fp, "%d", &cpu);
+	ret = fscanf(fp, "%d", &core_id);
 	fclose(fp);
-	if (ret != 1)
+
+	return ret == 1 ? core_id : -1;
+}
+
+int cpu_map__get_core(struct cpu_map *map, int idx)
+{
+	int cpu, s;
+
+	if (idx > map->nr)
 		return -1;
 
+	cpu = map->map[idx];
+
+	cpu = cpu_map__get_core_id(cpu);
+
 	s = cpu_map__get_socket(map, idx);
 	if (s == -1)
 		return -1;
diff --git a/tools/perf/util/cpumap.h b/tools/perf/util/cpumap.h
index 0af9cec..8982d53 100644
--- a/tools/perf/util/cpumap.h
+++ b/tools/perf/util/cpumap.h
@@ -18,7 +18,9 @@ struct cpu_map *cpu_map__new(const char *cpu_list);
 struct cpu_map *cpu_map__dummy_new(void);
 struct cpu_map *cpu_map__read(FILE *file);
 size_t cpu_map__fprintf(struct cpu_map *map, FILE *fp);
+int cpu_map__get_socket_id(int cpu);
 int cpu_map__get_socket(struct cpu_map *map, int idx);
+int cpu_map__get_core_id(int cpu);
 int cpu_map__get_core(struct cpu_map *map, int idx);
 int cpu_map__build_socket_map(struct cpu_map *cpus, struct cpu_map **sockp);
 int cpu_map__build_core_map(struct cpu_map *cpus, struct cpu_map **corep);

[tip:perf/core] perf tools: Store the cpu socket and core ids in the perf.data header

[tip-bot for Kan Liang]

Commit-ID:  2bb00d2f95193aea5bfa98392907273115c96920
Gitweb:     http://git.kernel.org/tip/2bb00d2f95193aea5bfa98392907273115c96920
Author:     Kan Liang <kan.liang@intel.com>
AuthorDate: Tue, 1 Sep 2015 09:58:12 -0400
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 2 Sep 2015 16:30:47 -0300

perf tools: Store the cpu socket and core ids in the perf.data header

This patch stores the cpu socket_id and core_id in a perf.data header,
and reads them into the perf_env struct when processing perf.data files.

The changes modifies the CPU_TOPOLOGY section, making sure it is
backward/forward compatible.

The patch checks the section size before reading the core and socket ids.

It never reads data crossing the section boundary.  An old perf binary
without this patch can also correctly read the perf.data from a new perf
with this patch.

Because the new info is added at the end of the cpu_topology section, an
old perf tool ignores the extra data.

Examples:

1. New perf with this patch read perf.data from an old perf without the
   patch:

  $ perf_new report -i perf_old.data --header-only -I
  ......
  # sibling threads : 33
  # sibling threads : 34
  # sibling threads : 35
  # Core ID and Socket ID information is not available
  # node0 meminfo  : total = 32823872 kB, free = 29315548 kB
  # node0 cpu list : 0-17,36-53
  ......

2. Old perf without the patch reads perf.data from a new perf with the
   patch:

  $ perf_old report -i perf_new.data --header-only -I
  ......
  # sibling threads : 33
  # sibling threads : 34
  # sibling threads : 35
  # node0 meminfo  : total = 32823872 kB, free = 29190932 kB
  # node0 cpu list : 0-17,36-53
  ......

3. New perf read new perf.data:

  $ perf_new report -i perf_new.data --header-only -I
  ......
  # sibling threads : 33
  # sibling threads : 34
  # sibling threads : 35
  # CPU 0: Core ID 0, Socket ID 0
  # CPU 1: Core ID 1, Socket ID 0
  ......
  # CPU 61: Core ID 10, Socket ID 1
  # CPU 62: Core ID 11, Socket ID 1
  # CPU 63: Core ID 16, Socket ID 1
  # node0 meminfo  : total = 32823872 kB, free = 29190932 kB
  # node0 cpu list : 0-17,36-53

Signed-off-by: Kan Liang <kan.liang@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Link: http://lkml.kernel.org/r/1441115893-22006-2-git-send-email-kan.liang@intel.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c  | 94 ++++++++++++++++++++++++++++++++++++++++++++---
 tools/perf/util/header.h  |  6 +++
 tools/perf/util/session.c |  1 +
 3 files changed, 96 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 4181454..8fd7b7d 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -88,6 +88,9 @@ int write_padded(int fd, const void *bf, size_t count, size_t count_aligned)
 	return err;
 }
 
+#define string_size(str)						\
+	(PERF_ALIGN((strlen(str) + 1), NAME_ALIGN) + sizeof(u32))
+
 static int do_write_string(int fd, const char *str)
 {
 	u32 len, olen;
@@ -441,10 +444,13 @@ static int write_cmdline(int fd, struct perf_header *h __maybe_unused,
 	"/sys/devices/system/cpu/cpu%d/topology/thread_siblings_list"
 
 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 build_cpu_topo(struct cpu_topo *tp, int cpu)
@@ -507,6 +513,9 @@ 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);
@@ -534,7 +543,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 +554,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 +612,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 +961,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 +978,13 @@ static void print_cpu_topology(struct perf_header *ph, int fd __maybe_unused,
 		fprintf(fp, "# sibling threads : %s\n", str);
 		str += strlen(str) + 1;
 	}
+
+	if (ph->env.cpu != NULL) {
+		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);
+	} else
+		fprintf(fp, "# Core ID and Socket ID information is not available\n");
 }
 
 static void free_event_desc(struct perf_evsel *events)
@@ -1582,7 +1613,7 @@ error:
 	return -1;
 }
 
-static int process_cpu_topology(struct perf_file_section *section __maybe_unused,
+static int process_cpu_topology(struct perf_file_section *section,
 				struct perf_header *ph, int fd,
 				void *data __maybe_unused)
 {
@@ -1590,15 +1621,22 @@ 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;
+	u64 size = 0;
+
+	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);
 
 	ph->env.nr_sibling_cores = nr;
+	size += sizeof(u32);
 	strbuf_init(&sb, 128);
 
 	for (i = 0; i < nr; i++) {
@@ -1608,6 +1646,7 @@ static int process_cpu_topology(struct perf_file_section *section __maybe_unused
 
 		/* include a NULL character at the end */
 		strbuf_add(&sb, str, strlen(str) + 1);
+		size += string_size(str);
 		free(str);
 	}
 	ph->env.sibling_cores = strbuf_detach(&sb, NULL);
@@ -1620,6 +1659,7 @@ static int process_cpu_topology(struct perf_file_section *section __maybe_unused
 		nr = bswap_32(nr);
 
 	ph->env.nr_sibling_threads = nr;
+	size += sizeof(u32);
 
 	for (i = 0; i < nr; i++) {
 		str = do_read_string(fd, ph);
@@ -1628,13 +1668,57 @@ static int process_cpu_topology(struct perf_file_section *section __maybe_unused
 
 		/* include a NULL character at the end */
 		strbuf_add(&sb, str, strlen(str) + 1);
+		size += string_size(str);
 		free(str);
 	}
 	ph->env.sibling_threads = strbuf_detach(&sb, NULL);
+
+	/*
+	 * The header may be from old perf,
+	 * which doesn't include core id and socket id information.
+	 */
+	if (section->size <= size) {
+		zfree(&ph->env.cpu);
+		return 0;
+	}
+
+	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:
+	zfree(&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)