From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Ingo Molnar <mingo@kernel.org>
Cc: linux-kernel@vger.kernel.org, Jan Stancek <jstancek@redhat.com>,
Alexander Shishkin <alexander.shishkin@linux.intel.com>,
Jiri Olsa <jolsa@kernel.org>,
Masami Hiramatsu <mhiramat@kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
Arnaldo Carvalho de Melo <acme@redhat.com>
Subject: [PATCH 07/18] perf tools: Replace _SC_NPROCESSORS_CONF with max_present_cpu in cpu_topology_map
Date: Mon, 20 Feb 2017 16:08:28 -0300 [thread overview]
Message-ID: <20170220190839.3928-8-acme@kernel.org> (raw)
In-Reply-To: <20170220190839.3928-1-acme@kernel.org>
From: Jan Stancek <jstancek@redhat.com>
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 a 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 <jstancek@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/d7c05c6445fca74a8442c2c73cfffd349c52c44f.1487146877.git.jstancek@redhat.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-stat.c | 2 +-
tools/perf/tests/topology.c | 4 +++-
tools/perf/util/env.c | 2 +-
| 16 +++++-----------
4 files changed, 10 insertions(+), 14 deletions(-)
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index f28719178b51..ca27a8a705ac 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)
--git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 1222f6c5e7b3..05714d548584 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -295,11 +295,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)
@@ -513,9 +509,7 @@ static struct cpu_topo *build_cpu_topology(void)
int ret = -1;
struct cpu_map *map;
- ncpus = sysconf(_SC_NPROCESSORS_CONF);
- if (ncpus < 0)
- return NULL;
+ ncpus = cpu__max_present_cpu();
/* build online CPU map */
map = cpu_map__new(NULL);
@@ -1139,7 +1133,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;
@@ -1794,7 +1788,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));
@@ -1875,7 +1869,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;
--
2.9.3
next prev parent reply other threads:[~2017-02-20 19:09 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-20 19:08 [GIT PULL 00/18] perf/core improvements and fixes Arnaldo Carvalho de Melo
2017-02-20 19:08 ` [PATCH 01/18] tools perf scripting python: clang doesn't have -spec, remove it Arnaldo Carvalho de Melo
2017-02-20 19:08 ` [PATCH 02/18] perf python: Filter out -specs=/a/b/c from the python binding cc options Arnaldo Carvalho de Melo
2017-02-20 19:08 ` [PATCH 03/18] tools lib traceevent: It's preempt not prempt Arnaldo Carvalho de Melo
2017-02-20 19:08 ` [PATCH 04/18] perf session: Fix DEBUG=1 build with clang Arnaldo Carvalho de Melo
2017-02-20 19:08 ` [PATCH 05/18] perf cpumap: Add cpu__max_present_cpu() Arnaldo Carvalho de Melo
2017-02-20 19:08 ` [PATCH 06/18] perf header: Make build_cpu_topology skip offline/absent CPUs Arnaldo Carvalho de Melo
2017-02-20 19:08 ` Arnaldo Carvalho de Melo [this message]
2017-02-20 19:08 ` [PATCH 08/18] perf build: Add special fixdep cleaning rule Arnaldo Carvalho de Melo
2017-02-20 19:08 ` [PATCH 09/18] perf tools: Move new_term arguments into struct parse_events_term template Arnaldo Carvalho de Melo
2017-02-20 19:08 ` [PATCH 10/18] perf tools: Fail on using multiple bits long terms without value Arnaldo Carvalho de Melo
2017-02-20 19:08 ` [PATCH 11/18] perf stat: Add -a as default target Arnaldo Carvalho de Melo
2017-02-20 19:15 ` Borislav Petkov
2017-02-20 19:55 ` Arnaldo Carvalho de Melo
2017-02-20 19:59 ` Borislav Petkov
2017-02-21 7:49 ` Jiri Olsa
2017-02-20 19:08 ` [PATCH 12/18] perf record: " Arnaldo Carvalho de Melo
2017-02-20 19:08 ` [PATCH 13/18] perf utils: Add perf_quiet_option() Arnaldo Carvalho de Melo
2017-02-20 19:08 ` [PATCH 14/18] perf utils: Check verbose flag properly Arnaldo Carvalho de Melo
2017-02-20 19:08 ` [PATCH 15/18] perf report: Add -q/--quiet option Arnaldo Carvalho de Melo
2017-02-20 19:08 ` [PATCH 16/18] perf diff: " Arnaldo Carvalho de Melo
2017-02-20 19:08 ` [PATCH 17/18] perf annotate: " Arnaldo Carvalho de Melo
2017-02-20 19:08 ` [PATCH 18/18] perf record: Honor --quiet option properly Arnaldo Carvalho de Melo
2017-02-21 8:08 ` [GIT PULL 00/18] perf/core improvements and fixes Ingo Molnar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170220190839.3928-8-acme@kernel.org \
--to=acme@kernel.org \
--cc=acme@redhat.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=jolsa@kernel.org \
--cc=jstancek@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mhiramat@kernel.org \
--cc=mingo@kernel.org \
--cc=peterz@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome