>From 4a43c50e547b4a0bc474d56fe3f07d137774d60a Mon Sep 17 00:00:00 2001 Message-Id: <4a43c50e547b4a0bc474d56fe3f07d137774d60a.1485877985.git.jstancek@redhat.com> In-Reply-To: <9bf8ece1e397b851beedaeceeb0cd07421ff6f43.1485877985.git.jstancek@redhat.com> References: <9bf8ece1e397b851beedaeceeb0cd07421ff6f43.1485877985.git.jstancek@redhat.com> From: Jan Stancek Date: Tue, 31 Jan 2017 14:46:54 +0100 Subject: [PATCH 3/3 v2] perf: replace _SC_NPROCESSORS_CONF with max_present_cpu in cpu_topology_map There are 2 problems wrt. cpu_topology_map on systems with sparse CPUs: 1. offline/absent CPUs will have their socket_id and core_id set to -1 which triggers: "socket_id number is too big.You may need to upgrade the perf tool." 2. size of cpu_topology_map (perf_env.cpu[]) is allocated based on _SC_NPROCESSORS_CONF, but can be indexed with CPU ids going above. Users of perf_env.cpu[] are using CPU id as index. This can lead to read beyond what was allocated: ==19991== Invalid read of size 4 ==19991== at 0x490CEB: check_cpu_topology (topology.c:69) ==19991== by 0x490CEB: test_session_topology (topology.c:106) ... For example: _SC_NPROCESSORS_CONF == 16 available: 2 nodes (0-1) node 0 cpus: 0 6 8 10 16 22 24 26 node 0 size: 12004 MB node 0 free: 9470 MB node 1 cpus: 1 7 9 11 23 25 27 node 1 size: 12093 MB node 1 free: 9406 MB node distances: node 0 1 0: 10 20 1: 20 10 This patch changes HEADER_NRCPUS.nr_cpus_available from _SC_NPROCESSORS_CONF to max_present_cpu and updates any user of cpu_topology_map to iterate with nr_cpus_avail. As consequence HEADER_CPU_TOPOLOGY core_id and socket_id lists get longer, but maintain compatibility with pre-patch state - index to cpu_topology_map is CPU id. ./perf test 36 -v 36: Session topology : --- start --- test child forked, pid 22211 templ file: /tmp/perf-test-gmdX5i CPU 0, core 0, socket 0 CPU 1, core 0, socket 1 CPU 6, core 10, socket 0 CPU 7, core 10, socket 1 CPU 8, core 1, socket 0 CPU 9, core 1, socket 1 CPU 10, core 9, socket 0 CPU 11, core 9, socket 1 CPU 16, core 0, socket 0 CPU 22, core 10, socket 0 CPU 23, core 10, socket 1 CPU 24, core 1, socket 0 CPU 25, core 1, socket 1 CPU 26, core 9, socket 0 CPU 27, core 9, socket 1 test child finished with 0 ---- end ---- Session topology: Ok Signed-off-by: Jan Stancek --- tools/perf/builtin-stat.c | 2 +- tools/perf/tests/topology.c | 4 +++- tools/perf/util/env.c | 2 +- tools/perf/util/header.c | 16 +++++----------- 4 files changed, 10 insertions(+), 14 deletions(-) diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c index a02f2e965628..9e2be1c52fbd 100644 --- a/tools/perf/builtin-stat.c +++ b/tools/perf/builtin-stat.c @@ -1765,7 +1765,7 @@ static inline int perf_env__get_cpu(struct perf_env *env, struct cpu_map *map, i cpu = map->map[idx]; - if (cpu >= env->nr_cpus_online) + if (cpu >= env->nr_cpus_avail) return -1; return cpu; diff --git a/tools/perf/tests/topology.c b/tools/perf/tests/topology.c index 98fe69ac553c..803f893550d6 100644 --- a/tools/perf/tests/topology.c +++ b/tools/perf/tests/topology.c @@ -65,7 +65,9 @@ static int check_cpu_topology(char *path, struct cpu_map *map) 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++) { + for (i = 0; i < session->header.env.nr_cpus_avail; i++) { + if (!cpu_map__has(map, i)) + continue; pr_debug("CPU %d, core %d, socket %d\n", i, session->header.env.cpu[i].core_id, session->header.env.cpu[i].socket_id); diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c index bb964e86b09d..075fc77286bf 100644 --- a/tools/perf/util/env.c +++ b/tools/perf/util/env.c @@ -66,7 +66,7 @@ int perf_env__read_cpu_topology_map(struct perf_env *env) return 0; if (env->nr_cpus_avail == 0) - env->nr_cpus_avail = sysconf(_SC_NPROCESSORS_CONF); + env->nr_cpus_avail = cpu__max_present_cpu(); nr_cpus = env->nr_cpus_avail; if (nr_cpus == -1) diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c index 2b7ed52df807..5c81e0853bff 100644 --- a/tools/perf/util/header.c +++ b/tools/perf/util/header.c @@ -293,11 +293,7 @@ static int write_nrcpus(int fd, struct perf_header *h __maybe_unused, u32 nrc, nra; int ret; - nr = sysconf(_SC_NPROCESSORS_CONF); - if (nr < 0) - return -1; - - nrc = (u32)(nr & UINT_MAX); + nrc = cpu__max_present_cpu(); nr = sysconf(_SC_NPROCESSORS_ONLN); if (nr < 0) @@ -511,9 +507,7 @@ static struct cpu_topo *build_cpu_topology(void) int ret = 0; struct cpu_map *map; - ncpus = sysconf(_SC_NPROCESSORS_CONF); - if (ncpus < 0) - goto out; + ncpus = cpu__max_present_cpu(); /* build online CPU map */ map = cpu_map__new(NULL); @@ -1138,7 +1132,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; + int cpu_nr = ph->env.nr_cpus_avail; nr = ph->env.nr_sibling_cores; str = ph->env.sibling_cores; @@ -1793,7 +1787,7 @@ static int process_cpu_topology(struct perf_file_section *section, u32 nr, i; char *str; struct strbuf sb; - int cpu_nr = ph->env.nr_cpus_online; + int cpu_nr = ph->env.nr_cpus_avail; u64 size = 0; ph->env.cpu = calloc(cpu_nr, sizeof(*ph->env.cpu)); @@ -1874,7 +1868,7 @@ static int process_cpu_topology(struct perf_file_section *section, if (ph->needs_swap) nr = bswap_32(nr); - if (nr > (u32)cpu_nr) { + if (nr != (u32)-1 && nr > (u32)cpu_nr) { pr_debug("socket_id number is too big." "You may need to upgrade the perf tool.\n"); goto free_cpu; -- 1.8.3.1