[PATCH] perf: fix topology test on systems with sparse CPUs

* [PATCH] perf: fix topology test on systems with sparse CPUs
@ 2017-01-30 16:53 Jan Stancek
  2017-01-30 18:49 ` Jiri Olsa
                   ` (2 more replies)
  0 siblings, 3 replies; 24+ messages in thread
From: Jan Stancek @ 2017-01-30 16:53 UTC (permalink / raw)
  To: linux-kernel
  Cc: peterz, mingo, acme, alexander.shishkin, jstancek, jolsa,
	mhiramat, rui.teng, sukadev

Topology test fails on systems with sparse CPUs, e.g.
CPU not present or offline:

36: Test topology in session                                 :
--- start ---
test child forked, pid 23703
templ file: /tmp/perf-test-i2rNki
failed to write feature 13
perf: Segmentation fault

available: 2 nodes (0-1)
node 0 cpus: 0 6 8 10 16 22 24 26
node 0 size: 11797 MB
node 0 free: 10526 MB
node 1 cpus: 1 7 9 11 17 23 25 27
node 1 size: 12065 MB
node 1 free: 10770 MB
node distances:
node   0   1
  0:  10  20
  1:  20  10

Enumerating CPU ids from 0 to _SC_NPROCESSORS_CONF-1 in header.env.cpu[]
doesn't work on system like one above, because some ids are higher than
number of CPUs, and there can be gaps.

On top of that, if CPU is offline, we can't get topology info from
sysfs entries, because they don't exist for offline CPUs.

This patch stores topology data only for online CPUs in header.env.cpu[]
list, regardless of their CPU ids, and then uses cpu_map to translate
index to actual CPU id.

Example:
coreid socketid for CPU0
coreid socketid for CPU1
coreid socketid for CPU6
coreid socketid for CPU7
...

Alternative is we go from 0 to highest CPU id, but CPUs which are
missing would contain some dummy values in topology data.

Example:
coreid socketid for CPU0
coreid socketid for CPU1
-1 -1
-1 -1
-1 -1
-1 -1
coreid socketid for CPU6
coreid socketid for CPU7
...

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 tools/perf/tests/topology.c |  7 ++++---
 tools/perf/util/env.c       | 40 ++++++++++++++++++++++++++++------------
 tools/perf/util/header.c    | 36 ++++++++++++++++++++----------------
 3 files changed, 52 insertions(+), 31 deletions(-)

diff --git a/tools/perf/tests/topology.c b/tools/perf/tests/topology.c
index 98fe69ac553c..7b0b621ea8c0 100644
--- a/tools/perf/tests/topology.c
+++ b/tools/perf/tests/topology.c
@@ -66,17 +66,18 @@ static int check_cpu_topology(char *path, struct cpu_map *map)
 	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,
+		pr_debug("CPU %d, core %d, socket %d\n", map->map[i],
 			 session->header.env.cpu[i].core_id,
 			 session->header.env.cpu[i].socket_id);
 	}
 
 	for (i = 0; i < map->nr; i++) {
+		int cpu = map->map[i];
 		TEST_ASSERT_VAL("Core ID doesn't match",
-			(session->header.env.cpu[map->map[i]].core_id == (cpu_map__get_core(map, i, NULL) & 0xffff)));
+			(session->header.env.cpu[i].core_id == (cpu_map__get_core_id(cpu) & 0xffff)));
 
 		TEST_ASSERT_VAL("Socket ID doesn't match",
-			(session->header.env.cpu[map->map[i]].socket_id == cpu_map__get_socket(map, i, NULL)));
+			(session->header.env.cpu[i].socket_id == cpu_map__get_socket_id(cpu)));
 	}
 
 	perf_session__delete(session);
diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
index bb964e86b09d..0c2cae807a61 100644
--- a/tools/perf/util/env.c
+++ b/tools/perf/util/env.c
@@ -60,29 +60,45 @@ 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)
 {
-	int cpu, nr_cpus;
+	int cpu, nr_cpus, i, err = 0;
+	struct cpu_map *map;
 
 	if (env->cpu != NULL)
 		return 0;
 
-	if (env->nr_cpus_avail == 0)
-		env->nr_cpus_avail = sysconf(_SC_NPROCESSORS_CONF);
+	map = cpu_map__new(NULL);
+	if (map == NULL) {
+		pr_debug("failed to get system cpumap\n");
+		err = -ENOMEM;
+		goto out;
+	}
+
+	if (env->nr_cpus_online == 0)
+		env->nr_cpus_online = map->nr;
 
-	nr_cpus = env->nr_cpus_avail;
-	if (nr_cpus == -1)
-		return -EINVAL;
+	nr_cpus = env->nr_cpus_online;
+	if (nr_cpus == -1 || map->nr < nr_cpus) {
+		err = -EINVAL;
+		goto out_free;
+	}
 
 	env->cpu = calloc(nr_cpus, sizeof(env->cpu[0]));
-	if (env->cpu == NULL)
-		return -ENOMEM;
+	if (env->cpu == NULL) {
+		err = -ENOMEM;
+		goto out_free;
+	}
 
-	for (cpu = 0; cpu < nr_cpus; ++cpu) {
-		env->cpu[cpu].core_id	= cpu_map__get_core_id(cpu);
-		env->cpu[cpu].socket_id	= cpu_map__get_socket_id(cpu);
+	for (i = 0; i < nr_cpus; i++) {
+		cpu = map->map[i];
+		env->cpu[i].core_id	= cpu_map__get_core_id(cpu);
+		env->cpu[i].socket_id	= cpu_map__get_socket_id(cpu);
 	}
 
 	env->nr_cpus_avail = nr_cpus;
-	return 0;
+out_free:
+	cpu_map__put(map);
+out:
+	return err;
 }
 
 void cpu_cache_level__free(struct cpu_cache_level *cache)
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index d89c9c7ef4e5..25faa93d143a 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -503,41 +503,45 @@ static void free_cpu_topo(struct cpu_topo *tp)
 
 static struct cpu_topo *build_cpu_topology(void)
 {
-	struct cpu_topo *tp;
+	struct cpu_topo *tp = NULL;
 	void *addr;
-	u32 nr, i;
+	u32 i;
 	size_t sz;
-	long ncpus;
-	int ret = -1;
-
-	ncpus = sysconf(_SC_NPROCESSORS_CONF);
-	if (ncpus < 0)
-		return NULL;
-
-	nr = (u32)(ncpus & UINT_MAX);
+	int ret = 0, cpu;
+	struct cpu_map *map;
 
-	sz = nr * sizeof(char *);
+	map = cpu_map__new(NULL);
+	if (map == NULL) {
+		pr_debug("failed to get system cpumap\n");
+		goto out;
+	}
 
+	sz = map->nr * sizeof(char *);
 	addr = calloc(1, sizeof(*tp) + 2 * sz);
 	if (!addr)
-		return NULL;
+		goto out_free;
 
 	tp = addr;
-	tp->cpu_nr = nr;
+	tp->cpu_nr = map->nr;
 	addr += sizeof(*tp);
 	tp->core_siblings = addr;
 	addr += sz;
 	tp->thread_siblings = addr;
 
-	for (i = 0; i < nr; i++) {
-		ret = build_cpu_topo(tp, i);
+	for (i = 0; i < tp->cpu_nr; i++) {
+		cpu = map->map[i];
+		ret = build_cpu_topo(tp, cpu);
 		if (ret < 0)
 			break;
 	}
+
+out_free:
+	cpu_map__put(map);
 	if (ret) {
 		free_cpu_topo(tp);
 		tp = NULL;
 	}
+out:
 	return tp;
 }
 
@@ -575,7 +579,7 @@ static int write_cpu_topology(int fd, struct perf_header *h __maybe_unused,
 	if (ret < 0)
 		goto done;
 
-	for (j = 0; j < perf_env.nr_cpus_avail; j++) {
+	for (j = 0; j < perf_env.nr_cpus_online; j++) {
 		ret = do_write(fd, &perf_env.cpu[j].core_id,
 			       sizeof(perf_env.cpu[j].core_id));
 		if (ret < 0)
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread