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

Groups > linux.kernel > #1221665 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [RFC 00/13] perf_env/CPU socket reorg/fixes Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-09 22:00 +0200
    [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-09 22:00 +0200
      RE: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method "Liang, Kan" <kan.liang@intel.com> - 2015-09-09 23:50 +0200
        Re: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-10 15:20 +0200
          RE: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method "Liang, Kan" <kan.liang@intel.com> - 2015-09-10 22:10 +0200
            RE: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method "Liang, Kan" <kan.liang@intel.com> - 2015-09-10 22:20 +0200
            Re: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-10 22:20 +0200
          Re: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method "Wangnan (F)" <wangnan0@huawei.com> - 2015-09-11 12:30 +0200
            Re: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-11 16:50 +0200
              Re: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-11 17:40 +0200
                Re: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method Namhyung Kim <namhyung@kernel.org> - 2015-09-11 18:20 +0200
                  Re: [PATCH 04/13] perf env: Introduce read_cpu_topology_map() method Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-11 18:40 +0200
                [tip:perf/urgent] perf header:   Fixup reading of HEADER_NRCPUS feature tip-bot for Arnaldo Carvalho de Melo <tipbot@zytor.com> - 2015-09-14 11:20 +0200
      [tip:perf/core] perf env: Introduce read_cpu_topology_map()   method tip-bot for Arnaldo Carvalho de Melo <tipbot@zytor.com> - 2015-09-15 09:10 +0200
    [PATCH 07/13] perf hists browser: Fixup the "cpu" column width calculation Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-09 22:00 +0200
      Re: [PATCH 07/13] perf hists browser: Fixup the "cpu" column width  calculation "Wangnan (F)" <wangnan0@huawei.com> - 2015-09-11 13:00 +0200
    [PATCH 12/13] perf evsel: Remove forward declaration of 'struct perf_evlist' Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-09 22:00 +0200
    [PATCH 02/13] perf env: Rename some leftovers from rename to perf_env Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-09 22:00 +0200
    [PATCH 01/13] perf env: Move perf_env out of header.h and session.c into separate object Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-09 22:00 +0200
    [PATCH 13/13] Revert "perf evlist: Add backpointer for perf_env to evlist" Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-09 22:00 +0200
    [PATCH 08/13] perf machine: Add pointer to sample's environment Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-09 22:00 +0200
    [PATCH 06/13] perf top: Cache the cpu topology info when "-s socket" is used Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-09 22:00 +0200
    [PATCH 03/13] perf env: Adopt perf_header__set_cmdline Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-09 22:00 +0200
    [PATCH 09/13] perf event: Use machine->env to find the cpu -> socket mapping Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-09 22:00 +0200
    [PATCH 10/13] perf report: Do not blindly use env->cpu[al.cpu].socket_id Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-09 22:00 +0200
      Re: [PATCH 10/13] perf report: Do not blindly use env->cpu[al.cpu].socket_id "Wangnan (F)" <wangnan0@huawei.com> - 2015-09-11 14:00 +0200
    Re: [RFC 00/13] perf_env/CPU socket reorg/fixes Jiri Olsa <jolsa@redhat.com> - 2015-09-10 11:20 +0200
      Re: [RFC 00/13] perf_env/CPU socket reorg/fixes Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-10 16:20 +0200
    Re: [RFC 00/13] perf_env/CPU socket reorg/fixes "Wangnan (F)" <wangnan0@huawei.com> - 2015-09-11 14:30 +0200
      Re: [RFC 00/13] perf_env/CPU socket reorg/fixes Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-11 15:10 +0200
        Re: [RFC 00/13] perf_env/CPU socket reorg/fixes Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-11 15:30 +0200
          Re: [RFC 00/13] perf_env/CPU socket reorg/fixes Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-11 15:40 +0200
            Re: [RFC 00/13] perf_env/CPU socket reorg/fixes Arnaldo Carvalho de Melo <acme@kernel.org> - 2015-09-11 15:40 +0200
              Re: [RFC 00/13] perf_env/CPU socket reorg/fixes "Wangnan (F)" <wangnan0@huawei.com> - 2015-09-14 03:40 +0200
        Re: [RFC 00/13] perf_env/CPU socket reorg/fixes "Wangnan (F)" <wangnan0@huawei.com> - 2015-09-14 03:30 +0200

Page 2 of 2 — ← Prev page 1 [2]

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

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1221681 — [PATCH 03/13] perf env: Adopt perf_header__set_cmdline

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

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

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


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1222052

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1222228

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1222792

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"


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1222830

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1222841

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;
 }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1222849

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;
>  }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1222852

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;
> >  }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1223790

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;
>>>   }


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1223787

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


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web