[RFC 00/13] perf_env/CPU socket reorg/fixes

[RFC 00/13] perf_env/CPU socket reorg/fixes

[Arnaldo Carvalho de Melo]

Hi,

	Please take a look at these changes to fix the problems reported by
Wang Nan wrt accesses to the cpu_topology_map information.

	The fixes are present on these following two csets:

  perf event: Use machine->env to find the cpu -> socket mapping
  perf report: Do not blindly use env->cpu[al.cpu].socket_id

	The rest are fixes made while working on this, infrastructure to enable
the fixes, reverts for things that ended up not being necessary and some
cleanups.

	It is available at:

 git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git perf/env

	Please let me know if I can have your Acked-by, Tested-by or
Reviewed-by.

- Arnaldo

Arnaldo Carvalho de Melo (13):
  perf env: Move perf_env out of header.h and session.c into separate object
  perf env: Rename some leftovers from rename to perf_env
  perf env: Adopt perf_header__set_cmdline
  perf env: Introduce read_cpu_topology_map() method
  perf sort: Set flag stating if the "socket" key is being used
  perf top: Cache the cpu topology info when "-s socket" is used
  perf hists browser: Fixup the "cpu" column width calculation
  perf machine: Add pointer to sample's environment
  perf event: Use machine->env to find the cpu -> socket mapping
  perf report: Do not blindly use env->cpu[al.cpu].socket_id
  Revert "perf evsel: Add a backpointer to the evlist a evsel is in"
  perf evsel: Remove forward declaration of 'struct perf_evlist'
  Revert "perf evlist: Add backpointer for perf_env to evlist"

[PATCH 01/13] perf env: Move perf_env out of header.h and session.c into separate object

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Since it can be used separately from 'perf_session' and 'perf_header',
move it to separate include file and object, next csets will try to move
a perf_env__init() routine.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/n/tip-ff2rw99tsn670y1b6gxbwdsi@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/Build     |  1 +
 tools/perf/util/env.c     | 19 +++++++++++++++++++
 tools/perf/util/env.h     | 37 +++++++++++++++++++++++++++++++++++++
 tools/perf/util/header.h  | 33 +--------------------------------
 tools/perf/util/session.c | 20 +-------------------
 5 files changed, 59 insertions(+), 51 deletions(-)
 create mode 100644 tools/perf/util/env.c
 create mode 100644 tools/perf/util/env.h

diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 349bc96ca1fe..4bc7a9ab45b1 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -5,6 +5,7 @@ libperf-y += build-id.o
 libperf-y += config.o
 libperf-y += ctype.o
 libperf-y += db-export.o
+libperf-y += env.o
 libperf-y += environment.o
 libperf-y += event.o
 libperf-y += evlist.o
diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
new file mode 100644
index 000000000000..0b3e1b2e5263
--- /dev/null
+++ b/tools/perf/util/env.c
@@ -0,0 +1,19 @@
+#include "env.h"
+#include "util.h"
+
+void perf_env__exit(struct perf_env *env)
+{
+	zfree(&env->hostname);
+	zfree(&env->os_release);
+	zfree(&env->version);
+	zfree(&env->arch);
+	zfree(&env->cpu_desc);
+	zfree(&env->cpuid);
+	zfree(&env->cmdline);
+	zfree(&env->cmdline_argv);
+	zfree(&env->sibling_cores);
+	zfree(&env->sibling_threads);
+	zfree(&env->numa_nodes);
+	zfree(&env->pmu_mappings);
+	zfree(&env->cpu);
+}
diff --git a/tools/perf/util/env.h b/tools/perf/util/env.h
new file mode 100644
index 000000000000..b1370516d99a
--- /dev/null
+++ b/tools/perf/util/env.h
@@ -0,0 +1,37 @@
+#ifndef __PERF_ENV_H
+#define __PERF_ENV_H
+
+struct cpu_topology_map {
+	int	socket_id;
+	int	core_id;
+};
+
+struct perf_env {
+	char			*hostname;
+	char			*os_release;
+	char			*version;
+	char			*arch;
+	int			nr_cpus_online;
+	int			nr_cpus_avail;
+	char			*cpu_desc;
+	char			*cpuid;
+	unsigned long long	total_mem;
+
+	int			nr_cmdline;
+	int			nr_sibling_cores;
+	int			nr_sibling_threads;
+	int			nr_numa_nodes;
+	int			nr_pmu_mappings;
+	int			nr_groups;
+	char			*cmdline;
+	const char		**cmdline_argv;
+	char			*sibling_cores;
+	char			*sibling_threads;
+	char			*numa_nodes;
+	char			*pmu_mappings;
+	struct cpu_topology_map	*cpu;
+};
+
+void perf_env__exit(struct perf_env *env);
+
+#endif /* __PERF_ENV_H */
diff --git a/tools/perf/util/header.h b/tools/perf/util/header.h
index 975d803f46c8..05f27cb6b7e3 100644
--- a/tools/perf/util/header.h
+++ b/tools/perf/util/header.h
@@ -7,7 +7,7 @@
 #include <linux/bitmap.h>
 #include <linux/types.h>
 #include "event.h"
-
+#include "env.h"
 
 enum {
 	HEADER_RESERVED		= 0,	/* always cleared */
@@ -66,37 +66,6 @@ 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;
-	char			*version;
-	char			*arch;
-	int			nr_cpus_online;
-	int			nr_cpus_avail;
-	char			*cpu_desc;
-	char			*cpuid;
-	unsigned long long	total_mem;
-
-	int			nr_cmdline;
-	int			nr_sibling_cores;
-	int			nr_sibling_threads;
-	int			nr_numa_nodes;
-	int			nr_pmu_mappings;
-	int			nr_groups;
-	char			*cmdline;
-	const char		**cmdline_argv;
-	char			*sibling_cores;
-	char			*sibling_threads;
-	char			*numa_nodes;
-	char			*pmu_mappings;
-	struct cpu_topology_map	*cpu;
-};
-
 struct perf_header {
 	enum perf_header_version	version;
 	bool				needs_swap;
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 23fed17307ff..728cb115fbb8 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -170,31 +170,13 @@ static void perf_session__delete_threads(struct perf_session *session)
 	machine__delete_threads(&session->machines.host);
 }
 
-static void perf_session_env__exit(struct perf_env *env)
-{
-	zfree(&env->hostname);
-	zfree(&env->os_release);
-	zfree(&env->version);
-	zfree(&env->arch);
-	zfree(&env->cpu_desc);
-	zfree(&env->cpuid);
-
-	zfree(&env->cmdline);
-	zfree(&env->cmdline_argv);
-	zfree(&env->sibling_cores);
-	zfree(&env->sibling_threads);
-	zfree(&env->numa_nodes);
-	zfree(&env->pmu_mappings);
-	zfree(&env->cpu);
-}
-
 void perf_session__delete(struct perf_session *session)
 {
 	auxtrace__free(session);
 	auxtrace_index__free(&session->auxtrace_index);
 	perf_session__destroy_kernel_maps(session);
 	perf_session__delete_threads(session);
-	perf_session_env__exit(&session->header.env);
+	perf_env__exit(&session->header.env);
 	machines__exit(&session->machines);
 	if (session->file)
 		perf_data_file__close(session->file);
-- 
2.1.0

[PATCH 02/13] perf env: Rename some leftovers from rename to perf_env

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

In ce80d3bef9ff ("perf tools: Rename perf_session_env to perf_env") we
forgot to rename a few functions to the "perf_env" prefix, do it now.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/n/tip-b3ui3z6ock89z1814pu2er98@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/arch/common.c       | 10 ++++------
 tools/perf/arch/common.h       |  4 ++--
 tools/perf/builtin-annotate.c  |  2 +-
 tools/perf/builtin-top.c       |  2 +-
 tools/perf/ui/browsers/hists.c |  2 +-
 5 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
index b00dfd92ea73..e83c8ce24303 100644
--- a/tools/perf/arch/common.c
+++ b/tools/perf/arch/common.c
@@ -128,9 +128,8 @@ static const char *normalize_arch(char *arch)
 	return arch;
 }
 
-static int perf_session_env__lookup_binutils_path(struct perf_env *env,
-						  const char *name,
-						  const char **path)
+static int perf_env__lookup_binutils_path(struct perf_env *env,
+					  const char *name, const char **path)
 {
 	int idx;
 	const char *arch, *cross_env;
@@ -206,7 +205,7 @@ out_error:
 	return -1;
 }
 
-int perf_session_env__lookup_objdump(struct perf_env *env)
+int perf_env__lookup_objdump(struct perf_env *env)
 {
 	/*
 	 * For live mode, env->arch will be NULL and we can use
@@ -215,6 +214,5 @@ int perf_session_env__lookup_objdump(struct perf_env *env)
 	if (env->arch == NULL)
 		return 0;
 
-	return perf_session_env__lookup_binutils_path(env, "objdump",
-						      &objdump_path);
+	return perf_env__lookup_binutils_path(env, "objdump", &objdump_path);
 }
diff --git a/tools/perf/arch/common.h b/tools/perf/arch/common.h
index 20176df69fc8..7529cfb143ce 100644
--- a/tools/perf/arch/common.h
+++ b/tools/perf/arch/common.h
@@ -1,10 +1,10 @@
 #ifndef ARCH_PERF_COMMON_H
 #define ARCH_PERF_COMMON_H
 
-#include "../util/session.h"
+#include "../util/env.h"
 
 extern const char *objdump_path;
 
-int perf_session_env__lookup_objdump(struct perf_env *env);
+int perf_env__lookup_objdump(struct perf_env *env);
 
 #endif /* ARCH_PERF_COMMON_H */
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 8edc205ff9a7..2bf9b3fd9e61 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -211,7 +211,7 @@ static int __cmd_annotate(struct perf_annotate *ann)
 	}
 
 	if (!objdump_path) {
-		ret = perf_session_env__lookup_objdump(&session->header.env);
+		ret = perf_env__lookup_objdump(&session->header.env);
 		if (ret)
 			goto out;
 	}
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 8c465c83aabf..e5ca6848f01d 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -952,7 +952,7 @@ static int __cmd_top(struct perf_top *top)
 	machines__set_symbol_filter(&top->session->machines, symbol_filter);
 
 	if (!objdump_path) {
-		ret = perf_session_env__lookup_objdump(&top->session->header.env);
+		ret = perf_env__lookup_objdump(&top->session->header.env);
 		if (ret)
 			goto out_delete;
 	}
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index e4fd40f72b4a..fea29fbb9d47 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1442,7 +1442,7 @@ do_annotate(struct hist_browser *browser, struct popup_action *act)
 	struct hist_entry *he;
 	int err;
 
-	if (!objdump_path && perf_session_env__lookup_objdump(browser->env))
+	if (!objdump_path && perf_env__lookup_objdump(browser->env))
 		return 0;
 
 	notes = symbol__annotation(act->ms.sym);
-- 
2.1.0

[PATCH 03/13] perf env: Adopt perf_header__set_cmdline

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Move this from two globals to perf_env global, that eventually will
be just perf_header->env or something else, to ease the refactoring
series, leave it as a global and go on reading more of its fields,
not as part of the header writing process but as a perf_env init one
that will be used for perf.data-less situations.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/n/tip-2j78tdf8zn1ci0y6ji15bifj@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/env.c           | 39 ++++++++++++++++++++++++++++++++++++
 tools/perf/util/env.h           |  4 ++++
 tools/perf/util/header.c        | 44 +++++------------------------------------
 tools/perf/util/parse-options.c |  2 +-
 4 files changed, 49 insertions(+), 40 deletions(-)

diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
index 0b3e1b2e5263..ca1e33a2203e 100644
--- a/tools/perf/util/env.c
+++ b/tools/perf/util/env.c
@@ -1,6 +1,8 @@
 #include "env.h"
 #include "util.h"
 
+struct perf_env perf_env;
+
 void perf_env__exit(struct perf_env *env)
 {
 	zfree(&env->hostname);
@@ -17,3 +19,40 @@ void perf_env__exit(struct perf_env *env)
 	zfree(&env->pmu_mappings);
 	zfree(&env->cpu);
 }
+
+int perf_env__set_cmdline(struct perf_env *env, int argc, const char *argv[])
+{
+	int i;
+
+	/*
+	 * If env->cmdline_argv has already been set, do not override it.  This allows
+	 * a command to set the cmdline, parse args and then call another
+	 * builtin function that implements a command -- e.g, cmd_kvm calling
+	 * cmd_record.
+	 */
+	if (env->cmdline_argv != NULL)
+		return 0;
+
+	/* do not include NULL termination */
+	env->cmdline_argv = calloc(argc, sizeof(char *));
+	if (env->cmdline_argv == NULL)
+		goto out_enomem;
+
+	/*
+	 * Must copy argv contents because it gets moved around during option
+	 * parsing:
+	 */
+	for (i = 0; i < argc ; i++) {
+		env->cmdline_argv[i] = argv[i];
+		if (env->cmdline_argv[i] == NULL)
+			goto out_free;
+	}
+
+	env->nr_cmdline = argc;
+
+	return 0;
+out_free:
+	zfree(&env->cmdline_argv);
+out_enomem:
+	return -ENOMEM;
+}
diff --git a/tools/perf/util/env.h b/tools/perf/util/env.h
index b1370516d99a..70124d9a1624 100644
--- a/tools/perf/util/env.h
+++ b/tools/perf/util/env.h
@@ -32,6 +32,10 @@ struct perf_env {
 	struct cpu_topology_map	*cpu;
 };
 
+extern struct perf_env perf_env;
+
 void perf_env__exit(struct perf_env *env);
 
+int perf_env__set_cmdline(struct perf_env *env, int argc, const char *argv[]);
+
 #endif /* __PERF_ENV_H */
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 8fd7b7de1acd..151b8310ac70 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -24,9 +24,6 @@
 #include "build-id.h"
 #include "data.h"
 
-static u32 header_argc;
-static const char **header_argv;
-
 /*
  * magic2 = "PERFILE2"
  * must be a numerical value to let the endianness
@@ -138,37 +135,6 @@ static char *do_read_string(int fd, struct perf_header *ph)
 	return NULL;
 }
 
-int
-perf_header__set_cmdline(int argc, const char **argv)
-{
-	int i;
-
-	/*
-	 * If header_argv has already been set, do not override it.
-	 * This allows a command to set the cmdline, parse args and
-	 * then call another builtin function that implements a
-	 * command -- e.g, cmd_kvm calling cmd_record.
-	 */
-	if (header_argv)
-		return 0;
-
-	header_argc = (u32)argc;
-
-	/* do not include NULL termination */
-	header_argv = calloc(argc, sizeof(char *));
-	if (!header_argv)
-		return -ENOMEM;
-
-	/*
-	 * must copy argv contents because it gets moved
-	 * around during option parsing
-	 */
-	for (i = 0; i < argc ; i++)
-		header_argv[i] = argv[i];
-
-	return 0;
-}
-
 static int write_tracing_data(int fd, struct perf_header *h __maybe_unused,
 			    struct perf_evlist *evlist)
 {
@@ -405,8 +371,8 @@ static int write_cmdline(int fd, struct perf_header *h __maybe_unused,
 {
 	char buf[MAXPATHLEN];
 	char proc[32];
-	u32 i, n;
-	int ret;
+	u32 n;
+	int i, ret;
 
 	/*
 	 * actual atual path to perf binary
@@ -420,7 +386,7 @@ static int write_cmdline(int fd, struct perf_header *h __maybe_unused,
 	buf[ret] = '\0';
 
 	/* account for binary path */
-	n = header_argc + 1;
+	n = perf_env.nr_cmdline + 1;
 
 	ret = do_write(fd, &n, sizeof(n));
 	if (ret < 0)
@@ -430,8 +396,8 @@ static int write_cmdline(int fd, struct perf_header *h __maybe_unused,
 	if (ret < 0)
 		return ret;
 
-	for (i = 0 ; i < header_argc; i++) {
-		ret = do_write_string(fd, header_argv[i]);
+	for (i = 0 ; i < perf_env.nr_cmdline; i++) {
+		ret = do_write_string(fd, perf_env.cmdline_argv[i]);
 		if (ret < 0)
 			return ret;
 	}
diff --git a/tools/perf/util/parse-options.c b/tools/perf/util/parse-options.c
index 01626be2a8eb..9a38b05f0273 100644
--- a/tools/perf/util/parse-options.c
+++ b/tools/perf/util/parse-options.c
@@ -496,7 +496,7 @@ int parse_options_subcommand(int argc, const char **argv, const struct option *o
 {
 	struct parse_opt_ctx_t ctx;
 
-	perf_header__set_cmdline(argc, argv);
+	perf_env__set_cmdline(&perf_env, argc, argv);
 
 	/* build usage string if it's not provided */
 	if (subcommands && !usagestr[0]) {
-- 
2.1.0

[PATCH 04/13] perf env: Introduce read_cpu_topology_map() method

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Out of the code to write the cpu topology map in the perf.data file
header.

Now if one needs the CPU topology map for the running machine, one needs
to call perf_env__read_cpu_topology_map(perf_env) and the info will be
stored in perf_env.cpu.

For now we're using a global perf_env variable, that will have its
contents freed after we run a builtin.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/n/tip-j181xo95og1w5q145dwjm3dw@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/perf.c        |  2 ++
 tools/perf/util/env.c    | 28 ++++++++++++++++++++++++++++
 tools/perf/util/env.h    |  2 ++
 tools/perf/util/header.c | 26 ++++++++++----------------
 4 files changed, 42 insertions(+), 16 deletions(-)

diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index f2fc019b3671..1fded922bcc8 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -8,6 +8,7 @@
  */
 #include "builtin.h"
 
+#include "util/env.h"
 #include "util/exec_cmd.h"
 #include "util/cache.h"
 #include "util/quote.h"
@@ -369,6 +370,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
 
 	status = p->fn(argc, argv, prefix);
 	exit_browser(status);
+	perf_env__exit(&perf_env);
 
 	if (status)
 		return status & 0xff;
diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
index ca1e33a2203e..6af4f7c36820 100644
--- a/tools/perf/util/env.c
+++ b/tools/perf/util/env.c
@@ -1,3 +1,4 @@
+#include "cpumap.h"
 #include "env.h"
 #include "util.h"
 
@@ -56,3 +57,30 @@ out_free:
 out_enomem:
 	return -ENOMEM;
 }
+
+int perf_env__read_cpu_topology_map(struct perf_env *env)
+{
+	int cpu, nr_cpus;
+
+	if (env->cpu != NULL)
+		return 0;
+
+	if (env->nr_cpus_avail == 0)
+		env->nr_cpus_avail = sysconf(_SC_NPROCESSORS_CONF);
+
+	nr_cpus = env->nr_cpus_avail;
+	if (nr_cpus == -1)
+		return -EINVAL;
+
+	env->cpu = calloc(nr_cpus, sizeof(env->cpu[0]));
+	if (env->cpu == NULL)
+		return -ENOMEM;
+
+	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);
+	}
+
+	env->nr_cpus_avail = nr_cpus;
+	return 0;
+}
diff --git a/tools/perf/util/env.h b/tools/perf/util/env.h
index 70124d9a1624..c4e36323d91e 100644
--- a/tools/perf/util/env.h
+++ b/tools/perf/util/env.h
@@ -38,4 +38,6 @@ void perf_env__exit(struct perf_env *env);
 
 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);
+
 #endif /* __PERF_ENV_H */
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 151b8310ac70..d4c8aa2f4db7 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -415,8 +415,6 @@ struct cpu_topo {
 	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)
@@ -479,9 +477,6 @@ 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);
@@ -509,7 +504,7 @@ static struct cpu_topo *build_cpu_topology(void)
 	struct cpu_topo *tp;
 	void *addr;
 	u32 nr, i;
-	size_t sz, sz_id;
+	size_t sz;
 	long ncpus;
 	int ret = -1;
 
@@ -520,9 +515,8 @@ 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 + 2 * sz_id);
+	addr = calloc(1, sizeof(*tp) + 2 * sz);
 	if (!addr)
 		return NULL;
 
@@ -532,10 +526,6 @@ static struct cpu_topo *build_cpu_topology(void)
 	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);
@@ -554,7 +544,7 @@ static int write_cpu_topology(int fd, struct perf_header *h __maybe_unused,
 {
 	struct cpu_topo *tp;
 	u32 i;
-	int ret;
+	int ret, j;
 
 	tp = build_cpu_topology();
 	if (!tp)
@@ -579,11 +569,15 @@ static int write_cpu_topology(int fd, struct perf_header *h __maybe_unused,
 			break;
 	}
 
-	for (i = 0; i < tp->cpu_nr; i++) {
-		ret = do_write(fd, &tp->core_id[i], sizeof(int));
+	perf_env__read_cpu_topology_map(&perf_env);
+
+	for (j = 0; j < perf_env.nr_cpus_avail; j++) {
+		ret = do_write(fd, &perf_env.cpu[j].core_id,
+			       sizeof(perf_env.cpu[j].core_id));
 		if (ret < 0)
 			return ret;
-		ret = do_write(fd, &tp->phy_pkg_id[i], sizeof(int));
+		ret = do_write(fd, &perf_env.cpu[j].socket_id,
+			       sizeof(perf_env.cpu[j].socket_id));
 		if (ret < 0)
 			return ret;
 	}
-- 
2.1.0

RE: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method

[Liang, Kan]

> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c index
> 151b8310ac70..d4c8aa2f4db7 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -415,8 +415,6 @@ struct cpu_topo {
>  	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) @@ -479,9 +477,6
> @@ 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);
> @@ -509,7 +504,7 @@ static struct cpu_topo *build_cpu_topology(void)
>  	struct cpu_topo *tp;
>  	void *addr;
>  	u32 nr, i;
> -	size_t sz, sz_id;
> +	size_t sz;
>  	long ncpus;
>  	int ret = -1;
> 
> @@ -520,9 +515,8 @@ 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 + 2 * sz_id);
> +	addr = calloc(1, sizeof(*tp) + 2 * sz);
>  	if (!addr)
>  		return NULL;
> 
> @@ -532,10 +526,6 @@ static struct cpu_topo *build_cpu_topology(void)
>  	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);
> @@ -554,7 +544,7 @@ static int write_cpu_topology(int fd, struct
> perf_header *h __maybe_unused,  {
>  	struct cpu_topo *tp;
>  	u32 i;
> -	int ret;
> +	int ret, j;
> 
>  	tp = build_cpu_topology();
>  	if (!tp)
> @@ -579,11 +569,15 @@ static int write_cpu_topology(int fd, struct
> perf_header *h __maybe_unused,
>  			break;
>  	}
> 
> -	for (i = 0; i < tp->cpu_nr; i++) {
> -		ret = do_write(fd, &tp->core_id[i], sizeof(int));
> +	perf_env__read_cpu_topology_map(&perf_env);
> +

I think we need to handle error here.

Re: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method

[Arnaldo Carvalho de Melo]

Em Wed, Sep 09, 2015 at 09:41:18PM +0000, Liang, Kan escreveu:
> > -	for (i = 0; i < tp->cpu_nr; i++) {
> > -		ret = do_write(fd, &tp->core_id[i], sizeof(int));
> > +	perf_env__read_cpu_topology_map(&perf_env);
> > +
> 
> I think we need to handle error here.

Ok, adding a test, updated patch below. While doing that, noticed that
the in the thread_sib case the "ret = do_something()" doesn't makes the
whole function fail, will fix later.

- Arnaldo

commit c98989f4a47a6fd3f3f8bd937b96faaced8b203e
Author: Arnaldo Carvalho de Melo <acme@redhat.com>
Date:   Wed Sep 9 10:37:01 2015 -0300

    perf env: Introduce read_cpu_topology_map() method
    
    Out of the code to write the cpu topology map in the perf.data file
    header.
    
    Now if one needs the CPU topology map for the running machine, one needs
    to call perf_env__read_cpu_topology_map(perf_env) and the info will be
    stored in perf_env.cpu.
    
    For now we're using a global perf_env variable, that will have its
    contents freed after we run a builtin.
    
    v2: Check perf_env__read_cpu_topology_map() return in
        write_cpu_topology() (Kan Liang)
    
    Cc: Adrian Hunter <adrian.hunter@intel.com>
    Cc: Borislav Petkov <bp@suse.de>
    Cc: David Ahern <dsahern@gmail.com>
    Cc: Frederic Weisbecker <fweisbec@gmail.com>
    Cc: Jiri Olsa <jolsa@redhat.com>
    Cc: Kan Liang <kan.liang@intel.com>
    Cc: Namhyung Kim <namhyung@kernel.org>
    Cc: Stephane Eranian <eranian@google.com>
    Cc: Wang Nan <wangnan0@huawei.com>
    Link: http://lkml.kernel.org/r/1441828225-667-5-git-send-email-acme@kernel.org
    Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index f2fc019b3671..1fded922bcc8 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -8,6 +8,7 @@
  */
 #include "builtin.h"
 
+#include "util/env.h"
 #include "util/exec_cmd.h"
 #include "util/cache.h"
 #include "util/quote.h"
@@ -369,6 +370,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
 
 	status = p->fn(argc, argv, prefix);
 	exit_browser(status);
+	perf_env__exit(&perf_env);
 
 	if (status)
 		return status & 0xff;
diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
index ca1e33a2203e..6af4f7c36820 100644
--- a/tools/perf/util/env.c
+++ b/tools/perf/util/env.c
@@ -1,3 +1,4 @@
+#include "cpumap.h"
 #include "env.h"
 #include "util.h"
 
@@ -56,3 +57,30 @@ out_free:
 out_enomem:
 	return -ENOMEM;
 }
+
+int perf_env__read_cpu_topology_map(struct perf_env *env)
+{
+	int cpu, nr_cpus;
+
+	if (env->cpu != NULL)
+		return 0;
+
+	if (env->nr_cpus_avail == 0)
+		env->nr_cpus_avail = sysconf(_SC_NPROCESSORS_CONF);
+
+	nr_cpus = env->nr_cpus_avail;
+	if (nr_cpus == -1)
+		return -EINVAL;
+
+	env->cpu = calloc(nr_cpus, sizeof(env->cpu[0]));
+	if (env->cpu == NULL)
+		return -ENOMEM;
+
+	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);
+	}
+
+	env->nr_cpus_avail = nr_cpus;
+	return 0;
+}
diff --git a/tools/perf/util/env.h b/tools/perf/util/env.h
index 70124d9a1624..c4e36323d91e 100644
--- a/tools/perf/util/env.h
+++ b/tools/perf/util/env.h
@@ -38,4 +38,6 @@ void perf_env__exit(struct perf_env *env);
 
 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);
+
 #endif /* __PERF_ENV_H */
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 151b8310ac70..d6437465f70f 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -415,8 +415,6 @@ struct cpu_topo {
 	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)
@@ -479,9 +477,6 @@ 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);
@@ -509,7 +504,7 @@ static struct cpu_topo *build_cpu_topology(void)
 	struct cpu_topo *tp;
 	void *addr;
 	u32 nr, i;
-	size_t sz, sz_id;
+	size_t sz;
 	long ncpus;
 	int ret = -1;
 
@@ -520,9 +515,8 @@ 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 + 2 * sz_id);
+	addr = calloc(1, sizeof(*tp) + 2 * sz);
 	if (!addr)
 		return NULL;
 
@@ -532,10 +526,6 @@ static struct cpu_topo *build_cpu_topology(void)
 	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);
@@ -554,7 +544,7 @@ static int write_cpu_topology(int fd, struct perf_header *h __maybe_unused,
 {
 	struct cpu_topo *tp;
 	u32 i;
-	int ret;
+	int ret, j;
 
 	tp = build_cpu_topology();
 	if (!tp)
@@ -579,11 +569,17 @@ static int write_cpu_topology(int fd, struct perf_header *h __maybe_unused,
 			break;
 	}
 
-	for (i = 0; i < tp->cpu_nr; i++) {
-		ret = do_write(fd, &tp->core_id[i], sizeof(int));
+	ret = perf_env__read_cpu_topology_map(&perf_env);
+	if (ret < 0)
+		goto done;
+
+	for (j = 0; j < perf_env.nr_cpus_avail; j++) {
+		ret = do_write(fd, &perf_env.cpu[j].core_id,
+			       sizeof(perf_env.cpu[j].core_id));
 		if (ret < 0)
 			return ret;
-		ret = do_write(fd, &tp->phy_pkg_id[i], sizeof(int));
+		ret = do_write(fd, &perf_env.cpu[j].socket_id,
+			       sizeof(perf_env.cpu[j].socket_id));
 		if (ret < 0)
 			return ret;
 	}

RE: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method

[Liang, Kan]

> 
> Em Wed, Sep 09, 2015 at 09:41:18PM +0000, Liang, Kan escreveu:
> > > -	for (i = 0; i < tp->cpu_nr; i++) {
> > > -		ret = do_write(fd, &tp->core_id[i], sizeof(int));
> > > +	perf_env__read_cpu_topology_map(&perf_env);
> > > +
> >
> > I think we need to handle error here.
> 
> Ok, adding a test, updated patch below. While doing that, noticed that the
> in the thread_sib case the "ret = do_something()" doesn't makes the
> whole function fail, will fix later.
> 

Thanks.
Except patch 11 & 13 (for reverting), the rest of patches are good to me.

Thanks,
Kan

> - Arnaldo
> 
> commit c98989f4a47a6fd3f3f8bd937b96faaced8b203e
> Author: Arnaldo Carvalho de Melo <acme@redhat.com>
> Date:   Wed Sep 9 10:37:01 2015 -0300
> 
>     perf env: Introduce read_cpu_topology_map() method
> 
>     Out of the code to write the cpu topology map in the perf.data file
>     header.
> 
>     Now if one needs the CPU topology map for the running machine, one
> needs
>     to call perf_env__read_cpu_topology_map(perf_env) and the info will
> be
>     stored in perf_env.cpu.
> 
>     For now we're using a global perf_env variable, that will have its
>     contents freed after we run a builtin.
> 
>     v2: Check perf_env__read_cpu_topology_map() return in
>         write_cpu_topology() (Kan Liang)
> 
>     Cc: Adrian Hunter <adrian.hunter@intel.com>
>     Cc: Borislav Petkov <bp@suse.de>
>     Cc: David Ahern <dsahern@gmail.com>
>     Cc: Frederic Weisbecker <fweisbec@gmail.com>
>     Cc: Jiri Olsa <jolsa@redhat.com>
>     Cc: Kan Liang <kan.liang@intel.com>
>     Cc: Namhyung Kim <namhyung@kernel.org>
>     Cc: Stephane Eranian <eranian@google.com>
>     Cc: Wang Nan <wangnan0@huawei.com>
>     Link: http://lkml.kernel.org/r/1441828225-667-5-git-send-email-
> acme@kernel.org
>     Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> diff --git a/tools/perf/perf.c b/tools/perf/perf.c index
> f2fc019b3671..1fded922bcc8 100644
> --- a/tools/perf/perf.c
> +++ b/tools/perf/perf.c
> @@ -8,6 +8,7 @@
>   */
>  #include "builtin.h"
> 
> +#include "util/env.h"
>  #include "util/exec_cmd.h"
>  #include "util/cache.h"
>  #include "util/quote.h"
> @@ -369,6 +370,7 @@ static int run_builtin(struct cmd_struct *p, int argc,
> const char **argv)
> 
>  	status = p->fn(argc, argv, prefix);
>  	exit_browser(status);
> +	perf_env__exit(&perf_env);
> 
>  	if (status)
>  		return status & 0xff;
> diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c index
> ca1e33a2203e..6af4f7c36820 100644
> --- a/tools/perf/util/env.c
> +++ b/tools/perf/util/env.c
> @@ -1,3 +1,4 @@
> +#include "cpumap.h"
>  #include "env.h"
>  #include "util.h"
> 
> @@ -56,3 +57,30 @@ out_free:
>  out_enomem:
>  	return -ENOMEM;
>  }
> +
> +int perf_env__read_cpu_topology_map(struct perf_env *env) {
> +	int cpu, nr_cpus;
> +
> +	if (env->cpu != NULL)
> +		return 0;
> +
> +	if (env->nr_cpus_avail == 0)
> +		env->nr_cpus_avail = sysconf(_SC_NPROCESSORS_CONF);
> +
> +	nr_cpus = env->nr_cpus_avail;
> +	if (nr_cpus == -1)
> +		return -EINVAL;
> +
> +	env->cpu = calloc(nr_cpus, sizeof(env->cpu[0]));
> +	if (env->cpu == NULL)
> +		return -ENOMEM;
> +
> +	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);
> +	}
> +
> +	env->nr_cpus_avail = nr_cpus;
> +	return 0;
> +}
> diff --git a/tools/perf/util/env.h b/tools/perf/util/env.h index
> 70124d9a1624..c4e36323d91e 100644
> --- a/tools/perf/util/env.h
> +++ b/tools/perf/util/env.h
> @@ -38,4 +38,6 @@ void perf_env__exit(struct perf_env *env);
> 
>  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);
> +
>  #endif /* __PERF_ENV_H */
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c index
> 151b8310ac70..d6437465f70f 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -415,8 +415,6 @@ struct cpu_topo {
>  	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) @@ -479,9 +477,6
> @@ 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);
> @@ -509,7 +504,7 @@ static struct cpu_topo *build_cpu_topology(void)
>  	struct cpu_topo *tp;
>  	void *addr;
>  	u32 nr, i;
> -	size_t sz, sz_id;
> +	size_t sz;
>  	long ncpus;
>  	int ret = -1;
> 
> @@ -520,9 +515,8 @@ 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 + 2 * sz_id);
> +	addr = calloc(1, sizeof(*tp) + 2 * sz);
>  	if (!addr)
>  		return NULL;
> 
> @@ -532,10 +526,6 @@ static struct cpu_topo *build_cpu_topology(void)
>  	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);
> @@ -554,7 +544,7 @@ static int write_cpu_topology(int fd, struct
> perf_header *h __maybe_unused,  {
>  	struct cpu_topo *tp;
>  	u32 i;
> -	int ret;
> +	int ret, j;
> 
>  	tp = build_cpu_topology();
>  	if (!tp)
> @@ -579,11 +569,17 @@ static int write_cpu_topology(int fd, struct
> perf_header *h __maybe_unused,
>  			break;
>  	}
> 
> -	for (i = 0; i < tp->cpu_nr; i++) {
> -		ret = do_write(fd, &tp->core_id[i], sizeof(int));
> +	ret = perf_env__read_cpu_topology_map(&perf_env);
> +	if (ret < 0)
> +		goto done;
> +
> +	for (j = 0; j < perf_env.nr_cpus_avail; j++) {
> +		ret = do_write(fd, &perf_env.cpu[j].core_id,
> +			       sizeof(perf_env.cpu[j].core_id));
>  		if (ret < 0)
>  			return ret;
> -		ret = do_write(fd, &tp->phy_pkg_id[i], sizeof(int));
> +		ret = do_write(fd, &perf_env.cpu[j].socket_id,
> +			       sizeof(perf_env.cpu[j].socket_id));
>  		if (ret < 0)
>  			return ret;
>  	}

Re: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method

[Arnaldo Carvalho de Melo]

Em Thu, Sep 10, 2015 at 08:00:54PM +0000, Liang, Kan escreveu:
> > Em Wed, Sep 09, 2015 at 09:41:18PM +0000, Liang, Kan escreveu:
> > > > -	for (i = 0; i < tp->cpu_nr; i++) {
> > > > -		ret = do_write(fd, &tp->core_id[i], sizeof(int));
> > > > +	perf_env__read_cpu_topology_map(&perf_env);
> > > > +

> > > I think we need to handle error here.

> > Ok, adding a test, updated patch below. While doing that, noticed that the
> > in the thread_sib case the "ret = do_something()" doesn't makes the
> > whole function fail, will fix later.

> Except patch 11 & 13 (for reverting), the rest of patches are good to me.

Ok, so, to take it more formally, can I turn this "good to me" as
justification for adding "Acked-by: Kan Liang", to those patches?

Please take a look at the other patches in the perf/env branch, I did it
while trying to figure it out if the reverts should be kept.

- Arnaldo

RE: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method

[Liang, Kan]

> Em Thu, Sep 10, 2015 at 08:00:54PM +0000, Liang, Kan escreveu:
> > > Em Wed, Sep 09, 2015 at 09:41:18PM +0000, Liang, Kan escreveu:
> > > > > -	for (i = 0; i < tp->cpu_nr; i++) {
> > > > > -		ret = do_write(fd, &tp->core_id[i], sizeof(int));
> > > > > +	perf_env__read_cpu_topology_map(&perf_env);
> > > > > +
> 
> > > > I think we need to handle error here.
> 
> > > Ok, adding a test, updated patch below. While doing that, noticed
> > > that the in the thread_sib case the "ret = do_something()" doesn't
> > > makes the whole function fail, will fix later.
> 
> > Except patch 11 & 13 (for reverting), the rest of patches are good to me.
> 
> Ok, so, to take it more formally, can I turn this "good to me" as justification
> for adding "Acked-by: Kan Liang", to those patches?

Yes.

> 
> Please take a look at the other patches in the perf/env branch, I did it while
> trying to figure it out if the reverts should be kept.
>
OK

Thanks,
Kan
 
> - Arnaldo

Re: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method

[Wangnan (F)]

On 2015/9/10 21:12, Arnaldo Carvalho de Melo wrote:

[SNIP]

> +
> +int perf_env__read_cpu_topology_map(struct perf_env *env)
> +{
> +	int cpu, nr_cpus;
> +
> +	if (env->cpu != NULL)
> +		return 0;
> +
> +	if (env->nr_cpus_avail == 0)
> +		env->nr_cpus_avail = sysconf(_SC_NPROCESSORS_CONF);
> +
> +	nr_cpus = env->nr_cpus_avail;
> +	if (nr_cpus == -1)
> +		return -EINVAL;
> +
> +	env->cpu = calloc(nr_cpus, sizeof(env->cpu[0]));
> +	if (env->cpu == NULL)
> +		return -ENOMEM;
> +
> +	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);
> +	}

Shouldn't we check the failure of these two functions?

At this point perf_env__read_cpu_topology_map and build_cpu_topology are 
doing
similar things. build_cpu_topology() reads 
/sys/xxxxx/cpu%d/topology/{core,thread}_siblings_list,
perf_env__read_cpu_topology_map() reads /sys/xxxxx/cpu%d/topology/core_id,
but build_cpu_topology() returns error if any read failed, but
perf_env__read_cpu_topology_map() fills core_id and socket_id with -1 if 
read fail.

I tried to offline a core between build_cpu_topology() and 
perf_env__read_cpu_topology_map(),
and perf report say:

  # perf report -v --header-only -I
  build id event received for [kernel.kallsyms]: (...)
  core_id number is too big.You may need to upgrade the perf tool. <-- 
*see this warning*
  # ========
  # captured on: Sun Feb 15 15:01:05 2009
  # hostname : localhost
  ...
  # sibling cores   : ...
  # sibling threads : 7
  # Core ID and Socket ID information is not available
  # pmu mappings: not available
  # ========

Thank you.

> +
> +	env->nr_cpus_avail = nr_cpus;
> +	return 0;
> +}
> diff --git a/tools/perf/util/env.h b/tools/perf/util/env.h
> index 70124d9a1624..c4e36323d91e 100644
> --- a/tools/perf/util/env.h
> +++ b/tools/perf/util/env.h
> @@ -38,4 +38,6 @@ void perf_env__exit(struct perf_env *env);
>   
>   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);
> +
>   #endif /* __PERF_ENV_H */
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index 151b8310ac70..d6437465f70f 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -415,8 +415,6 @@ struct cpu_topo {
>   	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)
> @@ -479,9 +477,6 @@ 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);
> @@ -509,7 +504,7 @@ static struct cpu_topo *build_cpu_topology(void)
>   	struct cpu_topo *tp;
>   	void *addr;
>   	u32 nr, i;
> -	size_t sz, sz_id;
> +	size_t sz;
>   	long ncpus;
>   	int ret = -1;
>   
> @@ -520,9 +515,8 @@ 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 + 2 * sz_id);
> +	addr = calloc(1, sizeof(*tp) + 2 * sz);
>   	if (!addr)
>   		return NULL;
>   
> @@ -532,10 +526,6 @@ static struct cpu_topo *build_cpu_topology(void)
>   	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);
> @@ -554,7 +544,7 @@ static int write_cpu_topology(int fd, struct perf_header *h __maybe_unused,
>   {
>   	struct cpu_topo *tp;
>   	u32 i;
> -	int ret;
> +	int ret, j;
>   
>   	tp = build_cpu_topology();
>   	if (!tp)
> @@ -579,11 +569,17 @@ static int write_cpu_topology(int fd, struct perf_header *h __maybe_unused,
>   			break;
>   	}
>   
> -	for (i = 0; i < tp->cpu_nr; i++) {
> -		ret = do_write(fd, &tp->core_id[i], sizeof(int));
> +	ret = perf_env__read_cpu_topology_map(&perf_env);
> +	if (ret < 0)
> +		goto done;
> +
> +	for (j = 0; j < perf_env.nr_cpus_avail; j++) {
> +		ret = do_write(fd, &perf_env.cpu[j].core_id,
> +			       sizeof(perf_env.cpu[j].core_id));
>   		if (ret < 0)
>   			return ret;
> -		ret = do_write(fd, &tp->phy_pkg_id[i], sizeof(int));
> +		ret = do_write(fd, &perf_env.cpu[j].socket_id,
> +			       sizeof(perf_env.cpu[j].socket_id));
>   		if (ret < 0)
>   			return ret;
>   	}

Re: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method

[Arnaldo Carvalho de Melo]

Em Fri, Sep 11, 2015 at 06:20:03PM +0800, Wangnan (F) escreveu:
> 
> 
> On 2015/9/10 21:12, Arnaldo Carvalho de Melo wrote:
> 
> [SNIP]
> 
> >+
> >+int perf_env__read_cpu_topology_map(struct perf_env *env)
> >+{
> >+	int cpu, nr_cpus;
> >+
> >+	if (env->cpu != NULL)
> >+		return 0;
> >+
> >+	if (env->nr_cpus_avail == 0)
> >+		env->nr_cpus_avail = sysconf(_SC_NPROCESSORS_CONF);
> >+
> >+	nr_cpus = env->nr_cpus_avail;
> >+	if (nr_cpus == -1)
> >+		return -EINVAL;
> >+
> >+	env->cpu = calloc(nr_cpus, sizeof(env->cpu[0]));
> >+	if (env->cpu == NULL)
> >+		return -ENOMEM;
> >+
> >+	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);
> >+	}
> 
> Shouldn't we check the failure of these two functions?

Humm, the original code, in build_cpu_topology() was not checking
that, i.e. if you tried it with or without my patches the result will be
the same, no?
 
> At this point perf_env__read_cpu_topology_map and build_cpu_topology are
> doing
> similar things. build_cpu_topology() reads
> /sys/xxxxx/cpu%d/topology/{core,thread}_siblings_list,
> perf_env__read_cpu_topology_map() reads /sys/xxxxx/cpu%d/topology/core_id,
> but build_cpu_topology() returns error if any read failed, but
> perf_env__read_cpu_topology_map() fills core_id and socket_id with -1 if
> read fail.

> I tried to offline a core between build_cpu_topology() and
> perf_env__read_cpu_topology_map(),
> and perf report say:
> 
>  # perf report -v --header-only -I
>  build id event received for [kernel.kallsyms]: (...)
>  core_id number is too big.You may need to upgrade the perf tool. <-- *see
> this warning*
>  # ========
>  # captured on: Sun Feb 15 15:01:05 2009
>  # hostname : localhost
>  ...
>  # sibling cores   : ...
>  # sibling threads : 7
>  # Core ID and Socket ID information is not available
>  # pmu mappings: not available
>  # ========

So this is a problem before and after my patches, i.e. If I go on and
do, with what we have in acme/perf/core, i.e. none of the changes I'm
playing with in perf/env:

  $ git remote update acme
  Fetching acme
  $ git checkout -b tmp acme/perf/core
  Branch tmp set up to track remote branch perf/core from acme.
  Switched to a new branch 'tmp'
  $ git log --oneline | head -5
  7e150fb33a91 perf tests: Introduce iterator function for tests
  1765d9b26f84 perf test: Add entry for hists socket filter
  207bb55e9193 perf hists browser: Zoom in/out for processor socket
  9a2843a5f421 perf report: Introduce --socket-filter option
  99851c76436a perf tools: Introduce new sort type "socket" for the processor socket
  $ rm -rf /tmp/build/perf ; mkdir -p /tmp/build/perf ; make DEBUG=1 -C tools/perf O=/tmp/build/perf install-bin
  # echo 0 > /sys/devices/system/cpu/cpu2/online
  $ cat /sys/devices/system/cpu/cpu2/topology/core_id
  cat: /sys/devices/system/cpu/cpu2/topology/core_id: No such file or directory
  $ ls -la /sys/devices/system/cpu/cpu2/topology/
  ls: cannot access /sys/devices/system/cpu/cpu2/topology/: No such file or directory
  $ perf record usleep 1
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.012 MB perf.data (7 samples) ]
  [acme@felicio linux]$ perf report --header-only -I 
  # ========
  # captured on: Fri Sep 11 11:34:18 2015
  # hostname : felicio.ghostprotocols.net
  # os release : 4.2.0
  # perf version : 4.2.g7e150f
  #  arch : x86_64
  # nrcpus online : 4
  # nrcpus avail : 3
  # cpudesc : Intel(R) Core(TM) i5-2400 CPU @ 3.10GHz
  <SNIP>
  # node0 meminfo  : total = 8085412 kB, free = 5317596 kB
  # node0 cpu list : 0-1,3
  <SNIP>
  $ 

We can see multiple bugs here, right? online/avail is swapped, and when
online != avail we simply do not record the cpu topology info at all!

If we get that CPU back online:

  # echo 1 > /sys/devices/system/cpu/cpu2/online

Then all works:

  $ perf record usleep 1
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.012 MB perf.data (7 samples) ]
  $ perf report --header-only -I
  # ========
  # captured on: Fri Sep 11 11:37:31 2015
  # hostname : felicio.ghostprotocols.net
  # os release : 4.2.0
  # perf version : 4.2.g7e150f
  # arch : x86_64
  # nrcpus online : 4
  # nrcpus avail : 4
  # cpudesc : Intel(R) Core(TM) i5-2400 CPU @ 3.10GHz
  <SNIP>
  # sibling cores   : 0-3
  # sibling threads : 0
  # sibling threads : 1
  # sibling threads : 2
  # sibling threads : 3
  # CPU 0: Core ID 0, Socket ID 0
  # CPU 1: Core ID 1, Socket ID 0
  # CPU 2: Core ID 2, Socket ID 0
  # CPU 3: Core ID 3, Socket ID 0
  # node0 meminfo  : total = 8085412 kB, free = 5316992 kB
  # node0 cpu list : 0-3

So, again, this was not introduced by this patchkit, but it is good that you did
these offline tests, so we can fix it!

- Arnaldo
 
 
> >+
> >+	env->nr_cpus_avail = nr_cpus;
> >+	return 0;
> >+}
> >diff --git a/tools/perf/util/env.h b/tools/perf/util/env.h
> >index 70124d9a1624..c4e36323d91e 100644
> >--- a/tools/perf/util/env.h
> >+++ b/tools/perf/util/env.h
> >@@ -38,4 +38,6 @@ void perf_env__exit(struct perf_env *env);
> >  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);
> >+
> >  #endif /* __PERF_ENV_H */
> >diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> >index 151b8310ac70..d6437465f70f 100644
> >--- a/tools/perf/util/header.c
> >+++ b/tools/perf/util/header.c
> >@@ -415,8 +415,6 @@ struct cpu_topo {
> >  	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)
> >@@ -479,9 +477,6 @@ 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);
> >@@ -509,7 +504,7 @@ static struct cpu_topo *build_cpu_topology(void)
> >  	struct cpu_topo *tp;
> >  	void *addr;
> >  	u32 nr, i;
> >-	size_t sz, sz_id;
> >+	size_t sz;
> >  	long ncpus;
> >  	int ret = -1;
> >@@ -520,9 +515,8 @@ 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 + 2 * sz_id);
> >+	addr = calloc(1, sizeof(*tp) + 2 * sz);
> >  	if (!addr)
> >  		return NULL;
> >@@ -532,10 +526,6 @@ static struct cpu_topo *build_cpu_topology(void)
> >  	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);
> >@@ -554,7 +544,7 @@ static int write_cpu_topology(int fd, struct perf_header *h __maybe_unused,
> >  {
> >  	struct cpu_topo *tp;
> >  	u32 i;
> >-	int ret;
> >+	int ret, j;
> >  	tp = build_cpu_topology();
> >  	if (!tp)
> >@@ -579,11 +569,17 @@ static int write_cpu_topology(int fd, struct perf_header *h __maybe_unused,
> >  			break;
> >  	}
> >-	for (i = 0; i < tp->cpu_nr; i++) {
> >-		ret = do_write(fd, &tp->core_id[i], sizeof(int));
> >+	ret = perf_env__read_cpu_topology_map(&perf_env);
> >+	if (ret < 0)
> >+		goto done;
> >+
> >+	for (j = 0; j < perf_env.nr_cpus_avail; j++) {
> >+		ret = do_write(fd, &perf_env.cpu[j].core_id,
> >+			       sizeof(perf_env.cpu[j].core_id));
> >  		if (ret < 0)
> >  			return ret;
> >-		ret = do_write(fd, &tp->phy_pkg_id[i], sizeof(int));
> >+		ret = do_write(fd, &perf_env.cpu[j].socket_id,
> >+			       sizeof(perf_env.cpu[j].socket_id));
> >  		if (ret < 0)
> >  			return ret;
> >  	}
> 

Re: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method

[Arnaldo Carvalho de Melo]

Em Fri, Sep 11, 2015 at 11:40:28AM -0300, Arnaldo Carvalho de Melo escreveu:
> So this is a problem before and after my patches, i.e. If I go on and
> do, with what we have in acme/perf/core, i.e. none of the changes I'm
> playing with in perf/env:
> 
>   $ git remote update acme
>   Fetching acme
>   $ git checkout -b tmp acme/perf/core
>   Branch tmp set up to track remote branch perf/core from acme.
>   Switched to a new branch 'tmp'
>   $ git log --oneline | head -5
>   7e150fb33a91 perf tests: Introduce iterator function for tests
>   1765d9b26f84 perf test: Add entry for hists socket filter
>   207bb55e9193 perf hists browser: Zoom in/out for processor socket
>   9a2843a5f421 perf report: Introduce --socket-filter option
>   99851c76436a perf tools: Introduce new sort type "socket" for the processor socket
>   $ rm -rf /tmp/build/perf ; mkdir -p /tmp/build/perf ; make DEBUG=1 -C tools/perf O=/tmp/build/perf install-bin
>   # echo 0 > /sys/devices/system/cpu/cpu2/online
>   $ cat /sys/devices/system/cpu/cpu2/topology/core_id
>   cat: /sys/devices/system/cpu/cpu2/topology/core_id: No such file or directory
>   $ ls -la /sys/devices/system/cpu/cpu2/topology/
>   ls: cannot access /sys/devices/system/cpu/cpu2/topology/: No such file or directory
>   $ perf record usleep 1
>   [ perf record: Woken up 1 times to write data ]
>   [ perf record: Captured and wrote 0.012 MB perf.data (7 samples) ]
>   [acme@felicio linux]$ perf report --header-only -I 
>   # ========
>   # captured on: Fri Sep 11 11:34:18 2015
>   # hostname : felicio.ghostprotocols.net
>   # os release : 4.2.0
>   # perf version : 4.2.g7e150f
>   #  arch : x86_64
>   # nrcpus online : 4
>   # nrcpus avail : 3
>   # cpudesc : Intel(R) Core(TM) i5-2400 CPU @ 3.10GHz
>   <SNIP>
>   # node0 meminfo  : total = 8085412 kB, free = 5317596 kB
>   # node0 cpu list : 0-1,3
>   <SNIP>
>   $ 

Stephane, Namhyung, the bug report below is about the online/avail
above, they are swapped, full explanation below.
 
> We can see multiple bugs here, right? online/avail is swapped, and when
> online != avail we simply do not record the cpu topology info at all!
 
> If we get that CPU back online:
> 
>   # echo 1 > /sys/devices/system/cpu/cpu2/online
> 
> Then all works:
> 
>   $ perf record usleep 1
>   [ perf record: Woken up 1 times to write data ]
>   [ perf record: Captured and wrote 0.012 MB perf.data (7 samples) ]
>   $ perf report --header-only -I
>   # ========
>   # captured on: Fri Sep 11 11:37:31 2015
>   # hostname : felicio.ghostprotocols.net
>   # os release : 4.2.0
>   # perf version : 4.2.g7e150f
>   # arch : x86_64
>   # nrcpus online : 4
>   # nrcpus avail : 4
>   # cpudesc : Intel(R) Core(TM) i5-2400 CPU @ 3.10GHz
>   <SNIP>
>   # sibling cores   : 0-3
>   # sibling threads : 0
>   # sibling threads : 1
>   # sibling threads : 2
>   # sibling threads : 3
>   # CPU 0: Core ID 0, Socket ID 0
>   # CPU 1: Core ID 1, Socket ID 0
>   # CPU 2: Core ID 2, Socket ID 0
>   # CPU 3: Core ID 3, Socket ID 0
>   # node0 meminfo  : total = 8085412 kB, free = 5316992 kB
>   # node0 cpu list : 0-3
> 
> So, again, this was not introduced by this patchkit, but it is good that you did
> these offline tests, so we can fix it!

Stephane, this was introduced in:

  commit fbe96f29ce4b33e0a22219cc7f5996d9157717e3
  Author: Stephane Eranian <eranian@google.com>
  Date:   Fri Sep 30 15:40:40 2011 +0200

    perf tools: Make perf.data more self-descriptive (v8)

------------------

When you write this part:

      - HEADER_NRCPUS: number of online/avail cpus

You do:

static int write_nrcpus(int fd, struct perf_header *h __used,
                       struct perf_evlist *evlist __used)
{
       long nr;
       u32 nrc, nra;
       int ret;

       nr = sysconf(_SC_NPROCESSORS_CONF);
       if (nr < 0)
               return -1;

       nrc = (u32)(nr & UINT_MAX);

       nr = sysconf(_SC_NPROCESSORS_ONLN);
       if (nr < 0)
               return -1;

       nra = (u32)(nr & UINT_MAX);

       ret = do_write(fd, &nrc, sizeof(nrc));
       if (ret < 0)
               return ret;

       return do_write(fd, &nra, sizeof(nra));
}

I.e. write what you called 'nrc' using what is in SC_NRPROCESSORS_CONF, that in
the documentation for glibc reads:

---------------------

http://www.gnu.org/software/libc/manual/html_node/Processor-Resources.html

  sysconf (_SC_NPROCESSORS_CONF)

which returns the number of processors the operating system configured. But it
might be possible for the operating system to disable individual processors and
so the call 

---------------------

Which menas "NR_AVAILABLE", right?

But then you call a variable 'nra' which sounds like you think that what is in
_SC_NPROCESSORS_ONLN is the "available" number of CPUs, which is confused a bit
more by the glibc docs when refering to _SC_NPROCESSORS_ONLN:

---------------------
  sysconf (_SC_NPROCESSORS_ONLN)

returns the number of processors which are currently online (i.e., available). 
---------------------

Then, when printing the number of CPUs encoded in the perf.data file by the
above write_nrcpus() routine, you did:

static void print_nrcpus(struct perf_header *ph, int fd, FILE *fp)
{
       ssize_t ret;
       u32 nr;

       ret = read(fd, &nr, sizeof(nr));
       if (ret != (ssize_t)sizeof(nr))
               nr = -1; /* interpreted as error */

       if (ph->needs_swap)
               nr = bswap_32(nr);

       fprintf(fp, "# nrcpus online : %u\n", nr);

       ret = read(fd, &nr, sizeof(nr));
       if (ret != (ssize_t)sizeof(nr))
               nr = -1; /* interpreted as error */

       if (ph->needs_swap)
               nr = bswap_32(nr);

       fprintf(fp, "# nrcpus avail : %u\n", nr);
}

You inverted it, no?

So, could you please check if the below patch can have your Acked-by?
Namhyung?

Thanks,

- Arnaldo

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 41814547da15..fce6634aebe2 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -1438,7 +1438,7 @@ static int process_nrcpus(struct perf_file_section *section __maybe_unused,
 	if (ph->needs_swap)
 		nr = bswap_32(nr);
 
-	ph->env.nr_cpus_online = nr;
+	ph->env.nr_cpus_avail = nr;
 
 	ret = readn(fd, &nr, sizeof(nr));
 	if (ret != sizeof(nr))
@@ -1447,7 +1447,7 @@ static int process_nrcpus(struct perf_file_section *section __maybe_unused,
 	if (ph->needs_swap)
 		nr = bswap_32(nr);
 
-	ph->env.nr_cpus_avail = nr;
+	ph->env.nr_cpus_online = nr;
 	return 0;
 }
 

Re: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method

[Namhyung Kim]

Hi Arnaldo,

On Fri, Sep 11, 2015 at 12:33:23PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Fri, Sep 11, 2015 at 11:40:28AM -0300, Arnaldo Carvalho de Melo escreveu:
> > So this is a problem before and after my patches, i.e. If I go on and
> > do, with what we have in acme/perf/core, i.e. none of the changes I'm
> > playing with in perf/env:
> > 
> >   $ git remote update acme
> >   Fetching acme
> >   $ git checkout -b tmp acme/perf/core
> >   Branch tmp set up to track remote branch perf/core from acme.
> >   Switched to a new branch 'tmp'
> >   $ git log --oneline | head -5
> >   7e150fb33a91 perf tests: Introduce iterator function for tests
> >   1765d9b26f84 perf test: Add entry for hists socket filter
> >   207bb55e9193 perf hists browser: Zoom in/out for processor socket
> >   9a2843a5f421 perf report: Introduce --socket-filter option
> >   99851c76436a perf tools: Introduce new sort type "socket" for the processor socket
> >   $ rm -rf /tmp/build/perf ; mkdir -p /tmp/build/perf ; make DEBUG=1 -C tools/perf O=/tmp/build/perf install-bin
> >   # echo 0 > /sys/devices/system/cpu/cpu2/online
> >   $ cat /sys/devices/system/cpu/cpu2/topology/core_id
> >   cat: /sys/devices/system/cpu/cpu2/topology/core_id: No such file or directory
> >   $ ls -la /sys/devices/system/cpu/cpu2/topology/
> >   ls: cannot access /sys/devices/system/cpu/cpu2/topology/: No such file or directory
> >   $ perf record usleep 1
> >   [ perf record: Woken up 1 times to write data ]
> >   [ perf record: Captured and wrote 0.012 MB perf.data (7 samples) ]
> >   [acme@felicio linux]$ perf report --header-only -I 
> >   # ========
> >   # captured on: Fri Sep 11 11:34:18 2015
> >   # hostname : felicio.ghostprotocols.net
> >   # os release : 4.2.0
> >   # perf version : 4.2.g7e150f
> >   #  arch : x86_64
> >   # nrcpus online : 4
> >   # nrcpus avail : 3
> >   # cpudesc : Intel(R) Core(TM) i5-2400 CPU @ 3.10GHz
> >   <SNIP>
> >   # node0 meminfo  : total = 8085412 kB, free = 5317596 kB
> >   # node0 cpu list : 0-1,3
> >   <SNIP>
> >   $ 
> 
> Stephane, Namhyung, the bug report below is about the online/avail
> above, they are swapped, full explanation below.
>  
> > We can see multiple bugs here, right? online/avail is swapped, and when
> > online != avail we simply do not record the cpu topology info at all!
>  
> > If we get that CPU back online:
> > 
> >   # echo 1 > /sys/devices/system/cpu/cpu2/online
> > 
> > Then all works:
> > 
> >   $ perf record usleep 1
> >   [ perf record: Woken up 1 times to write data ]
> >   [ perf record: Captured and wrote 0.012 MB perf.data (7 samples) ]
> >   $ perf report --header-only -I
> >   # ========
> >   # captured on: Fri Sep 11 11:37:31 2015
> >   # hostname : felicio.ghostprotocols.net
> >   # os release : 4.2.0
> >   # perf version : 4.2.g7e150f
> >   # arch : x86_64
> >   # nrcpus online : 4
> >   # nrcpus avail : 4
> >   # cpudesc : Intel(R) Core(TM) i5-2400 CPU @ 3.10GHz
> >   <SNIP>
> >   # sibling cores   : 0-3
> >   # sibling threads : 0
> >   # sibling threads : 1
> >   # sibling threads : 2
> >   # sibling threads : 3
> >   # CPU 0: Core ID 0, Socket ID 0
> >   # CPU 1: Core ID 1, Socket ID 0
> >   # CPU 2: Core ID 2, Socket ID 0
> >   # CPU 3: Core ID 3, Socket ID 0
> >   # node0 meminfo  : total = 8085412 kB, free = 5316992 kB
> >   # node0 cpu list : 0-3
> > 
> > So, again, this was not introduced by this patchkit, but it is good that you did
> > these offline tests, so we can fix it!
> 
> Stephane, this was introduced in:
> 
>   commit fbe96f29ce4b33e0a22219cc7f5996d9157717e3
>   Author: Stephane Eranian <eranian@google.com>
>   Date:   Fri Sep 30 15:40:40 2011 +0200
> 
>     perf tools: Make perf.data more self-descriptive (v8)
> 
> ------------------
> 
> When you write this part:
> 
>       - HEADER_NRCPUS: number of online/avail cpus
> 
> You do:
> 
> static int write_nrcpus(int fd, struct perf_header *h __used,
>                        struct perf_evlist *evlist __used)
> {
>        long nr;
>        u32 nrc, nra;
>        int ret;
> 
>        nr = sysconf(_SC_NPROCESSORS_CONF);
>        if (nr < 0)
>                return -1;
> 
>        nrc = (u32)(nr & UINT_MAX);
> 
>        nr = sysconf(_SC_NPROCESSORS_ONLN);
>        if (nr < 0)
>                return -1;
> 
>        nra = (u32)(nr & UINT_MAX);
> 
>        ret = do_write(fd, &nrc, sizeof(nrc));
>        if (ret < 0)
>                return ret;
> 
>        return do_write(fd, &nra, sizeof(nra));
> }
> 
> I.e. write what you called 'nrc' using what is in SC_NRPROCESSORS_CONF, that in
> the documentation for glibc reads:
> 
> ---------------------
> 
> http://www.gnu.org/software/libc/manual/html_node/Processor-Resources.html
> 
>   sysconf (_SC_NPROCESSORS_CONF)
> 
> which returns the number of processors the operating system configured. But it
> might be possible for the operating system to disable individual processors and
> so the call 
> 
> ---------------------
> 
> Which menas "NR_AVAILABLE", right?
> 
> But then you call a variable 'nra' which sounds like you think that what is in
> _SC_NPROCESSORS_ONLN is the "available" number of CPUs, which is confused a bit
> more by the glibc docs when refering to _SC_NPROCESSORS_ONLN:
> 
> ---------------------
>   sysconf (_SC_NPROCESSORS_ONLN)
> 
> returns the number of processors which are currently online (i.e., available). 
> ---------------------
> 
> Then, when printing the number of CPUs encoded in the perf.data file by the
> above write_nrcpus() routine, you did:
> 
> static void print_nrcpus(struct perf_header *ph, int fd, FILE *fp)
> {
>        ssize_t ret;
>        u32 nr;
> 
>        ret = read(fd, &nr, sizeof(nr));
>        if (ret != (ssize_t)sizeof(nr))
>                nr = -1; /* interpreted as error */
> 
>        if (ph->needs_swap)
>                nr = bswap_32(nr);
> 
>        fprintf(fp, "# nrcpus online : %u\n", nr);
> 
>        ret = read(fd, &nr, sizeof(nr));
>        if (ret != (ssize_t)sizeof(nr))
>                nr = -1; /* interpreted as error */
> 
>        if (ph->needs_swap)
>                nr = bswap_32(nr);
> 
>        fprintf(fp, "# nrcpus avail : %u\n", nr);
> }
> 
> You inverted it, no?
> 
> So, could you please check if the below patch can have your Acked-by?
> Namhyung?

Looks good to me.

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung


> 
> Thanks,
> 
> - Arnaldo
> 
> diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
> index 41814547da15..fce6634aebe2 100644
> --- a/tools/perf/util/header.c
> +++ b/tools/perf/util/header.c
> @@ -1438,7 +1438,7 @@ static int process_nrcpus(struct perf_file_section *section __maybe_unused,
>  	if (ph->needs_swap)
>  		nr = bswap_32(nr);
>  
> -	ph->env.nr_cpus_online = nr;
> +	ph->env.nr_cpus_avail = nr;
>  
>  	ret = readn(fd, &nr, sizeof(nr));
>  	if (ret != sizeof(nr))
> @@ -1447,7 +1447,7 @@ static int process_nrcpus(struct perf_file_section *section __maybe_unused,
>  	if (ph->needs_swap)
>  		nr = bswap_32(nr);
>  
> -	ph->env.nr_cpus_avail = nr;
> +	ph->env.nr_cpus_online = nr;
>  	return 0;
>  }
>  

Re: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method

[Arnaldo Carvalho de Melo]

Em Sat, Sep 12, 2015 at 01:14:02AM +0900, Namhyung Kim escreveu:
> > You inverted it, no?

> > So, could you please check if the below patch can have your Acked-by?
> > Namhyung?
 
> Looks good to me.
 
> Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks, added it to the patch, after lunch I should have another patch
for another bug introduced in the same patch, i.e. if one CPU is
offlined, we simply refuse to collect the topology information.

To fix it, I think, we need to insert "(offline)" where one expects to
find the thread and core siblings info.

- Arnaldo

[tip:perf/urgent] perf header: Fixup reading of HEADER_NRCPUS feature

[tip-bot for Arnaldo Carvalho de Melo]

Commit-ID:  caa470475d9b59eeff093ae650800d34612c4379
Gitweb:     http://git.kernel.org/tip/caa470475d9b59eeff093ae650800d34612c4379
Author:     Arnaldo Carvalho de Melo <acme@redhat.com>
AuthorDate: Fri, 11 Sep 2015 12:36:12 -0300
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Sun, 13 Sep 2015 11:41:34 -0300

perf header: Fixup reading of HEADER_NRCPUS feature

The original patch introducing this header wrote the number of CPUs available
and online in one order and then swapped those values when reading, fix it.

Before:

  # perf record usleep 1
  # perf report --header-only | grep 'nrcpus \(online\|avail\)'
  # nrcpus online : 4
  # nrcpus avail : 4
  # echo 0 > /sys/devices/system/cpu/cpu2/online
  # perf record usleep 1
  # perf report --header-only | grep 'nrcpus \(online\|avail\)'
  # nrcpus online : 4
  # nrcpus avail : 3
  # echo 0 > /sys/devices/system/cpu/cpu1/online
  # perf record usleep 1
  # perf report --header-only | grep 'nrcpus \(online\|avail\)'
  # nrcpus online : 4
  # nrcpus avail : 2

After the fix, bringing back the CPUs online:

  # perf report --header-only | grep 'nrcpus \(online\|avail\)'
  # nrcpus online : 2
  # nrcpus avail : 4
  # echo 1 > /sys/devices/system/cpu/cpu2/online
  # perf record usleep 1
  # perf report --header-only | grep 'nrcpus \(online\|avail\)'
  # nrcpus online : 3
  # nrcpus avail : 4
  # echo 1 > /sys/devices/system/cpu/cpu1/online
  # perf record usleep 1
  # perf report --header-only | grep 'nrcpus \(online\|avail\)'
  # nrcpus online : 4
  # nrcpus avail : 4

Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Fixes: fbe96f29ce4b ("perf tools: Make perf.data more self-descriptive (v8)")
Link: http://lkml.kernel.org/r/20150911153323.GP23511@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 4181454..fce6634 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -1438,7 +1438,7 @@ static int process_nrcpus(struct perf_file_section *section __maybe_unused,
 	if (ph->needs_swap)
 		nr = bswap_32(nr);
 
-	ph->env.nr_cpus_online = nr;
+	ph->env.nr_cpus_avail = nr;
 
 	ret = readn(fd, &nr, sizeof(nr));
 	if (ret != sizeof(nr))
@@ -1447,7 +1447,7 @@ static int process_nrcpus(struct perf_file_section *section __maybe_unused,
 	if (ph->needs_swap)
 		nr = bswap_32(nr);
 
-	ph->env.nr_cpus_avail = nr;
+	ph->env.nr_cpus_online = nr;
 	return 0;
 }
 

[tip:perf/core] perf env: Introduce read_cpu_topology_map() method

[tip-bot for Arnaldo Carvalho de Melo]

Commit-ID:  aa36ddd7afbb0a3db216c1391e28cd6d80ed1706
Gitweb:     http://git.kernel.org/tip/aa36ddd7afbb0a3db216c1391e28cd6d80ed1706
Author:     Arnaldo Carvalho de Melo <acme@redhat.com>
AuthorDate: Wed, 9 Sep 2015 10:37:01 -0300
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 14 Sep 2015 12:50:28 -0300

perf env: Introduce read_cpu_topology_map() method

Out of the code to write the cpu topology map in the perf.data file
header.

Now if one needs the CPU topology map for the running machine, one needs
to call perf_env__read_cpu_topology_map(perf_env) and the info will be
stored in perf_env.cpu.

For now we're using a global perf_env variable, that will have its
contents freed after we run a builtin.

v2: Check perf_env__read_cpu_topology_map() return in
    write_cpu_topology() (Kan Liang)

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1441828225-667-5-git-send-email-acme@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/perf.c        |  2 ++
 tools/perf/util/env.c    | 28 ++++++++++++++++++++++++++++
 tools/perf/util/env.h    |  2 ++
 tools/perf/util/header.c | 28 ++++++++++++----------------
 4 files changed, 44 insertions(+), 16 deletions(-)

diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index f2fc019..1fded92 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -8,6 +8,7 @@
  */
 #include "builtin.h"
 
+#include "util/env.h"
 #include "util/exec_cmd.h"
 #include "util/cache.h"
 #include "util/quote.h"
@@ -369,6 +370,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
 
 	status = p->fn(argc, argv, prefix);
 	exit_browser(status);
+	perf_env__exit(&perf_env);
 
 	if (status)
 		return status & 0xff;
diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
index ca1e33a..6af4f7c 100644
--- a/tools/perf/util/env.c
+++ b/tools/perf/util/env.c
@@ -1,3 +1,4 @@
+#include "cpumap.h"
 #include "env.h"
 #include "util.h"
 
@@ -56,3 +57,30 @@ out_free:
 out_enomem:
 	return -ENOMEM;
 }
+
+int perf_env__read_cpu_topology_map(struct perf_env *env)
+{
+	int cpu, nr_cpus;
+
+	if (env->cpu != NULL)
+		return 0;
+
+	if (env->nr_cpus_avail == 0)
+		env->nr_cpus_avail = sysconf(_SC_NPROCESSORS_CONF);
+
+	nr_cpus = env->nr_cpus_avail;
+	if (nr_cpus == -1)
+		return -EINVAL;
+
+	env->cpu = calloc(nr_cpus, sizeof(env->cpu[0]));
+	if (env->cpu == NULL)
+		return -ENOMEM;
+
+	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);
+	}
+
+	env->nr_cpus_avail = nr_cpus;
+	return 0;
+}
diff --git a/tools/perf/util/env.h b/tools/perf/util/env.h
index d0d1a96..0132b95 100644
--- a/tools/perf/util/env.h
+++ b/tools/perf/util/env.h
@@ -39,4 +39,6 @@ void perf_env__exit(struct perf_env *env);
 
 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);
+
 #endif /* __PERF_ENV_H */
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index f307b17..46ec6c5 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -415,8 +415,6 @@ struct cpu_topo {
 	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)
@@ -479,9 +477,6 @@ 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);
@@ -509,7 +504,7 @@ static struct cpu_topo *build_cpu_topology(void)
 	struct cpu_topo *tp;
 	void *addr;
 	u32 nr, i;
-	size_t sz, sz_id;
+	size_t sz;
 	long ncpus;
 	int ret = -1;
 
@@ -520,9 +515,8 @@ 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 + 2 * sz_id);
+	addr = calloc(1, sizeof(*tp) + 2 * sz);
 	if (!addr)
 		return NULL;
 
@@ -532,10 +526,6 @@ static struct cpu_topo *build_cpu_topology(void)
 	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);
@@ -554,7 +544,7 @@ static int write_cpu_topology(int fd, struct perf_header *h __maybe_unused,
 {
 	struct cpu_topo *tp;
 	u32 i;
-	int ret;
+	int ret, j;
 
 	tp = build_cpu_topology();
 	if (!tp)
@@ -579,11 +569,17 @@ static int write_cpu_topology(int fd, struct perf_header *h __maybe_unused,
 			break;
 	}
 
-	for (i = 0; i < tp->cpu_nr; i++) {
-		ret = do_write(fd, &tp->core_id[i], sizeof(int));
+	ret = perf_env__read_cpu_topology_map(&perf_env);
+	if (ret < 0)
+		goto done;
+
+	for (j = 0; j < perf_env.nr_cpus_avail; j++) {
+		ret = do_write(fd, &perf_env.cpu[j].core_id,
+			       sizeof(perf_env.cpu[j].core_id));
 		if (ret < 0)
 			return ret;
-		ret = do_write(fd, &tp->phy_pkg_id[i], sizeof(int));
+		ret = do_write(fd, &perf_env.cpu[j].socket_id,
+			       sizeof(perf_env.cpu[j].socket_id));
 		if (ret < 0)
 			return ret;
 	}

[PATCH 05/13] perf sort: Set flag stating if the "socket" key is being used

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Because it will require that we read the current machine CPU topology
map when the tool is not processing a perf.data file, from where it
would obtain that map.

Other tools may want to do some other initialization in such case.

We need to have a better way to figure out if a given sort key is being
used, perhaps a bitmap, but that is left for another patch.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/n/tip-zv0php3505x6lq8zpljz1bd9@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/sort.c | 3 +++
 tools/perf/util/sort.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 5e4ed1779f6f..6b9556d298c9 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -21,6 +21,7 @@ int		sort__need_collapse = 0;
 int		sort__has_parent = 0;
 int		sort__has_sym = 0;
 int		sort__has_dso = 0;
+int		sort__has_socket = 0;
 enum sort_mode	sort__mode = SORT_MODE__NORMAL;
 
 
@@ -1572,6 +1573,8 @@ int sort_dimension__add(const char *tok)
 
 		} else if (sd->entry == &sort_dso) {
 			sort__has_dso = 1;
+		} else if (sd->entry == &sort_socket) {
+			sort__has_socket = 1;
 		}
 
 		return __sort_dimension__add(sd);
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 654ac8a2c565..c06b75746613 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -34,6 +34,7 @@ extern int have_ignore_callees;
 extern int sort__need_collapse;
 extern int sort__has_parent;
 extern int sort__has_sym;
+extern int sort__has_socket;
 extern enum sort_mode sort__mode;
 extern struct sort_entry sort_comm;
 extern struct sort_entry sort_dso;
-- 
2.1.0

[PATCH 06/13] perf top: Cache the cpu topology info when "-s socket" is used

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

We need to cache that info to use in perf_event__preprocess_sample(), so
that we don't read sysfs files for each sample.

The next patches will add machine->env pointer that will point to either
the perf_env read from a perf.data file header or from the current
system.

Tools needing some specific info should call the perf_env methods that
cache the info, in this case perf_env__read_cpu_topology_map().

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/n/tip-54vvsb71b56v9d2e4qxwp49t@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-top.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index e5ca6848f01d..bdaf44f24d5d 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -963,6 +963,13 @@ static int __cmd_top(struct perf_top *top)
 
 	machine__synthesize_threads(&top->session->machines.host, &opts->target,
 				    top->evlist->threads, false, opts->proc_map_timeout);
+
+	if (sort__has_socket) {
+		ret = perf_env__read_cpu_topology_map(&perf_env);
+		if (ret < 0)
+			goto out_err_cpu_topo;
+	}
+
 	ret = perf_top__start_counters(top);
 	if (ret)
 		goto out_delete;
@@ -1020,6 +1027,14 @@ out_delete:
 	top->session = NULL;
 
 	return ret;
+
+out_err_cpu_topo: {
+	char errbuf[BUFSIZ];
+	const char *err = strerror_r(-ret, errbuf, sizeof(errbuf));
+
+	ui__error("Could not read the CPU topology map: %s\n", err);
+	goto out_delete;
+}
 }
 
 static int
-- 
2.1.0

[PATCH 07/13] perf hists browser: Fixup the "cpu" column width calculation

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Since we were not setting it to at least 3 chars ('CPU'), it was being
reset to zero when recalculating the columns width when refreshing the
screen, in 'perf top'. Fix it.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/n/tip-iqcdnkkqm6sew06x01fbijmy@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/hist.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 67b48616ab31..b3567a25f0c4 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -146,6 +146,7 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
 		hists__set_unres_dso_col_len(hists, HISTC_MEM_DADDR_DSO);
 	}
 
+	hists__new_col_len(hists, HISTC_CPU, 3);
 	hists__new_col_len(hists, HISTC_SOCKET, 6);
 	hists__new_col_len(hists, HISTC_MEM_LOCKED, 6);
 	hists__new_col_len(hists, HISTC_MEM_TLB, 22);
-- 
2.1.0

Re: [PATCH 07/13] perf hists browser: Fixup the "cpu" column width calculation

[Wangnan (F)]

On 2015/9/10 3:50, Arnaldo Carvalho de Melo wrote:
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> Since we were not setting it to at least 3 chars ('CPU'), it was being
> reset to zero when recalculating the columns width when refreshing the
> screen, in 'perf top'. Fix it.

Tested-by: Wang Nan <wangnan0@huawei.com>

> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Kan Liang <kan.liang@intel.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Stephane Eranian <eranian@google.com>
> Cc: Wang Nan <wangnan0@huawei.com>
> Link: http://lkml.kernel.org/n/tip-iqcdnkkqm6sew06x01fbijmy@git.kernel.org
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
>   tools/perf/util/hist.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index 67b48616ab31..b3567a25f0c4 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -146,6 +146,7 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
>   		hists__set_unres_dso_col_len(hists, HISTC_MEM_DADDR_DSO);
>   	}
>   
> +	hists__new_col_len(hists, HISTC_CPU, 3);
>   	hists__new_col_len(hists, HISTC_SOCKET, 6);
>   	hists__new_col_len(hists, HISTC_MEM_LOCKED, 6);
>   	hists__new_col_len(hists, HISTC_MEM_TLB, 22);

[PATCH 08/13] perf machine: Add pointer to sample's environment

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

The 'struct machine' represents the machine where the samples were/are
being collected, and we also have a 'struct perf_env' with extra details
about such machine, that we were collecting at 'perf.data' creation time
but we also needed when no perf.data file is being used, such as in
'perf top'.

So, get those structs closer together, as they provide a bigger picture
of the sample's environment.

In 'perf session', when the file argument is NULL, we can assume that
the tool is sampling the running machine, so point machine->env to
the global put in place in previous patches, while set it to the
perf_header.env one when reading from a file.

This paves the way for machine->env to be used in
perf_event__preprocess_sample to populate addr_location.socket.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/n/tip-2ajotl0khscutm68exictoy9@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/header.c  | 1 +
 tools/perf/util/machine.c | 1 +
 tools/perf/util/machine.h | 1 +
 tools/perf/util/session.c | 2 ++
 4 files changed, 5 insertions(+)

diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index d4c8aa2f4db7..085bbc35c186 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2559,6 +2559,7 @@ int perf_session__read_header(struct perf_session *session)
 		return -ENOMEM;
 
 	session->evlist->env = &header->env;
+	session->machines.host.env = &header->env;
 	if (perf_data_file__is_pipe(file))
 		return perf_header__read_pipe(session);
 
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 6309f7ceb08f..fd1efeafb343 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -35,6 +35,7 @@ int machine__init(struct machine *machine, const char *root_dir, pid_t pid)
 	machine->last_match = NULL;
 
 	machine->vdso_info = NULL;
+	machine->env = NULL;
 
 	machine->pid = pid;
 
diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
index ea5cb4a621db..9dfc4281f940 100644
--- a/tools/perf/util/machine.h
+++ b/tools/perf/util/machine.h
@@ -34,6 +34,7 @@ struct machine {
 	struct list_head  dead_threads;
 	struct thread	  *last_match;
 	struct vdso_info  *vdso_info;
+	struct perf_env   *env;
 	struct dsos	  dsos;
 	struct map_groups kmaps;
 	struct map	  *vmlinux_maps[MAP__NR_TYPES];
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 728cb115fbb8..d1a43a322f96 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -138,6 +138,8 @@ struct perf_session *perf_session__new(struct perf_data_file *file,
 			perf_session__set_id_hdr_size(session);
 			perf_session__set_comm_exec(session);
 		}
+	} else  {
+		session->machines.host.env = &perf_env;
 	}
 
 	if (!file || perf_data_file__is_write(file)) {
-- 
2.1.0

[PATCH 09/13] perf event: Use machine->env to find the cpu -> socket mapping

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

Instead of reading
/sysfs/devices/system/cpu/cpu%d/topology/physical_package_id for
each sample.

While at it, check that the sample has PERF_SAMPLE_CPU, i.e. that
sample.cpu >= 0, to avoid an out of bounds access.

Reported-by: Wang Nan <wangnan0@huawei.com>
Based-on-a-patch-by: Jiri Olsa <jolsa@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/n/tip-lkkb5iht6gbbngdpfv0nl7vh@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/event.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 0bf8c9889fc0..497157affc9c 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -1021,7 +1021,14 @@ int perf_event__preprocess_sample(const union perf_event *event,
 
 	al->sym = NULL;
 	al->cpu = sample->cpu;
-	al->socket = cpu_map__get_socket_id(al->cpu);
+	al->socket = -1;
+
+	if (al->cpu >= 0) {
+		struct perf_env *env = machine->env;
+
+		if (env && env->cpu)
+			al->socket = env->cpu[al->cpu].socket_id;
+	}
 
 	if (al->map) {
 		struct dso *dso = al->map->dso;
-- 
2.1.0

[PATCH 10/13] perf report: Do not blindly use env->cpu[al.cpu].socket_id

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

As al.cpu may be -1, i.e. no PERF_SAMPLE_CPU, and env->cpu may be NULL.

Rely instead on the work now done in perf_event__preprocess_sample(),
that does all those checks.

Reported-by: Wang Nan <wangnan0@huawei.com>
Based-on-a-patch-by: Jiri Olsa <jolsa@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/n/tip-2lw80g5ehsrec7tozhmnjgxw@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-report.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 4b432453922f..9b5083630a56 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -150,7 +150,6 @@ static int process_sample_event(struct perf_tool *tool,
 		.add_entry_cb 		= hist_iter__report_callback,
 	};
 	int ret = 0;
-	struct perf_env *env = evsel->evlist->env;
 
 	if (perf_event__preprocess_sample(event, machine, &al, sample) < 0) {
 		pr_debug("problem processing %d event, skipping it.\n",
@@ -158,9 +157,6 @@ static int process_sample_event(struct perf_tool *tool,
 		return -1;
 	}
 
-	/* read socket id from perf.data for perf report */
-	al.socket = env->cpu[al.cpu].socket_id;
-
 	if (rep->hide_unresolved && al.sym == NULL)
 		goto out_put;
 
-- 
2.1.0

Re: [PATCH 10/13] perf report: Do not blindly use env->cpu[al.cpu].socket_id

[Wangnan (F)]

On 2015/9/10 3:50, Arnaldo Carvalho de Melo wrote:
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> As al.cpu may be -1, i.e. no PERF_SAMPLE_CPU, and env->cpu may be NULL.
>
> Rely instead on the work now done in perf_event__preprocess_sample(),
> that does all those checks.
>
> Reported-by: Wang Nan <wangnan0@huawei.com>
> Based-on-a-patch-by: Jiri Olsa <jolsa@kernel.org>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Kan Liang <kan.liang@intel.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Stephane Eranian <eranian@google.com>
> Cc: Wang Nan <wangnan0@huawei.com>
> Link: http://lkml.kernel.org/n/tip-2lw80g5ehsrec7tozhmnjgxw@git.kernel.org
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
>   tools/perf/builtin-report.c | 4 ----
>   1 file changed, 4 deletions(-)
>
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 4b432453922f..9b5083630a56 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -150,7 +150,6 @@ static int process_sample_event(struct perf_tool *tool,
>   		.add_entry_cb 		= hist_iter__report_callback,
>   	};
>   	int ret = 0;
> -	struct perf_env *env = evsel->evlist->env;
>   
>   	if (perf_event__preprocess_sample(event, machine, &al, sample) < 0) {
>   		pr_debug("problem processing %d event, skipping it.\n",
> @@ -158,9 +157,6 @@ static int process_sample_event(struct perf_tool *tool,
>   		return -1;
>   	}
>   
> -	/* read socket id from perf.data for perf report */
> -	al.socket = env->cpu[al.cpu].socket_id;
> -
>   	if (rep->hide_unresolved && al.sym == NULL)
>   		goto out_put;
>   

I tested this patch on an 8 cores SOC. No segfault found now.

Normal case:

  # ./perf record -a ls
  # ...
  # ./perf_arm64 report -v  --stdio -s socket,cpu
  build id event received for [kernel.kallsyms]: ...
  # To display the perf.data header info, please use 
--header/--header-only options.
  #
  symsrc__init: cannot get elf header.
  Looking at the vmlinux_path (7 entries long)
  Failed to open /proc/kcore. Note /proc/kcore requires CAP_SYS_RAWIO 
capability to access.
  Using /proc/kallsyms for symbols
  Failed to open /sbin/adbd, continuing without symbols
  #
  # Total Lost Samples: 0
  #
  # Samples: 291  of event 'cycles'
  # Event count (approx.): 40968659
  #
  # Overhead  Socket  CPU
  # ........  ......  ...
  #
      58.01%     000  004
      35.61%     000  007
       3.65%     000  005
       1.23%     001  000
       1.18%     001  001
       0.19%     000  006
       0.08%     001  002
       0.07%     001  003

If one of CPUs is offlined before 'perf record':

  # ./perf record -a ls
  # ...
  # ./perf_arm64 report -v  --stdio -s socket,cpu
  build id event received for [kernel.kallsyms]: 
d287ff3393fb1a01d3a785c3a1dac6e63d973bce
  # To display the perf.data header info, please use 
--header/--header-only options.
  #
  symsrc__init: cannot get elf header.
  Looking at the vmlinux_path (7 entries long)
  Failed to open /proc/kcore. Note /proc/kcore requires CAP_SYS_RAWIO 
capability to access.
  Using /proc/kallsyms for symbols
  Failed to open /sbin/adbd, continuing without symbols
  #
  # Total Lost Samples: 0
  #
  # Samples: 304  of event 'cycles'
  # Event count (approx.): 42027298
  #
  # Overhead  Socket  CPU
  # ........  ......  ...
  #
      60.25%    -001  004
      32.69%    -001  007
       5.00%    -001  005
       1.34%    -001  003
       0.37%    -001  000
       0.20%    -001  006
       0.08%    -001  002
       0.07%    -001  001

And if CPU is offlined during perf record (between build_cpu_topology() 
and perf_env__read_cpu_topology_map()):

  # ./perf report -v  --stdio -s socket,cpu
  build id event received for [kernel.kallsyms]: ...
  core_id number is too big.You may need to upgrade the perf tool. <--- 
*please see this line*
  # To display the perf.data header info, please use 
--header/--header-only options.
  #
  symsrc__init: cannot get elf header.
  Looking at the vmlinux_path (7 entries long)
  Failed to open /proc/kcore. Note /proc/kcore requires CAP_SYS_RAWIO 
capability to access.
  Using /proc/kallsyms for symbols
  #
  # Total Lost Samples: 0
  #
  # Samples: 278  of event 'cycles'
  # Event count (approx.): 37729429
  #
  # Overhead  Socket  CPU
  # ........  ......  ...
  #
      59.06%    -001  004
      36.93%    -001  007
       1.33%    -001  000
       1.18%    -001  005
       0.82%    -001  006
       0.50%    -001  001
       0.08%    -001  002
       0.08%    -001  003

[PATCH 11/13] Revert "perf evsel: Add a backpointer to the evlist a evsel is in"

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

This reverts commit d49e4695077278ee3016cd242967de23072ec331.

We don't need it, using machine->env seems to be enough.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/evlist.c | 2 --
 tools/perf/util/evsel.c  | 2 --
 tools/perf/util/evsel.h  | 4 ----
 3 files changed, 8 deletions(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index d51a5200c8af..09b4ec221dda 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -98,7 +98,6 @@ static void perf_evlist__purge(struct perf_evlist *evlist)
 
 	evlist__for_each_safe(evlist, n, pos) {
 		list_del_init(&pos->node);
-		pos->evlist = NULL;
 		perf_evsel__delete(pos);
 	}
 
@@ -126,7 +125,6 @@ void perf_evlist__delete(struct perf_evlist *evlist)
 
 void perf_evlist__add(struct perf_evlist *evlist, struct perf_evsel *entry)
 {
-	entry->evlist = evlist;
 	list_add_tail(&entry->node, &evlist->entries);
 	entry->idx = evlist->nr_entries;
 	entry->tracking = !entry->idx;
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 771ade4d5966..8f4d45002461 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -206,7 +206,6 @@ void perf_evsel__init(struct perf_evsel *evsel,
 	evsel->leader	   = evsel;
 	evsel->unit	   = "";
 	evsel->scale	   = 1.0;
-	evsel->evlist	   = NULL;
 	INIT_LIST_HEAD(&evsel->node);
 	INIT_LIST_HEAD(&evsel->config_terms);
 	perf_evsel__object.init(evsel);
@@ -1027,7 +1026,6 @@ void perf_evsel__close_fd(struct perf_evsel *evsel, int ncpus, int nthreads)
 void perf_evsel__exit(struct perf_evsel *evsel)
 {
 	assert(list_empty(&evsel->node));
-	assert(evsel->evlist == NULL);
 	perf_evsel__free_fd(evsel);
 	perf_evsel__free_id(evsel);
 	perf_evsel__free_config_terms(evsel);
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 298e6bbca200..93ac6b128149 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -60,9 +60,6 @@ struct perf_evsel_config_term {
 
 /** struct perf_evsel - event selector
  *
- * @evlist - evlist this evsel is in, if it is in one.
- * @node - To insert it into evlist->entries or in other list_heads, say in
- *         the event parsing routines.
  * @name - Can be set to retain the original event name passed by the user,
  *         so that when showing results in tools such as 'perf stat', we
  *         show the name used, not some alias.
@@ -76,7 +73,6 @@ struct perf_evsel_config_term {
  */
 struct perf_evsel {
 	struct list_head	node;
-	struct perf_evlist	*evlist;
 	struct perf_event_attr	attr;
 	char			*filter;
 	struct xyarray		*fd;
-- 
2.1.0

RE: [PATCH 11/13] Revert "perf evsel: Add a backpointer to the evlist a evsel is in"

[Liang, Kan]

> 
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> This reverts commit d49e4695077278ee3016cd242967de23072ec331.
> 
> We don't need it, using machine->env seems to be enough.

The patchset to dump freq per sample need commit d49e469507.
It also needs commit 2c07144dfc which is revert by PATCH 13.
https://lkml.org/lkml/2015/9/8/758

Otherwise, we have to pass evlist and machine through dump_sample.

> 
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Kan Liang <kan.liang@intel.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Stephane Eranian <eranian@google.com>
> Cc: Wang Nan <wangnan0@huawei.com>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
>  tools/perf/util/evlist.c | 2 --
>  tools/perf/util/evsel.c  | 2 --
>  tools/perf/util/evsel.h  | 4 ----
>  3 files changed, 8 deletions(-)
> 
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index
> d51a5200c8af..09b4ec221dda 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -98,7 +98,6 @@ static void perf_evlist__purge(struct perf_evlist
> *evlist)
> 
>  	evlist__for_each_safe(evlist, n, pos) {
>  		list_del_init(&pos->node);
> -		pos->evlist = NULL;
>  		perf_evsel__delete(pos);
>  	}
> 
> @@ -126,7 +125,6 @@ void perf_evlist__delete(struct perf_evlist *evlist)
> 
>  void perf_evlist__add(struct perf_evlist *evlist, struct perf_evsel *entry)
> {
> -	entry->evlist = evlist;
>  	list_add_tail(&entry->node, &evlist->entries);
>  	entry->idx = evlist->nr_entries;
>  	entry->tracking = !entry->idx;
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c index
> 771ade4d5966..8f4d45002461 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -206,7 +206,6 @@ void perf_evsel__init(struct perf_evsel *evsel,
>  	evsel->leader	   = evsel;
>  	evsel->unit	   = "";
>  	evsel->scale	   = 1.0;
> -	evsel->evlist	   = NULL;
>  	INIT_LIST_HEAD(&evsel->node);
>  	INIT_LIST_HEAD(&evsel->config_terms);
>  	perf_evsel__object.init(evsel);
> @@ -1027,7 +1026,6 @@ void perf_evsel__close_fd(struct perf_evsel
> *evsel, int ncpus, int nthreads)  void perf_evsel__exit(struct perf_evsel
> *evsel)  {
>  	assert(list_empty(&evsel->node));
> -	assert(evsel->evlist == NULL);
>  	perf_evsel__free_fd(evsel);
>  	perf_evsel__free_id(evsel);
>  	perf_evsel__free_config_terms(evsel);
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h index
> 298e6bbca200..93ac6b128149 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -60,9 +60,6 @@ struct perf_evsel_config_term {
> 
>  /** struct perf_evsel - event selector
>   *
> - * @evlist - evlist this evsel is in, if it is in one.
> - * @node - To insert it into evlist->entries or in other list_heads, say in
> - *         the event parsing routines.
>   * @name - Can be set to retain the original event name passed by the
> user,
>   *         so that when showing results in tools such as 'perf stat', we
>   *         show the name used, not some alias.
> @@ -76,7 +73,6 @@ struct perf_evsel_config_term {
>   */
>  struct perf_evsel {
>  	struct list_head	node;
> -	struct perf_evlist	*evlist;
>  	struct perf_event_attr	attr;
>  	char			*filter;
>  	struct xyarray		*fd;
> --
> 2.1.0

[PATCH 12/13] perf evsel: Remove forward declaration of 'struct perf_evlist'

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

We have no use for it in evsel.h.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/n/tip-um03yjrgyi3bj1hzqiqs4dsu@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/evsel.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 93ac6b128149..306cfef044e3 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -125,7 +125,6 @@ union u64_swap {
 struct cpu_map;
 struct target;
 struct thread_map;
-struct perf_evlist;
 struct record_opts;
 
 static inline struct cpu_map *perf_evsel__cpus(struct perf_evsel *evsel)
-- 
2.1.0

[PATCH 13/13] Revert "perf evlist: Add backpointer for perf_env to evlist"

[Arnaldo Carvalho de Melo]

From: Arnaldo Carvalho de Melo <acme@redhat.com>

This reverts commit 2c07144dfce366e21465cc7b0ada9f0b6dc7b7ed.

We don't need it, machine->env provides what is needed.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/evlist.h | 1 -
 tools/perf/util/header.c | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h
index b39a6198f4ac..436e358300b1 100644
--- a/tools/perf/util/evlist.h
+++ b/tools/perf/util/evlist.h
@@ -56,7 +56,6 @@ struct perf_evlist {
 	struct cpu_map	  *cpus;
 	struct perf_evsel *selected;
 	struct events_stats stats;
-	struct perf_env	*env;
 };
 
 struct perf_evsel_str_handler {
diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c
index 085bbc35c186..64adbff36e04 100644
--- a/tools/perf/util/header.c
+++ b/tools/perf/util/header.c
@@ -2558,8 +2558,8 @@ int perf_session__read_header(struct perf_session *session)
 	if (session->evlist == NULL)
 		return -ENOMEM;
 
-	session->evlist->env = &header->env;
 	session->machines.host.env = &header->env;
+
 	if (perf_data_file__is_pipe(file))
 		return perf_header__read_pipe(session);
 
-- 
2.1.0

Re: [RFC 00/13] perf_env/CPU socket reorg/fixes

[Jiri Olsa]

On Wed, Sep 09, 2015 at 04:50:12PM -0300, Arnaldo Carvalho de Melo wrote:
> Hi,
> 
> 	Please take a look at these changes to fix the problems reported by
> Wang Nan wrt accesses to the cpu_topology_map information.
> 
> 	The fixes are present on these following two csets:
> 
>   perf event: Use machine->env to find the cpu -> socket mapping
>   perf report: Do not blindly use env->cpu[al.cpu].socket_id
> 
> 	The rest are fixes made while working on this, infrastructure to enable
> the fixes, reverts for things that ended up not being necessary and some
> cleanups.
> 
> 	It is available at:
> 
>  git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git perf/env
> 
> 	Please let me know if I can have your Acked-by, Tested-by or
> Reviewed-by.

perf_env holds the data for perf.data session.. is the plan to keep
it like this, or it's to be used in some other way? Moving it out of
session/header real suggest that.. just asking ;-)

jirka

Re: [RFC 00/13] perf_env/CPU socket reorg/fixes

[Arnaldo Carvalho de Melo]

Em Thu, Sep 10, 2015 at 11:19:06AM +0200, Jiri Olsa escreveu:
> On Wed, Sep 09, 2015 at 04:50:12PM -0300, Arnaldo Carvalho de Melo wrote:
> > 	Please take a look at these changes to fix the problems reported by
> > Wang Nan wrt accesses to the cpu_topology_map information.
> > 
> > 	The fixes are present on these following two csets:
> > 
> >   perf event: Use machine->env to find the cpu -> socket mapping
> >   perf report: Do not blindly use env->cpu[al.cpu].socket_id
> > 
> > 	The rest are fixes made while working on this, infrastructure to enable
> > the fixes, reverts for things that ended up not being necessary and some
> > cleanups.
> > 
> > 	It is available at:
> > 
> >  git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git perf/env
> > 
> > 	Please let me know if I can have your Acked-by, Tested-by or
> > Reviewed-by.
> 
> perf_env holds the data for perf.data session.. is the plan to keep

No, perf_env holds the data for the environment where samples were
taken.

It is needed to generate a perf.data file, and also now is needed to map
cpu -> socket_id, be it in a live session, where no perf.data file, nor
perf_session stuff is used (perf trace, I keep meaning to do the 'perf
top' conversion away from perf_session, others in the future).

I.e. perf_event__preprocess_sample() doesn't have to know if the samples
were freshly taken or if they come from another machine, possibly of a
different architecture.

> it like this, or it's to be used in some other way? Moving it out of
> session/header real suggest that.. just asking ;-)

They are already used in another way, i.e. we need to have that mapping
of cpu -> socket_id, to stop reading /sys/ to figure that out. I.e.
perf_env is, among other sampling/tracing environment, a cache of that
info, taken at perf.data generation time or at tool start time, if that
info is needed (i.e. if we use "socket" in -s).

- Arnaldo

Re: [RFC 00/13] perf_env/CPU socket reorg/fixes

[Wangnan (F)]

Hi Arnaldo,

I have tested patch 1 to 10. They looks good to me except patch 4/13. Please
see my email in that thread.

However, during the testing I found a limitation related to cpu
online/offline and 'perf top' that, if I offline most of cores before
'perf top', then online them during 'perf top' running, 'perf top'
dooesn't report new CPUs. It still reports the CPUs which are online
when 'perf top' starts consume 100% cycles.  So if CPUs are online and
offlined dynamically and there are many CPUs, user of 'perf top' may get
confusion result if he or she doesn't noticed that 'perf top' doesn't
listed all cores they have.

Here is how I did this:

  # for i in `seq 2 7` ; do echo 0 > 
/sys/devices/system/cpu/cpu$i/online ; done
  # perf top -s cpu,socket

  The result is something like:

  Samples: 28K of event 'cycles', Event count (approx.): 23640606383
  Overhead  Socket CPU
    67.14%     000  000
    32.86%     000  001

Then online them:

  # for i in `seq 2 7` ; do echo 1 > 
/sys/devices/system/cpu/cpu$i/online ; done

After a while, 'perf top' still reports two CPUs.

  Samples: 400K of event 'cycles', Event count (approx.): 38728257939
  Overhead  Socket CPU
    51.02%     000  001
    48.98%     000  000

And another 'perf top' report correct result:

Samples: 28K of event 'cycles', Event count (approx.): 24741565854
Overhead  Socket CPU
   27.26%     000  005
   21.07%     000  002
   13.07%     000  001
   12.69%     000  000
    8.07%     000  006
    6.75%     000  007
    5.64%     000  004
    5.45%     000  003

However, It is relatively a rare case. I don't think we have to fix it 
in this
patchset.

Thank you.

On 2015/9/10 3:50, Arnaldo Carvalho de Melo wrote:
> Hi,
>
> 	Please take a look at these changes to fix the problems reported by
> Wang Nan wrt accesses to the cpu_topology_map information.
>
> 	The fixes are present on these following two csets:
>
>    perf event: Use machine->env to find the cpu -> socket mapping
>    perf report: Do not blindly use env->cpu[al.cpu].socket_id
>
> 	The rest are fixes made while working on this, infrastructure to enable
> the fixes, reverts for things that ended up not being necessary and some
> cleanups.
>
> 	It is available at:
>
>   git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git perf/env
>
> 	Please let me know if I can have your Acked-by, Tested-by or
> Reviewed-by.
>
> - Arnaldo
>
> Arnaldo Carvalho de Melo (13):
>    perf env: Move perf_env out of header.h and session.c into separate object
>    perf env: Rename some leftovers from rename to perf_env
>    perf env: Adopt perf_header__set_cmdline
>    perf env: Introduce read_cpu_topology_map() method
>    perf sort: Set flag stating if the "socket" key is being used
>    perf top: Cache the cpu topology info when "-s socket" is used
>    perf hists browser: Fixup the "cpu" column width calculation
>    perf machine: Add pointer to sample's environment
>    perf event: Use machine->env to find the cpu -> socket mapping
>    perf report: Do not blindly use env->cpu[al.cpu].socket_id
>    Revert "perf evsel: Add a backpointer to the evlist a evsel is in"
>    perf evsel: Remove forward declaration of 'struct perf_evlist'
>    Revert "perf evlist: Add backpointer for perf_env to evlist"

Re: [RFC 00/13] perf_env/CPU socket reorg/fixes

[Arnaldo Carvalho de Melo]

Em Fri, Sep 11, 2015 at 08:20:54PM +0800, Wangnan (F) escreveu:
> I have tested patch 1 to 10. They looks good to me except patch 4/13. Please

Ok, I'll take that as a Tested-by: you for 1-10 with 4/13 having the
checks added, ok?

> see my email in that thread.

I add those checks.
 
> However, during the testing I found a limitation related to cpu
> online/offline and 'perf top' that, if I offline most of cores before
> 'perf top', then online them during 'perf top' running, 'perf top'
> dooesn't report new CPUs. It still reports the CPUs which are online

<SNIP>
 
> However, It is relatively a rare case. I don't think we have to fix it
> in this patchset.

Yup, unrelated to this patchset. But we need to seamlessly support that
situation, even telling the user that a CPU went offline/online.

- Arnaldo

Re: [RFC 00/13] perf_env/CPU socket reorg/fixes

[Arnaldo Carvalho de Melo]

Em Fri, Sep 11, 2015 at 10:03:39AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Fri, Sep 11, 2015 at 08:20:54PM +0800, Wangnan (F) escreveu:
> > I have tested patch 1 to 10. They looks good to me except patch 4/13. Please
> 
> Ok, I'll take that as a Tested-by: you for 1-10 with 4/13 having the
> checks added, ok?
 
> > see my email in that thread.
 
> I add those checks.

Ok, below is the diff for adding the checks. The get_{core,socket}_id
functions should be moved to tools/lib/api/cpu.[ch], using the same
interface as cpu__get_max_freq(&value), using the int return value to
propagate the precise error, etc. Will do it in a follow up patch.

- Arnaldo

diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
index 6af4f7c36820..2e4cad84197b 100644
--- a/tools/perf/util/env.c
+++ b/tools/perf/util/env.c
@@ -60,7 +60,7 @@ out_enomem:
 
 int perf_env__read_cpu_topology_map(struct perf_env *env)
 {
-	int cpu, nr_cpus;
+	int cpu, nr_cpus, err;
 
 	if (env->cpu != NULL)
 		return 0;
@@ -77,10 +77,17 @@ int perf_env__read_cpu_topology_map(struct perf_env *env)
 		return -ENOMEM;
 
 	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);
+		err = env->cpu[cpu].core_id = cpu_map__get_core_id(cpu);
+		if (err < 0)
+			goto out_free;
+		err = env->cpu[cpu].socket_id = cpu_map__get_socket_id(cpu);
+		if (err < 0)
+			goto out_free;
 	}
 
-	env->nr_cpus_avail = nr_cpus;
 	return 0;
+
+out_free:
+	zfree(&env->cpu);
+	return err;
 }

Re: [RFC 00/13] perf_env/CPU socket reorg/fixes

[Arnaldo Carvalho de Melo]

Em Fri, Sep 11, 2015 at 10:29:37AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Fri, Sep 11, 2015 at 10:03:39AM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Fri, Sep 11, 2015 at 08:20:54PM +0800, Wangnan (F) escreveu:
> > > I have tested patch 1 to 10. They looks good to me except patch 4/13. Please
> > 
> > Ok, I'll take that as a Tested-by: you for 1-10 with 4/13 having the
> > checks added, ok?
>  
> > > see my email in that thread.
>  
> > I add those checks.
> 
> Ok, below is the diff for adding the checks. The get_{core,socket}_id
> functions should be moved to tools/lib/api/cpu.[ch], using the same
> interface as cpu__get_max_freq(&value), using the int return value to
> propagate the precise error, etc. Will do it in a follow up patch.

Humm, but then, what happens if a CPU is offline? I'm checking it now...

- Arnaldo
 
> - Arnaldo
> 
> diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
> index 6af4f7c36820..2e4cad84197b 100644
> --- a/tools/perf/util/env.c
> +++ b/tools/perf/util/env.c
> @@ -60,7 +60,7 @@ out_enomem:
>  
>  int perf_env__read_cpu_topology_map(struct perf_env *env)
>  {
> -	int cpu, nr_cpus;
> +	int cpu, nr_cpus, err;
>  
>  	if (env->cpu != NULL)
>  		return 0;
> @@ -77,10 +77,17 @@ int perf_env__read_cpu_topology_map(struct perf_env *env)
>  		return -ENOMEM;
>  
>  	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);
> +		err = env->cpu[cpu].core_id = cpu_map__get_core_id(cpu);
> +		if (err < 0)
> +			goto out_free;
> +		err = env->cpu[cpu].socket_id = cpu_map__get_socket_id(cpu);
> +		if (err < 0)
> +			goto out_free;
>  	}
>  
> -	env->nr_cpus_avail = nr_cpus;
>  	return 0;
> +
> +out_free:
> +	zfree(&env->cpu);
> +	return err;
>  }

Re: [RFC 00/13] perf_env/CPU socket reorg/fixes

[Arnaldo Carvalho de Melo]

Em Fri, Sep 11, 2015 at 10:30:52AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Fri, Sep 11, 2015 at 10:29:37AM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Fri, Sep 11, 2015 at 10:03:39AM -0300, Arnaldo Carvalho de Melo escreveu:
> > > Em Fri, Sep 11, 2015 at 08:20:54PM +0800, Wangnan (F) escreveu:
> > > > I have tested patch 1 to 10. They looks good to me except patch 4/13. Please
> > > 
> > > Ok, I'll take that as a Tested-by: you for 1-10 with 4/13 having the
> > > checks added, ok?
> >  
> > > > see my email in that thread.
> >  
> > > I add those checks.
> > 
> > Ok, below is the diff for adding the checks. The get_{core,socket}_id
> > functions should be moved to tools/lib/api/cpu.[ch], using the same
> > interface as cpu__get_max_freq(&value), using the int return value to
> > propagate the precise error, etc. Will do it in a follow up patch.
> 
> Humm, but then, what happens if a CPU is offline? I'm checking it now...

  # cat /sys/devices/system/cpu/cpu3/topology/core_id 
  3
  # cat
  /sys/devices/system/cpu/cpu3/topology/physical_package_id 
  0
  # echo 0 > /sys/devices/system/cpu/cpu3/online
  # cat
  /sys/devices/system/cpu/cpu3/topology/physical_package_id 
  cat: /sys/devices/system/cpu/cpu3/topology/physical_package_id: No such file or directory
  # cat /sys/devices/system/cpu/cpu3/topology/core_id 
  cat: /sys/devices/system/cpu/cpu3/topology/core_id: No such file or directory
  # 

So we shouldn't check the result, right? We could further validate it by
checking:

  # cat /sys/devices/system/cpu/cpu3/online 
  0
  #

But assuming that not being able to access it means it is offline looks
almost reasonable, if not strictly correct, so I'm removing the tests
and will revisit this when I move those functions to
tools/lib/api/cpu.[ch].

- Arnaldo

> > diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
> > index 6af4f7c36820..2e4cad84197b 100644
> > --- a/tools/perf/util/env.c
> > +++ b/tools/perf/util/env.c
> > @@ -60,7 +60,7 @@ out_enomem:
> >  
> >  int perf_env__read_cpu_topology_map(struct perf_env *env)
> >  {
> > -	int cpu, nr_cpus;
> > +	int cpu, nr_cpus, err;
> >  
> >  	if (env->cpu != NULL)
> >  		return 0;
> > @@ -77,10 +77,17 @@ int perf_env__read_cpu_topology_map(struct perf_env *env)
> >  		return -ENOMEM;
> >  
> >  	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);
> > +		err = env->cpu[cpu].core_id = cpu_map__get_core_id(cpu);
> > +		if (err < 0)
> > +			goto out_free;
> > +		err = env->cpu[cpu].socket_id = cpu_map__get_socket_id(cpu);
> > +		if (err < 0)
> > +			goto out_free;
> >  	}
> >  
> > -	env->nr_cpus_avail = nr_cpus;
> >  	return 0;
> > +
> > +out_free:
> > +	zfree(&env->cpu);
> > +	return err;
> >  }

Re: [RFC 00/13] perf_env/CPU socket reorg/fixes

[Wangnan (F)]

On 2015/9/11 21:36, Arnaldo Carvalho de Melo wrote:
> Em Fri, Sep 11, 2015 at 10:30:52AM -0300, Arnaldo Carvalho de Melo escreveu:
>> Em Fri, Sep 11, 2015 at 10:29:37AM -0300, Arnaldo Carvalho de Melo escreveu:
>>> Em Fri, Sep 11, 2015 at 10:03:39AM -0300, Arnaldo Carvalho de Melo escreveu:
>>>> Em Fri, Sep 11, 2015 at 08:20:54PM +0800, Wangnan (F) escreveu:
>>>>> I have tested patch 1 to 10. They looks good to me except patch 4/13. Please
>>>> Ok, I'll take that as a Tested-by: you for 1-10 with 4/13 having the
>>>> checks added, ok?
>>>   
>>>>> see my email in that thread.
>>>   
>>>> I add those checks.
>>> Ok, below is the diff for adding the checks. The get_{core,socket}_id
>>> functions should be moved to tools/lib/api/cpu.[ch], using the same
>>> interface as cpu__get_max_freq(&value), using the int return value to
>>> propagate the precise error, etc. Will do it in a follow up patch.
>> Humm, but then, what happens if a CPU is offline? I'm checking it now...
>    # cat /sys/devices/system/cpu/cpu3/topology/core_id
>    3
>    # cat
>    /sys/devices/system/cpu/cpu3/topology/physical_package_id
>    0
>    # echo 0 > /sys/devices/system/cpu/cpu3/online
>    # cat
>    /sys/devices/system/cpu/cpu3/topology/physical_package_id
>    cat: /sys/devices/system/cpu/cpu3/topology/physical_package_id: No such file or directory
>    # cat /sys/devices/system/cpu/cpu3/topology/core_id
>    cat: /sys/devices/system/cpu/cpu3/topology/core_id: No such file or directory
>    #
>
> So we shouldn't check the result, right? We could further validate it by
> checking:
>
>    # cat /sys/devices/system/cpu/cpu3/online
>    0
>    #
>
> But assuming that not being able to access it means it is offline looks
> almost reasonable, if not strictly correct, so I'm removing the tests
> and will revisit this when I move those functions to
> tools/lib/api/cpu.[ch].

I highly suspect that setting 'online' to 0 is not the only reason of
the removal of topology directory. Testing the existance of the two
files should be better.

Thank you.

> - Arnaldo
>
>>> diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
>>> index 6af4f7c36820..2e4cad84197b 100644
>>> --- a/tools/perf/util/env.c
>>> +++ b/tools/perf/util/env.c
>>> @@ -60,7 +60,7 @@ out_enomem:
>>>   
>>>   int perf_env__read_cpu_topology_map(struct perf_env *env)
>>>   {
>>> -	int cpu, nr_cpus;
>>> +	int cpu, nr_cpus, err;
>>>   
>>>   	if (env->cpu != NULL)
>>>   		return 0;
>>> @@ -77,10 +77,17 @@ int perf_env__read_cpu_topology_map(struct perf_env *env)
>>>   		return -ENOMEM;
>>>   
>>>   	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);
>>> +		err = env->cpu[cpu].core_id = cpu_map__get_core_id(cpu);
>>> +		if (err < 0)
>>> +			goto out_free;
>>> +		err = env->cpu[cpu].socket_id = cpu_map__get_socket_id(cpu);
>>> +		if (err < 0)
>>> +			goto out_free;
>>>   	}
>>>   
>>> -	env->nr_cpus_avail = nr_cpus;
>>>   	return 0;
>>> +
>>> +out_free:
>>> +	zfree(&env->cpu);
>>> +	return err;
>>>   }

Re: [RFC 00/13] perf_env/CPU socket reorg/fixes

[Wangnan (F)]

On 2015/9/11 21:03, Arnaldo Carvalho de Melo wrote:
> Em Fri, Sep 11, 2015 at 08:20:54PM +0800, Wangnan (F) escreveu:
>> I have tested patch 1 to 10. They looks good to me except patch 4/13. Please
> Ok, I'll take that as a Tested-by: you for 1-10 with 4/13 having the
> checks added, ok?

Sure.

Tested-by: Wang Nan <wangnan0@huawei.com> // for patch 1-10 except 4

>> see my email in that thread.
> I add those checks.
>   
>> However, during the testing I found a limitation related to cpu
>> online/offline and 'perf top' that, if I offline most of cores before
>> 'perf top', then online them during 'perf top' running, 'perf top'
>> dooesn't report new CPUs. It still reports the CPUs which are online
> <SNIP>
>   
>> However, It is relatively a rare case. I don't think we have to fix it
>> in this patchset.
> Yup, unrelated to this patchset. But we need to seamlessly support that
> situation, even telling the user that a CPU went offline/online.
>
> - Arnaldo