Skip to contentSkip to conversation

perf tools: Assorted fixes

PATCH Applied Jiri Olsa 4 participants 41 messages Mar 7, 2018 – Mar 9, 2018

Subsystems (2)

v1 of 3 Applied Latest is v3 →

lore
Share & tools
lore

Jiri Olsa

author reviewer
#0 lore raw

cover perf tools: Assorted fixes

hi,
sending assorted general fixes that queued
up in my other branches.

Also available in here:
https://git.kernel.org/pub/scm/linux/kernel/git/jolsa/perf.git
perf/fixes

thanks,
jirka

---
Jiri Olsa (19):
perf report: Fix the output for stdio events list
perf report: Display perf.data header info
perf record: Move machine variable down the function perf record: Remove progname from struct record
perf tools: Add refcnt into struct mem_info
perf c2c: Use mem_info refcnt logic
perf tools: Add MEM_TOPOLOGY feature to perf data file perf tools: Add mem2node object
perf tests: Add mem2node object test
perf c2c record: Record physical addresses in samples perf c2c report: Make calc_width work with struct c2c_hist_entry perf c2c report: Call calc_width only for displayed entries perf c2c report: Display node for cacheline address perf c2c report: Add span header over cacheline data perf c2c report: Add cacheline address count column perf tools: Update tags with .cpp files
perf build: Add llvm/clang/cxx make tests into FEATURE_TESTS_EXTRA perf build: Add llvm/clang make targets to FILES
perf build: Force llvm/clang test compile output to .make.output

 tools/build/Makefile.feature          |   6 ++-
 tools/build/feature/Makefile          |  14 ++++--
 tools/include/linux/bitmap.h          |   2 +-
 tools/perf/Documentation/perf-c2c.txt |   2 +-
 tools/perf/Makefile.perf              |   6 +--
 tools/perf/builtin-c2c.c              | 249 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------
 tools/perf/builtin-record.c           |   7 +--
 tools/perf/builtin-report.c           |  17 +++++--
 tools/perf/tests/Build                |   1 +
 tools/perf/tests/builtin-test.c       |   4 ++
 tools/perf/tests/mem2node.c           |  73 +++++++++++++++++++++++++++
 tools/perf/tests/tests.h              |   1 +
 tools/perf/ui/browsers/hists.c        |   5 +-
 tools/perf/util/Build                 |   1 +
 tools/perf/util/env.h                 |   9 ++++
 tools/perf/util/evsel.c               |  20 ++++++--
 tools/perf/util/evsel.h               |   1 +
 tools/perf/util/header.c              | 312 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 tools/perf/util/header.h              |   1 +
 tools/perf/util/hist.c                |   4 +-
 tools/perf/util/machine.c             |   2 +-
 tools/perf/util/mem2node.c            | 129 +++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/mem2node.h            |  19 +++++++
 tools/perf/util/symbol.c              |  22 ++++++++
 tools/perf/util/symbol.h              |  19 +++++--
 25 files changed, 864 insertions(+), 62 deletions(-)
 create mode 100644 tools/perf/tests/mem2node.c
 create mode 100644 tools/perf/util/mem2node.c
 create mode 100644 tools/perf/util/mem2node.h
author reviewer
#1 lore raw

PATCH 1/19 perf report: Fix the output for stdio events list

Changing the output header for reporting forced groups via --groups option on non grouped events, like:

  $ perf record -e 'cycles,instructions'
  $ perf report --stdio --group

Before:
# Samples: 24 of event 'anon group { cycles:u, instructions:u }'

After:
# Samples: 24 of events 'cycles:u, instructions:u'

---
 tools/perf/builtin-report.c    | 17 ++++++++++++++---
 tools/perf/ui/browsers/hists.c |  5 +++--
 tools/perf/util/evsel.c        | 20 ++++++++++++++++----
 tools/perf/util/evsel.h        |  1 +
 4 files changed, 34 insertions(+), 9 deletions(-)
tools/perf/builtin-report.c +14 -3
index 1eedb1815c4c..c3603d4c0c57 100644
@@ -400,8 +400,10 @@ static size_t hists__fprintf_nr_sample_events(struct hists *hists, struct report
 
 	nr_samples = convert_unit(nr_samples, &unit);
 	ret = fprintf(fp, "# Samples: %lu%c", nr_samples, unit);
-	if (evname != NULL)
-		ret += fprintf(fp, " of event '%s'", evname);
+	if (evname != NULL) {
+		ret += fprintf(fp, " of event%s '%s'",
+			       evsel->nr_members > 1 ? "s" : "", evname);
+	}
 
 	if (rep->time_str)
 		ret += fprintf(fp, " (time slices: %s)", rep->time_str);
@@ -1175,8 +1177,17 @@ int cmd_report(int argc, const char **argv)
 	has_br_stack = perf_header__has_feat(&session->header,
 					     HEADER_BRANCH_STACK);
 
-	if (group_set && !session->evlist->nr_groups)
+	/*
+	 * Events in data file are not collect in groups, but we still want
+	 * the group display. Set the artificial group and set the leader's
+	 * forced_leader flag to notify the display code.
+	 */
+	if (group_set && !session->evlist->nr_groups) {
+		struct perf_evsel *leader = perf_evlist__first(session->evlist);
+
 		perf_evlist__set_leader(session->evlist);
+		leader->forced_leader = true;
+	}
 
 	if (itrace_synth_opts.last_branch)
 		has_br_stack = true;
tools/perf/ui/browsers/hists.c +3 -2
index de2bde232cb3..8b4e82548f8e 100644
@@ -2261,8 +2261,9 @@ static int perf_evsel_browser_title(struct hist_browser *browser,
 
 	nr_samples = convert_unit(nr_samples, &unit);
 	printed = scnprintf(bf, size,
-			   "Samples: %lu%c of event '%s',%s%sEvent count (approx.): %" PRIu64,
-			   nr_samples, unit, ev_name, sample_freq_str, enable_ref ? ref : " ", nr_events);
+			   "Samples: %lu%c of event%s '%s',%s%sEvent count (approx.): %" PRIu64,
+			   nr_samples, unit, evsel->nr_members > 1 ? "s" : "",
+			   ev_name, sample_freq_str, enable_ref ? ref : " ", nr_events);
 
 
 	if (hists->uid_filter_str)
tools/perf/util/evsel.c +16 -4
index f1f883bb41a8..8948a8c1c187 100644
@@ -621,22 +621,34 @@ const char *perf_evsel__group_name(struct perf_evsel *evsel)
 	return evsel->group_name ?: "anon group";
 }
 
+/*
+ * Returns the group details for the specified leader,
+ * with following rules.
+ *
+ *  For record -e '{cycles,instructions}'
+ *    'anon group { cycles:u, instructions:u }'
+ *
+ *  For record -e 'cycles,instructions' and report --group
+ *    'cycles:u, instructions:u'
+ */
 int perf_evsel__group_desc(struct perf_evsel *evsel, char *buf, size_t size)
 {
-	int ret;
+	int ret = 0;
 	struct perf_evsel *pos;
 	const char *group_name = perf_evsel__group_name(evsel);
 
-	ret = scnprintf(buf, size, "%s", group_name);
+	if (!evsel->forced_leader)
+		ret = scnprintf(buf, size, "%s { ", group_name);
 
-	ret += scnprintf(buf + ret, size - ret, " { %s",
+	ret += scnprintf(buf + ret, size - ret, "%s",
 			 perf_evsel__name(evsel));
 
 	for_each_group_member(pos, evsel)
 		ret += scnprintf(buf + ret, size - ret, ", %s",
 				 perf_evsel__name(pos));
 
-	ret += scnprintf(buf + ret, size - ret, " }");
+	if (!evsel->forced_leader)
+		ret += scnprintf(buf + ret, size - ret, " }");
 
 	return ret;
 }
tools/perf/util/evsel.h +1 -0
index 92ba001b627f..bb17afc429d9 100644
@@ -125,6 +125,7 @@ struct perf_evsel {
 	bool			per_pkg;
 	bool			precise_max;
 	bool			ignore_missing_thread;
+	bool			forced_leader;
 	/* parse modifier helper */
 	int			exclude_GH;
 	int			nr_members;
Signature · 1 line
2.13.6
#2 lore raw

[tip:perf/core] perf report: Fix the output for stdio events list

perf report: Fix the output for stdio events list
Commit-ID:  8ef278bb9305e1269f236013718801fe06a183d1
Gitweb:     https://git.kernel.org/tip/8ef278bb9305e1269f236013718801fe06a183d1
Author:     Jiri Olsa <[email protected]>
AuthorDate: Wed, 7 Mar 2018 16:50:02 +0100
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Thu, 8 Mar 2018 11:30:36 -0300

perf report: Fix the output for stdio events list

Changing the output header for reporting forced groups via --groups option on non grouped events, like:

  $ perf record -e 'cycles,instructions'
  $ perf report --stdio --group

Before:

# Samples: 24 of event 'anon group { cycles:u, instructions:u }'

After:

# Samples: 24 of events 'cycles:u, instructions:u'

Cc Alexander Shishkin <[email protected]>
Cc David Ahern <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
Fixes ad52b8cb4886 ("perf report: Add support to display group output for non group events")
---
 tools/perf/builtin-report.c    | 17 ++++++++++++++---
 tools/perf/ui/browsers/hists.c |  5 +++--
 tools/perf/util/evsel.c        | 20 ++++++++++++++++----
 tools/perf/util/evsel.h        |  1 +
 4 files changed, 34 insertions(+), 9 deletions(-)
tools/perf/builtin-report.c +14 -3
index 1eedb1815c4c..c3603d4c0c57 100644
@@ -400,8 +400,10 @@ static size_t hists__fprintf_nr_sample_events(struct hists *hists, struct report
 
 	nr_samples = convert_unit(nr_samples, &unit);
 	ret = fprintf(fp, "# Samples: %lu%c", nr_samples, unit);
-	if (evname != NULL)
-		ret += fprintf(fp, " of event '%s'", evname);
+	if (evname != NULL) {
+		ret += fprintf(fp, " of event%s '%s'",
+			       evsel->nr_members > 1 ? "s" : "", evname);
+	}
 
 	if (rep->time_str)
 		ret += fprintf(fp, " (time slices: %s)", rep->time_str);
@@ -1175,8 +1177,17 @@ repeat:
 	has_br_stack = perf_header__has_feat(&session->header,
 					     HEADER_BRANCH_STACK);
 
-	if (group_set && !session->evlist->nr_groups)
+	/*
+	 * Events in data file are not collect in groups, but we still want
+	 * the group display. Set the artificial group and set the leader's
+	 * forced_leader flag to notify the display code.
+	 */
+	if (group_set && !session->evlist->nr_groups) {
+		struct perf_evsel *leader = perf_evlist__first(session->evlist);
+
 		perf_evlist__set_leader(session->evlist);
+		leader->forced_leader = true;
+	}
 
 	if (itrace_synth_opts.last_branch)
 		has_br_stack = true;
tools/perf/ui/browsers/hists.c +3 -2
index de2bde232cb3..8b4e82548f8e 100644
@@ -2261,8 +2261,9 @@ static int perf_evsel_browser_title(struct hist_browser *browser,
 
 	nr_samples = convert_unit(nr_samples, &unit);
 	printed = scnprintf(bf, size,
-			   "Samples: %lu%c of event '%s',%s%sEvent count (approx.): %" PRIu64,
-			   nr_samples, unit, ev_name, sample_freq_str, enable_ref ? ref : " ", nr_events);
+			   "Samples: %lu%c of event%s '%s',%s%sEvent count (approx.): %" PRIu64,
+			   nr_samples, unit, evsel->nr_members > 1 ? "s" : "",
+			   ev_name, sample_freq_str, enable_ref ? ref : " ", nr_events);
 
 
 	if (hists->uid_filter_str)
tools/perf/util/evsel.c +16 -4
index e937894654b2..1ac8d9236efd 100644
@@ -622,22 +622,34 @@ const char *perf_evsel__group_name(struct perf_evsel *evsel)
 	return evsel->group_name ?: "anon group";
 }
 
+/*
+ * Returns the group details for the specified leader,
+ * with following rules.
+ *
+ *  For record -e '{cycles,instructions}'
+ *    'anon group { cycles:u, instructions:u }'
+ *
+ *  For record -e 'cycles,instructions' and report --group
+ *    'cycles:u, instructions:u'
+ */
 int perf_evsel__group_desc(struct perf_evsel *evsel, char *buf, size_t size)
 {
-	int ret;
+	int ret = 0;
 	struct perf_evsel *pos;
 	const char *group_name = perf_evsel__group_name(evsel);
 
-	ret = scnprintf(buf, size, "%s", group_name);
+	if (!evsel->forced_leader)
+		ret = scnprintf(buf, size, "%s { ", group_name);
 
-	ret += scnprintf(buf + ret, size - ret, " { %s",
+	ret += scnprintf(buf + ret, size - ret, "%s",
 			 perf_evsel__name(evsel));
 
 	for_each_group_member(pos, evsel)
 		ret += scnprintf(buf + ret, size - ret, ", %s",
 				 perf_evsel__name(pos));
 
-	ret += scnprintf(buf + ret, size - ret, " }");
+	if (!evsel->forced_leader)
+		ret += scnprintf(buf + ret, size - ret, " }");
 
 	return ret;
 }
tools/perf/util/evsel.h +1 -0
index 55ae1cda7396..d3ee3af618ef 100644
@@ -125,6 +125,7 @@ struct perf_evsel {
 	bool			per_pkg;
 	bool			precise_max;
 	bool			ignore_missing_thread;
+	bool			forced_leader;
 	/* parse modifier helper */
 	int			exclude_GH;
 	int			nr_members;
author reviewer
#3 lore raw

PATCH 2/19 perf report: Display perf.data header info

In reply to Jiri Olsa

Display more header info from perf.data file,
following values:

  $ perf report -i perf.data --header-only
  ...
  # header version : 1
  # data offset    : 424
  # data size      : 3364280
  # feat offset    : 3364704

It's handy for debuging.

---
 tools/perf/util/header.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
tools/perf/util/header.c +6 -1
index a326e0d8b5b6..e0c3cad0fd8d 100644
@@ -2318,7 +2318,12 @@ int perf_header__fprintf_info(struct perf_session *session, FILE *fp, bool full)
 	if (ret == -1)
 		return -1;
 
-	fprintf(fp, "# captured on: %s", ctime(&st.st_ctime));
+	fprintf(fp, "# captured on    : %s", ctime(&st.st_ctime));
+
+	fprintf(fp, "# header version : %u\n", header->version);
+	fprintf(fp, "# data offset    : %" PRIu64 "\n", header->data_offset);
+	fprintf(fp, "# data size      : %" PRIu64 "\n", header->data_size);
+	fprintf(fp, "# feat offset    : %" PRIu64 "\n", header->feat_offset);
 
 	perf_header__process_sections(header, fd, &hd,
 				      perf_file_section__fprintf_info);
Signature · 1 line
2.13.6
#4 lore raw

[tip:perf/core] perf report: Display perf.data header info

perf report: Display perf.data header info
Commit-ID:  e971a5a8399a5e84164239a5c2d59b8a51314241
Gitweb:     https://git.kernel.org/tip/e971a5a8399a5e84164239a5c2d59b8a51314241
Author:     Jiri Olsa <[email protected]>
AuthorDate: Wed, 7 Mar 2018 16:50:03 +0100
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Thu, 8 Mar 2018 11:30:41 -0300

perf report: Display perf.data header info

Display more header info from perf.data file, following values:

  $ perf report -i perf.data --header-only
  ...
  # header version : 1
  # data offset    : 424
  # data size      : 3364280
  # feat offset    : 3364704

It's handy for debuging.

---
 tools/perf/util/header.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
tools/perf/util/header.c +6 -1
index a326e0d8b5b6..e0c3cad0fd8d 100644
@@ -2318,7 +2318,12 @@ int perf_header__fprintf_info(struct perf_session *session, FILE *fp, bool full)
 	if (ret == -1)
 		return -1;
 
-	fprintf(fp, "# captured on: %s", ctime(&st.st_ctime));
+	fprintf(fp, "# captured on    : %s", ctime(&st.st_ctime));
+
+	fprintf(fp, "# header version : %u\n", header->version);
+	fprintf(fp, "# data offset    : %" PRIu64 "\n", header->data_offset);
+	fprintf(fp, "# data size      : %" PRIu64 "\n", header->data_size);
+	fprintf(fp, "# feat offset    : %" PRIu64 "\n", header->feat_offset);
 
 	perf_header__process_sections(header, fd, &hd,
 				      perf_file_section__fprintf_info);
author reviewer
#5 lore raw

PATCH 3/19 perf record: Move machine variable down the function

In reply to Jiri Olsa

It's used far more down to be declared on the top of the __cmd_record.

---
 tools/perf/builtin-record.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
tools/perf/builtin-record.c +1 -3
index 14d82f0fe5cc..b2ebd9b3feba 100644
@@ -854,7 +854,6 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 	int status = 0;
 	unsigned long waking = 0;
 	const bool forks = argc > 0;
-	struct machine *machine;
 	struct perf_tool *tool = &rec->tool;
 	struct record_opts *opts = &rec->opts;
 	struct perf_data *data = &rec->data;
@@ -959,8 +958,6 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 		goto out_child;
 	}
 
-	machine = &session->machines.host;
-
 	err = record__synthesize(rec, false);
 	if (err < 0)
 		goto out_child;
@@ -988,6 +985,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 	 * Let the child rip
 	 */
 	if (forks) {
+		struct machine *machine = &session->machines.host;
 		union perf_event *event;
 		pid_t tgid;
 
Signature · 1 line
2.13.6
#6 lore raw

[tip:perf/core] perf record: Move machine variable down the function

perf record: Move machine variable down the function
Commit-ID:  20a8a3cf90215cebd916048e51a5bfa6c3707778
Gitweb:     https://git.kernel.org/tip/20a8a3cf90215cebd916048e51a5bfa6c3707778
Author:     Jiri Olsa <[email protected]>
AuthorDate: Wed, 7 Mar 2018 16:50:04 +0100
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Thu, 8 Mar 2018 11:30:42 -0300

perf record: Move machine variable down the function

It's used far more down to be declared on the top of the __cmd_record.

---
 tools/perf/builtin-record.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
tools/perf/builtin-record.c +1 -3
index 753ffcecf254..31d4d70e2f6a 100644
@@ -854,7 +854,6 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 	int status = 0;
 	unsigned long waking = 0;
 	const bool forks = argc > 0;
-	struct machine *machine;
 	struct perf_tool *tool = &rec->tool;
 	struct record_opts *opts = &rec->opts;
 	struct perf_data *data = &rec->data;
@@ -959,8 +958,6 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 		goto out_child;
 	}
 
-	machine = &session->machines.host;
-
 	err = record__synthesize(rec, false);
 	if (err < 0)
 		goto out_child;
@@ -988,6 +985,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 	 * Let the child rip
 	 */
 	if (forks) {
+		struct machine *machine = &session->machines.host;
 		union perf_event *event;
 		pid_t tgid;
 
author reviewer
#7 lore raw

PATCH 4/19 perf record: Remove progname from struct record

In reply to Jiri Olsa

It's no longer used.

---
 tools/perf/builtin-record.c | 3 ---
 1 file changed, 3 deletions(-)
tools/perf/builtin-record.c +0 -3
index b2ebd9b3feba..68caf54aa191 100644
@@ -71,7 +71,6 @@ struct record {
 	struct auxtrace_record	*itr;
 	struct perf_evlist	*evlist;
 	struct perf_session	*session;
-	const char		*progname;
 	int			realtime_prio;
 	bool			no_buildid;
 	bool			no_buildid_set;
@@ -861,8 +860,6 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 	bool disabled = false, draining = false;
 	int fd;
 
-	rec->progname = argv[0];
-
 	atexit(record__sig_exit);
 	signal(SIGCHLD, sig_handler);
 	signal(SIGINT, sig_handler);
Signature · 1 line
2.13.6
#8 lore raw

[tip:perf/core] perf record: Remove progname from struct record

perf record: Remove progname from struct record
Commit-ID:  915b4e27f1d32b3c7de4874ac1e3fe1f801151ca
Gitweb:     https://git.kernel.org/tip/915b4e27f1d32b3c7de4874ac1e3fe1f801151ca
Author:     Jiri Olsa <[email protected]>
AuthorDate: Wed, 7 Mar 2018 16:50:05 +0100
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Thu, 8 Mar 2018 11:30:43 -0300

perf record: Remove progname from struct record

It's no longer used.

---
 tools/perf/builtin-record.c | 3 ---
 1 file changed, 3 deletions(-)
tools/perf/builtin-record.c +0 -3
index 31d4d70e2f6a..b81494587120 100644
@@ -71,7 +71,6 @@ struct record {
 	struct auxtrace_record	*itr;
 	struct perf_evlist	*evlist;
 	struct perf_session	*session;
-	const char		*progname;
 	int			realtime_prio;
 	bool			no_buildid;
 	bool			no_buildid_set;
@@ -861,8 +860,6 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 	bool disabled = false, draining = false;
 	int fd;
 
-	rec->progname = argv[0];
-
 	atexit(record__sig_exit);
 	signal(SIGCHLD, sig_handler);
 	signal(SIGINT, sig_handler);
author reviewer
#9 lore raw

PATCH 5/19 perf tools: Add refcnt into struct mem_info

In reply to Jiri Olsa

It's passed along several hists entries in --hierarchy mode, so it's better we keep track of it.

The current fail I see is that it gets removed in hierarchy --mem-mode mode, where it's shared in the different hierarchies, but removed from the template hist entry, so the report crashes.

---
 tools/perf/util/hist.c    |  4 ++--
 tools/perf/util/machine.c |  2 +-
 tools/perf/util/symbol.c  | 22 ++++++++++++++++++++++
 tools/perf/util/symbol.h  | 19 ++++++++++++++++---
 4 files changed, 41 insertions(+), 6 deletions(-)
tools/perf/util/hist.c +2 -2
index 44a8456cea10..7d968892ee39 100644
@@ -536,7 +536,7 @@ static struct hist_entry *hists__findnew_entry(struct hists *hists,
 			 * This mem info was allocated from sample__resolve_mem
 			 * and will not be used anymore.
 			 */
-			zfree(&entry->mem_info);
+			mem_info__zput(entry->mem_info);
 
 			/* If the map of an existing hist_entry has
 			 * become out-of-date due to an exec() or
@@ -1139,7 +1139,7 @@ void hist_entry__delete(struct hist_entry *he)
 	if (he->mem_info) {
 		map__zput(he->mem_info->iaddr.map);
 		map__zput(he->mem_info->daddr.map);
-		zfree(&he->mem_info);
+		mem_info__zput(he->mem_info);
 	}
 
 	zfree(&he->stat_acc);
tools/perf/util/machine.c +1 -1
index 12b7427444a3..d46c98d6dedd 100644
@@ -1697,7 +1697,7 @@ static void ip__resolve_data(struct thread *thread,
 struct mem_info *sample__resolve_mem(struct perf_sample *sample,
 				     struct addr_location *al)
 {
-	struct mem_info *mi = zalloc(sizeof(*mi));
+	struct mem_info *mi = mem_info__aloc();
 
 	if (!mi)
 		return NULL;
tools/perf/util/symbol.c +22 -0
index a1a312d99f30..117cd25cd21a 100644
@@ -2221,3 +2221,25 @@ int symbol__config_symfs(const struct option *opt __maybe_unused,
 	free(bf);
 	return 0;
 }
+
+struct mem_info *mem_info__get(struct mem_info *mi)
+{
+	if (mi)
+		refcount_inc(&mi->refcnt);
+	return mi;
+}
+
+void mem_info__put(struct mem_info *mi)
+{
+	if (mi && refcount_dec_and_test(&mi->refcnt))
+		free(mi);
+}
+
+struct mem_info *mem_info__aloc(void)
+{
+	struct mem_info *mi = zalloc(sizeof(*mi));
+
+	if (mi)
+		refcount_set(&mi->refcnt, 1);
+	return mi;
+}
tools/perf/util/symbol.h +16 -3
index 0563f33c1eb3..7e3ac7f35d63 100644
@@ -200,9 +200,10 @@ struct branch_info {
 };
 
 struct mem_info {
-	struct addr_map_symbol iaddr;
-	struct addr_map_symbol daddr;
-	union perf_mem_data_src data_src;
+	struct addr_map_symbol	iaddr;
+	struct addr_map_symbol	daddr;
+	union perf_mem_data_src	data_src;
+	refcount_t		refcnt;
 };
 
 struct addr_location {
@@ -389,4 +390,16 @@ int sdt_notes__get_count(struct list_head *start);
 #define SDT_NOTE_NAME "stapsdt"
 #define NR_ADDR 3
 
+struct mem_info *mem_info__aloc(void);
+struct mem_info *mem_info__get(struct mem_info *mi);
+void   mem_info__put(struct mem_info *mi);
+
+static inline void __mem_info__zput(struct mem_info **mi)
+{
+	mem_info__put(*mi);
+	*mi = NULL;
+}
+
+#define mem_info__zput(mi) __mem_info__zput(&mi)
+
 #endif /* __PERF_SYMBOL */
Signature · 1 line
2.13.6
#10 lore raw

Re: [PATCH 05/19] perf tools: Add refcnt into struct mem_info

Em Wed, Mar 07, 2018 at 04:50:06PM +0100, Jiri Olsa escreveu:

Quoted 2 lines

It's passed along several hists entries in --hierarchy mode, so it's better we keep track of it.

I like this, but you called it...

tools/perf/util/symbol.c int symbol__config_symfs(const struct option *opt __maybe_unused, · line 2244

Quoted 8 lines
tools/perf/util/symbol.c +8 -0
+
+struct mem_info *mem_info__aloc(void)
+{
+	struct mem_info *mi = zalloc(sizeof(*mi));
+
+	if (mi)
+		refcount_set(&mi->refcnt, 1);
+	return mi;

"aloc" with just one 'l', so I have to fix the typo, while at it I'll use what is used elsewhere for this operation, i.e.

struct mem_info *mem_info__new(void);

:-)

- Arnaldo

#11 lore raw

Re: [PATCH 05/19] perf tools: Add refcnt into struct mem_info

Quoted 18 lines · Arnaldo Carvalho de Melo

Em Wed, Mar 07, 2018 at 04:50:06PM +0100, Jiri Olsa escreveu:

Quoted 2 lines

It's passed along several hists entries in --hierarchy mode, so it's better we keep track of it.

I like this, but you called it...

tools/perf/util/symbol.c int symbol__config_symfs(const struct option *opt __maybe_unused, · line 2244

Quoted 8 lines
tools/perf/util/symbol.c +8 -0
+
+struct mem_info *mem_info__aloc(void)
+{
+	struct mem_info *mi = zalloc(sizeof(*mi));
+
+	if (mi)
+		refcount_set(&mi->refcnt, 1);
+	return mi;

"aloc" with just one 'l', so I have to fix the typo, while at it I'll use what is used elsewhere for this operation, i.e.

Quoted 1 line

struct mem_info *mem_info__new(void);

ok, thanks

jirka

#12 lore raw

[tip:perf/core] perf tools: Add refcnt into struct mem_info

In reply to Jiri Olsa
perf tools: Add refcnt into struct mem_info
Commit-ID:  9f87498f1cb72958c6f8725eb93d2f7ef81fa11e
Gitweb:     https://git.kernel.org/tip/9f87498f1cb72958c6f8725eb93d2f7ef81fa11e
Author:     Jiri Olsa <[email protected]>
AuthorDate: Wed, 7 Mar 2018 16:50:06 +0100
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Thu, 8 Mar 2018 11:30:44 -0300

perf tools: Add refcnt into struct mem_info

It's passed along several hists entries in --hierarchy mode, so it's better we keep track of it.

The current fail I see is that it gets removed in hierarchy --mem-mode mode, where it's shared in the different hierarchies, but removed from the template hist entry, so the report crashes.

Cc Alexander Shishkin <[email protected]>
Cc David Ahern <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Peter Zijlstra <[email protected]>
[ Rename mem_info__aloc() to mem_info__new(), to fix the typo and use the convention for constructors ]
---
 tools/perf/util/hist.c    |  4 ++--
 tools/perf/util/machine.c |  2 +-
 tools/perf/util/symbol.c  | 22 ++++++++++++++++++++++
 tools/perf/util/symbol.h  | 19 ++++++++++++++++---
 4 files changed, 41 insertions(+), 6 deletions(-)
tools/perf/util/hist.c +2 -2
index 44a8456cea10..7d968892ee39 100644
@@ -536,7 +536,7 @@ static struct hist_entry *hists__findnew_entry(struct hists *hists,
 			 * This mem info was allocated from sample__resolve_mem
 			 * and will not be used anymore.
 			 */
-			zfree(&entry->mem_info);
+			mem_info__zput(entry->mem_info);
 
 			/* If the map of an existing hist_entry has
 			 * become out-of-date due to an exec() or
@@ -1139,7 +1139,7 @@ void hist_entry__delete(struct hist_entry *he)
 	if (he->mem_info) {
 		map__zput(he->mem_info->iaddr.map);
 		map__zput(he->mem_info->daddr.map);
-		zfree(&he->mem_info);
+		mem_info__zput(he->mem_info);
 	}
 
 	zfree(&he->stat_acc);
tools/perf/util/machine.c +1 -1
index 12b7427444a3..43fbbee409ec 100644
@@ -1697,7 +1697,7 @@ static void ip__resolve_data(struct thread *thread,
 struct mem_info *sample__resolve_mem(struct perf_sample *sample,
 				     struct addr_location *al)
 {
-	struct mem_info *mi = zalloc(sizeof(*mi));
+	struct mem_info *mi = mem_info__new();
 
 	if (!mi)
 		return NULL;
tools/perf/util/symbol.c +22 -0
index a1a312d99f30..62b2dd2253eb 100644
@@ -2221,3 +2221,25 @@ int symbol__config_symfs(const struct option *opt __maybe_unused,
 	free(bf);
 	return 0;
 }
+
+struct mem_info *mem_info__get(struct mem_info *mi)
+{
+	if (mi)
+		refcount_inc(&mi->refcnt);
+	return mi;
+}
+
+void mem_info__put(struct mem_info *mi)
+{
+	if (mi && refcount_dec_and_test(&mi->refcnt))
+		free(mi);
+}
+
+struct mem_info *mem_info__new(void)
+{
+	struct mem_info *mi = zalloc(sizeof(*mi));
+
+	if (mi)
+		refcount_set(&mi->refcnt, 1);
+	return mi;
+}
tools/perf/util/symbol.h +16 -3
index 0563f33c1eb3..70c16741f50a 100644
@@ -200,9 +200,10 @@ struct branch_info {
 };
 
 struct mem_info {
-	struct addr_map_symbol iaddr;
-	struct addr_map_symbol daddr;
-	union perf_mem_data_src data_src;
+	struct addr_map_symbol	iaddr;
+	struct addr_map_symbol	daddr;
+	union perf_mem_data_src	data_src;
+	refcount_t		refcnt;
 };
 
 struct addr_location {
@@ -389,4 +390,16 @@ int sdt_notes__get_count(struct list_head *start);
 #define SDT_NOTE_NAME "stapsdt"
 #define NR_ADDR 3
 
+struct mem_info *mem_info__new(void);
+struct mem_info *mem_info__get(struct mem_info *mi);
+void   mem_info__put(struct mem_info *mi);
+
+static inline void __mem_info__zput(struct mem_info **mi)
+{
+	mem_info__put(*mi);
+	*mi = NULL;
+}
+
+#define mem_info__zput(mi) __mem_info__zput(&mi)
+
 #endif /* __PERF_SYMBOL */
author reviewer
#13 lore raw

PATCH 6/19 perf c2c: Use mem_info refcnt logic

In reply to Jiri Olsa

Switch to refcnt logic instead of duplicating
mem_info objects. No functional change, just
saving some memory.

---
 tools/perf/builtin-c2c.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)
tools/perf/builtin-c2c.c +11 -13
index 539c3d460158..98d243fa0c06 100644
@@ -237,9 +237,12 @@ static int process_sample_event(struct perf_tool *tool __maybe_unused,
 	if (mi == NULL)
 		return -ENOMEM;
 
-	mi_dup = memdup(mi, sizeof(*mi));
-	if (!mi_dup)
-		goto free_mi;
+	/*
+	 * The mi object is released in hists__add_entry_ops,
+	 * if it gets sorted out into existing data, so we need
+	 * to take the copy now.
+	 */
+	mi_dup = mem_info__get(mi);
 
 	c2c_decode_stats(&stats, mi);
 
@@ -247,7 +250,7 @@ static int process_sample_event(struct perf_tool *tool __maybe_unused,
 				  &al, NULL, NULL, mi,
 				  sample, true);
 	if (he == NULL)
-		goto free_mi_dup;
+		goto free_mi;
 
 	c2c_he = container_of(he, struct c2c_hist_entry, he);
 	c2c_add_stats(&c2c_he->stats, &stats);
@@ -272,19 +275,15 @@ static int process_sample_event(struct perf_tool *tool __maybe_unused,
 
 		mi = mi_dup;
 
-		mi_dup = memdup(mi, sizeof(*mi));
-		if (!mi_dup)
-			goto free_mi;
-
 		c2c_hists = he__get_c2c_hists(he, c2c.cl_sort, 2);
 		if (!c2c_hists)
-			goto free_mi_dup;
+			goto free_mi;
 
 		he = hists__add_entry_ops(&c2c_hists->hists, &c2c_entry_ops,
 					  &al, NULL, NULL, mi,
 					  sample, true);
 		if (he == NULL)
-			goto free_mi_dup;
+			goto free_mi;
 
 		c2c_he = container_of(he, struct c2c_hist_entry, he);
 		c2c_add_stats(&c2c_he->stats, &stats);
@@ -303,10 +302,9 @@ static int process_sample_event(struct perf_tool *tool __maybe_unused,
 	addr_location__put(&al);
 	return ret;
 
-free_mi_dup:
-	free(mi_dup);
 free_mi:
-	free(mi);
+	mem_info__put(mi_dup);
+	mem_info__put(mi);
 	ret = -ENOMEM;
 	goto out;
 }
Signature · 1 line
2.13.6
#14 lore raw

[tip:perf/core] perf c2c: Use mem_info refcnt logic

perf c2c: Use mem_info refcnt logic
Commit-ID:  5cedb413a63d452356aa634e0d4ffcc24f7dbcb5
Gitweb:     https://git.kernel.org/tip/5cedb413a63d452356aa634e0d4ffcc24f7dbcb5
Author:     Jiri Olsa <[email protected]>
AuthorDate: Wed, 7 Mar 2018 16:50:07 +0100
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Thu, 8 Mar 2018 11:30:45 -0300

perf c2c: Use mem_info refcnt logic

Switch to refcnt logic instead of duplicating mem_info objects. No functional change, just saving some memory.

---
 tools/perf/builtin-c2c.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)
tools/perf/builtin-c2c.c +11 -13
index 539c3d460158..98d243fa0c06 100644
@@ -237,9 +237,12 @@ static int process_sample_event(struct perf_tool *tool __maybe_unused,
 	if (mi == NULL)
 		return -ENOMEM;
 
-	mi_dup = memdup(mi, sizeof(*mi));
-	if (!mi_dup)
-		goto free_mi;
+	/*
+	 * The mi object is released in hists__add_entry_ops,
+	 * if it gets sorted out into existing data, so we need
+	 * to take the copy now.
+	 */
+	mi_dup = mem_info__get(mi);
 
 	c2c_decode_stats(&stats, mi);
 
@@ -247,7 +250,7 @@ static int process_sample_event(struct perf_tool *tool __maybe_unused,
 				  &al, NULL, NULL, mi,
 				  sample, true);
 	if (he == NULL)
-		goto free_mi_dup;
+		goto free_mi;
 
 	c2c_he = container_of(he, struct c2c_hist_entry, he);
 	c2c_add_stats(&c2c_he->stats, &stats);
@@ -272,19 +275,15 @@ static int process_sample_event(struct perf_tool *tool __maybe_unused,
 
 		mi = mi_dup;
 
-		mi_dup = memdup(mi, sizeof(*mi));
-		if (!mi_dup)
-			goto free_mi;
-
 		c2c_hists = he__get_c2c_hists(he, c2c.cl_sort, 2);
 		if (!c2c_hists)
-			goto free_mi_dup;
+			goto free_mi;
 
 		he = hists__add_entry_ops(&c2c_hists->hists, &c2c_entry_ops,
 					  &al, NULL, NULL, mi,
 					  sample, true);
 		if (he == NULL)
-			goto free_mi_dup;
+			goto free_mi;
 
 		c2c_he = container_of(he, struct c2c_hist_entry, he);
 		c2c_add_stats(&c2c_he->stats, &stats);
@@ -303,10 +302,9 @@ out:
 	addr_location__put(&al);
 	return ret;
 
-free_mi_dup:
-	free(mi_dup);
 free_mi:
-	free(mi);
+	mem_info__put(mi_dup);
+	mem_info__put(mi);
 	ret = -ENOMEM;
 	goto out;
 }
author reviewer
#15 lore raw

PATCH 7/19 perf tools: Add MEM_TOPOLOGY feature to perf data file

In reply to Jiri Olsa

Adding MEM_TOPOLOGY feature to perf data file,
that will carry physical memory map and its
node assignments.

The format of data in MEM_TOPOLOGY is as follows:

  0 - version          | for future changes
  8 - block_size_bytes | /sys/devices/system/memory/block_size_bytes
 16 - count            | number of nodes

For each node we store map of physical indexes for each node:

 32 - node id          | node index
 40 - size             | size of bitmap
 48 - bitmap           | bitmap of memory indexes that belongs to node
                       | /sys/devices/system/node/node<NODE>/memory<INDEX>

The MEM_TOPOLOGY could be displayed with following report command:

  $ perf report --header-only -I
  ...
  # memory nodes (nr 1, block size 0x8000000):
  #    0 [7G]: 0-23,32-69
---
 tools/include/linux/bitmap.h |   2 +-
 tools/perf/util/env.h        |   9 ++
 tools/perf/util/header.c     | 305 +++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/header.h     |   1 +
 4 files changed, 316 insertions(+), 1 deletion(-)
tools/include/linux/bitmap.h +1 -1
index ca160270fdfa..63440cc8d618 100644
@@ -98,7 +98,7 @@ static inline int test_and_set_bit(int nr, unsigned long *addr)
 
 /**
  * bitmap_alloc - Allocate bitmap
- * @nr: Bit to set
+ * @nbits: Number of bits
  */
 static inline unsigned long *bitmap_alloc(int nbits)
 {
tools/perf/util/env.h +9 -0
index bf970f57dce0..c4ef2e523367 100644
@@ -27,6 +27,12 @@ struct numa_node {
 	struct cpu_map	*map;
 };
 
+struct memory_node {
+	u64		 node;
+	u64		 size;
+	unsigned long	*set;
+};
+
 struct perf_env {
 	char			*hostname;
 	char			*os_release;
@@ -43,6 +49,7 @@ struct perf_env {
 	int			nr_sibling_cores;
 	int			nr_sibling_threads;
 	int			nr_numa_nodes;
+	int			nr_memory_nodes;
 	int			nr_pmu_mappings;
 	int			nr_groups;
 	char			*cmdline;
@@ -54,6 +61,8 @@ struct perf_env {
 	struct cpu_cache_level	*caches;
 	int			 caches_cnt;
 	struct numa_node	*numa_nodes;
+	struct memory_node	*memory_nodes;
+	unsigned long long	 memory_bsize;
 };
 
 extern struct perf_env perf_env;
tools/perf/util/header.c +305 -0

353 lines not on this page · raw message

tools/perf/util/header.h +1 -0
index 942bdec6d70d..90d4577a92dc 100644
@@ -36,6 +36,7 @@ enum {
 	HEADER_STAT,
 	HEADER_CACHE,
 	HEADER_SAMPLE_TIME,
+	HEADER_MEM_TOPOLOGY,
 	HEADER_LAST_FEATURE,
 	HEADER_FEAT_BITS	= 256,
 };
Signature · 1 line
2.13.6
#16 lore raw

Re: [PATCH 07/19] perf tools: Add MEM_TOPOLOGY feature to perf data file

Em Wed, Mar 07, 2018 at 04:50:08PM +0100, Jiri Olsa escreveu:

Quoted 3 lines

Adding MEM_TOPOLOGY feature to perf data file,
that will carry physical memory map and its
node assignments.

Good addition, applied 1-7 in this series, skipping a few after this, processing the rest.

- Arnaldo

Quoted 457 lines

The format of data in MEM_TOPOLOGY is as follows:

  0 - version          | for future changes
  8 - block_size_bytes | /sys/devices/system/memory/block_size_bytes
 16 - count            | number of nodes

For each node we store map of physical indexes for each node:

 32 - node id          | node index
 40 - size             | size of bitmap
 48 - bitmap           | bitmap of memory indexes that belongs to node
                       | /sys/devices/system/node/node<NODE>/memory<INDEX>

The MEM_TOPOLOGY could be displayed with following report command:

  $ perf report --header-only -I
  ...
  # memory nodes (nr 1, block size 0x8000000):
  #    0 [7G]: 0-23,32-69
---
 tools/include/linux/bitmap.h |   2 +-
 tools/perf/util/env.h        |   9 ++
 tools/perf/util/header.c     | 305 +++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/header.h     |   1 +
 4 files changed, 316 insertions(+), 1 deletion(-)
tools/include/linux/bitmap.h +1 -1
index ca160270fdfa..63440cc8d618 100644
@@ -98,7 +98,7 @@ static inline int test_and_set_bit(int nr, unsigned long *addr)
 
 /**
  * bitmap_alloc - Allocate bitmap
- * @nr: Bit to set
+ * @nbits: Number of bits
  */
 static inline unsigned long *bitmap_alloc(int nbits)
 {
tools/perf/util/env.h +9 -0
index bf970f57dce0..c4ef2e523367 100644
@@ -27,6 +27,12 @@ struct numa_node {
 	struct cpu_map	*map;
 };
 
+struct memory_node {
+	u64		 node;
+	u64		 size;
+	unsigned long	*set;
+};
+
 struct perf_env {
 	char			*hostname;
 	char			*os_release;
@@ -43,6 +49,7 @@ struct perf_env {
 	int			nr_sibling_cores;
 	int			nr_sibling_threads;
 	int			nr_numa_nodes;
+	int			nr_memory_nodes;
 	int			nr_pmu_mappings;
 	int			nr_groups;
 	char			*cmdline;
@@ -54,6 +61,8 @@ struct perf_env {
 	struct cpu_cache_level	*caches;
 	int			 caches_cnt;
 	struct numa_node	*numa_nodes;
+	struct memory_node	*memory_nodes;
+	unsigned long long	 memory_bsize;
 };
 
 extern struct perf_env perf_env;
tools/perf/util/header.c +305 -0
index e0c3cad0fd8d..3a107e7ac135 100644
@@ -17,6 +17,7 @@
 #include <sys/stat.h>
 #include <sys/utsname.h>
 #include <linux/time64.h>
+#include <dirent.h>
 
 #include "evlist.h"
 #include "evsel.h"
@@ -37,6 +38,7 @@
 #include "asm/bug.h"
 #include "tool.h"
 #include "time-utils.h"
+#include "units.h"
 
 #include "sane_ctype.h"
 
@@ -132,6 +134,25 @@ int do_write(struct feat_fd *ff, const void *buf, size_t size)
 }
 
 /* Return: 0 if succeded, -ERR if failed. */
+static int do_write_bitmap(struct feat_fd *ff, unsigned long *set, u64 size)
+{
+	u64 *p = (u64 *) set;
+	int i, ret;
+
+	ret = do_write(ff, &size, sizeof(size));
+	if (ret < 0)
+		return ret;
+
+	for (i = 0; (u64) i < BITS_TO_U64(size); i++) {
+		ret = do_write(ff, p + i, sizeof(*p));
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
+/* Return: 0 if succeded, -ERR if failed. */
 int write_padded(struct feat_fd *ff, const void *bf,
 		 size_t count, size_t count_aligned)
 {
@@ -243,6 +264,38 @@ static char *do_read_string(struct feat_fd *ff)
 	return NULL;
 }
 
+/* Return: 0 if succeded, -ERR if failed. */
+static int do_read_bitmap(struct feat_fd *ff, unsigned long **pset, u64 *psize)
+{
+	unsigned long *set;
+	u64 size, *p;
+	int i, ret;
+
+	ret = do_read_u64(ff, &size);
+	if (ret)
+		return ret;
+
+	set = bitmap_alloc(size);
+	if (!set)
+		return -ENOMEM;
+
+	bitmap_zero(set, size);
+
+	p = (u64 *) set;
+
+	for (i = 0; (u64) i < BITS_TO_U64(size); i++) {
+		ret = do_read_u64(ff, p + i);
+		if (ret < 0) {
+			free(set);
+			return ret;
+		}
+	}
+
+	*pset  = set;
+	*psize = size;
+	return 0;
+}
+
 static int write_tracing_data(struct feat_fd *ff,
 			      struct perf_evlist *evlist)
 {
@@ -1196,6 +1249,176 @@ static int write_sample_time(struct feat_fd *ff,
 			sizeof(evlist->last_sample_time));
 }
 
+
+static int memory_node__read(struct memory_node *n, unsigned long index)
+{
+	unsigned int phys, size = 0;
+	char path[PATH_MAX];
+	struct dirent *ent;
+	DIR *dir;
+
+#define for_each_memory(mem, dir)					\
+	while ((ent = readdir(dir)))					\
+		if (strcmp(ent->d_name, ".") &&				\
+		    strcmp(ent->d_name, "..") &&			\
+		    sscanf(ent->d_name, "memory%u", &mem) == 1)
+
+	scnprintf(path, PATH_MAX,
+		  "%s/devices/system/node/node%lu",
+		  sysfs__mountpoint(), index);
+
+	dir = opendir(path);
+	if (!dir) {
+		pr_warning("failed: cant' open memory sysfs data\n");
+		return -1;
+	}
+
+	for_each_memory(phys, dir) {
+		size = max(phys, size);
+	}
+
+	size++;
+
+	n->set = bitmap_alloc(size);
+	if (!n->set) {
+		closedir(dir);
+		return -ENOMEM;
+	}
+
+	bitmap_zero(n->set, size);
+	n->node = index;
+	n->size = size;
+
+	rewinddir(dir);
+
+	for_each_memory(phys, dir) {
+		set_bit(phys, n->set);
+	}
+
+	closedir(dir);
+	return 0;
+}
+
+static int memory_node__sort(const void *a, const void *b)
+{
+	const struct memory_node *na = a;
+	const struct memory_node *nb = b;
+
+	return na->node - nb->node;
+}
+
+static int build_mem_topology(struct memory_node *nodes, u64 size, u64 *cntp)
+{
+	char path[PATH_MAX];
+	struct dirent *ent;
+	DIR *dir;
+	u64 cnt = 0;
+	int ret = 0;
+
+	scnprintf(path, PATH_MAX, "%s/devices/system/node/",
+		  sysfs__mountpoint());
+
+	dir = opendir(path);
+	if (!dir) {
+		pr_warning("failed: can't open node sysfs data\n");
+		return -1;
+	}
+
+	while (!ret && (ent = readdir(dir))) {
+		unsigned int index;
+		int r;
+
+		if (!strcmp(ent->d_name, ".") ||
+		    !strcmp(ent->d_name, ".."))
+			continue;
+
+		r = sscanf(ent->d_name, "node%u", &index);
+		if (r != 1)
+			continue;
+
+		if (WARN_ONCE(cnt >= size,
+			      "failed to write MEM_TOPOLOGY, way too many nodes\n"))
+			return -1;
+
+		ret = memory_node__read(&nodes[cnt++], index);
+	}
+
+	*cntp = cnt;
+	closedir(dir);
+
+	if (!ret)
+		qsort(nodes, cnt, sizeof(nodes[0]), memory_node__sort);
+
+	return ret;
+}
+
+#define MAX_MEMORY_NODES 2000
+
+/*
+ * The MEM_TOPOLOGY holds physical memory map for every
+ * node in system. The format of data is as follows:
+ *
+ *  0 - version          | for future changes
+ *  8 - block_size_bytes | /sys/devices/system/memory/block_size_bytes
+ * 16 - count            | number of nodes
+ *
+ * For each node we store map of physical indexes for
+ * each node:
+ *
+ * 32 - node id          | node index
+ * 40 - size             | size of bitmap
+ * 48 - bitmap           | bitmap of memory indexes that belongs to node
+ */
+static int write_mem_topology(struct feat_fd *ff __maybe_unused,
+			      struct perf_evlist *evlist __maybe_unused)
+{
+	static struct memory_node nodes[MAX_MEMORY_NODES];
+	u64 bsize, version = 1, i, nr;
+	int ret;
+
+	ret = sysfs__read_xll("devices/system/memory/block_size_bytes",
+			      (unsigned long long *) &bsize);
+	if (ret)
+		return ret;
+
+	ret = build_mem_topology(&nodes[0], MAX_MEMORY_NODES, &nr);
+	if (ret)
+		return ret;
+
+	ret = do_write(ff, &version, sizeof(version));
+	if (ret < 0)
+		goto out;
+
+	ret = do_write(ff, &bsize, sizeof(bsize));
+	if (ret < 0)
+		goto out;
+
+	ret = do_write(ff, &nr, sizeof(nr));
+	if (ret < 0)
+		goto out;
+
+	for (i = 0; i < nr; i++) {
+		struct memory_node *n = &nodes[i];
+
+		#define _W(v)						\
+			ret = do_write(ff, &n->v, sizeof(n->v));	\
+			if (ret < 0)					\
+				goto out;
+
+		_W(node)
+		_W(size)
+
+		#undef _W
+
+		ret = do_write_bitmap(ff, n->set, n->size);
+		if (ret < 0)
+			goto out;
+	}
+
+out:
+	return ret;
+}
+
 static void print_hostname(struct feat_fd *ff, FILE *fp)
 {
 	fprintf(fp, "# hostname : %s\n", ff->ph->env.hostname);
@@ -1543,6 +1766,35 @@ static void print_sample_time(struct feat_fd *ff, FILE *fp)
 	fprintf(fp, "# sample duration : %10.3f ms\n", d);
 }
 
+static void memory_node__fprintf(struct memory_node *n,
+				 unsigned long long bsize, FILE *fp)
+{
+	char buf_map[100], buf_size[50];
+	unsigned long long size;
+
+	size = bsize * bitmap_weight(n->set, n->size);
+	unit_number__scnprintf(buf_size, 50, size);
+
+	bitmap_scnprintf(n->set, n->size, buf_map, 100);
+	fprintf(fp, "#  %3" PRIu64 " [%s]: %s\n", n->node, buf_size, buf_map);
+}
+
+static void print_mem_topology(struct feat_fd *ff, FILE *fp)
+{
+	struct memory_node *nodes;
+	int i, nr;
+
+	nodes = ff->ph->env.memory_nodes;
+	nr    = ff->ph->env.nr_memory_nodes;
+
+	fprintf(fp, "# memory nodes (nr %d, block size 0x%llx):\n",
+		nr, ff->ph->env.memory_bsize);
+
+	for (i = 0; i < nr; i++) {
+		memory_node__fprintf(&nodes[i], ff->ph->env.memory_bsize, fp);
+	}
+}
+
 static int __event_process_build_id(struct build_id_event *bev,
 				    char *filename,
 				    struct perf_session *session)
@@ -2205,6 +2457,58 @@ static int process_sample_time(struct feat_fd *ff, void *data __maybe_unused)
 	return 0;
 }
 
+static int process_mem_topology(struct feat_fd *ff,
+				void *data __maybe_unused)
+{
+	struct memory_node *nodes;
+	u64 version, i, nr, bsize;
+	int ret = -1;
+
+	if (do_read_u64(ff, &version))
+		return -1;
+
+	if (version != 1)
+		return -1;
+
+	if (do_read_u64(ff, &bsize))
+		return -1;
+
+	if (do_read_u64(ff, &nr))
+		return -1;
+
+	nodes = zalloc(sizeof(*nodes) * nr);
+	if (!nodes)
+		return -1;
+
+	for (i = 0; i < nr; i++) {
+		struct memory_node n;
+
+		#define _R(v)				\
+			if (do_read_u64(ff, &n.v))	\
+				goto out;		\
+
+		_R(node)
+		_R(size)
+
+		#undef _R
+
+		if (do_read_bitmap(ff, &n.set, &n.size))
+			goto out;
+
+		nodes[i] = n;
+	}
+
+	ff->ph->env.memory_bsize    = bsize;
+	ff->ph->env.memory_nodes    = nodes;
+	ff->ph->env.nr_memory_nodes = nr;
+	ret = 0;
+
+out:
+	if (ret)
+		free(nodes);
+	return ret;
+}
+
 struct feature_ops {
 	int (*write)(struct feat_fd *ff, struct perf_evlist *evlist);
 	void (*print)(struct feat_fd *ff, FILE *fp);
@@ -2263,6 +2567,7 @@ static const struct feature_ops feat_ops[HEADER_LAST_FEATURE] = {
 	FEAT_OPN(STAT,		stat,		false),
 	FEAT_OPN(CACHE,		cache,		true),
 	FEAT_OPR(SAMPLE_TIME,	sample_time,	false),
+	FEAT_OPR(MEM_TOPOLOGY,	mem_topology,	true),
 };
 
 struct header_print_data {
tools/perf/util/header.h +1 -0
index 942bdec6d70d..90d4577a92dc 100644
@@ -36,6 +36,7 @@ enum {
 	HEADER_STAT,
 	HEADER_CACHE,
 	HEADER_SAMPLE_TIME,
+	HEADER_MEM_TOPOLOGY,
 	HEADER_LAST_FEATURE,
 	HEADER_FEAT_BITS	= 256,
 };
Signature · 1 line
2.13.6
#17 lore raw

[tip:perf/core] perf tools: Add MEM_TOPOLOGY feature to perf data file

In reply to Jiri Olsa
perf tools: Add MEM_TOPOLOGY feature to perf data file
Commit-ID:  e2091cedd51bf5306bcd5dd498d2977abfe20e88
Gitweb:     https://git.kernel.org/tip/e2091cedd51bf5306bcd5dd498d2977abfe20e88
Author:     Jiri Olsa <[email protected]>
AuthorDate: Wed, 7 Mar 2018 16:50:08 +0100
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Thu, 8 Mar 2018 11:30:46 -0300

perf tools: Add MEM_TOPOLOGY feature to perf data file

Adding MEM_TOPOLOGY feature to perf data file,
that will carry physical memory map and its
node assignments.

The format of data in MEM_TOPOLOGY is as follows:

  0 - version          | for future changes
  8 - block_size_bytes | /sys/devices/system/memory/block_size_bytes
 16 - count            | number of nodes

For each node we store map of physical indexes for each node:

 32 - node id          | node index
 40 - size             | size of bitmap
 48 - bitmap           | bitmap of memory indexes that belongs to node
                       | /sys/devices/system/node/node<NODE>/memory<INDEX>

The MEM_TOPOLOGY could be displayed with following report command:

  $ perf report --header-only -I
  ...
  # memory nodes (nr 1, block size 0x8000000):
  #    0 [7G]: 0-23,32-69
Cc Alexander Shishkin <[email protected]>
Cc David Ahern <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Peter Zijlstra <[email protected]>
[ Rename 'index' to 'idx', as this breaks the build in rhel5, 6 and other systems where this is used by glibc headers ]
---
 tools/include/linux/bitmap.h |   2 +-
 tools/perf/util/env.h        |   9 ++
 tools/perf/util/header.c     | 305 +++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/header.h     |   1 +
 4 files changed, 316 insertions(+), 1 deletion(-)
tools/include/linux/bitmap.h +1 -1
index ca160270fdfa..63440cc8d618 100644
@@ -98,7 +98,7 @@ static inline int test_and_set_bit(int nr, unsigned long *addr)
 
 /**
  * bitmap_alloc - Allocate bitmap
- * @nr: Bit to set
+ * @nbits: Number of bits
  */
 static inline unsigned long *bitmap_alloc(int nbits)
 {
tools/perf/util/env.h +9 -0
index bf970f57dce0..c4ef2e523367 100644
@@ -27,6 +27,12 @@ struct numa_node {
 	struct cpu_map	*map;
 };
 
+struct memory_node {
+	u64		 node;
+	u64		 size;
+	unsigned long	*set;
+};
+
 struct perf_env {
 	char			*hostname;
 	char			*os_release;
@@ -43,6 +49,7 @@ struct perf_env {
 	int			nr_sibling_cores;
 	int			nr_sibling_threads;
 	int			nr_numa_nodes;
+	int			nr_memory_nodes;
 	int			nr_pmu_mappings;
 	int			nr_groups;
 	char			*cmdline;
@@ -54,6 +61,8 @@ struct perf_env {
 	struct cpu_cache_level	*caches;
 	int			 caches_cnt;
 	struct numa_node	*numa_nodes;
+	struct memory_node	*memory_nodes;
+	unsigned long long	 memory_bsize;
 };
 
 extern struct perf_env perf_env;
tools/perf/util/header.c +305 -0

353 lines not on this page · raw message

tools/perf/util/header.h +1 -0
index 942bdec6d70d..90d4577a92dc 100644
@@ -36,6 +36,7 @@ enum {
 	HEADER_STAT,
 	HEADER_CACHE,
 	HEADER_SAMPLE_TIME,
+	HEADER_MEM_TOPOLOGY,
 	HEADER_LAST_FEATURE,
 	HEADER_FEAT_BITS	= 256,
 };
author reviewer
#18 lore raw

PATCH 8/19 perf tools: Add mem2node object

In reply to Jiri Olsa

Adding mem2node object to allow the easy lookup
of the node for the physical address.

It has following interface:

  int  mem2node__init(struct mem2node *map, struct perf_env *env);
  void mem2node__exit(struct mem2node *map);
  int  mem2node__node(struct mem2node *map, u64 addr);

The mem2node__init initialize object from the perf data file MEM_TOPOLOGY feature data. Following calls to mem2node__node will return node number for given physical address. The mem2node__exit function frees the object.

---
 tools/perf/util/Build      |   1 +
 tools/perf/util/mem2node.c | 129 +++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/mem2node.h |  19 +++++++
 3 files changed, 149 insertions(+)
 create mode 100644 tools/perf/util/mem2node.c
 create mode 100644 tools/perf/util/mem2node.h
tools/perf/util/Build +1 -0
index ea0a452550b0..8052373bcd6a 100644
@@ -106,6 +106,7 @@ libperf-y += units.o
 libperf-y += time-utils.o
 libperf-y += expr-bison.o
 libperf-y += branch.o
+libperf-y += mem2node.o
 
 libperf-$(CONFIG_LIBBPF) += bpf-loader.o
 libperf-$(CONFIG_BPF_PROLOGUE) += bpf-prologue.o
tools/perf/util/mem2node.c new file +129 -0

129 lines not on this page · raw message

tools/perf/util/mem2node.h new file +19 -0
new file mode 100644
index 000000000000..e27333755a2c
@@ -0,0 +1,19 @@
+#ifndef __MEM2NODE_H
+#define __MEM2NODE_H
+
+#include <linux/rbtree.h>
+#include "env.h"
+
+struct entry;
+
+struct mem2node {
+	struct rb_root	 root;
+	struct entry	*entry;
+	int		 cnt;
+};
+
+int  mem2node__init(struct mem2node *map, struct perf_env *env);
+void mem2node__exit(struct mem2node *map);
+int  mem2node__node(struct mem2node *map, u64 addr);
+
+#endif /* __MEM2NODE_H */
Signature · 1 line
2.13.6
#19 lore raw

Re: [PATCH 08/19] perf tools: Add mem2node object

Em Wed, Mar 07, 2018 at 04:50:09PM +0100, Jiri Olsa escreveu:

Quoted 45 lines

Adding mem2node object to allow the easy lookup
of the node for the physical address.

It has following interface:

  int  mem2node__init(struct mem2node *map, struct perf_env *env);
  void mem2node__exit(struct mem2node *map);
  int  mem2node__node(struct mem2node *map, u64 addr);

The mem2node__init initialize object from the perf data file MEM_TOPOLOGY feature data. Following calls to mem2node__node will return node number for given physical address. The mem2node__exit function frees the object.

---
 tools/perf/util/Build      |   1 +
 tools/perf/util/mem2node.c | 129 +++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/mem2node.h |  19 +++++++
 3 files changed, 149 insertions(+)
 create mode 100644 tools/perf/util/mem2node.c
 create mode 100644 tools/perf/util/mem2node.h
tools/perf/util/Build +1 -0
index ea0a452550b0..8052373bcd6a 100644
@@ -106,6 +106,7 @@ libperf-y += units.o
 libperf-y += time-utils.o
 libperf-y += expr-bison.o
 libperf-y += branch.o
+libperf-y += mem2node.o
 
 libperf-$(CONFIG_LIBBPF) += bpf-loader.o
 libperf-$(CONFIG_BPF_PROLOGUE) += bpf-prologue.o
tools/perf/util/mem2node.c new file +2 -0
new file mode 100644
index 000000000000..6da8ddbb1182
@@ -0,0 +1,129 @@
+#include <errno.h>
+#include <inttypes.h>

tools/perf/util/mem2node.c line 12

Quoted 10 lines
tools/perf/util/mem2node.c new file +10 -0
+#include <linux/bitmap.h>
+#include "mem2node.h"
+#include "util.h"
+
+struct entry {
+	struct rb_node	rb_node;
+	u64	start;
+	u64	end;
+	u64	node;
+};

Hey, this name is way too generic, please rename it to something more descriptive

Quoted 29 lines
tools/perf/util/mem2node.c new file +29 -0
+
+static void entry__insert(struct entry *entry, struct rb_root *root)
+{
+	struct rb_node **p = &root->rb_node;
+	struct rb_node *parent = NULL;
+	struct entry *e;
+
+	while (*p != NULL) {
+		parent = *p;
+		e = rb_entry(parent, struct entry, rb_node);
+
+		if (entry->start < e->start)
+			p = &(*p)->rb_left;
+		else
+			p = &(*p)->rb_right;
+	}
+
+	rb_link_node(&entry->rb_node, parent, p);
+	rb_insert_color(&entry->rb_node, root);
+}
+
+int mem2node__init(struct mem2node *map, struct perf_env *env)
+{
+	struct memory_node *n, *nodes = &env->memory_nodes[0];
+	u64 bsize = env->memory_bsize;
+	struct entry *entry;
+	int i, j = 0, max = 0;
+
+	memset(map, 0x0, sizeof(*map));

tools/perf/util/mem2node.c line 51

Quoted 10 lines
tools/perf/util/mem2node.c new file +10 -0
+	map->root = RB_ROOT;
+
+	for (i = 0; i < env->nr_memory_nodes; i++) {
+		n = &nodes[i];
+		max += bitmap_weight(n->set, n->size);
+	}
+
+	entry = zalloc(sizeof(*entry) * max);
+	if (!entry)
+		return -ENOMEM;

Also this is not an 'entry', but max ones, please rename this variable to 'entries'.

Quoted 22 lines
tools/perf/util/mem2node.c new file +22 -0
+
+	for (i = 0; i < env->nr_memory_nodes; i++) {
+		u64 bit;
+
+		n = &nodes[i];
+
+		for (bit = 0; bit < n->size; bit++) {
+			u64 start;
+
+			if (!test_bit(bit, n->set))
+				continue;
+
+			start = bit * bsize;
+
+			/*
+			 * Merge nearby areas, we walk in order
+			 * through the bitmap, so no need to sort.
+			 */
+			if (j > 0) {
+				struct entry *prev = &entry[j - 1];
+
+				if ((prev->end == start) &&

tools/perf/util/mem2node.c line 83

Quoted 10 lines
tools/perf/util/mem2node.c new file +10 -0
+				    (prev->node == n->node)) {
+					prev->end += bsize;
+					continue;
+				}
+			}
+
+			entry[j].start = start;
+			entry[j].end   = start + bsize;
+			entry[j].node  = n->node;
+			RB_CLEAR_NODE(&entry[j].rb_node);

The previous four line could be done via the usual:

			krava_entry__init(&entries[j], start, bsize, n->node);

tools/perf/util/mem2node.c line 91

Quoted 8 lines
tools/perf/util/mem2node.c new file +8 -0
+			j++;
+		}
+	}
+
+	/* Cut unused entries, due to merging. */
+	entry = realloc(entry, sizeof(*entry) * j);
+	if (!entry)
+		return -ENOMEM;

Humm, so you lose it when not able to cut it short? Why not:

	nentries = realloc(entries, sizeof(entries[0) * j);
	if (nentries)
		entries = nentries;

And if you couldn't cut it (however unlikely that is) just go on and use what you have already.

Quoted 65 lines
tools/perf/util/mem2node.c new file +38 -0
+
+	for (i = 0; i < j; i++) {
+		pr_debug("mem2node %03" PRIu64 " [0x%016" PRIx64 "-0x%016" PRIx64 "]\n",
+			 entry[i].node, entry[i].start, entry[i].end);
+
+		entry__insert(&entry[i], &map->root);
+	}
+
+	map->entry = entry;
+	return 0;
+}
+
+void mem2node__exit(struct mem2node *map)
+{
+	zfree(&map->entry);
+}
+
+int mem2node__node(struct mem2node *map, u64 addr)
+{
+	struct rb_node **p, *parent = NULL;
+	struct entry *entry;
+
+	p = &map->root.rb_node;
+	while (*p != NULL) {
+		parent = *p;
+		entry = rb_entry(parent, struct entry, rb_node);
+		if (addr < entry->start)
+			p = &(*p)->rb_left;
+		else if (addr >= entry->end)
+			p = &(*p)->rb_right;
+		else
+			goto out;
+	}
+
+	entry = NULL;
+out:
+	return entry ? (int) entry->node : -1;
+}
tools/perf/util/mem2node.h new file +19 -0
new file mode 100644
index 000000000000..e27333755a2c
@@ -0,0 +1,19 @@
+#ifndef __MEM2NODE_H
+#define __MEM2NODE_H
+
+#include <linux/rbtree.h>
+#include "env.h"
+
+struct entry;
+
+struct mem2node {
+	struct rb_root	 root;
+	struct entry	*entry;
+	int		 cnt;
+};
+
+int  mem2node__init(struct mem2node *map, struct perf_env *env);
+void mem2node__exit(struct mem2node *map);
+int  mem2node__node(struct mem2node *map, u64 addr);
+
+#endif /* __MEM2NODE_H */
Signature · 1 line
2.13.6
#20 lore raw

Re: [PATCH 08/19] perf tools: Add mem2node object

On Wed, Mar 07, 2018 at 04:27:36PM -0300, Arnaldo Carvalho de Melo wrote:

SNIP

Quoted 13 lines
Quoted 13 lines
tools/perf/util/mem2node.c new file +7 -0
new file mode 100644
index 000000000000..6da8ddbb1182
@@ -0,0 +1,129 @@
+#include <errno.h>
+#include <inttypes.h>
+#include <linux/bitmap.h>
+#include "mem2node.h"
+#include "util.h"
+
+struct entry {
Quoted 8 lines

tools/perf/util/mem2node.c line 12

Quoted 5 lines
tools/perf/util/mem2node.c new file +5 -0
+	struct rb_node	rb_node;
+	u64	start;
+	u64	end;
+	u64	node;
+};

Hey, this name is way too generic, please rename it to something more descriptive

ok, will change

Quoted 35 lines
Quoted 35 lines
tools/perf/util/mem2node.c new file +34 -0
+
+static void entry__insert(struct entry *entry, struct rb_root *root)
+{
+	struct rb_node **p = &root->rb_node;
+	struct rb_node *parent = NULL;
+	struct entry *e;
+
+	while (*p != NULL) {
+		parent = *p;
+		e = rb_entry(parent, struct entry, rb_node);
+
+		if (entry->start < e->start)
+			p = &(*p)->rb_left;
+		else
+			p = &(*p)->rb_right;
+	}
+
+	rb_link_node(&entry->rb_node, parent, p);
+	rb_insert_color(&entry->rb_node, root);
+}
+
+int mem2node__init(struct mem2node *map, struct perf_env *env)
+{
+	struct memory_node *n, *nodes = &env->memory_nodes[0];
+	u64 bsize = env->memory_bsize;
+	struct entry *entry;
+	int i, j = 0, max = 0;
+
+	memset(map, 0x0, sizeof(*map));
+	map->root = RB_ROOT;
+
+	for (i = 0; i < env->nr_memory_nodes; i++) {
+		n = &nodes[i];
+		max += bitmap_weight(n->set, n->size);
Quoted 8 lines

tools/perf/util/mem2node.c line 51

Quoted 5 lines
tools/perf/util/mem2node.c new file +5 -0
+	}
+
+	entry = zalloc(sizeof(*entry) * max);
+	if (!entry)
+		return -ENOMEM;

Also this is not an 'entry', but max ones, please rename this variable to 'entries'.

ok

SNIP

Quoted 9 lines

tools/perf/util/mem2node.c line 83

Quoted 5 lines
tools/perf/util/mem2node.c new file +5 -0
+
+			entry[j].start = start;
+			entry[j].end   = start + bsize;
+			entry[j].node  = n->node;
+			RB_CLEAR_NODE(&entry[j].rb_node);

The previous four line could be done via the usual:

			krava_entry__init(&entries[j], start, bsize, n->node);

ook

Quoted 12 lines

tools/perf/util/mem2node.c line 91

Quoted 9 lines
tools/perf/util/mem2node.c new file +8 -0
+			j++;
+		}
+	}
+
+	/* Cut unused entries, due to merging. */
+	entry = realloc(entry, sizeof(*entry) * j);
+	if (!entry)
+		return -ENOMEM;

Humm, so you lose it when not able to cut it short? Why not:

just shortening the memory, because some entries merge together

Quoted 4 lines
	nentries = realloc(entries, sizeof(entries[0) * j);
	if (nentries)
		entries = nentries;

I don't think we need nentries.. AFAIK realloc works ok over single variable

jirka

#21 lore raw

Re: [PATCH 08/19] perf tools: Add mem2node object

Em Thu, Mar 08, 2018 at 12:03:07PM +0100, Jiri Olsa escreveu:

Quoted 106 lines · Arnaldo Carvalho de Melo

SNIP

Quoted 13 lines
Quoted 13 lines
tools/perf/util/mem2node.c new file +7 -0
new file mode 100644
index 000000000000..6da8ddbb1182
@@ -0,0 +1,129 @@
+#include <errno.h>
+#include <inttypes.h>
+#include <linux/bitmap.h>
+#include "mem2node.h"
+#include "util.h"
+
+struct entry {
Quoted 8 lines

tools/perf/util/mem2node.c line 12

Quoted 5 lines
tools/perf/util/mem2node.c new file +5 -0
+	struct rb_node	rb_node;
+	u64	start;
+	u64	end;
+	u64	node;
+};

Hey, this name is way too generic, please rename it to something more descriptive

ok, will change

Quoted 35 lines
Quoted 35 lines
tools/perf/util/mem2node.c new file +34 -0
+
+static void entry__insert(struct entry *entry, struct rb_root *root)
+{
+	struct rb_node **p = &root->rb_node;
+	struct rb_node *parent = NULL;
+	struct entry *e;
+
+	while (*p != NULL) {
+		parent = *p;
+		e = rb_entry(parent, struct entry, rb_node);
+
+		if (entry->start < e->start)
+			p = &(*p)->rb_left;
+		else
+			p = &(*p)->rb_right;
+	}
+
+	rb_link_node(&entry->rb_node, parent, p);
+	rb_insert_color(&entry->rb_node, root);
+}
+
+int mem2node__init(struct mem2node *map, struct perf_env *env)
+{
+	struct memory_node *n, *nodes = &env->memory_nodes[0];
+	u64 bsize = env->memory_bsize;
+	struct entry *entry;
+	int i, j = 0, max = 0;
+
+	memset(map, 0x0, sizeof(*map));
+	map->root = RB_ROOT;
+
+	for (i = 0; i < env->nr_memory_nodes; i++) {
+		n = &nodes[i];
+		max += bitmap_weight(n->set, n->size);
Quoted 8 lines

tools/perf/util/mem2node.c line 51

Quoted 5 lines
tools/perf/util/mem2node.c new file +5 -0
+	}
+
+	entry = zalloc(sizeof(*entry) * max);
+	if (!entry)
+		return -ENOMEM;

Also this is not an 'entry', but max ones, please rename this variable to 'entries'.

ok

SNIP

Quoted 9 lines

tools/perf/util/mem2node.c line 83

Quoted 5 lines
tools/perf/util/mem2node.c new file +5 -0
+
+			entry[j].start = start;
+			entry[j].end   = start + bsize;
+			entry[j].node  = n->node;
+			RB_CLEAR_NODE(&entry[j].rb_node);

The previous four line could be done via the usual:

			krava_entry__init(&entries[j], start, bsize, n->node);

ook

Quoted 12 lines

tools/perf/util/mem2node.c line 91

Quoted 9 lines
tools/perf/util/mem2node.c new file +8 -0
+			j++;
+		}
+	}
+
+	/* Cut unused entries, due to merging. */
+	entry = realloc(entry, sizeof(*entry) * j);
+	if (!entry)
+		return -ENOMEM;

Humm, so you lose it when not able to cut it short? Why not:

just shortening the memory, because some entries merge together

Quoted 1 line
Quoted 5 lines
Quoted 3 lines
	nentries = realloc(entries, sizeof(entries[0) * j);
	if (nentries)
		entries = nentries;

I don't think we need nentries.. AFAIK realloc works ok over single variable

So:

1) you alloc entries with a max number of entries

2) you go on populating it

3) there are some left, lets shrink it:

	entries = realloc(entries, nr_entries * sizeof(entries[0]);

Here it will probably not fail, but you check it anyway, and that is right, what happens if this returns NULL? entries gets set to NULL, we lose the reference to the allocated memory and you return -ENOMEM, right?

We end up leaking entries when what I'm suggesting you to do is to not clobber entries with the return of realloc() (doing it this way most of the time leads to bugs), but instead store it to a temp var (nentries), and if it succeeds, then you know that you can set nentries to entries and go ahead with your nicely shrunk block of memory.

If it fails, then you continue with the original block of memory, that continues to have what you just set up, etc.

Lemme look a third time to your original patch, I must be missing something...

- Arnaldo

#22 lore raw

Re: [PATCH 08/19] perf tools: Add mem2node object

Em Thu, Mar 08, 2018 at 09:58:49AM -0300, Arnaldo Carvalho de Melo escreveu:

Quoted 7 lines

We end up leaking entries when what I'm suggesting you to do is to not clobber entries with the return of realloc() (doing it this way most of the time leads to bugs), but instead store it to a temp var (nentries), and if it succeeds, then you know that you can set nentries to entries and go ahead with your nicely shrunk block of memory.

Quoted 5 lines

If it fails, then you continue with the original block of memory, that continues to have what you just set up, etc.

Lemme look a third time to your original patch, I must be missing something...

I don't think I'm missing anything ;-)

- Arnaldo

#23 lore raw

Re: [PATCH 08/19] perf tools: Add mem2node object

In reply to Arnaldo Carvalho de Melo

On Thu, Mar 08, 2018 at 09:58:49AM -0300, Arnaldo Carvalho de Melo wrote:

SNIP

Quoted 24 lines
Quoted 1 line

I don't think we need nentries.. AFAIK realloc works ok over single variable

So:

1) you alloc entries with a max number of entries

2) you go on populating it

3) there are some left, lets shrink it:

	entries = realloc(entries, nr_entries * sizeof(entries[0]);

Here it will probably not fail, but you check it anyway, and that is right, what happens if this returns NULL? entries gets set to NULL, we lose the reference to the allocated memory and you return -ENOMEM, right?

We end up leaking entries when what I'm suggesting you to do is to not clobber entries with the return of realloc() (doing it this way most of the time leads to bugs), but instead store it to a temp var (nentries), and if it succeeds, then you know that you can set nentries to entries and go ahead with your nicely shrunk block of memory.

Quoted 2 lines

If it fails, then you continue with the original block of memory, that continues to have what you just set up, etc.

ah that ;-) ok, will fix

thanks,
jirka

author reviewer
#24 lore raw

PATCH 9/19 perf tests: Add mem2node object test

In reply to Jiri Olsa

Adding mem2node object automated test.

The test prepares few artificial node - memory maps and verifies the mem2node object returns proper node values to given addresses.

---
 tools/perf/tests/Build          |  1 +
 tools/perf/tests/builtin-test.c |  4 +++
 tools/perf/tests/mem2node.c     | 73 +++++++++++++++++++++++++++++++++++++++++
 tools/perf/tests/tests.h        |  1 +
 4 files changed, 79 insertions(+)
 create mode 100644 tools/perf/tests/mem2node.c
tools/perf/tests/Build +1 -0
index 87bf3edb037c..45782220ac23 100644
@@ -47,6 +47,7 @@ perf-y += bitmap.o
 perf-y += perf-hooks.o
 perf-y += clang.o
 perf-y += unit_number__scnprintf.o
+perf-y += mem2node.o
 
 $(OUTPUT)tests/llvm-src-base.c: tests/bpf-script-example.c tests/Build
 	$(call rule_mkdir)
tools/perf/tests/builtin-test.c +4 -0
index fafa014240cd..09071ef4434f 100644
@@ -271,6 +271,10 @@ static struct test generic_tests[] = {
 		.func = test__unit_number__scnprint,
 	},
 	{
+		.desc = "mem2node",
+		.func = test__mem2node,
+	},
+	{
 		.func = NULL,
 	},
 };
tools/perf/tests/mem2node.c new file +73 -0

73 lines not on this page · raw message

tools/perf/tests/tests.h +1 -0
index 2862b80bc288..2e169819e647 100644
@@ -102,6 +102,7 @@ int test__clang(struct test *test, int subtest);
 const char *test__clang_subtest_get_desc(int subtest);
 int test__clang_subtest_get_nr(void);
 int test__unit_number__scnprint(struct test *test, int subtest);
+int test__mem2node(struct test *t, int subtest);
 
 bool test__bp_signal_is_supported(void);
 
Signature · 1 line
2.13.6
author reviewer
#25 lore raw

PATCH 10/19 perf c2c record: Record physical addresses in samples

In reply to Jiri Olsa

We are going to display NUMA node information in following patches. For this we need to have physical address data in the sample.

Adding --phys-data as a default option for perf c2c record.

---
 tools/perf/Documentation/perf-c2c.txt | 2 +-
 tools/perf/builtin-c2c.c              | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)
tools/perf/Documentation/perf-c2c.txt +1 -1
index 822414235170..095aebdc5bb7 100644
@@ -116,7 +116,7 @@ and calls standard perf record command.
 Following perf record options are configured by default:
 (check perf record man page for details)
 
-  -W,-d,--sample-cpu
+  -W,-d,--phys-data,--sample-cpu
 
 Unless specified otherwise with '-e' option, following events are monitored by
 default:
tools/perf/builtin-c2c.c +2 -1
index 98d243fa0c06..95765a1db903 100644
@@ -2704,7 +2704,7 @@ static int perf_c2c__record(int argc, const char **argv)
 	argc = parse_options(argc, argv, options, record_mem_usage,
 			     PARSE_OPT_KEEP_UNKNOWN);
 
-	rec_argc = argc + 10; /* max number of arguments */
+	rec_argc = argc + 11; /* max number of arguments */
 	rec_argv = calloc(rec_argc + 1, sizeof(char *));
 	if (!rec_argv)
 		return -1;
@@ -2720,6 +2720,7 @@ static int perf_c2c__record(int argc, const char **argv)
 		rec_argv[i++] = "-W";
 
 	rec_argv[i++] = "-d";
+	rec_argv[i++] = "--phys-data";
 	rec_argv[i++] = "--sample-cpu";
 
 	for (j = 0; j < PERF_MEM_EVENTS__MAX; j++) {
Signature · 1 line
2.13.6
author reviewer
#26 lore raw

PATCH 11/19 perf c2c report: Make calc_width work with struct c2c_hist_entry

In reply to Jiri Olsa

We are going to calculate column width based on the struct c2c_hist_entry data, so making calc_width to work with struct c2c_hist_entry.

---
 tools/perf/builtin-c2c.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)
tools/perf/builtin-c2c.c +9 -5
index 95765a1db903..43ce55550c9d 100644
@@ -1839,20 +1839,24 @@ static inline int valid_hitm_or_store(struct hist_entry *he)
 	return has_hitm || c2c_he->stats.store;
 }
 
-static void calc_width(struct hist_entry *he)
+static void calc_width(struct c2c_hist_entry *c2c_he)
 {
 	struct c2c_hists *c2c_hists;
 
-	c2c_hists = container_of(he->hists, struct c2c_hists, hists);
-	hists__calc_col_len(&c2c_hists->hists, he);
+	c2c_hists = container_of(c2c_he->he.hists, struct c2c_hists, hists);
+	hists__calc_col_len(&c2c_hists->hists, &c2c_he->he);
 }
 
 static int filter_cb(struct hist_entry *he)
 {
+	struct c2c_hist_entry *c2c_he;
+
+	c2c_he = container_of(he, struct c2c_hist_entry, he);
+
 	if (c2c.show_src && !he->srcline)
 		he->srcline = hist_entry__get_srcline(he);
 
-	calc_width(he);
+	calc_width(c2c_he);
 
 	if (!valid_hitm_or_store(he))
 		he->filtered = HIST_FILTER__C2C;
@@ -1869,7 +1873,7 @@ static int resort_cl_cb(struct hist_entry *he)
 	c2c_he = container_of(he, struct c2c_hist_entry, he);
 	c2c_hists = c2c_he->hists;
 
-	calc_width(he);
+	calc_width(c2c_he);
 
 	if (display && c2c_hists) {
 		static unsigned int idx;
Signature · 1 line
2.13.6
author reviewer
#27 lore raw

PATCH 12/19 perf c2c report: Call calc_width only for displayed entries

In reply to Jiri Olsa

There's no need to calculate column width on entries that are not going to be displayed.

---
 tools/perf/builtin-c2c.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
tools/perf/builtin-c2c.c +1 -2
index 43ce55550c9d..821112e8ba97 100644
@@ -1873,12 +1873,11 @@ static int resort_cl_cb(struct hist_entry *he)
 	c2c_he = container_of(he, struct c2c_hist_entry, he);
 	c2c_hists = c2c_he->hists;
 
-	calc_width(c2c_he);
-
 	if (display && c2c_hists) {
 		static unsigned int idx;
 
 		c2c_he->cacheline_idx = idx++;
+		calc_width(c2c_he);
 
 		c2c_hists__reinit(c2c_hists, c2c.cl_output, c2c.cl_resort);
 
Signature · 1 line
2.13.6
author reviewer
#28 lore raw

PATCH 13/19 perf c2c report: Display node for cacheline address

In reply to Jiri Olsa

Adding the NUMA node info for the data cacheline. Adding the new column to both Shared Data Cache Line Table and Shared Cache Line Distribution Pareto.

Note the new 'Node' column next to the 'Cacheline'.

  $ perf c2c report --stdio
  =================================================
             Shared Data Cache Line Table
  =================================================
  #
  #                                    Total      Tot  ----- LLC Load Hitm -----
  # Index           Cacheline  Node  records     Hitm    Total      Lcl      Rmt
  # .....  ..................  ....  .......  .......  .......  .......  .......
  #
        0      0x7f0830100000     0       84   10.53%        8        8        0
        1  0xffff922a93154200     0        3    2.63%        2        2        0
        2  0xffff922a93154500     0        4    2.63%        2        2        0
  ...

Note the new 'Node' column next to the 'Offset'.

=================================================
Shared Cache Line Distribution Pareto
=================================================
#
# ----- HITM ----- -- Store Refs -- Data address

  #   Num      Rmt      Lcl   L1 Hit  L1 Miss              Offset  Node      Pid
  # .....  .......  .......  .......  .......  ..................  ....  .......
  #
    -------------------------------------------------------------
        0        0        8       32        2      0x7f0830100000
    -------------------------------------------------------------
             0.00%   75.00%   21.88%    0.00%                0x18     0     1791
             0.00%   12.50%   37.50%    0.00%                0x18     0     1791
             0.00%    0.00%   34.38%    0.00%                0x18     0     1791

Using the mem2node object to get the NUMA node data.

---
 tools/perf/builtin-c2c.c | 119 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 114 insertions(+), 5 deletions(-)
tools/perf/builtin-c2c.c +114 -5
index 821112e8ba97..45c047fdd7ac 100644
@@ -32,6 +32,7 @@
 #include "evsel.h"
 #include "ui/browsers/hists.h"
 #include "thread.h"
+#include "mem2node.h"
 
 struct c2c_hists {
 	struct hists		hists;
@@ -49,6 +50,7 @@ struct c2c_hist_entry {
 	struct c2c_hists	*hists;
 	struct c2c_stats	 stats;
 	unsigned long		*cpuset;
+	unsigned long		*nodeset;
 	struct c2c_stats	*node_stats;
 	unsigned int		 cacheline_idx;
 
@@ -59,6 +61,11 @@ struct c2c_hist_entry {
 	 * because of its callchain dynamic entry
 	 */
 	struct hist_entry	he;
+
+	unsigned long		 paddr;
+	unsigned long		 paddr_cnt;
+	bool			 paddr_zero;
+	char			*nodestr;
 };
 
 static char const *coalesce_default = "pid,iaddr";
@@ -66,6 +73,7 @@ static char const *coalesce_default = "pid,iaddr";
 struct perf_c2c {
 	struct perf_tool	tool;
 	struct c2c_hists	hists;
+	struct mem2node		mem2node;
 
 	unsigned long		**nodes;
 	int			 nodes_cnt;
@@ -123,6 +131,10 @@ static void *c2c_he_zalloc(size_t size)
 	if (!c2c_he->cpuset)
 		return NULL;
 
+	c2c_he->nodeset = bitmap_alloc(c2c.nodes_cnt);
+	if (!c2c_he->nodeset)
+		return NULL;
+
 	c2c_he->node_stats = zalloc(c2c.nodes_cnt * sizeof(*c2c_he->node_stats));
 	if (!c2c_he->node_stats)
 		return NULL;
@@ -145,6 +157,8 @@ static void c2c_he_free(void *he)
 	}
 
 	free(c2c_he->cpuset);
+	free(c2c_he->nodeset);
+	free(c2c_he->nodestr);
 	free(c2c_he->node_stats);
 	free(c2c_he);
 }
@@ -194,6 +208,28 @@ static void c2c_he__set_cpu(struct c2c_hist_entry *c2c_he,
 	set_bit(sample->cpu, c2c_he->cpuset);
 }
 
+static void c2c_he__set_node(struct c2c_hist_entry *c2c_he,
+			     struct perf_sample *sample)
+{
+	int node;
+
+	if (!sample->phys_addr) {
+		c2c_he->paddr_zero = true;
+		return;
+	}
+
+	node = mem2node__node(&c2c.mem2node, sample->phys_addr);
+	if (WARN_ONCE(node < 0, "WARNING: failed to find node\n"))
+		return;
+
+	set_bit(node, c2c_he->nodeset);
+
+	if (c2c_he->paddr != sample->phys_addr) {
+		c2c_he->paddr_cnt++;
+		c2c_he->paddr = sample->phys_addr;
+	}
+}
+
 static void compute_stats(struct c2c_hist_entry *c2c_he,
 			  struct c2c_stats *stats,
 			  u64 weight)
@@ -257,6 +293,7 @@ static int process_sample_event(struct perf_tool *tool __maybe_unused,
 	c2c_add_stats(&c2c_hists->stats, &stats);
 
 	c2c_he__set_cpu(c2c_he, sample);
+	c2c_he__set_node(c2c_he, sample);
 
 	hists__inc_nr_samples(&c2c_hists->hists, he->filtered);
 	ret = hist_entry__append_callchain(he, sample);
@@ -293,6 +330,7 @@ static int process_sample_event(struct perf_tool *tool __maybe_unused,
 		compute_stats(c2c_he, &stats, sample->weight);
 
 		c2c_he__set_cpu(c2c_he, sample);
+		c2c_he__set_node(c2c_he, sample);
 
 		hists__inc_nr_samples(&c2c_hists->hists, he->filtered);
 		ret = hist_entry__append_callchain(he, sample);
@@ -455,6 +493,20 @@ static int dcacheline_entry(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
 	return scnprintf(hpp->buf, hpp->size, "%*s", width, HEX_STR(buf, addr));
 }
 
+static int
+dcacheline_node_entry(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
+		      struct hist_entry *he)
+{
+	struct c2c_hist_entry *c2c_he;
+	int width = c2c_width(fmt, hpp, he->hists);
+
+	c2c_he = container_of(he, struct c2c_hist_entry, he);
+	if (WARN_ON_ONCE(!c2c_he->nodestr))
+		return 0;
+
+	return scnprintf(hpp->buf, hpp->size, "%*s", width, c2c_he->nodestr);
+}
+
 static int offset_entry(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
 			struct hist_entry *he)
 {
@@ -1207,6 +1259,14 @@ static struct c2c_dimension dim_dcacheline = {
 	.width		= 18,
 };
 
+static struct c2c_dimension dim_dcacheline_node = {
+	.header		= HEADER_LOW("Node"),
+	.name		= "dcacheline_node",
+	.cmp		= empty_cmp,
+	.entry		= dcacheline_node_entry,
+	.width		= 4,
+};
+
 static struct c2c_header header_offset_tui = HEADER_LOW("Off");
 
 static struct c2c_dimension dim_offset = {
@@ -1217,6 +1277,14 @@ static struct c2c_dimension dim_offset = {
 	.width		= 18,
 };
 
+static struct c2c_dimension dim_offset_node = {
+	.header		= HEADER_LOW("Node"),
+	.name		= "offset_node",
+	.cmp		= empty_cmp,
+	.entry		= dcacheline_node_entry,
+	.width		= 4,
+};
+
 static struct c2c_dimension dim_iaddr = {
 	.header		= HEADER_LOW("Code address"),
 	.name		= "iaddr",
@@ -1536,7 +1604,9 @@ static struct c2c_dimension dim_dcacheline_num_empty = {
 
 static struct c2c_dimension *dimensions[] = {
 	&dim_dcacheline,
+	&dim_dcacheline_node,
 	&dim_offset,
+	&dim_offset_node,
 	&dim_iaddr,
 	&dim_tot_hitm,
 	&dim_lcl_hitm,
@@ -1839,12 +1909,44 @@ static inline int valid_hitm_or_store(struct hist_entry *he)
 	return has_hitm || c2c_he->stats.store;
 }
 
+static void set_node_width(struct c2c_hist_entry *c2c_he, int len)
+{
+	struct c2c_dimension *dim;
+
+	dim = &c2c.hists == c2c_he->hists ?
+	      &dim_dcacheline_node : &dim_offset_node;
+
+	if (len > dim->width)
+		dim->width = len;
+}
+
+static int set_nodestr(struct c2c_hist_entry *c2c_he)
+{
+	char buf[30];
+	int len;
+
+	if (c2c_he->nodestr)
+		return 0;
+
+	if (bitmap_weight(c2c_he->nodeset, c2c.nodes_cnt)) {
+		len = bitmap_scnprintf(c2c_he->nodeset, c2c.nodes_cnt,
+				      buf, sizeof(buf));
+	} else {
+		len = scnprintf(buf, sizeof(buf), "N/A");
+	}
+
+	set_node_width(c2c_he, len);
+	c2c_he->nodestr = strdup(buf);
+	return c2c_he->nodestr ? 0 : -ENOMEM;
+}
+
 static void calc_width(struct c2c_hist_entry *c2c_he)
 {
 	struct c2c_hists *c2c_hists;
 
 	c2c_hists = container_of(c2c_he->he.hists, struct c2c_hists, hists);
 	hists__calc_col_len(&c2c_hists->hists, &c2c_he->he);
+	set_nodestr(c2c_he);
 }
 
 static int filter_cb(struct hist_entry *he)
@@ -2474,7 +2576,7 @@ static int build_cl_output(char *cl_sort, bool no_source)
 		"percent_lcl_hitm,"
 		"percent_stores_l1hit,"
 		"percent_stores_l1miss,"
-		"offset,",
+		"offset,offset_node,",
 		add_pid   ? "pid," : "",
 		add_tid   ? "tid," : "",
 		add_iaddr ? "iaddr," : "",
@@ -2603,17 +2705,21 @@ static int perf_c2c__report(int argc, const char **argv)
 		goto out;
 	}
 
-	err = setup_callchain(session->evlist);
+	err = mem2node__init(&c2c.mem2node, &session->header.env);
 	if (err)
 		goto out_session;
 
+	err = setup_callchain(session->evlist);
+	if (err)
+		goto out_mem2node;
+
 	if (symbol__init(&session->header.env) < 0)
-		goto out_session;
+		goto out_mem2node;
 
 	/* No pipe support at the moment. */
 	if (perf_data__is_pipe(session->data)) {
 		pr_debug("No pipe support at the moment.\n");
-		goto out_session;
+		goto out_mem2node;
 	}
 
 	if (c2c.use_stdio)
@@ -2626,12 +2732,13 @@ static int perf_c2c__report(int argc, const char **argv)
 	err = perf_session__process_events(session);
 	if (err) {
 		pr_err("failed to process sample\n");
-		goto out_session;
+		goto out_mem2node;
 	}
 
 	c2c_hists__reinit(&c2c.hists,
 			"cl_idx,"
 			"dcacheline,"
+			"dcacheline_node,"
 			"tot_recs,"
 			"percent_hitm,"
 			"tot_hitm,lcl_hitm,rmt_hitm,"
@@ -2657,6 +2764,8 @@ static int perf_c2c__report(int argc, const char **argv)
 
 	perf_c2c_display(session);
 
+out_mem2node:
+	mem2node__exit(&c2c.mem2node);
 out_session:
 	perf_session__delete(session);
 out:
Signature · 1 line
2.13.6
author reviewer
#29 lore raw

PATCH 14/19 perf c2c report: Add span header over cacheline data

In reply to Jiri Olsa

Forcing the NUMA node output to be grouped with the Cacheline column in both Shared Data Cache Line Table and Shared Cache Line Distribution Pareto.

Before:
# Total Tot ----- LLC Load Hitm -----

  # Index           Cacheline  Node  records     Hitm    Total      Lcl      Rmt
  # .....  ..................  ....  .......  .......  .......  .......  .......
  #
        0      0x7f0830100000     0       84   10.53%        8        8        0
        1  0xffff922a93154200     0        3    2.63%        2        2        0
        2  0xffff922a93154500     0        4    2.63%        2        2        0

After:
# ------- Cacheline ------ Total Tot ----- LLC Load Hitm -----

  # Index             Address  Node  records     Hitm    Total      Lcl      Rmt
  # .....  ..................  ....  .......  .......  .......  .......  .......
  #
        0      0x7f0830100000     0       84   10.53%        8        8        0
        1  0xffff922a93154200     0        3    2.63%        2        2        0
        2  0xffff922a93154500     0        4    2.63%        2        2        0

Before:
# ----- HITM ----- -- Store Refs -- Data address

  #   Num      Rmt      Lcl   L1 Hit  L1 Miss              Offset  Node      Pid
  # .....  .......  .......  .......  .......  ..................  ....  .......
  #
    -------------------------------------------------------------
        0        0        8       32        2      0x7f0830100000
    -------------------------------------------------------------
             0.00%   75.00%   21.88%    0.00%                0x18     0     1791
             0.00%   12.50%   37.50%    0.00%                0x18     0     1791
             0.00%    0.00%   34.38%    0.00%                0x18     0     1791

After:
# ----- HITM ----- -- Store Refs -- ----- Data address -----

  #   Num      Rmt      Lcl   L1 Hit  L1 Miss              Offset  Node      Pid
  # .....  .......  .......  .......  .......  ..................  ....  .......
  #
    -------------------------------------------------------------
        0        0        8       32        2      0x7f0830100000
    -------------------------------------------------------------
             0.00%   75.00%   21.88%    0.00%                0x18     0     1791
             0.00%   12.50%   37.50%    0.00%                0x18     0     1791
             0.00%    0.00%   34.38%    0.00%                0x18     0     1791
---
 tools/perf/builtin-c2c.c | 65 ++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 60 insertions(+), 5 deletions(-)
tools/perf/builtin-c2c.c +60 -5
index 45c047fdd7ac..3790a7e0c51d 100644
@@ -1252,7 +1252,7 @@ cl_idx_empty_entry(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
 	}
 
 static struct c2c_dimension dim_dcacheline = {
-	.header		= HEADER_LOW("Cacheline"),
+	.header		= HEADER_SPAN("--- Cacheline ----", "Address", 1),
 	.name		= "dcacheline",
 	.cmp		= dcacheline_cmp,
 	.entry		= dcacheline_entry,
@@ -1267,10 +1267,10 @@ static struct c2c_dimension dim_dcacheline_node = {
 	.width		= 4,
 };
 
-static struct c2c_header header_offset_tui = HEADER_LOW("Off");
+static struct c2c_header header_offset_tui = HEADER_SPAN("-----", "Off", 1);
 
 static struct c2c_dimension dim_offset = {
-	.header		= HEADER_BOTH("Data address", "Offset"),
+	.header		= HEADER_SPAN("--- Data address -", "Offset", 1),
 	.name		= "offset",
 	.cmp		= offset_cmp,
 	.entry		= offset_entry,
@@ -2453,14 +2453,66 @@ static void perf_c2c_display(struct perf_session *session)
 }
 #endif /* HAVE_SLANG_SUPPORT */
 
-static void ui_quirks(void)
+static char *fill_line(const char *orig, int len)
 {
+	int i, j, olen = strlen(orig);
+	char *buf;
+
+	buf = zalloc(len + 1);
+	if (!buf)
+		return NULL;
+
+	j = len / 2 - olen / 2;
+
+	for (i = 0; i < j - 1; i++) {
+		buf[i] = '-';
+	}
+
+	buf[i++] = ' ';
+
+	strcpy(buf + i, orig);
+
+	i += olen;
+
+	buf[i++] = ' ';
+
+	for (; i < len; i++) {
+		buf[i] = '-';
+	}
+
+	return buf;
+}
+
+static int ui_quirks(void)
+{
+	const char *nodestr = "Data address";
+	char *buf;
+
 	if (!c2c.use_stdio) {
 		dim_offset.width  = 5;
 		dim_offset.header = header_offset_tui;
+		nodestr = "CL";
 	}
 
 	dim_percent_hitm.header = percent_hitm_header[c2c.display];
+
+	/* Fix the zero line for dcacheline column. */
+	buf = fill_line("Cacheline", dim_dcacheline.width +
+				     dim_dcacheline_node.width + 2);
+	if (!buf)
+		return -ENOMEM;
+
+	dim_dcacheline.header.line[0].text = buf;
+
+	/* Fix the zero line for offset column. */
+	buf = fill_line(nodestr, dim_offset.width +
+			      dim_offset_node.width + 2);
+	if (!buf)
+		return -ENOMEM;
+
+	dim_offset.header.line[0].text = buf;
+
+	return 0;
 }
 
 #define CALLCHAIN_DEFAULT_OPT  "graph,0.5,caller,function,percent"
@@ -2760,7 +2812,10 @@ static int perf_c2c__report(int argc, const char **argv)
 
 	ui_progress__finish();
 
-	ui_quirks();
+	if (ui_quirks()) {
+		pr_err("failed to setup UI\n");
+		goto out_mem2node;
+	}
 
 	perf_c2c_display(session);
 
Signature · 1 line
2.13.6
author reviewer
#30 lore raw

PATCH 15/19 perf c2c report: Add cacheline address count column

In reply to Jiri Olsa

Adding the 'PA cnt' column grouped under data cacheline address.

It shows how many times the physical addresses changed for the hist entry. It does not show the number of different physical addresses for entry, because we don't store those. We only track the number of times we got different address than we currently hold, which is not expensive and gives similar info.

  $ perf c2c report --stdio

# ----------- Cacheline ---------- Total Tot ----- LLC Load Hitm -----

  # Index             Address  Node  PA cnt  records     Hitm    Total      Lcl      Rmt
  # .....  ..................  ....  ......  .......  .......  .......  .......  .......
  #
        0  0xffff9ad56dca0a80     0       9       10    7.69%        2        2        0
        1  0xffff9ad56dce0a80     0       9        9    7.69%        2        2        0
        2  0xffff9ad37659ad80     0       1        2    3.85%        1        1        0

...

# ----- HITM ----- -- Store Refs -- --------- Data address ---------

  #   Num      Rmt      Lcl   L1 Hit  L1 Miss              Offset  Node  PA cnt      Pid
  # .....  .......  .......  .......  .......  ..................  ....  ......  .......
  #
    -------------------------------------------------------------
        0        0        2        3        0  0xffff9ad56dca0a80
    -------------------------------------------------------------
             0.00%    0.00%   33.33%    0.00%                 0x0     0       1     2510
             0.00%    0.00%   33.33%    0.00%                 0x4     0       1     2476
             0.00%    0.00%   33.33%    0.00%                0x20     0       1        0
             0.00%  100.00%    0.00%    0.00%                0x38     0       1        0
---
 tools/perf/builtin-c2c.c | 35 +++++++++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 6 deletions(-)
tools/perf/builtin-c2c.c +29 -6
index 3790a7e0c51d..5a0f5067dfe9 100644
@@ -507,6 +507,17 @@ dcacheline_node_entry(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
 	return scnprintf(hpp->buf, hpp->size, "%*s", width, c2c_he->nodestr);
 }
 
+static int
+dcacheline_node_count(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
+		      struct hist_entry *he)
+{
+	struct c2c_hist_entry *c2c_he;
+	int width = c2c_width(fmt, hpp, he->hists);
+
+	c2c_he = container_of(he, struct c2c_hist_entry, he);
+	return scnprintf(hpp->buf, hpp->size, "%*lu", width, c2c_he->paddr_cnt);
+}
+
 static int offset_entry(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
 			struct hist_entry *he)
 {
@@ -1252,7 +1263,7 @@ cl_idx_empty_entry(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
 	}
 
 static struct c2c_dimension dim_dcacheline = {
-	.header		= HEADER_SPAN("--- Cacheline ----", "Address", 1),
+	.header		= HEADER_SPAN("--- Cacheline ----", "Address", 2),
 	.name		= "dcacheline",
 	.cmp		= dcacheline_cmp,
 	.entry		= dcacheline_entry,
@@ -1267,10 +1278,18 @@ static struct c2c_dimension dim_dcacheline_node = {
 	.width		= 4,
 };
 
-static struct c2c_header header_offset_tui = HEADER_SPAN("-----", "Off", 1);
+static struct c2c_dimension dim_dcacheline_count = {
+	.header		= HEADER_LOW("PA cnt"),
+	.name		= "dcacheline_count",
+	.cmp		= empty_cmp,
+	.entry		= dcacheline_node_count,
+	.width		= 6,
+};
+
+static struct c2c_header header_offset_tui = HEADER_SPAN("-----", "Off", 2);
 
 static struct c2c_dimension dim_offset = {
-	.header		= HEADER_SPAN("--- Data address -", "Offset", 1),
+	.header		= HEADER_SPAN("--- Data address -", "Offset", 2),
 	.name		= "offset",
 	.cmp		= offset_cmp,
 	.entry		= offset_entry,
@@ -1605,6 +1624,7 @@ static struct c2c_dimension dim_dcacheline_num_empty = {
 static struct c2c_dimension *dimensions[] = {
 	&dim_dcacheline,
 	&dim_dcacheline_node,
+	&dim_dcacheline_count,
 	&dim_offset,
 	&dim_offset_node,
 	&dim_iaddr,
@@ -2498,7 +2518,8 @@ static int ui_quirks(void)
 
 	/* Fix the zero line for dcacheline column. */
 	buf = fill_line("Cacheline", dim_dcacheline.width +
-				     dim_dcacheline_node.width + 2);
+				     dim_dcacheline_node.width +
+				     dim_dcacheline_count.width + 4);
 	if (!buf)
 		return -ENOMEM;
 
@@ -2506,7 +2527,8 @@ static int ui_quirks(void)
 
 	/* Fix the zero line for offset column. */
 	buf = fill_line(nodestr, dim_offset.width +
-			      dim_offset_node.width + 2);
+			         dim_offset_node.width +
+				 dim_dcacheline_count.width + 4);
 	if (!buf)
 		return -ENOMEM;
 
@@ -2628,7 +2650,7 @@ static int build_cl_output(char *cl_sort, bool no_source)
 		"percent_lcl_hitm,"
 		"percent_stores_l1hit,"
 		"percent_stores_l1miss,"
-		"offset,offset_node,",
+		"offset,offset_node,dcacheline_count,",
 		add_pid   ? "pid," : "",
 		add_tid   ? "tid," : "",
 		add_iaddr ? "iaddr," : "",
@@ -2791,6 +2813,7 @@ static int perf_c2c__report(int argc, const char **argv)
 			"cl_idx,"
 			"dcacheline,"
 			"dcacheline_node,"
+			"dcacheline_count,"
 			"tot_recs,"
 			"percent_hitm,"
 			"tot_hitm,lcl_hitm,rmt_hitm,"
Signature · 1 line
2.13.6
author reviewer
#31 lore raw

PATCH 16/19 perf tools: Update tags with .cpp files

In reply to Jiri Olsa

We have some .cpp files, make ctags/cscope aware of them.

---
 tools/perf/Makefile.perf | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
tools/perf/Makefile.perf +3 -3
index 4679e237a7f5..f7517e1b73f8 100644
@@ -708,15 +708,15 @@ TAG_FILES= ../../include/uapi/linux/perf_event.h
 
 TAGS:
 	$(QUIET_GEN)$(RM) TAGS; \
-	$(FIND) $(TAG_FOLDERS) -name '*.[hcS]' -print | xargs etags -a $(TAG_FILES)
+	$(FIND) $(TAG_FOLDERS) -name '*.[hcS]' -print -o -name '*.cpp' -print | xargs etags -a $(TAG_FILES)
 
 tags:
 	$(QUIET_GEN)$(RM) tags; \
-	$(FIND) $(TAG_FOLDERS) -name '*.[hcS]' -print | xargs ctags -a $(TAG_FILES)
+	$(FIND) $(TAG_FOLDERS) -name '*.[hcS]' -print -o -name '*.cpp' -print | xargs ctags -a $(TAG_FILES)
 
 cscope:
 	$(QUIET_GEN)$(RM) cscope*; \
-	$(FIND) $(TAG_FOLDERS) -name '*.[hcS]' -print | xargs cscope -b $(TAG_FILES)
+	$(FIND) $(TAG_FOLDERS) -name '*.[hcS]' -print -o -name '*.cpp' -print | xargs cscope -b $(TAG_FILES)
 
 ### Testing rules
 
Signature · 1 line
2.13.6
maintainer Applied
#32 lore raw

Re: [PATCH 16/19] perf tools: Update tags with .cpp files

Em Wed, Mar 07, 2018 at 04:50:17PM +0100, Jiri Olsa escreveu:

Quoted 1 line

We have some .cpp files, make ctags/cscope aware of them.

Applied

Quoted 31 lines
---
 tools/perf/Makefile.perf | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
tools/perf/Makefile.perf +3 -3
index 4679e237a7f5..f7517e1b73f8 100644
@@ -708,15 +708,15 @@ TAG_FILES= ../../include/uapi/linux/perf_event.h
 
 TAGS:
 	$(QUIET_GEN)$(RM) TAGS; \
-	$(FIND) $(TAG_FOLDERS) -name '*.[hcS]' -print | xargs etags -a $(TAG_FILES)
+	$(FIND) $(TAG_FOLDERS) -name '*.[hcS]' -print -o -name '*.cpp' -print | xargs etags -a $(TAG_FILES)
 
 tags:
 	$(QUIET_GEN)$(RM) tags; \
-	$(FIND) $(TAG_FOLDERS) -name '*.[hcS]' -print | xargs ctags -a $(TAG_FILES)
+	$(FIND) $(TAG_FOLDERS) -name '*.[hcS]' -print -o -name '*.cpp' -print | xargs ctags -a $(TAG_FILES)
 
 cscope:
 	$(QUIET_GEN)$(RM) cscope*; \
-	$(FIND) $(TAG_FOLDERS) -name '*.[hcS]' -print | xargs cscope -b $(TAG_FILES)
+	$(FIND) $(TAG_FOLDERS) -name '*.[hcS]' -print -o -name '*.cpp' -print | xargs cscope -b $(TAG_FILES)
 
 ### Testing rules
 
Signature · 1 line
2.13.6
#33 lore raw

[tip:perf/core] perf tools: Update tags with .cpp files

In reply to Jiri Olsa
perf tools: Update tags with .cpp files
Commit-ID:  ed3956293f2913047c622c238b5bf7ff4fe250bf
Gitweb:     https://git.kernel.org/tip/ed3956293f2913047c622c238b5bf7ff4fe250bf
Author:     Jiri Olsa <[email protected]>
AuthorDate: Wed, 7 Mar 2018 16:50:17 +0100
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Thu, 8 Mar 2018 11:30:47 -0300

perf tools: Update tags with .cpp files

We have some .cpp files, make ctags/cscope aware of them.

---
 tools/perf/Makefile.perf | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
tools/perf/Makefile.perf +3 -3
index 4679e237a7f5..f7517e1b73f8 100644
@@ -708,15 +708,15 @@ TAG_FILES= ../../include/uapi/linux/perf_event.h
 
 TAGS:
 	$(QUIET_GEN)$(RM) TAGS; \
-	$(FIND) $(TAG_FOLDERS) -name '*.[hcS]' -print | xargs etags -a $(TAG_FILES)
+	$(FIND) $(TAG_FOLDERS) -name '*.[hcS]' -print -o -name '*.cpp' -print | xargs etags -a $(TAG_FILES)
 
 tags:
 	$(QUIET_GEN)$(RM) tags; \
-	$(FIND) $(TAG_FOLDERS) -name '*.[hcS]' -print | xargs ctags -a $(TAG_FILES)
+	$(FIND) $(TAG_FOLDERS) -name '*.[hcS]' -print -o -name '*.cpp' -print | xargs ctags -a $(TAG_FILES)
 
 cscope:
 	$(QUIET_GEN)$(RM) cscope*; \
-	$(FIND) $(TAG_FOLDERS) -name '*.[hcS]' -print | xargs cscope -b $(TAG_FILES)
+	$(FIND) $(TAG_FOLDERS) -name '*.[hcS]' -print -o -name '*.cpp' -print | xargs cscope -b $(TAG_FILES)
 
 ### Testing rules
 
author reviewer
#34 lore raw

PATCH 17/19 perf build: Add llvm/clang/cxx make tests into FEATURE_TESTS_EXTRA

In reply to Jiri Olsa

So we can see the status when we build perf, like:

  $ make LIBCLANGLLVM=1 VF=1
  ...                           cxx: [ on  ]
  ...                          llvm: [ on  ]
  ...                  llvm-version: [ on  ]
  ...                         clang: [ on  ]
---
 tools/build/Makefile.feature | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
tools/build/Makefile.feature +5 -1
index c378f003b007..5b6dda3b1ca8 100644
@@ -82,7 +82,11 @@ FEATURE_TESTS_EXTRA :=                  \
          liberty-z                      \
          libunwind-debug-frame          \
          libunwind-debug-frame-arm      \
-         libunwind-debug-frame-aarch64
+         libunwind-debug-frame-aarch64  \
+         cxx                            \
+         llvm                           \
+         llvm-version                   \
+         clang
 
 FEATURE_TESTS ?= $(FEATURE_TESTS_BASIC)
 
Signature · 1 line
2.13.6
#35 lore raw

[tip:perf/core] perf build: Add llvm/clang/cxx make tests into FEATURE_TESTS_EXTRA

perf build: Add llvm/clang/cxx make tests into FEATURE_TESTS_EXTRA
Commit-ID:  bd476684584ecc3e269167d595c6dd5375011ca0
Gitweb:     https://git.kernel.org/tip/bd476684584ecc3e269167d595c6dd5375011ca0
Author:     Jiri Olsa <[email protected]>
AuthorDate: Wed, 7 Mar 2018 16:50:18 +0100
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Thu, 8 Mar 2018 11:30:48 -0300

perf build: Add llvm/clang/cxx make tests into FEATURE_TESTS_EXTRA

So we can see the status when we build perf, like:

  $ make LIBCLANGLLVM=1 VF=1
  ...                           cxx: [ on  ]
  ...                          llvm: [ on  ]
  ...                  llvm-version: [ on  ]
  ...                         clang: [ on  ]
---
 tools/build/Makefile.feature | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
tools/build/Makefile.feature +5 -1
index c378f003b007..5b6dda3b1ca8 100644
@@ -82,7 +82,11 @@ FEATURE_TESTS_EXTRA :=                  \
          liberty-z                      \
          libunwind-debug-frame          \
          libunwind-debug-frame-arm      \
-         libunwind-debug-frame-aarch64
+         libunwind-debug-frame-aarch64  \
+         cxx                            \
+         llvm                           \
+         llvm-version                   \
+         clang
 
 FEATURE_TESTS ?= $(FEATURE_TESTS_BASIC)
 
author reviewer
#36 lore raw

PATCH 18/19 perf build: Add llvm/clang make targets to FILES

In reply to Jiri Olsa

So they can follow the OUTPUT variable setup as the rest of the features.

---
 tools/build/feature/Makefile | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
tools/build/feature/Makefile +4 -1
index 0a490cb15149..f8ad640ffe5d 100644
@@ -54,7 +54,10 @@ FILES=                                          \
          test-jvmti.bin				\
          test-sched_getcpu.bin			\
          test-setns.bin				\
-         test-libopencsd.bin
+         test-libopencsd.bin			\
+         test-clang.bin				\
+         test-llvm.bin				\
+         test-llvm-version.bin
 
 FILES := $(addprefix $(OUTPUT),$(FILES))
 
Signature · 1 line
2.13.6
#37 lore raw

[tip:perf/core] perf build: Add llvm/clang make targets to FILES

perf build: Add llvm/clang make targets to FILES
Commit-ID:  36f9dc33b9128b85e096f6fff83e05d49448b28e
Gitweb:     https://git.kernel.org/tip/36f9dc33b9128b85e096f6fff83e05d49448b28e
Author:     Jiri Olsa <[email protected]>
AuthorDate: Wed, 7 Mar 2018 16:50:19 +0100
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Thu, 8 Mar 2018 11:30:49 -0300

perf build: Add llvm/clang make targets to FILES

So they can follow the OUTPUT variable setup as the rest of the features.

---
 tools/build/feature/Makefile | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
tools/build/feature/Makefile +4 -1
index 0a490cb15149..f8ad640ffe5d 100644
@@ -54,7 +54,10 @@ FILES=                                          \
          test-jvmti.bin				\
          test-sched_getcpu.bin			\
          test-setns.bin				\
-         test-libopencsd.bin
+         test-libopencsd.bin			\
+         test-clang.bin				\
+         test-llvm.bin				\
+         test-llvm-version.bin
 
 FILES := $(addprefix $(OUTPUT),$(FILES))
 
author reviewer
#38 lore raw

PATCH 19/19 perf build: Force llvm/clang test compile output to .make.output

In reply to Jiri Olsa

So we can see the output of feature compile
in following files:

tools/build/feature/test-llvm.make.output tools/build/feature/test-llvm-version.make.output tools/build/feature/test-clang.make.output

---
 tools/build/feature/Makefile | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
tools/build/feature/Makefile +6 -3
index f8ad640ffe5d..dac9563b5470 100644
@@ -260,11 +260,13 @@ $(OUTPUT)test-llvm.bin:
 		-I$(shell $(LLVM_CONFIG) --includedir) 		\
 		-L$(shell $(LLVM_CONFIG) --libdir)		\
 		$(shell $(LLVM_CONFIG) --libs Core BPF)		\
-		$(shell $(LLVM_CONFIG) --system-libs)
+		$(shell $(LLVM_CONFIG) --system-libs)		\
+		> $(@:.bin=.make.output) 2>&1
 
 $(OUTPUT)test-llvm-version.bin:
 	$(BUILDXX) -std=gnu++11 				\
-		-I$(shell $(LLVM_CONFIG) --includedir)
+		-I$(shell $(LLVM_CONFIG) --includedir)		\
+		> $(@:.bin=.make.output) 2>&1
 
 $(OUTPUT)test-clang.bin:
 	$(BUILDXX) -std=gnu++11 				\
@@ -274,7 +276,8 @@ $(OUTPUT)test-clang.bin:
 		  -lclangFrontend -lclangEdit -lclangLex	\
 		  -lclangAST -Wl,--end-group 			\
 		$(shell $(LLVM_CONFIG) --libs Core option)	\
-		$(shell $(LLVM_CONFIG) --system-libs)
+		$(shell $(LLVM_CONFIG) --system-libs)		\
+		> $(@:.bin=.make.output) 2>&1
 
 -include $(OUTPUT)*.d
 
Signature · 1 line
2.13.6
#39 lore raw

Re: [PATCH 19/19] perf build: Force llvm/clang test compile output to .make.output

Em Wed, Mar 07, 2018 at 04:50:20PM +0100, Jiri Olsa escreveu:

Quoted 2 lines

So we can see the output of feature compile
in following files:

Applied 16-19

- Arnaldo

#40 lore raw

[tip:perf/core] perf build: Force llvm/clang test compile output to .make.output

In reply to Jiri Olsa
perf build: Force llvm/clang test compile output to .make.output
Commit-ID:  5fb3d8b7b50d2101b0c15d2e90264deb7a01c2d8
Gitweb:     https://git.kernel.org/tip/5fb3d8b7b50d2101b0c15d2e90264deb7a01c2d8
Author:     Jiri Olsa <[email protected]>
AuthorDate: Wed, 7 Mar 2018 16:50:20 +0100
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Thu, 8 Mar 2018 11:30:50 -0300

perf build: Force llvm/clang test compile output to .make.output

So we can see the output of feature compile in following files:

tools/build/feature/test-llvm.make.output tools/build/feature/test-llvm-version.make.output tools/build/feature/test-clang.make.output

---
 tools/build/feature/Makefile | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
tools/build/feature/Makefile +6 -3
index f8ad640ffe5d..dac9563b5470 100644
@@ -260,11 +260,13 @@ $(OUTPUT)test-llvm.bin:
 		-I$(shell $(LLVM_CONFIG) --includedir) 		\
 		-L$(shell $(LLVM_CONFIG) --libdir)		\
 		$(shell $(LLVM_CONFIG) --libs Core BPF)		\
-		$(shell $(LLVM_CONFIG) --system-libs)
+		$(shell $(LLVM_CONFIG) --system-libs)		\
+		> $(@:.bin=.make.output) 2>&1
 
 $(OUTPUT)test-llvm-version.bin:
 	$(BUILDXX) -std=gnu++11 				\
-		-I$(shell $(LLVM_CONFIG) --includedir)
+		-I$(shell $(LLVM_CONFIG) --includedir)		\
+		> $(@:.bin=.make.output) 2>&1
 
 $(OUTPUT)test-clang.bin:
 	$(BUILDXX) -std=gnu++11 				\
@@ -274,7 +276,8 @@ $(OUTPUT)test-clang.bin:
 		  -lclangFrontend -lclangEdit -lclangLex	\
 		  -lclangAST -Wl,--end-group 			\
 		$(shell $(LLVM_CONFIG) --libs Core option)	\
-		$(shell $(LLVM_CONFIG) --system-libs)
+		$(shell $(LLVM_CONFIG) --system-libs)		\
+		> $(@:.bin=.make.output) 2>&1
 
 -include $(OUTPUT)*.d