2012-11-09 21:44:43

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [GIT PULL 00/21] perf/core improvements and fixes

Hi Ingo,

Please consider pulling.

- Arnaldo

The following changes since commit 8dfec403e39b7c37fd6e8813bacc01da1e1210ab:

perf tests: Removing 'optional' field (2012-11-05 14:03:59 -0300)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux tags/perf-core-for-mingo

for you to fetch changes up to 27f94d52394003d444a383eaf8d4824daf32432e:

tools lib traceevent: Use 'const' in variables pointing to const strings (2012-11-09 17:42:47 -0300)

----------------------------------------------------------------
perf/core improvements and fixes:

. Add a 'link' method for hists, so that we can have the leader with
buckets for all the entries in all the hists. This new method
is now used in the default 'diff' output, making the sum of the 'baseline'
column be 100%, eliminating blind spots. Now we need to use this
for 'diff' with > 2 perf.data files and for multi event 'report' and
'annotate'.

. libtraceevent fixes for compiler warnings trying to make perf it build
on some distros, like fedora 14, 32-bit, some of the warnings really
pointed to real bugs.

. Remove temp dir on failure in 'perf test', fix from Jiri Olsa.

. Fixes for handling data, stack mmaps, from Namhyung Kim.

. Fix live annotation bug related to recent objdump lookup patches, from
Namhyung Kim

. Don't try to follow jump target on PLT symbols in the annotation browser,
fix from Namhyung Kim.

. Fix leak on hist_entry delete, from Namhyung Kim.

. Fix a CPU_ALLOC related build error on builtin-test, from Zheng Liu.

Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>

----------------------------------------------------------------
Andi Kleen (1):
perf tools: Add arbitary aliases and support names with -

Arnaldo Carvalho de Melo (10):
perf diff: Start moving to support matching more than two hists
perf diff: Move hists__match to the hists lib
perf hists: Introduce hists__link
perf diff: Use hists__link when not pairing just with baseline
perf machine: Move more methods to machine.[ch]
tools lib traceevent: Add __maybe_unused to unused parameters
tools lib traceevent: Avoid comparisions between signed/unsigned
tools lib traceevent: No need to check for < 0 on an unsigned enum
tools lib traceevent: Handle INVALID_ARG_TYPE errno in pevent_strerror
tools lib traceevent: Use 'const' in variables pointing to const strings

Jiri Olsa (2):
perf tests: Move attr.py temp dir cleanup into finally section
perf tools: Add LIBDW_DIR Makefile variable to for alternate libdw

Namhyung Kim (7):
perf machine: Set kernel data mapping length
perf tools: Fix detection of stack area
perf hists: Free branch_info when freeing hist_entry
perf tools: Don't try to lookup objdump for live mode
perf annotate: Whitespace fixups
perf annotate: Don't try to follow jump target on PLT symbols
perf annotate: Merge same lines in summary view

Zheng Liu (1):
perf test: fix a build error on builtin-test

tools/lib/traceevent/event-parse.c | 22 ++--
tools/perf/Makefile | 12 ++-
tools/perf/arch/common.c | 7 ++
tools/perf/builtin-diff.c | 48 ++-------
tools/perf/tests/attr.py | 30 +++---
tools/perf/tests/builtin-test.c | 39 +++----
tools/perf/tests/dso-data.c | 1 +
tools/perf/ui/browsers/annotate.c | 12 +++
tools/perf/ui/hist.c | 10 +-
tools/perf/util/annotate.c | 69 ++++++++++--
tools/perf/util/annotate.h | 1 +
tools/perf/util/dso.c | 1 +
tools/perf/util/hist.c | 100 ++++++++++++++++++
tools/perf/util/hist.h | 3 +
tools/perf/util/machine.c | 205 ++++++++++++++++++++++++++++++++++--
tools/perf/util/machine.h | 131 ++++++++++++++++++++++-
tools/perf/util/map.c | 181 +------------------------------
tools/perf/util/map.h | 93 ----------------
tools/perf/util/parse-events.l | 2 +
tools/perf/util/session.h | 5 +-
tools/perf/util/sort.h | 27 ++++-
tools/perf/util/symbol.c | 1 +
tools/perf/util/symbol.h | 20 ----
23 files changed, 604 insertions(+), 416 deletions(-)


2012-11-09 21:43:38

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 16/21] perf annotate: Merge same lines in summary view

From: Namhyung Kim <[email protected]>

The --print-line option of perf annotate command shows summary for
each source line. But it didn't merge same lines so that it can
appear multiple times.

* before:

Sorted summary for file /home/namhyung/bin/mcol
----------------------------------------------
21.71 /home/namhyung/tmp/mcol.c:26
20.66 /home/namhyung/tmp/mcol.c:25
9.53 /home/namhyung/tmp/mcol.c:24
7.68 /home/namhyung/tmp/mcol.c:25
7.67 /home/namhyung/tmp/mcol.c:25
7.66 /home/namhyung/tmp/mcol.c:26
7.49 /home/namhyung/tmp/mcol.c:26
6.92 /home/namhyung/tmp/mcol.c:25
6.81 /home/namhyung/tmp/mcol.c:25
1.07 /home/namhyung/tmp/mcol.c:26
0.52 /home/namhyung/tmp/mcol.c:25
0.51 /home/namhyung/tmp/mcol.c:25
0.51 /home/namhyung/tmp/mcol.c:24

* after:

Sorted summary for file /home/namhyung/bin/mcol
----------------------------------------------
50.77 /home/namhyung/tmp/mcol.c:25
37.94 /home/namhyung/tmp/mcol.c:26
10.04 /home/namhyung/tmp/mcol.c:24

To do that, introduce percent_sum field so that the normal
line-by-line output doesn't get changed.

Signed-off-by: Namhyung Kim <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/annotate.c | 55 +++++++++++++++++++++++++++++++++++++++++---
tools/perf/util/annotate.h | 1 +
2 files changed, 53 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 435bf6d..07aaeea 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -858,12 +858,41 @@ static void insert_source_line(struct rb_root *root, struct source_line *src_lin
struct source_line *iter;
struct rb_node **p = &root->rb_node;
struct rb_node *parent = NULL;
+ int ret;

while (*p != NULL) {
parent = *p;
iter = rb_entry(parent, struct source_line, node);

- if (src_line->percent > iter->percent)
+ ret = strcmp(iter->path, src_line->path);
+ if (ret == 0) {
+ iter->percent_sum += src_line->percent;
+ return;
+ }
+
+ if (ret < 0)
+ p = &(*p)->rb_left;
+ else
+ p = &(*p)->rb_right;
+ }
+
+ src_line->percent_sum = src_line->percent;
+
+ rb_link_node(&src_line->node, parent, p);
+ rb_insert_color(&src_line->node, root);
+}
+
+static void __resort_source_line(struct rb_root *root, struct source_line *src_line)
+{
+ struct source_line *iter;
+ struct rb_node **p = &root->rb_node;
+ struct rb_node *parent = NULL;
+
+ while (*p != NULL) {
+ parent = *p;
+ iter = rb_entry(parent, struct source_line, node);
+
+ if (src_line->percent_sum > iter->percent_sum)
p = &(*p)->rb_left;
else
p = &(*p)->rb_right;
@@ -873,6 +902,24 @@ static void insert_source_line(struct rb_root *root, struct source_line *src_lin
rb_insert_color(&src_line->node, root);
}

+static void resort_source_line(struct rb_root *dest_root, struct rb_root *src_root)
+{
+ struct source_line *src_line;
+ struct rb_node *node;
+
+ node = rb_first(src_root);
+ while (node) {
+ struct rb_node *next;
+
+ src_line = rb_entry(node, struct source_line, node);
+ next = rb_next(node);
+ rb_erase(node, src_root);
+
+ __resort_source_line(dest_root, src_line);
+ node = next;
+ }
+}
+
static void symbol__free_source_line(struct symbol *sym, int len)
{
struct annotation *notes = symbol__annotation(sym);
@@ -897,6 +944,7 @@ static int symbol__get_source_line(struct symbol *sym, struct map *map,
struct source_line *src_line;
struct annotation *notes = symbol__annotation(sym);
struct sym_hist *h = annotation__histogram(notes, evidx);
+ struct rb_root tmp_root = RB_ROOT;

if (!h->sum)
return 0;
@@ -931,12 +979,13 @@ static int symbol__get_source_line(struct symbol *sym, struct map *map,
goto next;

strcpy(src_line[i].path, path);
- insert_source_line(root, &src_line[i]);
+ insert_source_line(&tmp_root, &src_line[i]);

next:
pclose(fp);
}

+ resort_source_line(root, &tmp_root);
return 0;
}

@@ -960,7 +1009,7 @@ static void print_summary(struct rb_root *root, const char *filename)
char *path;

src_line = rb_entry(node, struct source_line, node);
- percent = src_line->percent;
+ percent = src_line->percent_sum;
color = get_percent_color(percent);
path = src_line->path;

diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index c627201..8eec943 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -76,6 +76,7 @@ struct sym_hist {
struct source_line {
struct rb_node node;
double percent;
+ double percent_sum;
char *path;
};

--
1.7.9.2.358.g22243

2012-11-09 21:43:42

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 06/21] perf tools: Add arbitary aliases and support names with -

From: Andi Kleen <[email protected]>

- Add missing scanner symbol for arbitrary aliases inside the config
region.

- looks nicer than _, so allow - in the event names. Used for various of
the arch perfmon and Haswell events.

Signed-off-by: Andi Kleen <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/parse-events.l | 2 ++
1 file changed, 2 insertions(+)

diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index c87efc1..66959fa 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -81,6 +81,7 @@ num_dec [0-9]+
num_hex 0x[a-fA-F0-9]+
num_raw_hex [a-fA-F0-9]+
name [a-zA-Z_*?][a-zA-Z0-9_*?]*
+name_minus [a-zA-Z_*?][a-zA-Z0-9\-_*?]*
modifier_event [ukhpGH]{1,8}
modifier_bp [rwx]{1,3}

@@ -168,6 +169,7 @@ period { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD); }
branch_type { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE); }
, { return ','; }
"/" { BEGIN(INITIAL); return '/'; }
+{name_minus} { return str(yyscanner, PE_NAME); }
}

mem: { BEGIN(mem); return PE_PREFIX_MEM; }
--
1.7.9.2.358.g22243

2012-11-09 21:43:44

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 02/21] perf tools: Fix detection of stack area

From: Namhyung Kim <[email protected]>

Output of /proc/<pid>/maps contains helpful information to anonymous
mappings like stack, heap, ... For the case of stack, it can show
multiple stack area for each thread in the process:

$ cat /proc/$(pidof gnome-shell)/maps | grep stack
7fe019946000-7fe01a146000 rw-p 00000000 00:00 0 [stack:1624]
7fe040e32000-7fe041632000 rw-p 00000000 00:00 0 [stack:1451]
7fe041643000-7fe041e43000 rw-p 00000000 00:00 0 [stack:1450]
7fe04204b000-7fe04284b000 rw-p 00000000 00:00 0 [stack:1449]
7fe042a7e000-7fe04327e000 rw-p 00000000 00:00 0 [stack:1446]
7fe0432ff000-7fe043aff000 rw-p 00000000 00:00 0 [stack:1445]
7fe043b00000-7fe044300000 rw-p 00000000 00:00 0 [stack:1444]
7fe044301000-7fe044b01000 rw-p 00000000 00:00 0 [stack:1443]
7fe044b02000-7fe045302000 rw-p 00000000 00:00 0 [stack:1442]
7fe045303000-7fe045b03000 rw-p 00000000 00:00 0 [stack:1441]
7fe045b04000-7fe046304000 rw-p 00000000 00:00 0 [stack:1440]
7fe046305000-7fe046b05000 rw-p 00000000 00:00 0 [stack:1439]
7fe046b06000-7fe047306000 rw-p 00000000 00:00 0 [stack:1438]
7fff4b16f000-7fff4b190000 rw-p 00000000 00:00 0 [stack]

However perf only knew about the main thread's. Fix it.

Signed-off-by: Namhyung Kim <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/map.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 9b40c44..5791878 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -24,7 +24,7 @@ static inline int is_anon_memory(const char *filename)

static inline int is_no_dso_memory(const char *filename)
{
- return !strcmp(filename, "[stack]") ||
+ return !strncmp(filename, "[stack", 6) ||
!strcmp(filename, "[heap]");
}

--
1.7.9.2.358.g22243

2012-11-09 21:43:47

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 03/21] perf hists: Free branch_info when freeing hist_entry

From: Namhyung Kim <[email protected]>

Those data should be free along with the associated hist_entry,
otherwise it'll be leaked.

Signed-off-by: Namhyung Kim <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
[ committer note: mem_info is not yet in perf/core, free just branch_info ]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/hist.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 277947a..a1b823f 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -410,6 +410,7 @@ hist_entry__collapse(struct hist_entry *left, struct hist_entry *right)

void hist_entry__free(struct hist_entry *he)
{
+ free(he->branch_info);
free(he);
}

--
1.7.9.2.358.g22243

2012-11-09 21:43:36

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 01/21] perf machine: Set kernel data mapping length

From: Namhyung Kim <[email protected]>

Currently only text (function) mapping was set, so that the kernel data
addresses couldn't parsed correctly. Fix it.

Signed-off-by: Namhyung Kim <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/machine.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 502eec0..4c6754a 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -84,15 +84,19 @@ int machine__process_lost_event(struct machine *machine __maybe_unused,
static void machine__set_kernel_mmap_len(struct machine *machine,
union perf_event *event)
{
- machine->vmlinux_maps[MAP__FUNCTION]->start = event->mmap.start;
- machine->vmlinux_maps[MAP__FUNCTION]->end = (event->mmap.start +
- event->mmap.len);
- /*
- * Be a bit paranoid here, some perf.data file came with
- * a zero sized synthesized MMAP event for the kernel.
- */
- if (machine->vmlinux_maps[MAP__FUNCTION]->end == 0)
- machine->vmlinux_maps[MAP__FUNCTION]->end = ~0ULL;
+ int i;
+
+ for (i = 0; i < MAP__NR_TYPES; i++) {
+ machine->vmlinux_maps[i]->start = event->mmap.start;
+ machine->vmlinux_maps[i]->end = (event->mmap.start +
+ event->mmap.len);
+ /*
+ * Be a bit paranoid here, some perf.data file came with
+ * a zero sized synthesized MMAP event for the kernel.
+ */
+ if (machine->vmlinux_maps[i]->end == 0)
+ machine->vmlinux_maps[i]->end = ~0ULL;
+ }
}

static int machine__process_kernel_mmap_event(struct machine *machine,
--
1.7.9.2.358.g22243

2012-11-09 21:43:33

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 07/21] perf tools: Don't try to lookup objdump for live mode

From: Namhyung Kim <[email protected]>

Arnaldo reported that annotation during perf top resulted in a segfault.
It was because the env->arch was NULL and we don't set it for a live
session. In fact, no need to look up objdump in this case since we can
use system's default (native) objdump.

Reported-by: Arnaldo Carvalho de Melo <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Irina Tirdea <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/arch/common.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/tools/perf/arch/common.c b/tools/perf/arch/common.c
index 5683529..3e975cb 100644
--- a/tools/perf/arch/common.c
+++ b/tools/perf/arch/common.c
@@ -199,6 +199,13 @@ out_error:

int perf_session_env__lookup_objdump(struct perf_session_env *env)
{
+ /*
+ * For live mode, env->arch will be NULL and we can use
+ * the native objdump tool.
+ */
+ if (env->arch == NULL)
+ return 0;
+
return perf_session_env__lookup_binutils_path(env, "objdump",
&objdump_path);
}
--
1.7.9.2.358.g22243

2012-11-09 21:45:55

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 18/21] tools lib traceevent: Avoid comparisions between signed/unsigned

From: Arnaldo Carvalho de Melo <[email protected]>

Fixing this warning-as-error on f14 32-bit:

tools/lib/traceevent/event-parse.c:5564:17: error: comparison between signed and unsigned integer expressions
tools/lib/traceevent/event-parse.c:5586:17: error: comparison between signed and unsigned integer expressions

Cc: David Ahern <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Cc: Steven Rostedt <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/lib/traceevent/event-parse.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 0dcc18c..6b647c1 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -5561,7 +5561,7 @@ void pevent_free(struct pevent *pevent)
}

if (pevent->func_map) {
- for (i = 0; i < pevent->func_count; i++) {
+ for (i = 0; i < (int)pevent->func_count; i++) {
free(pevent->func_map[i].func);
free(pevent->func_map[i].mod);
}
@@ -5583,7 +5583,7 @@ void pevent_free(struct pevent *pevent)
}

if (pevent->printk_map) {
- for (i = 0; i < pevent->printk_count; i++)
+ for (i = 0; i < (int)pevent->printk_count; i++)
free(pevent->printk_map[i].printk);
free(pevent->printk_map);
}
--
1.7.9.2.358.g22243

2012-11-09 21:46:20

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 21/21] tools lib traceevent: Use 'const' in variables pointing to const strings

From: Arnaldo Carvalho de Melo <[email protected]>

Fixing the build on fedora 14, 32-bit:

tools/lib/traceevent/event-parse.c: In function ‘find_cmdline’:
tools/lib/traceevent/event-parse.c:183:3: error: return discards qualifiers from pointer target type
tools/lib/traceevent/event-parse.c:186:3: error: return discards qualifiers from pointer target type
tools/lib/traceevent/event-parse.c:195:2: error: return discards qualifiers from pointer target type
tools/lib/traceevent/event-parse.c: In function ‘process_func_handler’:
tools/lib/traceevent/event-parse.c:2658:9: error: assignment discards qualifiers from pointer target type
tools/lib/traceevent/event-parse.c:2660:9: error: assignment discards qualifiers from pointer target type
tools/lib/traceevent/event-parse.c: In function ‘print_mac_arg’:
tools/lib/traceevent/event-parse.c:3892:14: error: initialization discards qualifiers from pointer target type
tools/lib/traceevent/event-parse.c:3906:7: error: assignment discards qualifiers from pointer target type
tools/lib/traceevent/event-parse.c: In function ‘pevent_print_event’:
tools/lib/traceevent/event-parse.c:4412:24: error: initialization discards qualifiers from pointer target type

Cc: David Ahern <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Cc: Steven Rostedt <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/lib/traceevent/event-parse.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 669953a..5a824e3 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -174,7 +174,7 @@ static int cmdline_init(struct pevent *pevent)
return 0;
}

-static char *find_cmdline(struct pevent *pevent, int pid)
+static const char *find_cmdline(struct pevent *pevent, int pid)
{
const struct cmdline *comm;
struct cmdline key;
@@ -2637,7 +2637,7 @@ process_func_handler(struct event_format *event, struct pevent_function_handler
struct print_arg *farg;
enum event_type type;
char *token;
- char *test;
+ const char *test;
int i;

arg->type = PRINT_FUNC;
@@ -3889,7 +3889,7 @@ static void print_mac_arg(struct trace_seq *s, int mac, void *data, int size,
struct event_format *event, struct print_arg *arg)
{
unsigned char *buf;
- char *fmt = "%.2x:%.2x:%.2x:%.2x:%.2x:%.2x";
+ const char *fmt = "%.2x:%.2x:%.2x:%.2x:%.2x:%.2x";

if (arg->type == PRINT_FUNC) {
process_defined_func(s, data, size, event, arg);
@@ -4409,7 +4409,7 @@ void pevent_event_info(struct trace_seq *s, struct event_format *event,
void pevent_print_event(struct pevent *pevent, struct trace_seq *s,
struct pevent_record *record)
{
- static char *spaces = " "; /* 20 spaces */
+ static const char *spaces = " "; /* 20 spaces */
struct event_format *event;
unsigned long secs;
unsigned long usecs;
--
1.7.9.2.358.g22243

2012-11-09 21:46:22

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 20/21] tools lib traceevent: Handle INVALID_ARG_TYPE errno in pevent_strerror

From: Arnaldo Carvalho de Melo <[email protected]>

gcc on f14 32-bit rightly complains:

tools/lib/traceevent/event-parse.c:5097:2: error: enumeration value ‘PEVENT_ERRNO__INVALID_ARG_TYPE’ not handled in switch

The entry for it is in the error strings array pevent_error_str[]:

_PE(INVALID_ARG_TYPE, "invalid argument type")

It was just not being handled on the pevent_strerror switch, fix it.

Cc: David Ahern <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Cc: Steven Rostedt <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/lib/traceevent/event-parse.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 38d65958..669953a 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -5101,6 +5101,7 @@ int pevent_strerror(struct pevent *pevent __maybe_unused,
case PEVENT_ERRNO__READ_FORMAT_FAILED:
case PEVENT_ERRNO__READ_PRINT_FAILED:
case PEVENT_ERRNO__OLD_FTRACE_ARG_FAILED:
+ case PEVENT_ERRNO__INVALID_ARG_TYPE:
snprintf(buf, buflen, "%s", msg);
break;

--
1.7.9.2.358.g22243

2012-11-09 21:46:27

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 17/21] tools lib traceevent: Add __maybe_unused to unused parameters

From: Arnaldo Carvalho de Melo <[email protected]>

Fixing the build on 32-bit Fedora 14:

tools/lib/traceevent/event-parse.c: In function ‘print_event_fields’:
tools/lib/traceevent/event-parse.c:3934:69: error: unused parameter ‘size’
tools/lib/traceevent/event-parse.c: In function ‘pevent_strerror’:
tools/lib/traceevent/event-parse.c:5074:36: error: unused parameter ‘pevent’

Cc: David Ahern <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Cc: Steven Rostedt <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/lib/traceevent/event-parse.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index f2989c5..0dcc18c 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -3931,7 +3931,8 @@ static int is_printable_array(char *p, unsigned int len)
return 1;
}

-static void print_event_fields(struct trace_seq *s, void *data, int size,
+static void print_event_fields(struct trace_seq *s, void *data,
+ int size __maybe_unused,
struct event_format *event)
{
struct format_field *field;
@@ -5070,8 +5071,8 @@ static const char * const pevent_error_str[] = {
};
#undef _PE

-int pevent_strerror(struct pevent *pevent, enum pevent_errno errnum,
- char *buf, size_t buflen)
+int pevent_strerror(struct pevent *pevent __maybe_unused,
+ enum pevent_errno errnum, char *buf, size_t buflen)
{
int idx;
const char *msg;
--
1.7.9.2.358.g22243

2012-11-09 21:47:18

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 09/21] perf diff: Move hists__match to the hists lib

From: Arnaldo Carvalho de Melo <[email protected]>

Its not 'diff' specific and will be useful for other use cases, like
bucketizing multiple events in a single session.

Cc: David Ahern <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-diff.c | 35 +----------------------------------
tools/perf/util/hist.c | 37 +++++++++++++++++++++++++++++++++++++
tools/perf/util/hist.h | 2 ++
3 files changed, 40 insertions(+), 34 deletions(-)

diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 8a9db38..e99fb3b 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -334,39 +334,6 @@ static void hists__name_resort(struct hists *self, bool sort)
self->entries = tmp;
}

-static struct hist_entry *hists__find_entry(struct hists *self,
- struct hist_entry *he)
-{
- struct rb_node *n = self->entries.rb_node;
-
- while (n) {
- struct hist_entry *iter = rb_entry(n, struct hist_entry, rb_node);
- int64_t cmp = hist_entry__cmp(he, iter);
-
- if (cmp < 0)
- n = n->rb_left;
- else if (cmp > 0)
- n = n->rb_right;
- else
- return iter;
- }
-
- return NULL;
-}
-
-static void hists__match(struct hists *older, struct hists *newer)
-{
- struct rb_node *nd;
-
- for (nd = rb_first(&newer->entries); nd; nd = rb_next(nd)) {
- struct hist_entry *pos = rb_entry(nd, struct hist_entry, rb_node),
- *pair = hists__find_entry(older, pos);
-
- if (pair)
- hist__entry_add_pair(pos, pair);
- }
-}
-
static struct perf_evsel *evsel_match(struct perf_evsel *evsel,
struct perf_evlist *evlist)
{
@@ -520,7 +487,7 @@ static void hists__compute_resort(struct hists *hists)

static void hists__process(struct hists *old, struct hists *new)
{
- hists__match(old, new);
+ hists__match(new, old);

if (show_baseline_only)
hists__baseline_only(new);
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index f42de79..c1de3b0 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -716,3 +716,40 @@ void hists__inc_nr_events(struct hists *hists, u32 type)
++hists->stats.nr_events[0];
++hists->stats.nr_events[type];
}
+
+static struct hist_entry *hists__find_entry(struct hists *hists,
+ struct hist_entry *he)
+{
+ struct rb_node *n = hists->entries.rb_node;
+
+ while (n) {
+ struct hist_entry *iter = rb_entry(n, struct hist_entry, rb_node);
+ int64_t cmp = hist_entry__cmp(he, iter);
+
+ if (cmp < 0)
+ n = n->rb_left;
+ else if (cmp > 0)
+ n = n->rb_right;
+ else
+ return iter;
+ }
+
+ return NULL;
+}
+
+/*
+ * Look for pairs to link to the leader buckets (hist_entries):
+ */
+void hists__match(struct hists *leader, struct hists *other)
+{
+ struct rb_node *nd;
+ struct hist_entry *pos, *pair;
+
+ for (nd = rb_first(&leader->entries); nd; nd = rb_next(nd)) {
+ pos = rb_entry(nd, struct hist_entry, rb_node);
+ pair = hists__find_entry(other, pos);
+
+ if (pair)
+ hist__entry_add_pair(pos, pair);
+ }
+}
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index a4f45dd..ff1c396 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -115,6 +115,8 @@ bool hists__new_col_len(struct hists *self, enum hist_column col, u16 len);
void hists__reset_col_len(struct hists *hists);
void hists__calc_col_len(struct hists *hists, struct hist_entry *he);

+void hists__match(struct hists *leader, struct hists *other);
+
struct perf_hpp {
char *buf;
size_t size;
--
1.7.9.2.358.g22243

2012-11-09 21:47:22

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 08/21] perf diff: Start moving to support matching more than two hists

From: Arnaldo Carvalho de Melo <[email protected]>

We want to match more than two hists, so that we can match more than two
perf.data files and moreover, match hist_entries (buckets) in multiple
events in a group.

So the "baseline"/"leader" will instead of a ->pair pointer, use a
list_head, that will link to the pairs and hists__match use it.

Following that perf_evlist__link will link the hists in its evsel
groups.

Cc: David Ahern <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-diff.c | 21 ++++++++++++---------
tools/perf/ui/hist.c | 10 +++++-----
tools/perf/util/hist.c | 2 ++
tools/perf/util/sort.h | 27 +++++++++++++++++++++++----
4 files changed, 42 insertions(+), 18 deletions(-)

diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 380683d..8a9db38 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -154,7 +154,7 @@ static double get_period_percent(struct hist_entry *he, u64 period)

double perf_diff__compute_delta(struct hist_entry *he)
{
- struct hist_entry *pair = he->pair;
+ struct hist_entry *pair = hist_entry__next_pair(he);
double new_percent = get_period_percent(he, he->stat.period);
double old_percent = pair ? get_period_percent(pair, pair->stat.period) : 0.0;

@@ -165,7 +165,7 @@ double perf_diff__compute_delta(struct hist_entry *he)

double perf_diff__compute_ratio(struct hist_entry *he)
{
- struct hist_entry *pair = he->pair;
+ struct hist_entry *pair = hist_entry__next_pair(he);
double new_period = he->stat.period;
double old_period = pair ? pair->stat.period : 0;

@@ -176,7 +176,7 @@ double perf_diff__compute_ratio(struct hist_entry *he)

s64 perf_diff__compute_wdiff(struct hist_entry *he)
{
- struct hist_entry *pair = he->pair;
+ struct hist_entry *pair = hist_entry__next_pair(he);
u64 new_period = he->stat.period;
u64 old_period = pair ? pair->stat.period : 0;

@@ -193,7 +193,7 @@ s64 perf_diff__compute_wdiff(struct hist_entry *he)

static int formula_delta(struct hist_entry *he, char *buf, size_t size)
{
- struct hist_entry *pair = he->pair;
+ struct hist_entry *pair = hist_entry__next_pair(he);

if (!pair)
return -1;
@@ -207,7 +207,7 @@ static int formula_delta(struct hist_entry *he, char *buf, size_t size)

static int formula_ratio(struct hist_entry *he, char *buf, size_t size)
{
- struct hist_entry *pair = he->pair;
+ struct hist_entry *pair = hist_entry__next_pair(he);
double new_period = he->stat.period;
double old_period = pair ? pair->stat.period : 0;

@@ -219,7 +219,7 @@ static int formula_ratio(struct hist_entry *he, char *buf, size_t size)

static int formula_wdiff(struct hist_entry *he, char *buf, size_t size)
{
- struct hist_entry *pair = he->pair;
+ struct hist_entry *pair = hist_entry__next_pair(he);
u64 new_period = he->stat.period;
u64 old_period = pair ? pair->stat.period : 0;

@@ -359,8 +359,11 @@ static void hists__match(struct hists *older, struct hists *newer)
struct rb_node *nd;

for (nd = rb_first(&newer->entries); nd; nd = rb_next(nd)) {
- struct hist_entry *pos = rb_entry(nd, struct hist_entry, rb_node);
- pos->pair = hists__find_entry(older, pos);
+ struct hist_entry *pos = rb_entry(nd, struct hist_entry, rb_node),
+ *pair = hists__find_entry(older, pos);
+
+ if (pair)
+ hist__entry_add_pair(pos, pair);
}
}

@@ -402,7 +405,7 @@ static void hists__baseline_only(struct hists *hists)
struct hist_entry *he = rb_entry(next, struct hist_entry, rb_node);

next = rb_next(&he->rb_node);
- if (!he->pair) {
+ if (!hist_entry__next_pair(he)) {
rb_erase(&he->rb_node, &hists->entries);
hist_entry__free(he);
}
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 4f5f475..aa84130 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -161,7 +161,7 @@ static int hpp__width_baseline(struct perf_hpp *hpp __maybe_unused)

static double baseline_percent(struct hist_entry *he)
{
- struct hist_entry *pair = he->pair;
+ struct hist_entry *pair = hist_entry__next_pair(he);
struct hists *pair_hists = pair ? pair->hists : NULL;
double percent = 0.0;

@@ -179,7 +179,7 @@ static int hpp__color_baseline(struct perf_hpp *hpp, struct hist_entry *he)
{
double percent = baseline_percent(he);

- if (he->pair)
+ if (hist_entry__has_pairs(he))
return percent_color_snprintf(hpp->buf, hpp->size, " %6.2f%%", percent);
else
return scnprintf(hpp->buf, hpp->size, " ");
@@ -190,7 +190,7 @@ static int hpp__entry_baseline(struct perf_hpp *hpp, struct hist_entry *he)
double percent = baseline_percent(he);
const char *fmt = symbol_conf.field_sep ? "%.2f" : " %6.2f%%";

- if (he->pair || symbol_conf.field_sep)
+ if (hist_entry__has_pairs(he) || symbol_conf.field_sep)
return scnprintf(hpp->buf, hpp->size, fmt, percent);
else
return scnprintf(hpp->buf, hpp->size, " ");
@@ -248,7 +248,7 @@ static int hpp__width_period_baseline(struct perf_hpp *hpp __maybe_unused)

static int hpp__entry_period_baseline(struct perf_hpp *hpp, struct hist_entry *he)
{
- struct hist_entry *pair = he->pair;
+ struct hist_entry *pair = hist_entry__next_pair(he);
u64 period = pair ? pair->stat.period : 0;
const char *fmt = symbol_conf.field_sep ? "%" PRIu64 : "%12" PRIu64;

@@ -354,7 +354,7 @@ static int hpp__width_displ(struct perf_hpp *hpp __maybe_unused)
static int hpp__entry_displ(struct perf_hpp *hpp,
struct hist_entry *he)
{
- struct hist_entry *pair = he->pair;
+ struct hist_entry *pair = hist_entry__next_pair(he);
long displacement = pair ? pair->position - he->position : 0;
const char *fmt = symbol_conf.field_sep ? "%s" : "%6.6s";
char buf[32] = " ";
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index a1b823f..f42de79 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -244,6 +244,8 @@ static struct hist_entry *hist_entry__new(struct hist_entry *template)
he->ms.map->referenced = true;
if (symbol_conf.use_callchain)
callchain_init(he->callchain);
+
+ INIT_LIST_HEAD(&he->pairs.node);
}

return he;
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 13761d8..b4e8c3b 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -77,6 +77,10 @@ struct hist_entry_diff {
struct hist_entry {
struct rb_node rb_node_in;
struct rb_node rb_node;
+ union {
+ struct list_head node;
+ struct list_head head;
+ } pairs;
struct he_stat stat;
struct map_symbol ms;
struct thread *thread;
@@ -96,15 +100,30 @@ struct hist_entry {
char *srcline;
struct symbol *parent;
unsigned long position;
- union {
- struct hist_entry *pair;
- struct rb_root sorted_chain;
- };
+ struct rb_root sorted_chain;
struct branch_info *branch_info;
struct hists *hists;
struct callchain_root callchain[0];
};

+static inline bool hist_entry__has_pairs(struct hist_entry *he)
+{
+ return !list_empty(&he->pairs.node);
+}
+
+static inline struct hist_entry *hist_entry__next_pair(struct hist_entry *he)
+{
+ if (hist_entry__has_pairs(he))
+ return list_entry(he->pairs.node.next, struct hist_entry, pairs.node);
+ return NULL;
+}
+
+static inline void hist__entry_add_pair(struct hist_entry *he,
+ struct hist_entry *pair)
+{
+ list_add_tail(&he->pairs.head, &pair->pairs.node);
+}
+
enum sort_type {
SORT_PID,
SORT_COMM,
--
1.7.9.2.358.g22243

2012-11-09 21:47:25

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 10/21] perf hists: Introduce hists__link

From: Arnaldo Carvalho de Melo <[email protected]>

That given two hists will find the hist_entries (buckets) in the second
hists that are for the same bucket in the first and link them, then it
will look for all buckets in the second that don't have a counterpart in
the first and will create a dummy counterpart that will then be linked
to the entry in the second.

For multiple events this will be done pairing the leader with all the
other events in the group, so that in the end the leader will have all
the buckets in all the hists in a group, dummy or not while the other
hists will be left untouched.

Cc: David Ahern <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/hist.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++
tools/perf/util/hist.h | 1 +
2 files changed, 61 insertions(+)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index c1de3b0..7c6e73b 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -717,6 +717,42 @@ void hists__inc_nr_events(struct hists *hists, u32 type)
++hists->stats.nr_events[type];
}

+static struct hist_entry *hists__add_dummy_entry(struct hists *hists,
+ struct hist_entry *pair)
+{
+ struct rb_node **p = &hists->entries.rb_node;
+ struct rb_node *parent = NULL;
+ struct hist_entry *he;
+ int cmp;
+
+ while (*p != NULL) {
+ parent = *p;
+ he = rb_entry(parent, struct hist_entry, rb_node);
+
+ cmp = hist_entry__cmp(pair, he);
+
+ if (!cmp)
+ goto out;
+
+ if (cmp < 0)
+ p = &(*p)->rb_left;
+ else
+ p = &(*p)->rb_right;
+ }
+
+ he = hist_entry__new(pair);
+ if (he) {
+ he->stat.nr_events = 0;
+ he->stat.period = 0;
+ he->hists = hists;
+ rb_link_node(&he->rb_node, parent, p);
+ rb_insert_color(&he->rb_node, &hists->entries);
+ hists__inc_nr_entries(hists, he);
+ }
+out:
+ return he;
+}
+
static struct hist_entry *hists__find_entry(struct hists *hists,
struct hist_entry *he)
{
@@ -753,3 +789,27 @@ void hists__match(struct hists *leader, struct hists *other)
hist__entry_add_pair(pos, pair);
}
}
+
+/*
+ * Look for entries in the other hists that are not present in the leader, if
+ * we find them, just add a dummy entry on the leader hists, with period=0,
+ * nr_events=0, to serve as the list header.
+ */
+int hists__link(struct hists *leader, struct hists *other)
+{
+ struct rb_node *nd;
+ struct hist_entry *pos, *pair;
+
+ for (nd = rb_first(&other->entries); nd; nd = rb_next(nd)) {
+ pos = rb_entry(nd, struct hist_entry, rb_node);
+
+ if (!hist_entry__has_pairs(pos)) {
+ pair = hists__add_dummy_entry(leader, pos);
+ if (pair == NULL)
+ return -1;
+ hist__entry_add_pair(pair, pos);
+ }
+ }
+
+ return 0;
+}
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index ff1c396..1278c2c 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -116,6 +116,7 @@ void hists__reset_col_len(struct hists *hists);
void hists__calc_col_len(struct hists *hists, struct hist_entry *he);

void hists__match(struct hists *leader, struct hists *other);
+int hists__link(struct hists *leader, struct hists *other);

struct perf_hpp {
char *buf;
--
1.7.9.2.358.g22243

2012-11-09 21:47:30

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 14/21] perf annotate: Whitespace fixups

From: Namhyung Kim <[email protected]>

Some lines are indented by whitespace characters rather than tabs. Fix
them.

Signed-off-by: Namhyung Kim <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/annotate.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index b14d4df..435bf6d 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -171,15 +171,15 @@ static int lock__parse(struct ins_operands *ops)
if (disasm_line__parse(ops->raw, &name, &ops->locked.ops->raw) < 0)
goto out_free_ops;

- ops->locked.ins = ins__find(name);
- if (ops->locked.ins == NULL)
- goto out_free_ops;
+ ops->locked.ins = ins__find(name);
+ if (ops->locked.ins == NULL)
+ goto out_free_ops;

- if (!ops->locked.ins->ops)
- return 0;
+ if (!ops->locked.ins->ops)
+ return 0;

- if (ops->locked.ins->ops->parse)
- ops->locked.ins->ops->parse(ops->locked.ops);
+ if (ops->locked.ins->ops->parse)
+ ops->locked.ins->ops->parse(ops->locked.ops);

return 0;

--
1.7.9.2.358.g22243

2012-11-09 21:47:48

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 11/21] perf diff: Use hists__link when not pairing just with baseline

From: Arnaldo Carvalho de Melo <[email protected]>

Previously there were blind spots because we were not looking at symbols
that didn't ocurred in the latest run:

# perf record usleep 1
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.018 MB perf.data (~801 samples) ]
# perf record usleep 1
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.018 MB perf.data (~801 samples) ]

Before:

# perf diff
# Event 'cycles'
#
# Baseline Delta Shared Object Symbol
# ........ ....... ................. .............................
#
+10.38% [kernel.kallsyms] [k] get_empty_filp
+9.51% [kernel.kallsyms] [k] update_sd_lb_stats
+9.41% libpopt.so.0.0.0 [.] _init
+9.29% [kernel.kallsyms] [k] vma_interval_tree_insert
9.05% +0.12% [kernel.kallsyms] [k] do_sys_open
+9.14% [kernel.kallsyms] [k] kfree
+8.98% [kernel.kallsyms] [k] free_pages_and_swap_cache
+8.78% [kernel.kallsyms] [k] unmap_page_range
9.36% -0.90% [kernel.kallsyms] [k] zap_pte_range
7.60% +0.09% [kernel.kallsyms] [k] find_next_bit
+4.37% [kernel.kallsyms] [k] place_entity
+3.38% [kernel.kallsyms] [k] __do_page_fault
+0.80% [kernel.kallsyms] [k] native_apic_mem_write
0.21% +0.43% [kernel.kallsyms] [k] native_write_msr_safe
#

So 9.05 + 9.36 + 7.60 + 0.21 != 100%

Now using the recently introduced hists__link we can see the whole
picture:

# perf diff
# Event 'cycles'
#
# Baseline Delta Shared Object Symbol
# ........ ....... ................. .............................
#
8.44% -8.44% [kernel.kallsyms] [k] _raw_spin_lock
9.05% -9.05% [kernel.kallsyms] [k] sha_transform
10.55% -10.55% [kernel.kallsyms] [k] __d_lookup_rcu
+10.38% [kernel.kallsyms] [k] get_empty_filp
17.70% -17.70% [kernel.kallsyms] [k] kmem_cache_free
+9.51% [kernel.kallsyms] [k] update_sd_lb_stats
+9.41% libpopt.so.0.0.0 [.] _init
+9.29% [kernel.kallsyms] [k] vma_interval_tree_insert
9.05% +0.12% [kernel.kallsyms] [k] do_sys_open
+9.14% [kernel.kallsyms] [k] kfree
+8.98% [kernel.kallsyms] [k] free_pages_and_swap_cache
+8.78% [kernel.kallsyms] [k] unmap_page_range
9.36% -0.90% [kernel.kallsyms] [k] zap_pte_range
7.60% +0.09% [kernel.kallsyms] [k] find_next_bit
+4.37% [kernel.kallsyms] [k] place_entity
+3.38% [kernel.kallsyms] [k] __do_page_fault
4.01% -4.01% [kernel.kallsyms] [k] handle_pte_fault
9.27% -9.27% [kernel.kallsyms] [k] find_get_page
0.78% -0.78% [kernel.kallsyms] [k] rcu_irq_enter
0.57% -0.57% [kernel.kallsyms] [k] finish_task_switch
4.25% -4.25% [kernel.kallsyms] [k] run_timer_softirq
+0.80% [kernel.kallsyms] [k] native_apic_mem_write
0.21% +0.43% [kernel.kallsyms] [k] native_write_msr_safe
9.16% -9.16% ld-2.12.so [.] close
#

Now:

8.44 + 9.05 + 10.55 + 17.70 + 9.05 + 9.36 +
7.60 + 4.01 + 9.27 + 0.78 + 0.57 + 4.25 + 0.21 + 9.16 == 100%

Cc: David Ahern <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-diff.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index e99fb3b..93b852f 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -491,6 +491,8 @@ static void hists__process(struct hists *old, struct hists *new)

if (show_baseline_only)
hists__baseline_only(new);
+ else
+ hists__link(new, old);

if (sort_compute) {
hists__precompute(new);
--
1.7.9.2.358.g22243

2012-11-09 21:47:46

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 15/21] perf annotate: Don't try to follow jump target on PLT symbols

From: Namhyung Kim <[email protected]>

The perf annotate browser on TUI can identify a jump target for a
selected instruction. It assumes that the jump target is within the
function but it's not the case of PLT symbols which have offset out of
the function as a target.

Since it caused a segmentation fault, do not try to follow jump target
on the PLT symbols.

Signed-off-by: Namhyung Kim <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/ui/browsers/annotate.c | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 3eff17f..5dab3ca 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -188,6 +188,12 @@ static void annotate_browser__draw_current_jump(struct ui_browser *browser)
struct disasm_line *cursor = ab->selection, *target;
struct browser_disasm_line *btarget, *bcursor;
unsigned int from, to;
+ struct map_symbol *ms = ab->b.priv;
+ struct symbol *sym = ms->sym;
+
+ /* PLT symbols contain external offsets */
+ if (strstr(sym->name, "@plt"))
+ return;

if (!cursor || !cursor->ins || !ins__is_jump(cursor->ins) ||
!disasm_line__has_offset(cursor))
@@ -771,6 +777,12 @@ static void annotate_browser__mark_jump_targets(struct annotate_browser *browser
size_t size)
{
u64 offset;
+ struct map_symbol *ms = browser->b.priv;
+ struct symbol *sym = ms->sym;
+
+ /* PLT symbols contain external offsets */
+ if (strstr(sym->name, "@plt"))
+ return;

for (offset = 0; offset < size; ++offset) {
struct disasm_line *dl = browser->offsets[offset], *dlt;
--
1.7.9.2.358.g22243

2012-11-09 21:47:42

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 13/21] perf test: fix a build error on builtin-test

From: Zheng Liu <[email protected]>

Recently I build perf and get a build error on builtin-test.c. The error is as
following:

$ make
CC perf.o
CC builtin-test.o
cc1: warnings being treated as errors
builtin-test.c: In function ‘sched__get_first_possible_cpu’:
builtin-test.c:977: warning: implicit declaration of function ‘CPU_ALLOC’
builtin-test.c:977: warning: nested extern declaration of ‘CPU_ALLOC’
builtin-test.c:977: warning: assignment makes pointer from integer without a cast
builtin-test.c:978: warning: implicit declaration of function ‘CPU_ALLOC_SIZE’
builtin-test.c:978: warning: nested extern declaration of ‘CPU_ALLOC_SIZE’
builtin-test.c:979: warning: implicit declaration of function ‘CPU_ZERO_S’
builtin-test.c:979: warning: nested extern declaration of ‘CPU_ZERO_S’
builtin-test.c:982: warning: implicit declaration of function ‘CPU_FREE’
builtin-test.c:982: warning: nested extern declaration of ‘CPU_FREE’
builtin-test.c:992: warning: implicit declaration of function ‘CPU_ISSET_S’
builtin-test.c:992: warning: nested extern declaration of ‘CPU_ISSET_S’
builtin-test.c:998: warning: implicit declaration of function ‘CPU_CLR_S’
builtin-test.c:998: warning: nested extern declaration of ‘CPU_CLR_S’
make: *** [builtin-test.o] Error 1

This problem is introduced in 3e7c439a. CPU_ALLOC and related macros are
missing in sched__get_first_possible_cpu function. In 54489c18, commiter
mentioned that CPU_ALLOC has been removed. So CPU_ALLOC calls in this
function are removed to let perf to be built.

Signed-off-by: Vinson Lee <[email protected]>
Signed-off-by: Zheng Liu <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Cc: Vinson Lee <[email protected]>
Cc: Zheng Liu <[email protected]>
Cc: [email protected]
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/tests/builtin-test.c | 38 ++++++++++++--------------------------
1 file changed, 12 insertions(+), 26 deletions(-)

diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index b5a544d..5d4354e 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -605,19 +605,13 @@ out_free_threads:
#undef nsyscalls
}

-static int sched__get_first_possible_cpu(pid_t pid, cpu_set_t **maskp,
- size_t *sizep)
+static int sched__get_first_possible_cpu(pid_t pid, cpu_set_t *maskp)
{
- cpu_set_t *mask;
- size_t size;
int i, cpu = -1, nrcpus = 1024;
realloc:
- mask = CPU_ALLOC(nrcpus);
- size = CPU_ALLOC_SIZE(nrcpus);
- CPU_ZERO_S(size, mask);
+ CPU_ZERO(maskp);

- if (sched_getaffinity(pid, size, mask) == -1) {
- CPU_FREE(mask);
+ if (sched_getaffinity(pid, sizeof(*maskp), maskp) == -1) {
if (errno == EINVAL && nrcpus < (1024 << 8)) {
nrcpus = nrcpus << 2;
goto realloc;
@@ -627,19 +621,14 @@ realloc:
}

for (i = 0; i < nrcpus; i++) {
- if (CPU_ISSET_S(i, size, mask)) {
- if (cpu == -1) {
+ if (CPU_ISSET(i, maskp)) {
+ if (cpu == -1)
cpu = i;
- *maskp = mask;
- *sizep = size;
- } else
- CPU_CLR_S(i, size, mask);
+ else
+ CPU_CLR(i, maskp);
}
}

- if (cpu == -1)
- CPU_FREE(mask);
-
return cpu;
}

@@ -654,8 +643,8 @@ static int test__PERF_RECORD(void)
.freq = 10,
.mmap_pages = 256,
};
- cpu_set_t *cpu_mask = NULL;
- size_t cpu_mask_size = 0;
+ cpu_set_t cpu_mask;
+ size_t cpu_mask_size = sizeof(cpu_mask);
struct perf_evlist *evlist = perf_evlist__new(NULL, NULL);
struct perf_evsel *evsel;
struct perf_sample sample;
@@ -719,8 +708,7 @@ static int test__PERF_RECORD(void)
evsel->attr.sample_type |= PERF_SAMPLE_TIME;
perf_evlist__config_attrs(evlist, &opts);

- err = sched__get_first_possible_cpu(evlist->workload.pid, &cpu_mask,
- &cpu_mask_size);
+ err = sched__get_first_possible_cpu(evlist->workload.pid, &cpu_mask);
if (err < 0) {
pr_debug("sched__get_first_possible_cpu: %s\n", strerror(errno));
goto out_delete_evlist;
@@ -731,9 +719,9 @@ static int test__PERF_RECORD(void)
/*
* So that we can check perf_sample.cpu on all the samples.
*/
- if (sched_setaffinity(evlist->workload.pid, cpu_mask_size, cpu_mask) < 0) {
+ if (sched_setaffinity(evlist->workload.pid, cpu_mask_size, &cpu_mask) < 0) {
pr_debug("sched_setaffinity: %s\n", strerror(errno));
- goto out_free_cpu_mask;
+ goto out_delete_evlist;
}

/*
@@ -917,8 +905,6 @@ found_exit:
}
out_err:
perf_evlist__munmap(evlist);
-out_free_cpu_mask:
- CPU_FREE(cpu_mask);
out_delete_evlist:
perf_evlist__delete(evlist);
out:
--
1.7.9.2.358.g22243

2012-11-09 21:47:39

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 19/21] tools lib traceevent: No need to check for < 0 on an unsigned enum

From: Arnaldo Carvalho de Melo <[email protected]>

gcc on f14 32-bit complains:

tools/lib/traceevent/event-parse.c: In function ‘pevent_register_print_function’:
tools/lib/traceevent/event-parse.c:5366:3: error: comparison of unsigned expression < 0 is always false

This is because:

enum pevent_func_arg_type type;

this enum doesn't have any negative value, so gcc makes it an 'unsigned
int'. Fix it by removing the < 0 test.

Cc: David Ahern <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Cc: Steven Rostedt <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/lib/traceevent/event-parse.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 6b647c1..38d65958 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -5363,7 +5363,7 @@ int pevent_register_print_function(struct pevent *pevent,
if (type == PEVENT_FUNC_ARG_VOID)
break;

- if (type < 0 || type >= PEVENT_FUNC_ARG_MAX_TYPES) {
+ if (type >= PEVENT_FUNC_ARG_MAX_TYPES) {
do_warning("Invalid argument type %d", type);
ret = PEVENT_ERRNO__INVALID_ARG_TYPE;
goto out_free;
--
1.7.9.2.358.g22243

2012-11-09 21:47:34

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 04/21] perf tests: Move attr.py temp dir cleanup into finally section

From: Jiri Olsa <[email protected]>

Currently if there's 'Unsup' exception raised, we do not clean up the
temp directory. Solving this by adding 'finally' to make the cleanup in
any case.

Signed-off-by: Jiri Olsa <[email protected]>
Cc: Corey Ashford <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/tests/attr.py | 30 ++++++++++++++++--------------
1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/tools/perf/tests/attr.py b/tools/perf/tests/attr.py
index 9b25b33c..e702b82 100644
--- a/tools/perf/tests/attr.py
+++ b/tools/perf/tests/attr.py
@@ -228,24 +228,26 @@ class Test(object):
def run(self):
tempdir = tempfile.mkdtemp();

- # run the test script
- self.run_cmd(tempdir);
+ try:
+ # run the test script
+ self.run_cmd(tempdir);

- # load events expectation for the test
- log.info(" loading result events");
- for f in glob.glob(tempdir + '/event*'):
- self.load_events(f, self.result);
+ # load events expectation for the test
+ log.info(" loading result events");
+ for f in glob.glob(tempdir + '/event*'):
+ self.load_events(f, self.result);

- # resolve group_fd to event names
- self.resolve_groups(self.expect);
- self.resolve_groups(self.result);
+ # resolve group_fd to event names
+ self.resolve_groups(self.expect);
+ self.resolve_groups(self.result);

- # do the expectation - results matching - both ways
- self.compare(self.expect, self.result)
- self.compare(self.result, self.expect)
+ # do the expectation - results matching - both ways
+ self.compare(self.expect, self.result)
+ self.compare(self.result, self.expect)

- # cleanup
- shutil.rmtree(tempdir)
+ finally:
+ # cleanup
+ shutil.rmtree(tempdir)


def run_tests(options):
--
1.7.9.2.358.g22243

2012-11-09 21:47:15

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 12/21] perf machine: Move more methods to machine.[ch]

From: Arnaldo Carvalho de Melo <[email protected]>

This time out of map.[ch] mostly, just code move plus a buch of 'self'
removal, using machine or machines instead.

Cc: David Ahern <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/tests/builtin-test.c | 1 +
tools/perf/tests/dso-data.c | 1 +
tools/perf/util/dso.c | 1 +
tools/perf/util/machine.c | 183 +++++++++++++++++++++++++++++++++++++++
tools/perf/util/machine.h | 131 +++++++++++++++++++++++++++-
tools/perf/util/map.c | 179 --------------------------------------
tools/perf/util/map.h | 93 --------------------
tools/perf/util/session.h | 5 +-
tools/perf/util/symbol.c | 1 +
tools/perf/util/symbol.h | 20 -----
10 files changed, 318 insertions(+), 297 deletions(-)

diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 1aa9e99..b5a544d 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -10,6 +10,7 @@
#include "util/debug.h"
#include "util/debugfs.h"
#include "util/evlist.h"
+#include "util/machine.h"
#include "util/parse-options.h"
#include "util/parse-events.h"
#include "util/symbol.h"
diff --git a/tools/perf/tests/dso-data.c b/tools/perf/tests/dso-data.c
index c6caede..0cd42fc 100644
--- a/tools/perf/tests/dso-data.c
+++ b/tools/perf/tests/dso-data.c
@@ -6,6 +6,7 @@
#include <fcntl.h>
#include <string.h>

+#include "machine.h"
#include "symbol.h"

#define TEST_ASSERT_VAL(text, cond) \
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index db24a3f..d6d9a46 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -1,5 +1,6 @@
#include "symbol.h"
#include "dso.h"
+#include "machine.h"
#include "util.h"
#include "debug.h"

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 4c6754a..1f09d05 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -2,9 +2,192 @@
#include "event.h"
#include "machine.h"
#include "map.h"
+#include "strlist.h"
#include "thread.h"
#include <stdbool.h>

+int machine__init(struct machine *machine, const char *root_dir, pid_t pid)
+{
+ map_groups__init(&machine->kmaps);
+ RB_CLEAR_NODE(&machine->rb_node);
+ INIT_LIST_HEAD(&machine->user_dsos);
+ INIT_LIST_HEAD(&machine->kernel_dsos);
+
+ machine->threads = RB_ROOT;
+ INIT_LIST_HEAD(&machine->dead_threads);
+ machine->last_match = NULL;
+
+ machine->kmaps.machine = machine;
+ machine->pid = pid;
+
+ machine->root_dir = strdup(root_dir);
+ if (machine->root_dir == NULL)
+ return -ENOMEM;
+
+ if (pid != HOST_KERNEL_ID) {
+ struct thread *thread = machine__findnew_thread(machine, pid);
+ char comm[64];
+
+ if (thread == NULL)
+ return -ENOMEM;
+
+ snprintf(comm, sizeof(comm), "[guest/%d]", pid);
+ thread__set_comm(thread, comm);
+ }
+
+ return 0;
+}
+
+static void dsos__delete(struct list_head *dsos)
+{
+ struct dso *pos, *n;
+
+ list_for_each_entry_safe(pos, n, dsos, node) {
+ list_del(&pos->node);
+ dso__delete(pos);
+ }
+}
+
+void machine__exit(struct machine *machine)
+{
+ map_groups__exit(&machine->kmaps);
+ dsos__delete(&machine->user_dsos);
+ dsos__delete(&machine->kernel_dsos);
+ free(machine->root_dir);
+ machine->root_dir = NULL;
+}
+
+void machine__delete(struct machine *machine)
+{
+ machine__exit(machine);
+ free(machine);
+}
+
+struct machine *machines__add(struct rb_root *machines, pid_t pid,
+ const char *root_dir)
+{
+ struct rb_node **p = &machines->rb_node;
+ struct rb_node *parent = NULL;
+ struct machine *pos, *machine = malloc(sizeof(*machine));
+
+ if (machine == NULL)
+ return NULL;
+
+ if (machine__init(machine, root_dir, pid) != 0) {
+ free(machine);
+ return NULL;
+ }
+
+ while (*p != NULL) {
+ parent = *p;
+ pos = rb_entry(parent, struct machine, rb_node);
+ if (pid < pos->pid)
+ p = &(*p)->rb_left;
+ else
+ p = &(*p)->rb_right;
+ }
+
+ rb_link_node(&machine->rb_node, parent, p);
+ rb_insert_color(&machine->rb_node, machines);
+
+ return machine;
+}
+
+struct machine *machines__find(struct rb_root *machines, pid_t pid)
+{
+ struct rb_node **p = &machines->rb_node;
+ struct rb_node *parent = NULL;
+ struct machine *machine;
+ struct machine *default_machine = NULL;
+
+ while (*p != NULL) {
+ parent = *p;
+ machine = rb_entry(parent, struct machine, rb_node);
+ if (pid < machine->pid)
+ p = &(*p)->rb_left;
+ else if (pid > machine->pid)
+ p = &(*p)->rb_right;
+ else
+ return machine;
+ if (!machine->pid)
+ default_machine = machine;
+ }
+
+ return default_machine;
+}
+
+struct machine *machines__findnew(struct rb_root *machines, pid_t pid)
+{
+ char path[PATH_MAX];
+ const char *root_dir = "";
+ struct machine *machine = machines__find(machines, pid);
+
+ if (machine && (machine->pid == pid))
+ goto out;
+
+ if ((pid != HOST_KERNEL_ID) &&
+ (pid != DEFAULT_GUEST_KERNEL_ID) &&
+ (symbol_conf.guestmount)) {
+ sprintf(path, "%s/%d", symbol_conf.guestmount, pid);
+ if (access(path, R_OK)) {
+ static struct strlist *seen;
+
+ if (!seen)
+ seen = strlist__new(true, NULL);
+
+ if (!strlist__has_entry(seen, path)) {
+ pr_err("Can't access file %s\n", path);
+ strlist__add(seen, path);
+ }
+ machine = NULL;
+ goto out;
+ }
+ root_dir = path;
+ }
+
+ machine = machines__add(machines, pid, root_dir);
+out:
+ return machine;
+}
+
+void machines__process(struct rb_root *machines,
+ machine__process_t process, void *data)
+{
+ struct rb_node *nd;
+
+ for (nd = rb_first(machines); nd; nd = rb_next(nd)) {
+ struct machine *pos = rb_entry(nd, struct machine, rb_node);
+ process(pos, data);
+ }
+}
+
+char *machine__mmap_name(struct machine *machine, char *bf, size_t size)
+{
+ if (machine__is_host(machine))
+ snprintf(bf, size, "[%s]", "kernel.kallsyms");
+ else if (machine__is_default_guest(machine))
+ snprintf(bf, size, "[%s]", "guest.kernel.kallsyms");
+ else {
+ snprintf(bf, size, "[%s.%d]", "guest.kernel.kallsyms",
+ machine->pid);
+ }
+
+ return bf;
+}
+
+void machines__set_id_hdr_size(struct rb_root *machines, u16 id_hdr_size)
+{
+ struct rb_node *node;
+ struct machine *machine;
+
+ for (node = rb_first(machines); node; node = rb_next(node)) {
+ machine = rb_entry(node, struct machine, rb_node);
+ machine->id_hdr_size = id_hdr_size;
+ }
+
+ return;
+}
+
static struct thread *__machine__findnew_thread(struct machine *machine, pid_t pid,
bool create)
{
diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h
index df152f1..b7cde74 100644
--- a/tools/perf/util/machine.h
+++ b/tools/perf/util/machine.h
@@ -2,11 +2,40 @@
#define __PERF_MACHINE_H

#include <sys/types.h>
+#include <linux/rbtree.h>
+#include "map.h"

+struct branch_stack;
+struct perf_evsel;
+struct perf_sample;
+struct symbol;
struct thread;
-struct machine;
union perf_event;

+/* Native host kernel uses -1 as pid index in machine */
+#define HOST_KERNEL_ID (-1)
+#define DEFAULT_GUEST_KERNEL_ID (0)
+
+struct machine {
+ struct rb_node rb_node;
+ pid_t pid;
+ u16 id_hdr_size;
+ char *root_dir;
+ struct rb_root threads;
+ struct list_head dead_threads;
+ struct thread *last_match;
+ struct list_head user_dsos;
+ struct list_head kernel_dsos;
+ struct map_groups kmaps;
+ struct map *vmlinux_maps[MAP__NR_TYPES];
+};
+
+static inline
+struct map *machine__kernel_map(struct machine *machine, enum map_type type)
+{
+ return machine->vmlinux_maps[type];
+}
+
struct thread *machine__find_thread(struct machine *machine, pid_t pid);

int machine__process_comm_event(struct machine *machine, union perf_event *event);
@@ -16,4 +45,104 @@ int machine__process_lost_event(struct machine *machine, union perf_event *event
int machine__process_mmap_event(struct machine *machine, union perf_event *event);
int machine__process_event(struct machine *machine, union perf_event *event);

+typedef void (*machine__process_t)(struct machine *machine, void *data);
+
+void machines__process(struct rb_root *machines,
+ machine__process_t process, void *data);
+
+struct machine *machines__add(struct rb_root *machines, pid_t pid,
+ const char *root_dir);
+struct machine *machines__find_host(struct rb_root *machines);
+struct machine *machines__find(struct rb_root *machines, pid_t pid);
+struct machine *machines__findnew(struct rb_root *machines, pid_t pid);
+
+void machines__set_id_hdr_size(struct rb_root *machines, u16 id_hdr_size);
+char *machine__mmap_name(struct machine *machine, char *bf, size_t size);
+
+int machine__init(struct machine *machine, const char *root_dir, pid_t pid);
+void machine__exit(struct machine *machine);
+void machine__delete(struct machine *machine);
+
+
+struct branch_info *machine__resolve_bstack(struct machine *machine,
+ struct thread *thread,
+ struct branch_stack *bs);
+int machine__resolve_callchain(struct machine *machine,
+ struct perf_evsel *evsel,
+ struct thread *thread,
+ struct perf_sample *sample,
+ struct symbol **parent);
+
+/*
+ * Default guest kernel is defined by parameter --guestkallsyms
+ * and --guestmodules
+ */
+static inline bool machine__is_default_guest(struct machine *machine)
+{
+ return machine ? machine->pid == DEFAULT_GUEST_KERNEL_ID : false;
+}
+
+static inline bool machine__is_host(struct machine *machine)
+{
+ return machine ? machine->pid == HOST_KERNEL_ID : false;
+}
+
+struct thread *machine__findnew_thread(struct machine *machine, pid_t pid);
+void machine__remove_thread(struct machine *machine, struct thread *th);
+
+size_t machine__fprintf(struct machine *machine, FILE *fp);
+
+static inline
+struct symbol *machine__find_kernel_symbol(struct machine *machine,
+ enum map_type type, u64 addr,
+ struct map **mapp,
+ symbol_filter_t filter)
+{
+ return map_groups__find_symbol(&machine->kmaps, type, addr,
+ mapp, filter);
+}
+
+static inline
+struct symbol *machine__find_kernel_function(struct machine *machine, u64 addr,
+ struct map **mapp,
+ symbol_filter_t filter)
+{
+ return machine__find_kernel_symbol(machine, MAP__FUNCTION, addr,
+ mapp, filter);
+}
+
+static inline
+struct symbol *machine__find_kernel_function_by_name(struct machine *machine,
+ const char *name,
+ struct map **mapp,
+ symbol_filter_t filter)
+{
+ return map_groups__find_function_by_name(&machine->kmaps, name, mapp,
+ filter);
+}
+
+struct map *machine__new_module(struct machine *machine, u64 start,
+ const char *filename);
+
+int machine__load_kallsyms(struct machine *machine, const char *filename,
+ enum map_type type, symbol_filter_t filter);
+int machine__load_vmlinux_path(struct machine *machine, enum map_type type,
+ symbol_filter_t filter);
+
+size_t machine__fprintf_dsos_buildid(struct machine *machine,
+ FILE *fp, bool with_hits);
+size_t machines__fprintf_dsos(struct rb_root *machines, FILE *fp);
+size_t machines__fprintf_dsos_buildid(struct rb_root *machines,
+ FILE *fp, bool with_hits);
+
+void machine__destroy_kernel_maps(struct machine *machine);
+int __machine__create_kernel_maps(struct machine *machine, struct dso *kernel);
+int machine__create_kernel_maps(struct machine *machine);
+
+int machines__create_kernel_maps(struct rb_root *machines, pid_t pid);
+int machines__create_guest_kernel_maps(struct rb_root *machines);
+void machines__destroy_guest_kernel_maps(struct rb_root *machines);
+
+size_t machine__fprintf_vmlinux_path(struct machine *machine, FILE *fp);
+
#endif /* __PERF_MACHINE_H */
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index 5791878..0328d45 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -590,182 +590,3 @@ struct map *maps__find(struct rb_root *maps, u64 ip)

return NULL;
}
-
-int machine__init(struct machine *self, const char *root_dir, pid_t pid)
-{
- map_groups__init(&self->kmaps);
- RB_CLEAR_NODE(&self->rb_node);
- INIT_LIST_HEAD(&self->user_dsos);
- INIT_LIST_HEAD(&self->kernel_dsos);
-
- self->threads = RB_ROOT;
- INIT_LIST_HEAD(&self->dead_threads);
- self->last_match = NULL;
-
- self->kmaps.machine = self;
- self->pid = pid;
- self->root_dir = strdup(root_dir);
- if (self->root_dir == NULL)
- return -ENOMEM;
-
- if (pid != HOST_KERNEL_ID) {
- struct thread *thread = machine__findnew_thread(self, pid);
- char comm[64];
-
- if (thread == NULL)
- return -ENOMEM;
-
- snprintf(comm, sizeof(comm), "[guest/%d]", pid);
- thread__set_comm(thread, comm);
- }
-
- return 0;
-}
-
-static void dsos__delete(struct list_head *self)
-{
- struct dso *pos, *n;
-
- list_for_each_entry_safe(pos, n, self, node) {
- list_del(&pos->node);
- dso__delete(pos);
- }
-}
-
-void machine__exit(struct machine *self)
-{
- map_groups__exit(&self->kmaps);
- dsos__delete(&self->user_dsos);
- dsos__delete(&self->kernel_dsos);
- free(self->root_dir);
- self->root_dir = NULL;
-}
-
-void machine__delete(struct machine *self)
-{
- machine__exit(self);
- free(self);
-}
-
-struct machine *machines__add(struct rb_root *self, pid_t pid,
- const char *root_dir)
-{
- struct rb_node **p = &self->rb_node;
- struct rb_node *parent = NULL;
- struct machine *pos, *machine = malloc(sizeof(*machine));
-
- if (!machine)
- return NULL;
-
- if (machine__init(machine, root_dir, pid) != 0) {
- free(machine);
- return NULL;
- }
-
- while (*p != NULL) {
- parent = *p;
- pos = rb_entry(parent, struct machine, rb_node);
- if (pid < pos->pid)
- p = &(*p)->rb_left;
- else
- p = &(*p)->rb_right;
- }
-
- rb_link_node(&machine->rb_node, parent, p);
- rb_insert_color(&machine->rb_node, self);
-
- return machine;
-}
-
-struct machine *machines__find(struct rb_root *self, pid_t pid)
-{
- struct rb_node **p = &self->rb_node;
- struct rb_node *parent = NULL;
- struct machine *machine;
- struct machine *default_machine = NULL;
-
- while (*p != NULL) {
- parent = *p;
- machine = rb_entry(parent, struct machine, rb_node);
- if (pid < machine->pid)
- p = &(*p)->rb_left;
- else if (pid > machine->pid)
- p = &(*p)->rb_right;
- else
- return machine;
- if (!machine->pid)
- default_machine = machine;
- }
-
- return default_machine;
-}
-
-struct machine *machines__findnew(struct rb_root *self, pid_t pid)
-{
- char path[PATH_MAX];
- const char *root_dir = "";
- struct machine *machine = machines__find(self, pid);
-
- if (machine && (machine->pid == pid))
- goto out;
-
- if ((pid != HOST_KERNEL_ID) &&
- (pid != DEFAULT_GUEST_KERNEL_ID) &&
- (symbol_conf.guestmount)) {
- sprintf(path, "%s/%d", symbol_conf.guestmount, pid);
- if (access(path, R_OK)) {
- static struct strlist *seen;
-
- if (!seen)
- seen = strlist__new(true, NULL);
-
- if (!strlist__has_entry(seen, path)) {
- pr_err("Can't access file %s\n", path);
- strlist__add(seen, path);
- }
- machine = NULL;
- goto out;
- }
- root_dir = path;
- }
-
- machine = machines__add(self, pid, root_dir);
-
-out:
- return machine;
-}
-
-void machines__process(struct rb_root *self, machine__process_t process, void *data)
-{
- struct rb_node *nd;
-
- for (nd = rb_first(self); nd; nd = rb_next(nd)) {
- struct machine *pos = rb_entry(nd, struct machine, rb_node);
- process(pos, data);
- }
-}
-
-char *machine__mmap_name(struct machine *self, char *bf, size_t size)
-{
- if (machine__is_host(self))
- snprintf(bf, size, "[%s]", "kernel.kallsyms");
- else if (machine__is_default_guest(self))
- snprintf(bf, size, "[%s]", "guest.kernel.kallsyms");
- else
- snprintf(bf, size, "[%s.%d]", "guest.kernel.kallsyms", self->pid);
-
- return bf;
-}
-
-void machines__set_id_hdr_size(struct rb_root *machines, u16 id_hdr_size)
-{
- struct rb_node *node;
- struct machine *machine;
-
- for (node = rb_first(machines); node; node = rb_next(node)) {
- machine = rb_entry(node, struct machine, rb_node);
- machine->id_hdr_size = id_hdr_size;
- }
-
- return;
-}
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index d2250fc..bcb39e2 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -57,30 +57,6 @@ struct map_groups {
struct machine *machine;
};

-/* Native host kernel uses -1 as pid index in machine */
-#define HOST_KERNEL_ID (-1)
-#define DEFAULT_GUEST_KERNEL_ID (0)
-
-struct machine {
- struct rb_node rb_node;
- pid_t pid;
- u16 id_hdr_size;
- char *root_dir;
- struct rb_root threads;
- struct list_head dead_threads;
- struct thread *last_match;
- struct list_head user_dsos;
- struct list_head kernel_dsos;
- struct map_groups kmaps;
- struct map *vmlinux_maps[MAP__NR_TYPES];
-};
-
-static inline
-struct map *machine__kernel_map(struct machine *self, enum map_type type)
-{
- return self->vmlinux_maps[type];
-}
-
static inline struct kmap *map__kmap(struct map *self)
{
return (struct kmap *)(self + 1);
@@ -143,44 +119,9 @@ int map_groups__clone(struct map_groups *mg,
size_t map_groups__fprintf(struct map_groups *mg, int verbose, FILE *fp);
size_t map_groups__fprintf_maps(struct map_groups *mg, int verbose, FILE *fp);

-typedef void (*machine__process_t)(struct machine *self, void *data);
-
-void machines__process(struct rb_root *self, machine__process_t process, void *data);
-struct machine *machines__add(struct rb_root *self, pid_t pid,
- const char *root_dir);
-struct machine *machines__find_host(struct rb_root *self);
-struct machine *machines__find(struct rb_root *self, pid_t pid);
-struct machine *machines__findnew(struct rb_root *self, pid_t pid);
-void machines__set_id_hdr_size(struct rb_root *self, u16 id_hdr_size);
-char *machine__mmap_name(struct machine *self, char *bf, size_t size);
-int machine__init(struct machine *self, const char *root_dir, pid_t pid);
-void machine__exit(struct machine *self);
-void machine__delete(struct machine *self);
-
-struct perf_evsel;
-struct perf_sample;
-int machine__resolve_callchain(struct machine *machine,
- struct perf_evsel *evsel,
- struct thread *thread,
- struct perf_sample *sample,
- struct symbol **parent);
int maps__set_kallsyms_ref_reloc_sym(struct map **maps, const char *symbol_name,
u64 addr);

-/*
- * Default guest kernel is defined by parameter --guestkallsyms
- * and --guestmodules
- */
-static inline bool machine__is_default_guest(struct machine *self)
-{
- return self ? self->pid == DEFAULT_GUEST_KERNEL_ID : false;
-}
-
-static inline bool machine__is_host(struct machine *self)
-{
- return self ? self->pid == HOST_KERNEL_ID : false;
-}
-
static inline void map_groups__insert(struct map_groups *mg, struct map *map)
{
maps__insert(&mg->maps[map->type], map);
@@ -209,29 +150,6 @@ struct symbol *map_groups__find_symbol_by_name(struct map_groups *mg,
struct map **mapp,
symbol_filter_t filter);

-
-struct thread *machine__findnew_thread(struct machine *machine, pid_t pid);
-void machine__remove_thread(struct machine *machine, struct thread *th);
-
-size_t machine__fprintf(struct machine *machine, FILE *fp);
-
-static inline
-struct symbol *machine__find_kernel_symbol(struct machine *self,
- enum map_type type, u64 addr,
- struct map **mapp,
- symbol_filter_t filter)
-{
- return map_groups__find_symbol(&self->kmaps, type, addr, mapp, filter);
-}
-
-static inline
-struct symbol *machine__find_kernel_function(struct machine *self, u64 addr,
- struct map **mapp,
- symbol_filter_t filter)
-{
- return machine__find_kernel_symbol(self, MAP__FUNCTION, addr, mapp, filter);
-}
-
static inline
struct symbol *map_groups__find_function_by_name(struct map_groups *mg,
const char *name, struct map **mapp,
@@ -240,22 +158,11 @@ struct symbol *map_groups__find_function_by_name(struct map_groups *mg,
return map_groups__find_symbol_by_name(mg, MAP__FUNCTION, name, mapp, filter);
}

-static inline
-struct symbol *machine__find_kernel_function_by_name(struct machine *self,
- const char *name,
- struct map **mapp,
- symbol_filter_t filter)
-{
- return map_groups__find_function_by_name(&self->kmaps, name, mapp,
- filter);
-}
-
int map_groups__fixup_overlappings(struct map_groups *mg, struct map *map,
int verbose, FILE *fp);

struct map *map_groups__find_by_name(struct map_groups *mg,
enum map_type type, const char *name);
-struct map *machine__new_module(struct machine *self, u64 start, const char *filename);

void map_groups__flush(struct map_groups *mg);

diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
index dd64261..18d1222 100644
--- a/tools/perf/util/session.h
+++ b/tools/perf/util/session.h
@@ -4,6 +4,7 @@
#include "hist.h"
#include "event.h"
#include "header.h"
+#include "machine.h"
#include "symbol.h"
#include "thread.h"
#include <linux/rbtree.h>
@@ -68,10 +69,6 @@ int perf_session__resolve_callchain(struct perf_session *self, struct perf_evsel
struct ip_callchain *chain,
struct symbol **parent);

-struct branch_info *machine__resolve_bstack(struct machine *self,
- struct thread *thread,
- struct branch_stack *bs);
-
bool perf_session__has_traces(struct perf_session *self, const char *msg);

void mem_bswap_64(void *src, int byte_size);
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 624c65e..295f8d4 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -12,6 +12,7 @@
#include "build-id.h"
#include "util.h"
#include "debug.h"
+#include "machine.h"
#include "symbol.h"
#include "strlist.h"

diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 863b05b..04ccf29 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -200,16 +200,6 @@ int dso__load_vmlinux_path(struct dso *dso, struct map *map,
symbol_filter_t filter);
int dso__load_kallsyms(struct dso *dso, const char *filename, struct map *map,
symbol_filter_t filter);
-int machine__load_kallsyms(struct machine *machine, const char *filename,
- enum map_type type, symbol_filter_t filter);
-int machine__load_vmlinux_path(struct machine *machine, enum map_type type,
- symbol_filter_t filter);
-
-size_t machine__fprintf_dsos_buildid(struct machine *machine,
- FILE *fp, bool with_hits);
-size_t machines__fprintf_dsos(struct rb_root *machines, FILE *fp);
-size_t machines__fprintf_dsos_buildid(struct rb_root *machines,
- FILE *fp, bool with_hits);

struct symbol *dso__find_symbol(struct dso *dso, enum map_type type,
u64 addr);
@@ -224,14 +214,6 @@ int kallsyms__parse(const char *filename, void *arg,
int filename__read_debuglink(const char *filename, char *debuglink,
size_t size);

-void machine__destroy_kernel_maps(struct machine *machine);
-int __machine__create_kernel_maps(struct machine *machine, struct dso *kernel);
-int machine__create_kernel_maps(struct machine *machine);
-
-int machines__create_kernel_maps(struct rb_root *machines, pid_t pid);
-int machines__create_guest_kernel_maps(struct rb_root *machines);
-void machines__destroy_guest_kernel_maps(struct rb_root *machines);
-
int symbol__init(void);
void symbol__exit(void);
void symbol__elf_init(void);
@@ -242,8 +224,6 @@ size_t symbol__fprintf_symname(const struct symbol *sym, FILE *fp);
size_t symbol__fprintf(struct symbol *sym, FILE *fp);
bool symbol_type__is_a(char symbol_type, enum map_type map_type);

-size_t machine__fprintf_vmlinux_path(struct machine *machine, FILE *fp);
-
int dso__test_data(void);
int dso__load_sym(struct dso *dso, struct map *map, struct symsrc *syms_ss,
struct symsrc *runtime_ss, symbol_filter_t filter,
--
1.7.9.2.358.g22243

2012-11-09 21:47:13

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: [PATCH 05/21] perf tools: Add LIBDW_DIR Makefile variable to for alternate libdw

From: Jiri Olsa <[email protected]>

Adding LIBDW_DIR Makefile variable to be able to specify
alternate libdw library location.

To use it run make like:
$ make LIBDW_DIR=/opt/libdw/

Signed-off-by: Jiri Olsa <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Corey Ashford <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/Makefile | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 4ffcd02..cca5bb8 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -501,7 +501,14 @@ ifneq ($(call try-cc,$(SOURCE_LIBELF),$(FLAGS_LIBELF),libelf),y)
msg := $(error No gnu/libc-version.h found, please install glibc-dev[el]/glibc-static);
endif
else
- FLAGS_DWARF=$(ALL_CFLAGS) -ldw -lelf $(ALL_LDFLAGS) $(EXTLIBS)
+ # for linking with debug library, run like:
+ # make DEBUG=1 LIBDW_DIR=/opt/libdw/
+ ifdef LIBDW_DIR
+ LIBDW_CFLAGS := -I$(LIBDW_DIR)/include
+ LIBDW_LDFLAGS := -L$(LIBDW_DIR)/lib
+ endif
+
+ FLAGS_DWARF=$(ALL_CFLAGS) $(LIBDW_CFLAGS) -ldw -lelf $(LIBDW_LDFLAGS) $(ALL_LDFLAGS) $(EXTLIBS)
ifneq ($(call try-cc,$(SOURCE_DWARF),$(FLAGS_DWARF),libdw),y)
msg := $(warning No libdw.h found or old libdw.h found or elfutils is older than 0.138, disables dwarf support. Please install new elfutils-devel/libdw-dev);
NO_DWARF := 1
@@ -556,7 +563,8 @@ ifndef NO_DWARF
ifeq ($(origin PERF_HAVE_DWARF_REGS), undefined)
msg := $(warning DWARF register mappings have not been defined for architecture $(ARCH), DWARF support disabled);
else
- BASIC_CFLAGS += -DDWARF_SUPPORT
+ BASIC_CFLAGS := -DDWARF_SUPPORT $(LIBDW_CFLAGS) $(BASIC_CFLAGS)
+ BASIC_LDFLAGS := $(LIBDW_LDFLAGS) $(BASIC_LDFLAGS)
EXTLIBS += -lelf -ldw
LIB_OBJS += $(OUTPUT)util/probe-finder.o
LIB_OBJS += $(OUTPUT)util/dwarf-aux.o
--
1.7.9.2.358.g22243

2012-11-12 02:10:56

by Namhyung Kim

[permalink] [raw]
Subject: Re: [GIT PULL 00/21] perf/core improvements and fixes

Hi Arnaldo,

On Fri, 9 Nov 2012 18:42:49 -0300, Arnaldo Carvalho de Melo wrote:
> Hi Ingo,
>
> Please consider pulling.
>
> - Arnaldo
>
> The following changes since commit 8dfec403e39b7c37fd6e8813bacc01da1e1210ab:
>
> perf tests: Removing 'optional' field (2012-11-05 14:03:59 -0300)
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux tags/perf-core-for-mingo
>
> for you to fetch changes up to 27f94d52394003d444a383eaf8d4824daf32432e:
>
> tools lib traceevent: Use 'const' in variables pointing to const strings (2012-11-09 17:42:47 -0300)
>
> ----------------------------------------------------------------
> perf/core improvements and fixes:
>
> . Add a 'link' method for hists, so that we can have the leader with
> buckets for all the entries in all the hists. This new method
> is now used in the default 'diff' output, making the sum of the 'baseline'
> column be 100%, eliminating blind spots. Now we need to use this
> for 'diff' with > 2 perf.data files and for multi event 'report' and
> 'annotate'.

I'm not sure it can be used for group report at least in its current
form. IIUC it connects multiple hist entries using a list head and
create a dummy entry in the leader if need be. But it didn't handle
non-leader entries so it's hard to tell which is which if less entries
are present only. For example consider following case:

leader member1 member2
A A A
B
C
D

where leader, member1 and member2 are evsel/hists and A, B, C and D are
hist entries. After 'linking' the entries the leader will have
following linkage:

leader
A -> A -> A
B
C (dummy) -> C
D (dummy) -> D

In this case, for entry A the leader can determine which entry came from
which hists by looking its order in the list. For entry B the leader
can use zero value for them since the list is empty. However for
entries C and D, it cannot know which one is the right hists unless it
records a hist index or creates dummy entry and insert it in a correct
order (looks far from an optimal solution). Am I missing something?

Thanks,
Namhyung

2012-11-12 02:41:00

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH 10/21] perf hists: Introduce hists__link

On Fri, 9 Nov 2012 18:42:59 -0300, Arnaldo Carvalho de Melo wrote:
> From: Arnaldo Carvalho de Melo <[email protected]>
>
> That given two hists will find the hist_entries (buckets) in the second
> hists that are for the same bucket in the first and link them, then it
> will look for all buckets in the second that don't have a counterpart in
> the first and will create a dummy counterpart that will then be linked
> to the entry in the second.
>
> For multiple events this will be done pairing the leader with all the
> other events in the group, so that in the end the leader will have all
> the buckets in all the hists in a group, dummy or not while the other
> hists will be left untouched.
[snip]
> + he = hist_entry__new(pair);
> + if (he) {
> + he->stat.nr_events = 0;
> + he->stat.period = 0;

What about other fields?
Why not use "memset(&he->stat, 0, sizeof(he->stat))" for this?

Thanks,
Namhyung


> + he->hists = hists;
> + rb_link_node(&he->rb_node, parent, p);
> + rb_insert_color(&he->rb_node, &hists->entries);
> + hists__inc_nr_entries(hists, he);
> + }
> +out:
> + return he;
> +}
> +

2012-11-12 13:56:26

by Jiri Olsa

[permalink] [raw]
Subject: Re: [GIT PULL 00/21] perf/core improvements and fixes

On Mon, Nov 12, 2012 at 11:10:52AM +0900, Namhyung Kim wrote:
> Hi Arnaldo,
>
> On Fri, 9 Nov 2012 18:42:49 -0300, Arnaldo Carvalho de Melo wrote:
> > Hi Ingo,
> >
> > Please consider pulling.
> >
> > - Arnaldo
> >
> > The following changes since commit 8dfec403e39b7c37fd6e8813bacc01da1e1210ab:
> >
> > perf tests: Removing 'optional' field (2012-11-05 14:03:59 -0300)
> >
> > are available in the git repository at:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux tags/perf-core-for-mingo
> >
> > for you to fetch changes up to 27f94d52394003d444a383eaf8d4824daf32432e:
> >
> > tools lib traceevent: Use 'const' in variables pointing to const strings (2012-11-09 17:42:47 -0300)
> >
> > ----------------------------------------------------------------
> > perf/core improvements and fixes:
> >
> > . Add a 'link' method for hists, so that we can have the leader with
> > buckets for all the entries in all the hists. This new method
> > is now used in the default 'diff' output, making the sum of the 'baseline'
> > column be 100%, eliminating blind spots. Now we need to use this
> > for 'diff' with > 2 perf.data files and for multi event 'report' and
> > 'annotate'.
>
> I'm not sure it can be used for group report at least in its current
> form. IIUC it connects multiple hist entries using a list head and
> create a dummy entry in the leader if need be. But it didn't handle
> non-leader entries so it's hard to tell which is which if less entries
> are present only. For example consider following case:
>
> leader member1 member2
> A A A
> B
> C
> D
>
> where leader, member1 and member2 are evsel/hists and A, B, C and D are
> hist entries. After 'linking' the entries the leader will have
> following linkage:
>
> leader
> A -> A -> A
> B
> C (dummy) -> C
> D (dummy) -> D
>
> In this case, for entry A the leader can determine which entry came from
> which hists by looking its order in the list. For entry B the leader
> can use zero value for them since the list is empty. However for
> entries C and D, it cannot know which one is the right hists unless it
> records a hist index or creates dummy entry and insert it in a correct
> order (looks far from an optimal solution). Am I missing something?

there's hists pointer in hist_entry if that's what you look for

jirka

2012-11-12 16:02:06

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [GIT PULL 00/21] perf/core improvements and fixes

Em Mon, Nov 12, 2012 at 02:55:46PM +0100, Jiri Olsa escreveu:
> On Mon, Nov 12, 2012 at 11:10:52AM +0900, Namhyung Kim wrote:
> > On Fri, 9 Nov 2012 18:42:49 -0300, Arnaldo Carvalho de Melo wrote:
> > > . Add a 'link' method for hists, so that we can have the leader with
> > > buckets for all the entries in all the hists. This new method
> > > is now used in the default 'diff' output, making the sum of the 'baseline'
> > > column be 100%, eliminating blind spots. Now we need to use this
> > > for 'diff' with > 2 perf.data files and for multi event 'report' and
> > > 'annotate'.

> > I'm not sure it can be used for group report at least in its current
> > form. IIUC it connects multiple hist entries using a list head and
> > create a dummy entry in the leader if need be. But it didn't handle
> > non-leader entries so it's hard to tell which is which if less entries
> > are present only. For example consider following case:

> > leader member1 member2
> > A A A
> > B
> > C
> > D

> > where leader, member1 and member2 are evsel/hists and A, B, C and D are
> > hist entries. After 'linking' the entries the leader will have
> > following linkage:

> > leader
> > A -> A -> A
> > B
> > C (dummy) -> C
> > D (dummy) -> D

> > In this case, for entry A the leader can determine which entry came from
> > which hists by looking its order in the list. For entry B the leader
> > can use zero value for them since the list is empty. However for
> > entries C and D, it cannot know which one is the right hists unless it
> > records a hist index or creates dummy entry and insert it in a correct
> > order (looks far from an optimal solution). Am I missing something?

> there's hists pointer in hist_entry if that's what you look for

And from there to evsel->idx. In your patchset you even introduce
hists_2_evsel(), right?

- Arnaldo

2012-11-12 16:04:18

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 10/21] perf hists: Introduce hists__link

Em Mon, Nov 12, 2012 at 11:40:57AM +0900, Namhyung Kim escreveu:
> On Fri, 9 Nov 2012 18:42:59 -0300, Arnaldo Carvalho de Melo wrote:
> > From: Arnaldo Carvalho de Melo <[email protected]>

> > That given two hists will find the hist_entries (buckets) in the second
> > hists that are for the same bucket in the first and link them, then it
> > will look for all buckets in the second that don't have a counterpart in
> > the first and will create a dummy counterpart that will then be linked
> > to the entry in the second.

> > For multiple events this will be done pairing the leader with all the
> > other events in the group, so that in the end the leader will have all
> > the buckets in all the hists in a group, dummy or not while the other
> > hists will be left untouched.
> [snip]
> > + he = hist_entry__new(pair);
> > + if (he) {
> > + he->stat.nr_events = 0;
> > + he->stat.period = 0;
>
> What about other fields?
> Why not use "memset(&he->stat, 0, sizeof(he->stat))" for this?

Right, will do.

- Arnaldo

2012-11-13 01:20:36

by Namhyung Kim

[permalink] [raw]
Subject: Re: [GIT PULL 00/21] perf/core improvements and fixes

On Mon, 12 Nov 2012 13:01:39 -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Nov 12, 2012 at 02:55:46PM +0100, Jiri Olsa escreveu:
>> On Mon, Nov 12, 2012 at 11:10:52AM +0900, Namhyung Kim wrote:
>> > On Fri, 9 Nov 2012 18:42:49 -0300, Arnaldo Carvalho de Melo wrote:
>> > > . Add a 'link' method for hists, so that we can have the leader with
>> > > buckets for all the entries in all the hists. This new method
>> > > is now used in the default 'diff' output, making the sum of the 'baseline'
>> > > column be 100%, eliminating blind spots. Now we need to use this
>> > > for 'diff' with > 2 perf.data files and for multi event 'report' and
>> > > 'annotate'.
>
>> > I'm not sure it can be used for group report at least in its current
>> > form. IIUC it connects multiple hist entries using a list head and
>> > create a dummy entry in the leader if need be. But it didn't handle
>> > non-leader entries so it's hard to tell which is which if less entries
>> > are present only. For example consider following case:
>
>> > leader member1 member2
>> > A A A
>> > B
>> > C
>> > D
>
>> > where leader, member1 and member2 are evsel/hists and A, B, C and D are
>> > hist entries. After 'linking' the entries the leader will have
>> > following linkage:
>
>> > leader
>> > A -> A -> A
>> > B
>> > C (dummy) -> C
>> > D (dummy) -> D
>
>> > In this case, for entry A the leader can determine which entry came from
>> > which hists by looking its order in the list. For entry B the leader
>> > can use zero value for them since the list is empty. However for
>> > entries C and D, it cannot know which one is the right hists unless it
>> > records a hist index or creates dummy entry and insert it in a correct
>> > order (looks far from an optimal solution). Am I missing something?
>
>> there's hists pointer in hist_entry if that's what you look for
>
> And from there to evsel->idx. In your patchset you even introduce
> hists_2_evsel(), right?

Ah, okay. I worried about a possiblity of non-consecutive event groups
for some reason, but that's not gonna happen in the future?

Thanks,
Namhyung

2012-11-13 18:11:23

by Ingo Molnar

[permalink] [raw]
Subject: Re: [GIT PULL 00/21] perf/core improvements and fixes


* Arnaldo Carvalho de Melo <[email protected]> wrote:

> Hi Ingo,
>
> Please consider pulling.
>
> - Arnaldo
>
> The following changes since commit 8dfec403e39b7c37fd6e8813bacc01da1e1210ab:
>
> perf tests: Removing 'optional' field (2012-11-05 14:03:59 -0300)
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux tags/perf-core-for-mingo
>
> for you to fetch changes up to 27f94d52394003d444a383eaf8d4824daf32432e:
>
> tools lib traceevent: Use 'const' in variables pointing to const strings (2012-11-09 17:42:47 -0300)
>
> ----------------------------------------------------------------
> perf/core improvements and fixes:
>
> . Add a 'link' method for hists, so that we can have the leader with
> buckets for all the entries in all the hists. This new method
> is now used in the default 'diff' output, making the sum of the 'baseline'
> column be 100%, eliminating blind spots. Now we need to use this
> for 'diff' with > 2 perf.data files and for multi event 'report' and
> 'annotate'.
>
> . libtraceevent fixes for compiler warnings trying to make perf it build
> on some distros, like fedora 14, 32-bit, some of the warnings really
> pointed to real bugs.
>
> . Remove temp dir on failure in 'perf test', fix from Jiri Olsa.
>
> . Fixes for handling data, stack mmaps, from Namhyung Kim.
>
> . Fix live annotation bug related to recent objdump lookup patches, from
> Namhyung Kim
>
> . Don't try to follow jump target on PLT symbols in the annotation browser,
> fix from Namhyung Kim.
>
> . Fix leak on hist_entry delete, from Namhyung Kim.
>
> . Fix a CPU_ALLOC related build error on builtin-test, from Zheng Liu.
>
> Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
>
> ----------------------------------------------------------------
> Andi Kleen (1):
> perf tools: Add arbitary aliases and support names with -
>
> Arnaldo Carvalho de Melo (10):
> perf diff: Start moving to support matching more than two hists
> perf diff: Move hists__match to the hists lib
> perf hists: Introduce hists__link
> perf diff: Use hists__link when not pairing just with baseline
> perf machine: Move more methods to machine.[ch]
> tools lib traceevent: Add __maybe_unused to unused parameters
> tools lib traceevent: Avoid comparisions between signed/unsigned
> tools lib traceevent: No need to check for < 0 on an unsigned enum
> tools lib traceevent: Handle INVALID_ARG_TYPE errno in pevent_strerror
> tools lib traceevent: Use 'const' in variables pointing to const strings
>
> Jiri Olsa (2):
> perf tests: Move attr.py temp dir cleanup into finally section
> perf tools: Add LIBDW_DIR Makefile variable to for alternate libdw
>
> Namhyung Kim (7):
> perf machine: Set kernel data mapping length
> perf tools: Fix detection of stack area
> perf hists: Free branch_info when freeing hist_entry
> perf tools: Don't try to lookup objdump for live mode
> perf annotate: Whitespace fixups
> perf annotate: Don't try to follow jump target on PLT symbols
> perf annotate: Merge same lines in summary view
>
> Zheng Liu (1):
> perf test: fix a build error on builtin-test
>
> tools/lib/traceevent/event-parse.c | 22 ++--
> tools/perf/Makefile | 12 ++-
> tools/perf/arch/common.c | 7 ++
> tools/perf/builtin-diff.c | 48 ++-------
> tools/perf/tests/attr.py | 30 +++---
> tools/perf/tests/builtin-test.c | 39 +++----
> tools/perf/tests/dso-data.c | 1 +
> tools/perf/ui/browsers/annotate.c | 12 +++
> tools/perf/ui/hist.c | 10 +-
> tools/perf/util/annotate.c | 69 ++++++++++--
> tools/perf/util/annotate.h | 1 +
> tools/perf/util/dso.c | 1 +
> tools/perf/util/hist.c | 100 ++++++++++++++++++
> tools/perf/util/hist.h | 3 +
> tools/perf/util/machine.c | 205 ++++++++++++++++++++++++++++++++++--
> tools/perf/util/machine.h | 131 ++++++++++++++++++++++-
> tools/perf/util/map.c | 181 +------------------------------
> tools/perf/util/map.h | 93 ----------------
> tools/perf/util/parse-events.l | 2 +
> tools/perf/util/session.h | 5 +-
> tools/perf/util/sort.h | 27 ++++-
> tools/perf/util/symbol.c | 1 +
> tools/perf/util/symbol.h | 20 ----
> 23 files changed, 604 insertions(+), 416 deletions(-)

Pulled, thanks Arnaldo!

Ingo