Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1585118

[tip:perf/urgent] perf tools: Replace _SC_NPROCESSORS_CONF with max_present_cpu in cpu_topology_map

From tip-bot for Jan Stancek <tipbot@zytor.com>
Newsgroups linux.kernel
Subject [tip:perf/urgent] perf tools: Replace _SC_NPROCESSORS_CONF with max_present_cpu in cpu_topology_map
Date 2017-02-21 09:20 +0100
Message-ID <tddPZ-8nM-41@gated-at.bofh.it> (permalink)
References <tbOJY-3B2-19@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


Commit-ID:  da8a58b56c661681f9b2fd2fa59c6da3a5bac8d1
Gitweb:     http://git.kernel.org/tip/da8a58b56c661681f9b2fd2fa59c6da3a5bac8d1
Author:     Jan Stancek <jstancek@redhat.com>
AuthorDate: Fri, 17 Feb 2017 12:10:26 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 17 Feb 2017 12:56:35 -0300

perf tools: 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 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 +-
 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 f287191..ca27a8a 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 98fe69a..803f893 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 bb964e8..075fc77 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 1222f6c..05714d5 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;

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

Re: [PATCH v2 2/3] perf: make build_cpu_topology skip offline/absent  CPUs Jiri Olsa <jolsa@redhat.com> - 2017-02-14 12:10 +0100
  Re: [PATCH v2 2/3] perf: make build_cpu_topology skip  offline/absent CPUs Jan Stancek <jstancek@redhat.com> - 2017-02-15 09:50 +0100
    [PATCH v3 1/3] perf: add cpu__max_present_cpu() Jan Stancek <jstancek@redhat.com> - 2017-02-17 12:20 +0100
      [PATCH v3 2/3] perf: make build_cpu_topology skip offline/absent CPUs Jan Stancek <jstancek@redhat.com> - 2017-02-17 12:20 +0100
        Re: [PATCH v3 2/3] perf: make build_cpu_topology skip offline/absent  CPUs Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-02-17 16:40 +0100
        [tip:perf/urgent] perf header: Make build_cpu_topology skip  offline/absent CPUs tip-bot for Jan Stancek <tipbot@zytor.com> - 2017-02-21 09:20 +0100
      [PATCH v3 3/3] perf: replace _SC_NPROCESSORS_CONF with max_present_cpu in cpu_topology_map Jan Stancek <jstancek@redhat.com> - 2017-02-17 12:20 +0100
        Re: [PATCH v3 3/3] perf: replace _SC_NPROCESSORS_CONF with  max_present_cpu in cpu_topology_map Jiri Olsa <jolsa@redhat.com> - 2017-02-17 16:10 +0100
        [tip:perf/urgent] perf tools: Replace _SC_NPROCESSORS_CONF with  max_present_cpu in cpu_topology_map tip-bot for Jan Stancek <tipbot@zytor.com> - 2017-02-21 09:20 +0100
      [tip:perf/urgent] perf cpumap: Add cpu__max_present_cpu() tip-bot for Jan Stancek <tipbot@zytor.com> - 2017-02-21 09:20 +0100

csiph-web