=?y?q?=5BGIT=20PULL=2000/13=5D=20Annotation=20improvements=20=28G+=20edition=29?=

=?y?q?=5BGIT=20PULL=2000/13=5D=20Annotation=20improvements=20=28G+=20edition=29?=

[Arnaldo Carvalho de Melo]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 3525 bytes --]

Hi Ingo,

	Please consider pulling,

- Arnaldo

The following changes since commit a385ec4f11bdcf81af094c03e2444ee9b7fad2e5:

  Merge tag 'v3.4-rc2' into perf/core (2012-04-13 09:57:10 +0200)

are available in the git repository at:


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

for you to fetch changes up to 3f862fd076275c442dfe295eddb5650a6e0aecd4:

  perf annotate: Add missing jump variants (2012-04-19 17:10:12 -0300)

----------------------------------------------------------------
Annotate improvements

Now the default annotate browser uses a much more compact format, implementing
suggestions made made by several people, notably Linus.

Here is part of the new __list_del_entry annotation:

__list_del_entry
    8.47 │      push   %rbp
    8.47 │      mov    (%rdi),%rdx
   20.34 │      mov    $0xdead000000100100,%rcx
    3.39 │      mov    0x8(%rdi),%rax
    0.00 │      mov    %rsp,%rbp
    1.69 │      cmp    %rcx,%rdx
    0.00 │      je     43
    1.69 │      mov    $0xdead000000200200,%rcx
    3.39 │      cmp    %rcx,%rax
    0.00 │      je     a3
    5.08 │      mov    (%rax),%r8
   18.64 │      cmp    %r8,%rdi
    0.00 │      jne    84
    1.69 │      mov    0x8(%rdx),%r8
   25.42 │      cmp    %r8,%rdi
    0.00 │      jne    65
    1.69 │      mov    %rax,0x8(%rdx)
    0.00 │      mov    %rdx,(%rax)
    0.00 │      leaveq
    0.00 │      retq
    0.00 │ 43:  mov    %rdx,%r8
    0.00 │      mov    %rdi,%rcx
    0.00 │      mov    $0xffffffff817cd6a8,%rdx
    0.00 │      mov    $0x31,%esi
    0.00 │      mov    $0xffffffff817cd6e0,%rdi
    0.00 │      xor    %eax,%eax
    0.00 │      callq  ffffffff8104eab0 <warn_slowpath_fmt>
    0.00 │      leaveq
    0.00 │      retq
    0.00 │ 65:  mov    %rdi,%rcx
    0.00 │      mov    $0xffffffff817cd780,%rdx
    0.00 │      mov    $0x3a,%esi
    0.00 │      mov    $0xffffffff817cd6e0,%rdi
    0.00 │      xor    %eax,%eax
    0.00 │      callq  ffffffff8104eab0 <warn_slowpath_fmt>
    0.00 │      leaveq
    0.00 │      retq

The infrastructure is there to provide formatters for any instruction,
like the one I'll do for call functions to elide the address.

Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

----------------------------------------------------------------
Arnaldo Carvalho de Melo (13):
      perf annotate: Rename objdump_line to disasm_line
      perf annotate: Parse instruction
      perf annotate browser: Use the disasm_line instruction name and operand fields
      perf annotate: Disassembler instruction parsing
      perf annotate: Parse call targets earlier
      perf annotate: Introduce scnprintf ins_ops method
      perf annotate browser: Rename disasm_line_rb_node
      perf symbols: Introduce symbol__size method
      perf annotate browser: Hide non jump target addresses in offset mode
      perf annotate browser: Align jump labels
      perf annotate browser: Make lines more compact
      perf annotate browser: Use a vertical line as percentage separator
      perf annotate: Add missing jump variants

 tools/perf/ui/browsers/annotate.c |  323 +++++++++++++++++++++----------------
 tools/perf/util/annotate.c        |  263 +++++++++++++++++++++++++-----
 tools/perf/util/annotate.h        |   32 +++-
 tools/perf/util/symbol.h          |    5 +
 tools/perf/util/util.c            |   10 ++
 tools/perf/util/util.h            |    2 +
 6 files changed, 446 insertions(+), 189 deletions(-)

[PATCH 01/13] perf annotate: Rename objdump_line to disasm_line

[Arnaldo Carvalho de Melo]

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

We want to move away from using 'objdump -dS' as the only disassembler
supported.

Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-lsn9pjuxxm5ezsubyhkmprw7@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/annotate.c |  179 ++++++++++++++++++-------------------
 tools/perf/util/annotate.c        |   72 ++++++++-------
 tools/perf/util/annotate.h        |   11 ++-
 3 files changed, 128 insertions(+), 134 deletions(-)

diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 4db5186..bc540b1 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -15,7 +15,7 @@ struct annotate_browser {
 	struct ui_browser b;
 	struct rb_root	  entries;
 	struct rb_node	  *curr_hot;
-	struct objdump_line *selection;
+	struct disasm_line	  *selection;
 	u64		    start;
 	int		    nr_asm_entries;
 	int		    nr_entries;
@@ -25,26 +25,25 @@ struct annotate_browser {
 	char		    search_bf[128];
 };
 
-struct objdump_line_rb_node {
+struct disasm_line_rb_node {
 	struct rb_node	rb_node;
 	double		percent;
 	u32		idx;
 	int		idx_asm;
 };
 
-static inline
-struct objdump_line_rb_node *objdump_line__rb(struct objdump_line *self)
+static inline struct disasm_line_rb_node *disasm_line__rb(struct disasm_line *dl)
 {
-	return (struct objdump_line_rb_node *)(self + 1);
+	return (struct disasm_line_rb_node *)(dl + 1);
 }
 
-static bool objdump_line__filter(struct ui_browser *browser, void *entry)
+static bool disasm_line__filter(struct ui_browser *browser, void *entry)
 {
 	struct annotate_browser *ab = container_of(browser, struct annotate_browser, b);
 
 	if (ab->hide_src_code) {
-		struct objdump_line *ol = list_entry(entry, struct objdump_line, node);
-		return ol->offset == -1;
+		struct disasm_line *dl = list_entry(entry, struct disasm_line, node);
+		return dl->offset == -1;
 	}
 
 	return false;
@@ -53,17 +52,17 @@ static bool objdump_line__filter(struct ui_browser *browser, void *entry)
 static void annotate_browser__write(struct ui_browser *self, void *entry, int row)
 {
 	struct annotate_browser *ab = container_of(self, struct annotate_browser, b);
-	struct objdump_line *ol = list_entry(entry, struct objdump_line, node);
+	struct disasm_line *dl = list_entry(entry, struct disasm_line, node);
 	bool current_entry = ui_browser__is_current_entry(self, row);
 	bool change_color = (!ab->hide_src_code &&
 			     (!current_entry || (self->use_navkeypressed &&
 					         !self->navkeypressed)));
 	int width = self->width;
 
-	if (ol->offset != -1) {
-		struct objdump_line_rb_node *olrb = objdump_line__rb(ol);
-		ui_browser__set_percent_color(self, olrb->percent, current_entry);
-		slsmg_printf(" %7.2f ", olrb->percent);
+	if (dl->offset != -1) {
+		struct disasm_line_rb_node *dlrb = disasm_line__rb(dl);
+		ui_browser__set_percent_color(self, dlrb->percent, current_entry);
+		slsmg_printf(" %7.2f ", dlrb->percent);
 	} else {
 		ui_browser__set_percent_color(self, 0, current_entry);
 		slsmg_write_nstring(" ", 9);
@@ -76,16 +75,16 @@ static void annotate_browser__write(struct ui_browser *self, void *entry, int ro
 	if (!self->navkeypressed)
 		width += 1;
 
-	if (ol->offset != -1 && change_color)
+	if (dl->offset != -1 && change_color)
 		ui_browser__set_color(self, HE_COLORSET_CODE);
 
-	if (!*ol->line)
+	if (!*dl->line)
 		slsmg_write_nstring(" ", width - 18);
-	else if (ol->offset == -1)
-		slsmg_write_nstring(ol->line, width - 18);
+	else if (dl->offset == -1)
+		slsmg_write_nstring(dl->line, width - 18);
 	else {
 		char bf[64];
-		u64 addr = ol->offset;
+		u64 addr = dl->offset;
 		int printed, color = -1;
 
 		if (!ab->use_offset)
@@ -97,28 +96,27 @@ static void annotate_browser__write(struct ui_browser *self, void *entry, int ro
 		slsmg_write_nstring(bf, printed);
 		if (change_color)
 			ui_browser__set_color(self, color);
-		slsmg_write_nstring(ol->line, width - 18 - printed);
+		slsmg_write_nstring(dl->line, width - 18 - printed);
 	}
 
 	if (current_entry)
-		ab->selection = ol;
+		ab->selection = dl;
 }
 
-static double objdump_line__calc_percent(struct objdump_line *self,
-					 struct symbol *sym, int evidx)
+static double disasm_line__calc_percent(struct disasm_line *dl, struct symbol *sym, int evidx)
 {
 	double percent = 0.0;
 
-	if (self->offset != -1) {
+	if (dl->offset != -1) {
 		int len = sym->end - sym->start;
 		unsigned int hits = 0;
 		struct annotation *notes = symbol__annotation(sym);
 		struct source_line *src_line = notes->src->lines;
 		struct sym_hist *h = annotation__histogram(notes, evidx);
-		s64 offset = self->offset;
-		struct objdump_line *next;
+		s64 offset = dl->offset;
+		struct disasm_line *next;
 
-		next = objdump__get_next_ip_line(&notes->src->source, self);
+		next = disasm__get_next_ip_line(&notes->src->source, dl);
 		while (offset < (s64)len &&
 		       (next == NULL || offset < next->offset)) {
 			if (src_line) {
@@ -139,27 +137,26 @@ static double objdump_line__calc_percent(struct objdump_line *self,
 	return percent;
 }
 
-static void objdump__insert_line(struct rb_root *self,
-				 struct objdump_line_rb_node *line)
+static void disasm_rb_tree__insert(struct rb_root *root, struct disasm_line_rb_node *dlrb)
 {
-	struct rb_node **p = &self->rb_node;
+	struct rb_node **p = &root->rb_node;
 	struct rb_node *parent = NULL;
-	struct objdump_line_rb_node *l;
+	struct disasm_line_rb_node *l;
 
 	while (*p != NULL) {
 		parent = *p;
-		l = rb_entry(parent, struct objdump_line_rb_node, rb_node);
-		if (line->percent < l->percent)
+		l = rb_entry(parent, struct disasm_line_rb_node, rb_node);
+		if (dlrb->percent < l->percent)
 			p = &(*p)->rb_left;
 		else
 			p = &(*p)->rb_right;
 	}
-	rb_link_node(&line->rb_node, parent, p);
-	rb_insert_color(&line->rb_node, self);
+	rb_link_node(&dlrb->rb_node, parent, p);
+	rb_insert_color(&dlrb->rb_node, root);
 }
 
 static void annotate_browser__set_top(struct annotate_browser *self,
-				      struct objdump_line *pos, u32 idx)
+				      struct disasm_line *pos, u32 idx)
 {
 	unsigned back;
 
@@ -168,9 +165,9 @@ static void annotate_browser__set_top(struct annotate_browser *self,
 	self->b.top_idx = self->b.index = idx;
 
 	while (self->b.top_idx != 0 && back != 0) {
-		pos = list_entry(pos->node.prev, struct objdump_line, node);
+		pos = list_entry(pos->node.prev, struct disasm_line, node);
 
-		if (objdump_line__filter(&self->b, &pos->node))
+		if (disasm_line__filter(&self->b, &pos->node))
 			continue;
 
 		--self->b.top_idx;
@@ -184,11 +181,11 @@ static void annotate_browser__set_top(struct annotate_browser *self,
 static void annotate_browser__set_rb_top(struct annotate_browser *browser,
 					 struct rb_node *nd)
 {
-	struct objdump_line_rb_node *rbpos;
-	struct objdump_line *pos;
+	struct disasm_line_rb_node *rbpos;
+	struct disasm_line *pos;
 
-	rbpos = rb_entry(nd, struct objdump_line_rb_node, rb_node);
-	pos = ((struct objdump_line *)rbpos) - 1;
+	rbpos = rb_entry(nd, struct disasm_line_rb_node, rb_node);
+	pos = ((struct disasm_line *)rbpos) - 1;
 	annotate_browser__set_top(browser, pos, rbpos->idx);
 	browser->curr_hot = nd;
 }
@@ -199,20 +196,20 @@ static void annotate_browser__calc_percent(struct annotate_browser *browser,
 	struct map_symbol *ms = browser->b.priv;
 	struct symbol *sym = ms->sym;
 	struct annotation *notes = symbol__annotation(sym);
-	struct objdump_line *pos;
+	struct disasm_line *pos;
 
 	browser->entries = RB_ROOT;
 
 	pthread_mutex_lock(&notes->lock);
 
 	list_for_each_entry(pos, &notes->src->source, node) {
-		struct objdump_line_rb_node *rbpos = objdump_line__rb(pos);
-		rbpos->percent = objdump_line__calc_percent(pos, sym, evidx);
+		struct disasm_line_rb_node *rbpos = disasm_line__rb(pos);
+		rbpos->percent = disasm_line__calc_percent(pos, sym, evidx);
 		if (rbpos->percent < 0.01) {
 			RB_CLEAR_NODE(&rbpos->rb_node);
 			continue;
 		}
-		objdump__insert_line(&browser->entries, rbpos);
+		disasm_rb_tree__insert(&browser->entries, rbpos);
 	}
 	pthread_mutex_unlock(&notes->lock);
 
@@ -221,38 +218,38 @@ static void annotate_browser__calc_percent(struct annotate_browser *browser,
 
 static bool annotate_browser__toggle_source(struct annotate_browser *browser)
 {
-	struct objdump_line *ol;
-	struct objdump_line_rb_node *olrb;
+	struct disasm_line *dl;
+	struct disasm_line_rb_node *dlrb;
 	off_t offset = browser->b.index - browser->b.top_idx;
 
 	browser->b.seek(&browser->b, offset, SEEK_CUR);
-	ol = list_entry(browser->b.top, struct objdump_line, node);
-	olrb = objdump_line__rb(ol);
+	dl = list_entry(browser->b.top, struct disasm_line, node);
+	dlrb = disasm_line__rb(dl);
 
 	if (browser->hide_src_code) {
-		if (olrb->idx_asm < offset)
-			offset = olrb->idx;
+		if (dlrb->idx_asm < offset)
+			offset = dlrb->idx;
 
 		browser->b.nr_entries = browser->nr_entries;
 		browser->hide_src_code = false;
 		browser->b.seek(&browser->b, -offset, SEEK_CUR);
-		browser->b.top_idx = olrb->idx - offset;
-		browser->b.index = olrb->idx;
+		browser->b.top_idx = dlrb->idx - offset;
+		browser->b.index = dlrb->idx;
 	} else {
-		if (olrb->idx_asm < 0) {
+		if (dlrb->idx_asm < 0) {
 			ui_helpline__puts("Only available for assembly lines.");
 			browser->b.seek(&browser->b, -offset, SEEK_CUR);
 			return false;
 		}
 
-		if (olrb->idx_asm < offset)
-			offset = olrb->idx_asm;
+		if (dlrb->idx_asm < offset)
+			offset = dlrb->idx_asm;
 
 		browser->b.nr_entries = browser->nr_asm_entries;
 		browser->hide_src_code = true;
 		browser->b.seek(&browser->b, -offset, SEEK_CUR);
-		browser->b.top_idx = olrb->idx_asm - offset;
-		browser->b.index = olrb->idx_asm;
+		browser->b.top_idx = dlrb->idx_asm - offset;
+		browser->b.index = dlrb->idx_asm;
 	}
 
 	return true;
@@ -302,20 +299,20 @@ static bool annotate_browser__callq(struct annotate_browser *browser,
 	return true;
 }
 
-static struct objdump_line *
-	annotate_browser__find_offset(struct annotate_browser *browser,
-				      s64 offset, s64 *idx)
+static
+struct disasm_line *annotate_browser__find_offset(struct annotate_browser *browser,
+					  s64 offset, s64 *idx)
 {
 	struct map_symbol *ms = browser->b.priv;
 	struct symbol *sym = ms->sym;
 	struct annotation *notes = symbol__annotation(sym);
-	struct objdump_line *pos;
+	struct disasm_line *pos;
 
 	*idx = 0;
 	list_for_each_entry(pos, &notes->src->source, node) {
 		if (pos->offset == offset)
 			return pos;
-		if (!objdump_line__filter(&browser->b, &pos->node))
+		if (!disasm_line__filter(&browser->b, &pos->node))
 			++*idx;
 	}
 
@@ -325,7 +322,7 @@ static struct objdump_line *
 static bool annotate_browser__jump(struct annotate_browser *browser)
 {
 	const char *jumps[] = { "je ", "jne ", "ja ", "jmpq ", "js ", "jmp ", NULL };
-	struct objdump_line *line;
+	struct disasm_line *dl;
 	s64 idx, offset;
 	char *s = NULL;
 	int i = 0;
@@ -346,29 +343,29 @@ static bool annotate_browser__jump(struct annotate_browser *browser)
 	}
 
 	offset = strtoll(s, NULL, 16);
-	line = annotate_browser__find_offset(browser, offset, &idx);
-	if (line == NULL) {
+	dl = annotate_browser__find_offset(browser, offset, &idx);
+	if (dl == NULL) {
 		ui_helpline__puts("Invallid jump offset");
 		return true;
 	}
 
-	annotate_browser__set_top(browser, line, idx);
+	annotate_browser__set_top(browser, dl, idx);
 	
 	return true;
 }
 
-static struct objdump_line *
-	annotate_browser__find_string(struct annotate_browser *browser,
-				      char *s, s64 *idx)
+static
+struct disasm_line *annotate_browser__find_string(struct annotate_browser *browser,
+					  char *s, s64 *idx)
 {
 	struct map_symbol *ms = browser->b.priv;
 	struct symbol *sym = ms->sym;
 	struct annotation *notes = symbol__annotation(sym);
-	struct objdump_line *pos = browser->selection;
+	struct disasm_line *pos = browser->selection;
 
 	*idx = browser->b.index;
 	list_for_each_entry_continue(pos, &notes->src->source, node) {
-		if (objdump_line__filter(&browser->b, &pos->node))
+		if (disasm_line__filter(&browser->b, &pos->node))
 			continue;
 
 		++*idx;
@@ -382,32 +379,32 @@ static struct objdump_line *
 
 static bool __annotate_browser__search(struct annotate_browser *browser)
 {
-	struct objdump_line *line;
+	struct disasm_line *dl;
 	s64 idx;
 
-	line = annotate_browser__find_string(browser, browser->search_bf, &idx);
-	if (line == NULL) {
+	dl = annotate_browser__find_string(browser, browser->search_bf, &idx);
+	if (dl == NULL) {
 		ui_helpline__puts("String not found!");
 		return false;
 	}
 
-	annotate_browser__set_top(browser, line, idx);
+	annotate_browser__set_top(browser, dl, idx);
 	browser->searching_backwards = false;
 	return true;
 }
 
-static struct objdump_line *
-	annotate_browser__find_string_reverse(struct annotate_browser *browser,
-					      char *s, s64 *idx)
+static
+struct disasm_line *annotate_browser__find_string_reverse(struct annotate_browser *browser,
+						  char *s, s64 *idx)
 {
 	struct map_symbol *ms = browser->b.priv;
 	struct symbol *sym = ms->sym;
 	struct annotation *notes = symbol__annotation(sym);
-	struct objdump_line *pos = browser->selection;
+	struct disasm_line *pos = browser->selection;
 
 	*idx = browser->b.index;
 	list_for_each_entry_continue_reverse(pos, &notes->src->source, node) {
-		if (objdump_line__filter(&browser->b, &pos->node))
+		if (disasm_line__filter(&browser->b, &pos->node))
 			continue;
 
 		--*idx;
@@ -421,16 +418,16 @@ static struct objdump_line *
 
 static bool __annotate_browser__search_reverse(struct annotate_browser *browser)
 {
-	struct objdump_line *line;
+	struct disasm_line *dl;
 	s64 idx;
 
-	line = annotate_browser__find_string_reverse(browser, browser->search_bf, &idx);
-	if (line == NULL) {
+	dl = annotate_browser__find_string_reverse(browser, browser->search_bf, &idx);
+	if (dl == NULL) {
 		ui_helpline__puts("String not found!");
 		return false;
 	}
 
-	annotate_browser__set_top(browser, line, idx);
+	annotate_browser__set_top(browser, dl, idx);
 	browser->searching_backwards = true;
 	return true;
 }
@@ -613,7 +610,7 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map, int evidx,
 			 void(*timer)(void *arg), void *arg,
 			 int delay_secs)
 {
-	struct objdump_line *pos, *n;
+	struct disasm_line *pos, *n;
 	struct annotation *notes;
 	struct map_symbol ms = {
 		.map = map,
@@ -624,7 +621,7 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map, int evidx,
 			.refresh = ui_browser__list_head_refresh,
 			.seek	 = ui_browser__list_head_seek,
 			.write	 = annotate_browser__write,
-			.filter  = objdump_line__filter,
+			.filter  = disasm_line__filter,
 			.priv	 = &ms,
 			.use_navkeypressed = true,
 		},
@@ -637,7 +634,7 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map, int evidx,
 	if (map->dso->annotate_warned)
 		return -1;
 
-	if (symbol__annotate(sym, map, sizeof(struct objdump_line_rb_node)) < 0) {
+	if (symbol__annotate(sym, map, sizeof(struct disasm_line_rb_node)) < 0) {
 		ui__error("%s", ui_helpline__last_msg);
 		return -1;
 	}
@@ -648,12 +645,12 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map, int evidx,
 	browser.start = map__rip_2objdump(map, sym->start);
 
 	list_for_each_entry(pos, &notes->src->source, node) {
-		struct objdump_line_rb_node *rbpos;
+		struct disasm_line_rb_node *rbpos;
 		size_t line_len = strlen(pos->line);
 
 		if (browser.b.width < line_len)
 			browser.b.width = line_len;
-		rbpos = objdump_line__rb(pos);
+		rbpos = disasm_line__rb(pos);
 		rbpos->idx = browser.nr_entries++;
 		if (pos->offset != -1)
 			rbpos->idx_asm = browser.nr_asm_entries++;
@@ -667,7 +664,7 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map, int evidx,
 	ret = annotate_browser__run(&browser, evidx, timer, arg, delay_secs);
 	list_for_each_entry_safe(pos, n, &notes->src->source, node) {
 		list_del(&pos->node);
-		objdump_line__free(pos);
+		disasm_line__free(pos);
 	}
 	return ret;
 }
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 1e7fd52..ef1d57d 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -78,36 +78,35 @@ int symbol__inc_addr_samples(struct symbol *sym, struct map *map,
 	return 0;
 }
 
-static struct objdump_line *objdump_line__new(s64 offset, char *line, size_t privsize)
+static struct disasm_line *disasm_line__new(s64 offset, char *line, size_t privsize)
 {
-	struct objdump_line *self = malloc(sizeof(*self) + privsize);
+	struct disasm_line *dl = malloc(sizeof(*dl) + privsize);
 
-	if (self != NULL) {
-		self->offset = offset;
-		self->line = strdup(line);
-		if (self->line == NULL)
+	if (dl != NULL) {
+		dl->offset = offset;
+		dl->line = strdup(line);
+		if (dl->line == NULL)
 			goto out_delete;
 	}
 
-	return self;
+	return dl;
 out_delete:
-	free(self);
+	free(dl);
 	return NULL;
 }
 
-void objdump_line__free(struct objdump_line *self)
+void disasm_line__free(struct disasm_line *dl)
 {
-	free(self->line);
-	free(self);
+	free(dl->line);
+	free(dl);
 }
 
-static void objdump__add_line(struct list_head *head, struct objdump_line *line)
+static void disasm__add(struct list_head *head, struct disasm_line *line)
 {
 	list_add_tail(&line->node, head);
 }
 
-struct objdump_line *objdump__get_next_ip_line(struct list_head *head,
-					       struct objdump_line *pos)
+struct disasm_line *disasm__get_next_ip_line(struct list_head *head, struct disasm_line *pos)
 {
 	list_for_each_entry_continue(pos, head, node)
 		if (pos->offset >= 0)
@@ -116,15 +115,14 @@ struct objdump_line *objdump__get_next_ip_line(struct list_head *head,
 	return NULL;
 }
 
-static int objdump_line__print(struct objdump_line *oline, struct symbol *sym,
-			       u64 start, int evidx, u64 len, int min_pcnt,
-			       int printed, int max_lines,
-			       struct objdump_line *queue)
+static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 start,
+		      int evidx, u64 len, int min_pcnt, int printed,
+		      int max_lines, struct disasm_line *queue)
 {
 	static const char *prev_line;
 	static const char *prev_color;
 
-	if (oline->offset != -1) {
+	if (dl->offset != -1) {
 		const char *path = NULL;
 		unsigned int hits = 0;
 		double percent = 0.0;
@@ -132,11 +130,11 @@ static int objdump_line__print(struct objdump_line *oline, struct symbol *sym,
 		struct annotation *notes = symbol__annotation(sym);
 		struct source_line *src_line = notes->src->lines;
 		struct sym_hist *h = annotation__histogram(notes, evidx);
-		s64 offset = oline->offset;
+		s64 offset = dl->offset;
 		const u64 addr = start + offset;
-		struct objdump_line *next;
+		struct disasm_line *next;
 
-		next = objdump__get_next_ip_line(&notes->src->source, oline);
+		next = disasm__get_next_ip_line(&notes->src->source, dl);
 
 		while (offset < (s64)len &&
 		       (next == NULL || offset < next->offset)) {
@@ -161,9 +159,9 @@ static int objdump_line__print(struct objdump_line *oline, struct symbol *sym,
 
 		if (queue != NULL) {
 			list_for_each_entry_from(queue, &notes->src->source, node) {
-				if (queue == oline)
+				if (queue == dl)
 					break;
-				objdump_line__print(queue, sym, start, evidx, len,
+				disasm_line__print(queue, sym, start, evidx, len,
 						    0, 0, 1, NULL);
 			}
 		}
@@ -187,17 +185,17 @@ static int objdump_line__print(struct objdump_line *oline, struct symbol *sym,
 		color_fprintf(stdout, color, " %7.2f", percent);
 		printf(" :	");
 		color_fprintf(stdout, PERF_COLOR_MAGENTA, "  %" PRIx64 ":", addr);
-		color_fprintf(stdout, PERF_COLOR_BLUE, "%s\n", oline->line);
+		color_fprintf(stdout, PERF_COLOR_BLUE, "%s\n", dl->line);
 	} else if (max_lines && printed >= max_lines)
 		return 1;
 	else {
 		if (queue)
 			return -1;
 
-		if (!*oline->line)
+		if (!*dl->line)
 			printf("         :\n");
 		else
-			printf("         :	%s\n", oline->line);
+			printf("         :	%s\n", dl->line);
 	}
 
 	return 0;
@@ -207,7 +205,7 @@ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map,
 				      FILE *file, size_t privsize)
 {
 	struct annotation *notes = symbol__annotation(sym);
-	struct objdump_line *objdump_line;
+	struct disasm_line *dl;
 	char *line = NULL, *parsed_line, *tmp, *tmp2, *c;
 	size_t line_len;
 	s64 line_ip, offset = -1;
@@ -258,13 +256,13 @@ static int symbol__parse_objdump_line(struct symbol *sym, struct map *map,
 			parsed_line = tmp2 + 1;
 	}
 
-	objdump_line = objdump_line__new(offset, parsed_line, privsize);
+	dl = disasm_line__new(offset, parsed_line, privsize);
 	free(line);
 
-	if (objdump_line == NULL)
+	if (dl == NULL)
 		return -1;
 
-	objdump__add_line(&notes->src->source, objdump_line);
+	disasm__add(&notes->src->source, dl);
 
 	return 0;
 }
@@ -503,7 +501,7 @@ int symbol__annotate_printf(struct symbol *sym, struct map *map, int evidx,
 	struct dso *dso = map->dso;
 	const char *filename = dso->long_name, *d_filename;
 	struct annotation *notes = symbol__annotation(sym);
-	struct objdump_line *pos, *queue = NULL;
+	struct disasm_line *pos, *queue = NULL;
 	u64 start = map__rip_2objdump(map, sym->start);
 	int printed = 2, queue_len = 0;
 	int more = 0;
@@ -528,7 +526,7 @@ int symbol__annotate_printf(struct symbol *sym, struct map *map, int evidx,
 			queue_len = 0;
 		}
 
-		switch (objdump_line__print(pos, sym, start, evidx, len,
+		switch (disasm_line__print(pos, sym, start, evidx, len,
 					    min_pcnt, printed, max_lines,
 					    queue)) {
 		case 0:
@@ -583,13 +581,13 @@ void symbol__annotate_decay_histogram(struct symbol *sym, int evidx)
 	}
 }
 
-void objdump_line_list__purge(struct list_head *head)
+void disasm__purge(struct list_head *head)
 {
-	struct objdump_line *pos, *n;
+	struct disasm_line *pos, *n;
 
 	list_for_each_entry_safe(pos, n, head, node) {
 		list_del(&pos->node);
-		objdump_line__free(pos);
+		disasm_line__free(pos);
 	}
 }
 
@@ -618,7 +616,7 @@ int symbol__tty_annotate(struct symbol *sym, struct map *map, int evidx,
 	if (print_lines)
 		symbol__free_source_line(sym, len);
 
-	objdump_line_list__purge(&symbol__annotation(sym)->src->source);
+	disasm__purge(&symbol__annotation(sym)->src->source);
 
 	return 0;
 }
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index efa5dc8..8bb68bb 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -7,15 +7,14 @@
 #include <linux/list.h>
 #include <linux/rbtree.h>
 
-struct objdump_line {
+struct disasm_line {
 	struct list_head node;
 	s64		 offset;
 	char		 *line;
 };
 
-void objdump_line__free(struct objdump_line *self);
-struct objdump_line *objdump__get_next_ip_line(struct list_head *head,
-					       struct objdump_line *pos);
+void disasm_line__free(struct disasm_line *dl);
+struct disasm_line *disasm__get_next_ip_line(struct list_head *head, struct disasm_line *pos);
 
 struct sym_hist {
 	u64		sum;
@@ -32,7 +31,7 @@ struct source_line {
  *
  * @histogram: Array of addr hit histograms per event being monitored
  * @lines: If 'print_lines' is specified, per source code line percentages
- * @source: source parsed from objdump -dS
+ * @source: source parsed from a disassembler like objdump -dS
  *
  * lines is allocated, percentages calculated and all sorted by percentage
  * when the annotation is about to be presented, so the percentages are for
@@ -82,7 +81,7 @@ int symbol__annotate_printf(struct symbol *sym, struct map *map, int evidx,
 			    int context);
 void symbol__annotate_zero_histogram(struct symbol *sym, int evidx);
 void symbol__annotate_decay_histogram(struct symbol *sym, int evidx);
-void objdump_line_list__purge(struct list_head *head);
+void disasm__purge(struct list_head *head);
 
 int symbol__tty_annotate(struct symbol *sym, struct map *map, int evidx,
 			 bool print_lines, bool full_paths, int min_pcnt,
-- 
1.7.9.2.358.g22243

[PATCH 02/13] perf annotate: Parse instruction

[Arnaldo Carvalho de Melo]

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

For lines with instructions find the name and operands, breaking those
tokens for consumption by the browser.

Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-6aazb9f5o3d9zi28e6rruv12@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/annotate.c |   65 +++++++++++++++++++++++++++++++++++++++++++-
 tools/perf/util/annotate.h |    3 ++
 2 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index ef1d57d..a72585a 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -80,16 +80,50 @@ int symbol__inc_addr_samples(struct symbol *sym, struct map *map,
 
 static struct disasm_line *disasm_line__new(s64 offset, char *line, size_t privsize)
 {
-	struct disasm_line *dl = malloc(sizeof(*dl) + privsize);
+	struct disasm_line *dl = zalloc(sizeof(*dl) + privsize);
 
 	if (dl != NULL) {
 		dl->offset = offset;
 		dl->line = strdup(line);
 		if (dl->line == NULL)
 			goto out_delete;
+
+		if (offset != -1) {
+			char *name = dl->line, tmp;
+
+			while (isspace(name[0]))
+				++name;
+
+			if (name[0] == '\0')
+				goto out_delete;
+
+			dl->operands = name + 1;
+
+			while (dl->operands[0] != '\0' &&
+			       !isspace(dl->operands[0]))
+				++dl->operands;
+
+			tmp = dl->operands[0];
+			dl->operands[0] = '\0';
+			dl->name = strdup(name);
+
+			if (dl->name == NULL)
+				goto out_free_line;
+
+			dl->operands[0] = tmp;
+
+			if (dl->operands[0] != '\0') {
+				dl->operands++;
+				while (isspace(dl->operands[0]))
+					++dl->operands;
+			}
+		}
 	}
 
 	return dl;
+
+out_free_line:
+	free(dl->line);
 out_delete:
 	free(dl);
 	return NULL;
@@ -98,6 +132,7 @@ out_delete:
 void disasm_line__free(struct disasm_line *dl)
 {
 	free(dl->line);
+	free(dl->name);
 	free(dl);
 }
 
@@ -591,6 +626,34 @@ void disasm__purge(struct list_head *head)
 	}
 }
 
+static size_t disasm_line__fprintf(struct disasm_line *dl, FILE *fp)
+{
+	size_t printed;
+
+	if (dl->offset == -1)
+		return fprintf(fp, "%s\n", dl->line);
+
+	printed = fprintf(fp, "%#" PRIx64 " %s", dl->offset, dl->name);
+
+	if (dl->operands[0] != '\0') {
+		printed += fprintf(fp, "%.*s %s\n", 6 - (int)printed, " ",
+				   dl->operands);
+	}
+
+	return printed + fprintf(fp, "\n");
+}
+
+size_t disasm__fprintf(struct list_head *head, FILE *fp)
+{
+	struct disasm_line *pos;
+	size_t printed = 0;
+
+	list_for_each_entry(pos, head, node)
+		printed += disasm_line__fprintf(pos, fp);
+
+	return printed;
+}
+
 int symbol__tty_annotate(struct symbol *sym, struct map *map, int evidx,
 			 bool print_lines, bool full_paths, int min_pcnt,
 			 int max_lines)
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 8bb68bb..dd7636d 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -11,10 +11,13 @@ struct disasm_line {
 	struct list_head node;
 	s64		 offset;
 	char		 *line;
+	char		 *name;
+	char		 *operands;
 };
 
 void disasm_line__free(struct disasm_line *dl);
 struct disasm_line *disasm__get_next_ip_line(struct list_head *head, struct disasm_line *pos);
+size_t disasm__fprintf(struct list_head *head, FILE *fp);
 
 struct sym_hist {
 	u64		sum;
-- 
1.7.9.2.358.g22243

Re: [PATCH 02/13] perf annotate: Parse instruction

[David Ahern]

On 4/19/12 2:33 PM, Arnaldo Carvalho de Melo wrote:
> From: Arnaldo Carvalho de Melo<acme@redhat.com>
>
> For lines with instructions find the name and operands, breaking those
> tokens for consumption by the browser.
>
> Cc: David Ahern<dsahern@gmail.com>
> Cc: Frederic Weisbecker<fweisbec@gmail.com>
> Cc: Mike Galbraith<efault@gmx.de>
> Cc: Namhyung Kim<namhyung@gmail.com>
> Cc: Paul Mackerras<paulus@samba.org>
> Cc: Peter Zijlstra<peterz@infradead.org>
> Cc: Stephane Eranian<eranian@google.com>
> Link: http://lkml.kernel.org/n/tip-6aazb9f5o3d9zi28e6rruv12@git.kernel.org
> Signed-off-by: Arnaldo Carvalho de Melo<acme@redhat.com>
> ---
>   tools/perf/util/annotate.c |   65 +++++++++++++++++++++++++++++++++++++++++++-
>   tools/perf/util/annotate.h |    3 ++
>   2 files changed, 67 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index ef1d57d..a72585a 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -80,16 +80,50 @@ int symbol__inc_addr_samples(struct symbol *sym, struct map *map,
>
>   static struct disasm_line *disasm_line__new(s64 offset, char *line, size_t privsize)
>   {
> -	struct disasm_line *dl = malloc(sizeof(*dl) + privsize);
> +	struct disasm_line *dl = zalloc(sizeof(*dl) + privsize);
>
>   	if (dl != NULL) {
>   		dl->offset = offset;
>   		dl->line = strdup(line);
>   		if (dl->line == NULL)
>   			goto out_delete;
> +
> +		if (offset != -1) {
> +			char *name = dl->line, tmp;
> +
> +			while (isspace(name[0]))
> +				++name;
> +
> +			if (name[0] == '\0')
> +				goto out_delete;
> +
> +			dl->operands = name + 1;
> +
> +			while (dl->operands[0] != '\0'&&
> +			       !isspace(dl->operands[0]))
> +				++dl->operands;
> +
> +			tmp = dl->operands[0];
> +			dl->operands[0] = '\0';
> +			dl->name = strdup(name);

This strdup seems unnecessary. The parsing seems to be:

dl->line = "\W*name\W*operands"
    dl->name = -^    ^- = dl_operands

so name and operands are 2 different parts of the same buffer.

David

Re: [PATCH 02/13] perf annotate: Parse instruction

[Arnaldo Carvalho de Melo]

Em Thu, Apr 19, 2012 at 05:55:57PM -0600, David Ahern escreveu:
> On 4/19/12 2:33 PM, Arnaldo Carvalho de Melo wrote:
> >  		dl->offset = offset;
> >  		dl->line = strdup(line);
> >  		if (dl->line == NULL)
> >  			goto out_delete;
> >+
> >+		if (offset != -1) {
> >+			char *name = dl->line, tmp;
> >+
> >+			while (isspace(name[0]))
> >+				++name;
> >+
> >+			if (name[0] == '\0')
> >+				goto out_delete;
> >+
> >+			dl->operands = name + 1;
> >+
> >+			while (dl->operands[0] != '\0'&&
> >+			       !isspace(dl->operands[0]))
> >+				++dl->operands;
> >+
> >+			tmp = dl->operands[0];
> >+			dl->operands[0] = '\0';
> >+			dl->name = strdup(name);
> 
> This strdup seems unnecessary. The parsing seems to be:
> 
> dl->line = "\W*name\W*operands"
>    dl->name = -^    ^- = dl_operands
> 
> so name and operands are 2 different parts of the same buffer.

Yeah, but we still want to do searches in the whole line, sure we could
do that using the name + operands parts, but I got lazy at that point.

- Arnaldo

Re: [PATCH 02/13] perf annotate: Parse instruction

[Namhyung Kim]

Hi,

On Thu, 19 Apr 2012 17:33:57 -0300, Arnaldo Carvalho de Melo wrote:
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> For lines with instructions find the name and operands, breaking those
> tokens for consumption by the browser.
>

Just a minor nit below...


> Cc: David Ahern <dsahern@gmail.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Mike Galbraith <efault@gmx.de>
> Cc: Namhyung Kim <namhyung@gmail.com>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Stephane Eranian <eranian@google.com>
> Link: http://lkml.kernel.org/n/tip-6aazb9f5o3d9zi28e6rruv12@git.kernel.org
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
>  tools/perf/util/annotate.c |   65 +++++++++++++++++++++++++++++++++++++++++++-
>  tools/perf/util/annotate.h |    3 ++
>  2 files changed, 67 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index ef1d57d..a72585a 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -80,16 +80,50 @@ int symbol__inc_addr_samples(struct symbol *sym, struct map *map,
>  
>  static struct disasm_line *disasm_line__new(s64 offset, char *line, size_t privsize)
>  {
> -	struct disasm_line *dl = malloc(sizeof(*dl) + privsize);
> +	struct disasm_line *dl = zalloc(sizeof(*dl) + privsize);
>  
>  	if (dl != NULL) {
>  		dl->offset = offset;
>  		dl->line = strdup(line);
>  		if (dl->line == NULL)
>  			goto out_delete;
> +
> +		if (offset != -1) {

To avoid deep nesting, how about this:

	if (dl == NULL)
		return NULL;

	<snip>

	if (offset == -1)
		return dl;

Thanks,
Namhyung


> +			char *name = dl->line, tmp;
> +
> +			while (isspace(name[0]))
> +				++name;
> +
> +			if (name[0] == '\0')
> +				goto out_delete;
> +
> +			dl->operands = name + 1;
> +
> +			while (dl->operands[0] != '\0' &&
> +			       !isspace(dl->operands[0]))
> +				++dl->operands;
> +
> +			tmp = dl->operands[0];
> +			dl->operands[0] = '\0';
> +			dl->name = strdup(name);
> +
> +			if (dl->name == NULL)
> +				goto out_free_line;
> +
> +			dl->operands[0] = tmp;
> +
> +			if (dl->operands[0] != '\0') {
> +				dl->operands++;
> +				while (isspace(dl->operands[0]))
> +					++dl->operands;
> +			}
> +		}
>  	}
>  
>  	return dl;
> +
> +out_free_line:
> +	free(dl->line);
>  out_delete:
>  	free(dl);
>  	return NULL;
> @@ -98,6 +132,7 @@ out_delete:
>  void disasm_line__free(struct disasm_line *dl)
>  {
>  	free(dl->line);
> +	free(dl->name);
>  	free(dl);
>  }
>  
> @@ -591,6 +626,34 @@ void disasm__purge(struct list_head *head)
>  	}
>  }
>  
> +static size_t disasm_line__fprintf(struct disasm_line *dl, FILE *fp)
> +{
> +	size_t printed;
> +
> +	if (dl->offset == -1)
> +		return fprintf(fp, "%s\n", dl->line);
> +
> +	printed = fprintf(fp, "%#" PRIx64 " %s", dl->offset, dl->name);
> +
> +	if (dl->operands[0] != '\0') {
> +		printed += fprintf(fp, "%.*s %s\n", 6 - (int)printed, " ",
> +				   dl->operands);
> +	}
> +
> +	return printed + fprintf(fp, "\n");
> +}
> +
> +size_t disasm__fprintf(struct list_head *head, FILE *fp)
> +{
> +	struct disasm_line *pos;
> +	size_t printed = 0;
> +
> +	list_for_each_entry(pos, head, node)
> +		printed += disasm_line__fprintf(pos, fp);
> +
> +	return printed;
> +}
> +
>  int symbol__tty_annotate(struct symbol *sym, struct map *map, int evidx,
>  			 bool print_lines, bool full_paths, int min_pcnt,
>  			 int max_lines)
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index 8bb68bb..dd7636d 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -11,10 +11,13 @@ struct disasm_line {
>  	struct list_head node;
>  	s64		 offset;
>  	char		 *line;
> +	char		 *name;
> +	char		 *operands;
>  };
>  
>  void disasm_line__free(struct disasm_line *dl);
>  struct disasm_line *disasm__get_next_ip_line(struct list_head *head, struct disasm_line *pos);
> +size_t disasm__fprintf(struct list_head *head, FILE *fp);
>  
>  struct sym_hist {
>  	u64		sum;

[PATCH 03/13] perf annotate browser: Use the disasm_line instruction name and operand fields

[Arnaldo Carvalho de Melo]

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

No need to reparse it everytime.

Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-90ncot487p4h5rzkn8h2whou@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/annotate.c |   29 ++++++++++-------------------
 1 file changed, 10 insertions(+), 19 deletions(-)

diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index bc540b1..0bc3e65 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -260,22 +260,16 @@ static bool annotate_browser__callq(struct annotate_browser *browser,
 				    void *arg, int delay_secs)
 {
 	struct map_symbol *ms = browser->b.priv;
+	struct disasm_line *dl = browser->selection;
 	struct symbol *sym = ms->sym;
 	struct annotation *notes;
 	struct symbol *target;
-	char *s = strstr(browser->selection->line, "callq ");
 	u64 ip;
 
-	if (s == NULL)
+	if (strcmp(dl->name, "callq"))
 		return false;
 
-	s = strchr(s, ' ');
-	if (s++ == NULL) {
-		ui_helpline__puts("Invallid callq instruction.");
-		return true;
-	}
-
-	ip = strtoull(s, NULL, 16);
+	ip = strtoull(dl->operands, NULL, 16);
 	ip = ms->map->map_ip(ms->map, ip);
 	target = map__find_symbol(ms->map, ip, NULL);
 	if (target == NULL) {
@@ -321,22 +315,19 @@ struct disasm_line *annotate_browser__find_offset(struct annotate_browser *brows
 
 static bool annotate_browser__jump(struct annotate_browser *browser)
 {
-	const char *jumps[] = { "je ", "jne ", "ja ", "jmpq ", "js ", "jmp ", NULL };
-	struct disasm_line *dl;
+	const char *jumps[] = { "je", "jne", "ja", "jmpq", "js", "jmp", NULL };
+	struct disasm_line *dl = browser->selection;
 	s64 idx, offset;
-	char *s = NULL;
+	char *s;
 	int i = 0;
 
-	while (jumps[i]) {
-		s = strstr(browser->selection->line, jumps[i++]);
-		if (s)
-			break;
-	}
+	while (jumps[i] && strcmp(dl->name, jumps[i]))
+		++i;
 
-	if (s == NULL)
+	if (jumps[i] == NULL)
 		return false;
 
-	s = strchr(s, '+');
+	s = strchr(dl->operands, '+');
 	if (s++ == NULL) {
 		ui_helpline__puts("Invallid jump instruction.");
 		return true;
-- 
1.7.9.2.358.g22243

[PATCH 04/13] perf annotate: Disassembler instruction parsing

[Arnaldo Carvalho de Melo]

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

So that at disassembly time we parse targets, etc.

Supporting jump instructions initially, call functions are next.

Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-7vzlh66n5or46n27ji658cnl@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/annotate.c |   19 ++---------
 tools/perf/util/annotate.c        |   63 +++++++++++++++++++++++++++++++++++++
 tools/perf/util/annotate.h        |   13 ++++++++
 3 files changed, 79 insertions(+), 16 deletions(-)

diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 0bc3e65..bdbb54f 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -315,26 +315,13 @@ struct disasm_line *annotate_browser__find_offset(struct annotate_browser *brows
 
 static bool annotate_browser__jump(struct annotate_browser *browser)
 {
-	const char *jumps[] = { "je", "jne", "ja", "jmpq", "js", "jmp", NULL };
 	struct disasm_line *dl = browser->selection;
-	s64 idx, offset;
-	char *s;
-	int i = 0;
-
-	while (jumps[i] && strcmp(dl->name, jumps[i]))
-		++i;
+	s64 idx;
 
-	if (jumps[i] == NULL)
+	if (!dl->ins || !ins__is_jump(dl->ins))
 		return false;
 
-	s = strchr(dl->operands, '+');
-	if (s++ == NULL) {
-		ui_helpline__puts("Invallid jump instruction.");
-		return true;
-	}
-
-	offset = strtoll(s, NULL, 16);
-	dl = annotate_browser__find_offset(browser, offset, &idx);
+	dl = annotate_browser__find_offset(browser, dl->target, &idx);
 	if (dl == NULL) {
 		ui_helpline__puts("Invallid jump offset");
 		return true;
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index a72585a..4ee2c07 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -18,6 +18,53 @@
 
 const char 	*disassembler_style;
 
+static int jump_ops__parse_target(const char *operands, u64 *target)
+{
+	const char *s = strchr(operands, '+');
+
+	if (s++ == NULL)
+		return -1;
+
+	*target = strtoll(s, NULL, 16);
+	return 0;
+}
+
+static struct ins_ops jump_ops = {
+	.parse_target = jump_ops__parse_target,
+};
+
+bool ins__is_jump(const struct ins *ins)
+{
+	return ins->ops == &jump_ops;
+}
+
+
+/*
+ * Must be sorted by name!
+ */
+static struct ins instructions[] = {
+	{ .name = "ja",	   .ops  = &jump_ops, },
+	{ .name = "je",	   .ops  = &jump_ops, },
+	{ .name = "jmp",   .ops  = &jump_ops, },
+	{ .name = "jmpq",  .ops  = &jump_ops, },
+	{ .name = "jne",   .ops  = &jump_ops, },
+	{ .name = "js",	   .ops  = &jump_ops, },
+};
+
+static int ins__cmp(const void *name, const void *insp)
+{
+	const struct ins *ins = insp;
+
+	return strcmp(name, ins->name);
+}
+
+static struct ins *ins__find(const char *name)
+{
+	const int nmemb = ARRAY_SIZE(instructions);
+
+	return bsearch(name, instructions, nmemb, sizeof(struct ins), ins__cmp);
+}
+
 int symbol__annotate_init(struct map *map __used, struct symbol *sym)
 {
 	struct annotation *notes = symbol__annotation(sym);
@@ -78,6 +125,20 @@ int symbol__inc_addr_samples(struct symbol *sym, struct map *map,
 	return 0;
 }
 
+static void disasm_line__init_ins(struct disasm_line *dl)
+{
+	dl->ins = ins__find(dl->name);
+
+	if (dl->ins == NULL)
+		return;
+
+	if (!dl->ins->ops)
+		return;
+
+	if (dl->ins->ops->parse_target)
+		dl->ins->ops->parse_target(dl->operands, &dl->target);
+}
+
 static struct disasm_line *disasm_line__new(s64 offset, char *line, size_t privsize)
 {
 	struct disasm_line *dl = zalloc(sizeof(*dl) + privsize);
@@ -117,6 +178,8 @@ static struct disasm_line *disasm_line__new(s64 offset, char *line, size_t privs
 				while (isspace(dl->operands[0]))
 					++dl->operands;
 			}
+
+			disasm_line__init_ins(dl);
 		}
 	}
 
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index dd7636d..934c6fe 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -7,11 +7,24 @@
 #include <linux/list.h>
 #include <linux/rbtree.h>
 
+struct ins_ops {
+	int (*parse_target)(const char *operands, u64 *target);
+};
+
+struct ins {
+	const char     *name;
+	struct ins_ops *ops;
+};
+
+bool ins__is_jump(const struct ins *ins);
+
 struct disasm_line {
 	struct list_head node;
 	s64		 offset;
+	u64		 target;
 	char		 *line;
 	char		 *name;
+	struct ins	 *ins;
 	char		 *operands;
 };
 
-- 
1.7.9.2.358.g22243

[PATCH 05/13] perf annotate: Parse call targets earlier

[Arnaldo Carvalho de Melo]

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

No need to do it everytime the user presses enter/-> on a call
instruction.

Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-ybgss44m5ycry8mk7b1qdbre@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/annotate.c |   10 +++++-----
 tools/perf/util/annotate.c        |   18 +++++++++++++++++-
 tools/perf/util/annotate.h        |    1 +
 3 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index bdbb54f..46ef966 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -266,11 +266,10 @@ static bool annotate_browser__callq(struct annotate_browser *browser,
 	struct symbol *target;
 	u64 ip;
 
-	if (strcmp(dl->name, "callq"))
+	if (!ins__is_call(dl->ins))
 		return false;
 
-	ip = strtoull(dl->operands, NULL, 16);
-	ip = ms->map->map_ip(ms->map, ip);
+	ip = ms->map->map_ip(ms->map, dl->target);
 	target = map__find_symbol(ms->map, ip, NULL);
 	if (target == NULL) {
 		ui_helpline__puts("The called function was not found.");
@@ -318,7 +317,7 @@ static bool annotate_browser__jump(struct annotate_browser *browser)
 	struct disasm_line *dl = browser->selection;
 	s64 idx;
 
-	if (!dl->ins || !ins__is_jump(dl->ins))
+	if (!ins__is_jump(dl->ins))
 		return false;
 
 	dl = annotate_browser__find_offset(browser, dl->target, &idx);
@@ -556,7 +555,8 @@ show_help:
 				ui_helpline__puts("Huh? No selection. Report to linux-kernel@vger.kernel.org");
 			else if (self->selection->offset == -1)
 				ui_helpline__puts("Actions are only available for assembly lines.");
-			else if (!(annotate_browser__jump(self) ||
+			else if (!self->selection->ins ||
+				 !(annotate_browser__jump(self) ||
 				   annotate_browser__callq(self, evidx, timer, arg, delay_secs)))
 				ui_helpline__puts("Actions are only available for the 'callq' and jump instructions.");
 			continue;
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 4ee2c07..a4296fd 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -18,6 +18,21 @@
 
 const char 	*disassembler_style;
 
+static int call_ops__parse_target(const char *operands, u64 *target)
+{
+	*target = strtoull(operands, NULL, 16);
+	return 0;
+}
+
+static struct ins_ops call_ops = {
+	.parse_target = call_ops__parse_target,
+};
+
+bool ins__is_call(const struct ins *ins)
+{
+	return ins->ops == &call_ops;
+}
+
 static int jump_ops__parse_target(const char *operands, u64 *target)
 {
 	const char *s = strchr(operands, '+');
@@ -38,11 +53,12 @@ bool ins__is_jump(const struct ins *ins)
 	return ins->ops == &jump_ops;
 }
 
-
 /*
  * Must be sorted by name!
  */
 static struct ins instructions[] = {
+	{ .name = "call",  .ops  = &call_ops, },
+	{ .name = "callq", .ops  = &call_ops, },
 	{ .name = "ja",	   .ops  = &jump_ops, },
 	{ .name = "je",	   .ops  = &jump_ops, },
 	{ .name = "jmp",   .ops  = &jump_ops, },
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 934c6fe..a2105f2 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -17,6 +17,7 @@ struct ins {
 };
 
 bool ins__is_jump(const struct ins *ins);
+bool ins__is_call(const struct ins *ins);
 
 struct disasm_line {
 	struct list_head node;
-- 
1.7.9.2.358.g22243

[PATCH 06/13] perf annotate: Introduce scnprintf ins_ops method

[Arnaldo Carvalho de Melo]

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

And implement the jump one, where if the operands string is not passed,
a compact form that uses just the target address is used.

Right now this is toggled via the 'o' option in the annotate browser,
switching from:

    0.00 :         ffffffff811661e8:       je     ffffffff81166204 <mem_cgroup_count_vm_event+0x44>
    0.00 :         ffffffff811661ea:       cmp    $0xb,%esi
    0.00 :         ffffffff811661ed:       je     ffffffff811661f8 <mem_cgroup_count_vm_event+0x38>

To:

    0.00 :         28:       je     44
    0.00 :         2a:       cmp    $0xb,%esi
    0.00 :         2d:       je     38

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-o88q46yh4kxgpd1chk5gvjl5@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/annotate.c |   13 +++++++++++--
 tools/perf/util/annotate.c        |   10 ++++++++++
 tools/perf/util/annotate.h        |    4 ++++
 3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 46ef966..f7d2db3 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -83,7 +83,7 @@ static void annotate_browser__write(struct ui_browser *self, void *entry, int ro
 	else if (dl->offset == -1)
 		slsmg_write_nstring(dl->line, width - 18);
 	else {
-		char bf[64];
+		char bf[256], *line = dl->line;
 		u64 addr = dl->offset;
 		int printed, color = -1;
 
@@ -96,7 +96,16 @@ static void annotate_browser__write(struct ui_browser *self, void *entry, int ro
 		slsmg_write_nstring(bf, printed);
 		if (change_color)
 			ui_browser__set_color(self, color);
-		slsmg_write_nstring(dl->line, width - 18 - printed);
+		if (dl->ins && dl->ins->ops->scnprintf) {
+			dl->ins->ops->scnprintf(dl->ins, bf, sizeof(bf),
+						!ab->use_offset ? dl->operands : NULL,
+						dl->target);
+			line = bf;
+			slsmg_write_nstring(" ", 7);
+			printed += 7;
+		}
+
+		slsmg_write_nstring(line, width - 18 - printed);
 	}
 
 	if (current_entry)
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index a4296fd..ed1f89d 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -44,8 +44,18 @@ static int jump_ops__parse_target(const char *operands, u64 *target)
 	return 0;
 }
 
+static int jump_ops__scnprintf(struct ins *ins, char *bf, size_t size,
+			       const char *operands, u64 target)
+{
+	if (operands)
+		return scnprintf(bf, size, "%-6.6s %s", ins->name, operands);
+
+	return scnprintf(bf, size, "%-6.6s %" PRIx64, ins->name, target);
+}
+
 static struct ins_ops jump_ops = {
 	.parse_target = jump_ops__parse_target,
+	.scnprintf = jump_ops__scnprintf,
 };
 
 bool ins__is_jump(const struct ins *ins)
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index a2105f2..6314335 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -7,8 +7,12 @@
 #include <linux/list.h>
 #include <linux/rbtree.h>
 
+struct ins;
+
 struct ins_ops {
 	int (*parse_target)(const char *operands, u64 *target);
+	int (*scnprintf)(struct ins *ins, char *bf, size_t size,
+			 const char *operands, u64 target);
 };
 
 struct ins {
-- 
1.7.9.2.358.g22243

[PATCH 07/13] perf annotate browser: Rename disasm_line_rb_node

[Arnaldo Carvalho de Melo]

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

Its not just an rb_node, it carries extra state that is private to the
browser. And will carry some more in the next patches.

Better name it browser_disasm_line, i.e. something derived from
disasm_line, that specializes it.

Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-nev4b97vdvv35we1qmooym52@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/annotate.c |   76 ++++++++++++++++++-------------------
 1 file changed, 38 insertions(+), 38 deletions(-)

diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index f7d2db3..58ebfd0 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -25,16 +25,16 @@ struct annotate_browser {
 	char		    search_bf[128];
 };
 
-struct disasm_line_rb_node {
+struct browser_disasm_line {
 	struct rb_node	rb_node;
 	double		percent;
 	u32		idx;
 	int		idx_asm;
 };
 
-static inline struct disasm_line_rb_node *disasm_line__rb(struct disasm_line *dl)
+static inline struct browser_disasm_line *disasm_line__browser(struct disasm_line *dl)
 {
-	return (struct disasm_line_rb_node *)(dl + 1);
+	return (struct browser_disasm_line *)(dl + 1);
 }
 
 static bool disasm_line__filter(struct ui_browser *browser, void *entry)
@@ -60,9 +60,9 @@ static void annotate_browser__write(struct ui_browser *self, void *entry, int ro
 	int width = self->width;
 
 	if (dl->offset != -1) {
-		struct disasm_line_rb_node *dlrb = disasm_line__rb(dl);
-		ui_browser__set_percent_color(self, dlrb->percent, current_entry);
-		slsmg_printf(" %7.2f ", dlrb->percent);
+		struct browser_disasm_line *bdl = disasm_line__browser(dl);
+		ui_browser__set_percent_color(self, bdl->percent, current_entry);
+		slsmg_printf(" %7.2f ", bdl->percent);
 	} else {
 		ui_browser__set_percent_color(self, 0, current_entry);
 		slsmg_write_nstring(" ", 9);
@@ -146,22 +146,22 @@ static double disasm_line__calc_percent(struct disasm_line *dl, struct symbol *s
 	return percent;
 }
 
-static void disasm_rb_tree__insert(struct rb_root *root, struct disasm_line_rb_node *dlrb)
+static void disasm_rb_tree__insert(struct rb_root *root, struct browser_disasm_line *bdl)
 {
 	struct rb_node **p = &root->rb_node;
 	struct rb_node *parent = NULL;
-	struct disasm_line_rb_node *l;
+	struct browser_disasm_line *l;
 
 	while (*p != NULL) {
 		parent = *p;
-		l = rb_entry(parent, struct disasm_line_rb_node, rb_node);
-		if (dlrb->percent < l->percent)
+		l = rb_entry(parent, struct browser_disasm_line, rb_node);
+		if (bdl->percent < l->percent)
 			p = &(*p)->rb_left;
 		else
 			p = &(*p)->rb_right;
 	}
-	rb_link_node(&dlrb->rb_node, parent, p);
-	rb_insert_color(&dlrb->rb_node, root);
+	rb_link_node(&bdl->rb_node, parent, p);
+	rb_insert_color(&bdl->rb_node, root);
 }
 
 static void annotate_browser__set_top(struct annotate_browser *self,
@@ -190,12 +190,12 @@ static void annotate_browser__set_top(struct annotate_browser *self,
 static void annotate_browser__set_rb_top(struct annotate_browser *browser,
 					 struct rb_node *nd)
 {
-	struct disasm_line_rb_node *rbpos;
+	struct browser_disasm_line *bpos;
 	struct disasm_line *pos;
 
-	rbpos = rb_entry(nd, struct disasm_line_rb_node, rb_node);
-	pos = ((struct disasm_line *)rbpos) - 1;
-	annotate_browser__set_top(browser, pos, rbpos->idx);
+	bpos = rb_entry(nd, struct browser_disasm_line, rb_node);
+	pos = ((struct disasm_line *)bpos) - 1;
+	annotate_browser__set_top(browser, pos, bpos->idx);
 	browser->curr_hot = nd;
 }
 
@@ -212,13 +212,13 @@ static void annotate_browser__calc_percent(struct annotate_browser *browser,
 	pthread_mutex_lock(&notes->lock);
 
 	list_for_each_entry(pos, &notes->src->source, node) {
-		struct disasm_line_rb_node *rbpos = disasm_line__rb(pos);
-		rbpos->percent = disasm_line__calc_percent(pos, sym, evidx);
-		if (rbpos->percent < 0.01) {
-			RB_CLEAR_NODE(&rbpos->rb_node);
+		struct browser_disasm_line *bpos = disasm_line__browser(pos);
+		bpos->percent = disasm_line__calc_percent(pos, sym, evidx);
+		if (bpos->percent < 0.01) {
+			RB_CLEAR_NODE(&bpos->rb_node);
 			continue;
 		}
-		disasm_rb_tree__insert(&browser->entries, rbpos);
+		disasm_rb_tree__insert(&browser->entries, bpos);
 	}
 	pthread_mutex_unlock(&notes->lock);
 
@@ -228,37 +228,37 @@ static void annotate_browser__calc_percent(struct annotate_browser *browser,
 static bool annotate_browser__toggle_source(struct annotate_browser *browser)
 {
 	struct disasm_line *dl;
-	struct disasm_line_rb_node *dlrb;
+	struct browser_disasm_line *bdl;
 	off_t offset = browser->b.index - browser->b.top_idx;
 
 	browser->b.seek(&browser->b, offset, SEEK_CUR);
 	dl = list_entry(browser->b.top, struct disasm_line, node);
-	dlrb = disasm_line__rb(dl);
+	bdl = disasm_line__browser(dl);
 
 	if (browser->hide_src_code) {
-		if (dlrb->idx_asm < offset)
-			offset = dlrb->idx;
+		if (bdl->idx_asm < offset)
+			offset = bdl->idx;
 
 		browser->b.nr_entries = browser->nr_entries;
 		browser->hide_src_code = false;
 		browser->b.seek(&browser->b, -offset, SEEK_CUR);
-		browser->b.top_idx = dlrb->idx - offset;
-		browser->b.index = dlrb->idx;
+		browser->b.top_idx = bdl->idx - offset;
+		browser->b.index = bdl->idx;
 	} else {
-		if (dlrb->idx_asm < 0) {
+		if (bdl->idx_asm < 0) {
 			ui_helpline__puts("Only available for assembly lines.");
 			browser->b.seek(&browser->b, -offset, SEEK_CUR);
 			return false;
 		}
 
-		if (dlrb->idx_asm < offset)
-			offset = dlrb->idx_asm;
+		if (bdl->idx_asm < offset)
+			offset = bdl->idx_asm;
 
 		browser->b.nr_entries = browser->nr_asm_entries;
 		browser->hide_src_code = true;
 		browser->b.seek(&browser->b, -offset, SEEK_CUR);
-		browser->b.top_idx = dlrb->idx_asm - offset;
-		browser->b.index = dlrb->idx_asm;
+		browser->b.top_idx = bdl->idx_asm - offset;
+		browser->b.index = bdl->idx_asm;
 	}
 
 	return true;
@@ -621,7 +621,7 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map, int evidx,
 	if (map->dso->annotate_warned)
 		return -1;
 
-	if (symbol__annotate(sym, map, sizeof(struct disasm_line_rb_node)) < 0) {
+	if (symbol__annotate(sym, map, sizeof(struct browser_disasm_line)) < 0) {
 		ui__error("%s", ui_helpline__last_msg);
 		return -1;
 	}
@@ -632,17 +632,17 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map, int evidx,
 	browser.start = map__rip_2objdump(map, sym->start);
 
 	list_for_each_entry(pos, &notes->src->source, node) {
-		struct disasm_line_rb_node *rbpos;
+		struct browser_disasm_line *bpos;
 		size_t line_len = strlen(pos->line);
 
 		if (browser.b.width < line_len)
 			browser.b.width = line_len;
-		rbpos = disasm_line__rb(pos);
-		rbpos->idx = browser.nr_entries++;
+		bpos = disasm_line__browser(pos);
+		bpos->idx = browser.nr_entries++;
 		if (pos->offset != -1)
-			rbpos->idx_asm = browser.nr_asm_entries++;
+			bpos->idx_asm = browser.nr_asm_entries++;
 		else
-			rbpos->idx_asm = -1;
+			bpos->idx_asm = -1;
 	}
 
 	browser.b.nr_entries = browser.nr_entries;
-- 
1.7.9.2.358.g22243

[PATCH 08/13] perf symbols: Introduce symbol__size method

[Arnaldo Carvalho de Melo]

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

Fixing some off by one cases in the process.

Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-fxumzufhk829z0q9anmvemea@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/annotate.c |   10 +++++-----
 tools/perf/util/symbol.h   |    5 +++++
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index ed1f89d..d8e2f41 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -101,7 +101,7 @@ int symbol__annotate_init(struct map *map __used, struct symbol *sym)
 int symbol__alloc_hist(struct symbol *sym)
 {
 	struct annotation *notes = symbol__annotation(sym);
-	const size_t size = sym->end - sym->start + 1;
+	const size_t size = symbol__size(sym);
 	size_t sizeof_sym_hist = (sizeof(struct sym_hist) + size * sizeof(u64));
 
 	notes->src = zalloc(sizeof(*notes->src) + symbol_conf.nr_events * sizeof_sym_hist);
@@ -609,7 +609,7 @@ static void symbol__annotate_hits(struct symbol *sym, int evidx)
 {
 	struct annotation *notes = symbol__annotation(sym);
 	struct sym_hist *h = annotation__histogram(notes, evidx);
-	u64 len = sym->end - sym->start, offset;
+	u64 len = symbol__size(sym), offset;
 
 	for (offset = 0; offset < len; ++offset)
 		if (h->addr[offset] != 0)
@@ -636,7 +636,7 @@ int symbol__annotate_printf(struct symbol *sym, struct map *map, int evidx,
 	else
 		d_filename = basename(filename);
 
-	len = sym->end - sym->start;
+	len = symbol__size(sym);
 
 	printf(" Percent |	Source code & Disassembly of %s\n", d_filename);
 	printf("------------------------------------------------\n");
@@ -696,7 +696,7 @@ void symbol__annotate_decay_histogram(struct symbol *sym, int evidx)
 {
 	struct annotation *notes = symbol__annotation(sym);
 	struct sym_hist *h = annotation__histogram(notes, evidx);
-	int len = sym->end - sym->start, offset;
+	int len = symbol__size(sym), offset;
 
 	h->sum = 0;
 	for (offset = 0; offset < len; ++offset) {
@@ -755,7 +755,7 @@ int symbol__tty_annotate(struct symbol *sym, struct map *map, int evidx,
 	if (symbol__annotate(sym, map, 0) < 0)
 		return -1;
 
-	len = sym->end - sym->start;
+	len = symbol__size(sym);
 
 	if (print_lines) {
 		symbol__get_source_line(sym, map, evidx, &source_line,
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index ac49ef2..1f00388 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -65,6 +65,11 @@ struct symbol {
 
 void symbol__delete(struct symbol *sym);
 
+static inline size_t symbol__size(const struct symbol *sym)
+{
+	return sym->end - sym->start + 1;
+}
+
 struct strlist;
 
 struct symbol_conf {
-- 
1.7.9.2.358.g22243

[PATCH 09/13] perf annotate browser: Hide non jump target addresses in offset mode

[Arnaldo Carvalho de Melo]

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

This:

    0.00 :         ffffffff8116bd00:       lock btsl $0x0,(%r12)
  100.00 :         ffffffff8116bd07:       sbb    %eax,%eax
    0.00 :         ffffffff8116bd09:       test   %eax,%eax
    0.00 :         ffffffff8116bd0b:       jne    ffffffff8116bf5f <__mem_cgroup_commit_charge+0x28f>
    0.00 :         ffffffff8116bd11:       mov    (%r12),%rax
    0.00 :         ffffffff8116bd15:       test   $0x2,%al
    0.00 :         ffffffff8116bd17:       jne    ffffffff8116bf6e <__mem_cgroup_commit_charge+0x29e>
    0.00 :         ffffffff8116bd1d:       test   %r9b,%r9b
    0.00 :         ffffffff8116bd20:       jne    ffffffff8116be30 <__mem_cgroup_commit_charge+0x160>
    0.00 :         ffffffff8116bd26:       xor    %eax,%eax
    0.00 :         ffffffff8116bd28:       mov    %r13,0x8(%r12)
    0.00 :         ffffffff8116bd2d:       lock orb $0x2,(%r12)
    0.00 :         ffffffff8116bd33:       test   %r9b,%r9b
    0.00 :         ffffffff8116bd36:       je     ffffffff8116bdf3 <__mem_cgroup_commit_charge+0x123>

Becomes:

    0.00 :         30:       lock btsl $0x0,(%r12)
  100.00 :                   sbb    %eax,%eax
    0.00 :                   test   %eax,%eax
    0.00 :                   jne    28f
    0.00 :                   mov    (%r12),%rax
    0.00 :                   test   $0x2,%al
    0.00 :                   jne    29e
    0.00 :                   test   %r9b,%r9b
    0.00 :                   jne    160
    0.00 :         56:       xor    %eax,%eax
    0.00 :         58:       mov    %r13,0x8(%r12)
    0.00 :                   lock orb $0x2,(%r12)
    0.00 :                   test   %r9b,%r9b
    0.00 :                   je     123

I.e. We trow away all those useless addresses and keep just jump labels.

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-r2vmbtgz0l8coluj8flztgrn@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/annotate.c |   71 ++++++++++++++++++++++++++++++-------
 1 file changed, 58 insertions(+), 13 deletions(-)

diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 58ebfd0..a1e942b 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -11,11 +11,20 @@
 #include <pthread.h>
 #include <newt.h>
 
+struct browser_disasm_line {
+	struct rb_node	rb_node;
+	double		percent;
+	u32		idx;
+	int		idx_asm;
+	bool		jump_target;
+};
+
 struct annotate_browser {
 	struct ui_browser b;
 	struct rb_root	  entries;
 	struct rb_node	  *curr_hot;
 	struct disasm_line	  *selection;
+	struct disasm_line  **offsets;
 	u64		    start;
 	int		    nr_asm_entries;
 	int		    nr_entries;
@@ -25,13 +34,6 @@ struct annotate_browser {
 	char		    search_bf[128];
 };
 
-struct browser_disasm_line {
-	struct rb_node	rb_node;
-	double		percent;
-	u32		idx;
-	int		idx_asm;
-};
-
 static inline struct browser_disasm_line *disasm_line__browser(struct disasm_line *dl)
 {
 	return (struct browser_disasm_line *)(dl + 1);
@@ -53,6 +55,7 @@ static void annotate_browser__write(struct ui_browser *self, void *entry, int ro
 {
 	struct annotate_browser *ab = container_of(self, struct annotate_browser, b);
 	struct disasm_line *dl = list_entry(entry, struct disasm_line, node);
+	struct browser_disasm_line *bdl = disasm_line__browser(dl);
 	bool current_entry = ui_browser__is_current_entry(self, row);
 	bool change_color = (!ab->hide_src_code &&
 			     (!current_entry || (self->use_navkeypressed &&
@@ -60,7 +63,6 @@ static void annotate_browser__write(struct ui_browser *self, void *entry, int ro
 	int width = self->width;
 
 	if (dl->offset != -1) {
-		struct browser_disasm_line *bdl = disasm_line__browser(dl);
 		ui_browser__set_percent_color(self, bdl->percent, current_entry);
 		slsmg_printf(" %7.2f ", bdl->percent);
 	} else {
@@ -90,7 +92,11 @@ static void annotate_browser__write(struct ui_browser *self, void *entry, int ro
 		if (!ab->use_offset)
 			addr += ab->start;
 
-		printed = scnprintf(bf, sizeof(bf), " %" PRIx64 ":", addr);
+		if (bdl->jump_target || !ab->use_offset)
+			printed = scnprintf(bf, sizeof(bf), " %" PRIx64 ":", addr);
+		else
+			printed = scnprintf(bf, sizeof(bf), "    ");
+
 		if (change_color)
 			color = ui_browser__set_color(self, HE_COLORSET_ADDR);
 		slsmg_write_nstring(bf, printed);
@@ -593,12 +599,39 @@ int hist_entry__tui_annotate(struct hist_entry *he, int evidx,
 				    timer, arg, delay_secs);
 }
 
+static void annotate_browser__mark_jump_targets(struct annotate_browser *browser,
+						size_t size)
+{
+	u64 offset;
+
+	for (offset = 0; offset < size; ++offset) {
+		struct disasm_line *dl = browser->offsets[offset], *dlt;
+		struct browser_disasm_line *bdlt;
+
+		if (!dl || !dl->ins || !ins__is_jump(dl->ins))
+			continue;
+
+		if (dl->target >= size) {
+			ui__error("jump to after symbol!\n"
+				  "size: %zx, jump target: %" PRIx64,
+				  size, dl->target);
+			continue;
+		}
+
+		dlt = browser->offsets[dl->target];
+		bdlt = disasm_line__browser(dlt);
+		bdlt->jump_target = true;
+	}
+		
+}
+
 int symbol__tui_annotate(struct symbol *sym, struct map *map, int evidx,
 			 void(*timer)(void *arg), void *arg,
 			 int delay_secs)
 {
 	struct disasm_line *pos, *n;
 	struct annotation *notes;
+	const size_t size = symbol__size(sym);
 	struct map_symbol ms = {
 		.map = map,
 		.sym = sym,
@@ -613,7 +646,7 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map, int evidx,
 			.use_navkeypressed = true,
 		},
 	};
-	int ret;
+	int ret = -1;
 
 	if (sym == NULL)
 		return -1;
@@ -621,9 +654,15 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map, int evidx,
 	if (map->dso->annotate_warned)
 		return -1;
 
+	browser.offsets = zalloc(size * sizeof(struct disasm_line *));
+	if (browser.offsets == NULL) {
+		ui__error("Not enough memory!");
+		return -1;
+	}
+
 	if (symbol__annotate(sym, map, sizeof(struct browser_disasm_line)) < 0) {
 		ui__error("%s", ui_helpline__last_msg);
-		return -1;
+		goto out_free_offsets;
 	}
 
 	ui_helpline__push("Press <- or ESC to exit");
@@ -639,12 +678,15 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map, int evidx,
 			browser.b.width = line_len;
 		bpos = disasm_line__browser(pos);
 		bpos->idx = browser.nr_entries++;
-		if (pos->offset != -1)
+		if (pos->offset != -1) {
 			bpos->idx_asm = browser.nr_asm_entries++;
-		else
+			browser.offsets[pos->offset] = pos;
+		} else
 			bpos->idx_asm = -1;
 	}
 
+	annotate_browser__mark_jump_targets(&browser, size);
+
 	browser.b.nr_entries = browser.nr_entries;
 	browser.b.entries = &notes->src->source,
 	browser.b.width += 18; /* Percentage */
@@ -653,5 +695,8 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map, int evidx,
 		list_del(&pos->node);
 		disasm_line__free(pos);
 	}
+
+out_free_offsets:
+	free(browser.offsets);
 	return ret;
 }
-- 
1.7.9.2.358.g22243

Re: [GIT PULL 00/13] Annotation improvements (G+ edition)

[David Ahern]

Something hosed up on your send: I only have 9 of the 13, I don't see 
the missing ones here: https://lkml.org/lkml/2012/4/19/, and the subject 
line on the lkml summary page is messed up.

David


On 4/19/12 2:33 PM, Arnaldo Carvalho de Melo wrote:
> Hi Ingo,
>
> 	Please consider pulling,
>
> - Arnaldo
>
> The following changes since commit a385ec4f11bdcf81af094c03e2444ee9b7fad2e5:
>
>    Merge tag 'v3.4-rc2' into perf/core (2012-04-13 09:57:10 +0200)
>
> are available in the git repository at:
>
>
>    git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux perf/annotate
>
> for you to fetch changes up to 3f862fd076275c442dfe295eddb5650a6e0aecd4:
>
>    perf annotate: Add missing jump variants (2012-04-19 17:10:12 -0300)
>
> ----------------------------------------------------------------
> Annotate improvements
>
> Now the default annotate browser uses a much more compact format, implementing
> suggestions made made by several people, notably Linus.
>
> Here is part of the new __list_del_entry annotation:
>
> __list_del_entry
>      8.47 │      push   %rbp
>      8.47 │      mov    (%rdi),%rdx
>     20.34 │      mov    $0xdead000000100100,%rcx
>      3.39 │      mov    0x8(%rdi),%rax
>      0.00 │      mov    %rsp,%rbp
>      1.69 │      cmp    %rcx,%rdx
>      0.00 │      je     43
>      1.69 │      mov    $0xdead000000200200,%rcx
>      3.39 │      cmp    %rcx,%rax
>      0.00 │      je     a3
>      5.08 │      mov    (%rax),%r8
>     18.64 │      cmp    %r8,%rdi
>      0.00 │      jne    84
>      1.69 │      mov    0x8(%rdx),%r8
>     25.42 │      cmp    %r8,%rdi
>      0.00 │      jne    65
>      1.69 │      mov    %rax,0x8(%rdx)
>      0.00 │      mov    %rdx,(%rax)
>      0.00 │      leaveq
>      0.00 │      retq
>      0.00 │ 43:  mov    %rdx,%r8
>      0.00 │      mov    %rdi,%rcx
>      0.00 │      mov    $0xffffffff817cd6a8,%rdx
>      0.00 │      mov    $0x31,%esi
>      0.00 │      mov    $0xffffffff817cd6e0,%rdi
>      0.00 │      xor    %eax,%eax
>      0.00 │      callq  ffffffff8104eab0<warn_slowpath_fmt>
>      0.00 │      leaveq
>      0.00 │      retq
>      0.00 │ 65:  mov    %rdi,%rcx
>      0.00 │      mov    $0xffffffff817cd780,%rdx
>      0.00 │      mov    $0x3a,%esi
>      0.00 │      mov    $0xffffffff817cd6e0,%rdi
>      0.00 │      xor    %eax,%eax
>      0.00 │      callq  ffffffff8104eab0<warn_slowpath_fmt>
>      0.00 │      leaveq
>      0.00 │      retq
>
> The infrastructure is there to provide formatters for any instruction,
> like the one I'll do for call functions to elide the address.
>
> Signed-off-by: Arnaldo Carvalho de Melo<acme@redhat.com>
>
> ----------------------------------------------------------------
> Arnaldo Carvalho de Melo (13):
>        perf annotate: Rename objdump_line to disasm_line
>        perf annotate: Parse instruction
>        perf annotate browser: Use the disasm_line instruction name and operand fields
>        perf annotate: Disassembler instruction parsing
>        perf annotate: Parse call targets earlier
>        perf annotate: Introduce scnprintf ins_ops method
>        perf annotate browser: Rename disasm_line_rb_node
>        perf symbols: Introduce symbol__size method
>        perf annotate browser: Hide non jump target addresses in offset mode
>        perf annotate browser: Align jump labels
>        perf annotate browser: Make lines more compact
>        perf annotate browser: Use a vertical line as percentage separator
>        perf annotate: Add missing jump variants
>
>   tools/perf/ui/browsers/annotate.c |  323 +++++++++++++++++++++----------------
>   tools/perf/util/annotate.c        |  263 +++++++++++++++++++++++++-----
>   tools/perf/util/annotate.h        |   32 +++-
>   tools/perf/util/symbol.h          |    5 +
>   tools/perf/util/util.c            |   10 ++
>   tools/perf/util/util.h            |    2 +
>   6 files changed, 446 insertions(+), 189 deletions(-)

Re: [GIT PULL 00/13] Annotation improvements (G+ edition)

[Arnaldo Carvalho de Melo]

Em Thu, Apr 19, 2012 at 06:01:15PM -0600, David Ahern escreveu:
> Something hosed up on your send: I only have 9 of the 13, I don't
> see the missing ones here: https://lkml.org/lkml/2012/4/19/, and the
> subject line on the lkml summary page is messed up.

Subject line probably was git send-email asking if I wanted it to be
UTF-8, due to the vertical line characters :-\

The others I'm checking now, but its all at git.k.o.

- Arnaldo
 
> David
> 
> 
> On 4/19/12 2:33 PM, Arnaldo Carvalho de Melo wrote:
> >Hi Ingo,
> >
> >	Please consider pulling,
> >
> >- Arnaldo
> >
> >The following changes since commit a385ec4f11bdcf81af094c03e2444ee9b7fad2e5:
> >
> >   Merge tag 'v3.4-rc2' into perf/core (2012-04-13 09:57:10 +0200)
> >
> >are available in the git repository at:
> >
> >
> >   git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux perf/annotate
> >
> >for you to fetch changes up to 3f862fd076275c442dfe295eddb5650a6e0aecd4:
> >
> >   perf annotate: Add missing jump variants (2012-04-19 17:10:12 -0300)
> >
> >----------------------------------------------------------------
> >Annotate improvements
> >
> >Now the default annotate browser uses a much more compact format, implementing
> >suggestions made made by several people, notably Linus.
> >
> >Here is part of the new __list_del_entry annotation:
> >
> >__list_del_entry
> >     8.47 │      push   %rbp
> >     8.47 │      mov    (%rdi),%rdx
> >    20.34 │      mov    $0xdead000000100100,%rcx
> >     3.39 │      mov    0x8(%rdi),%rax
> >     0.00 │      mov    %rsp,%rbp
> >     1.69 │      cmp    %rcx,%rdx
> >     0.00 │      je     43
> >     1.69 │      mov    $0xdead000000200200,%rcx
> >     3.39 │      cmp    %rcx,%rax
> >     0.00 │      je     a3
> >     5.08 │      mov    (%rax),%r8
> >    18.64 │      cmp    %r8,%rdi
> >     0.00 │      jne    84
> >     1.69 │      mov    0x8(%rdx),%r8
> >    25.42 │      cmp    %r8,%rdi
> >     0.00 │      jne    65
> >     1.69 │      mov    %rax,0x8(%rdx)
> >     0.00 │      mov    %rdx,(%rax)
> >     0.00 │      leaveq
> >     0.00 │      retq
> >     0.00 │ 43:  mov    %rdx,%r8
> >     0.00 │      mov    %rdi,%rcx
> >     0.00 │      mov    $0xffffffff817cd6a8,%rdx
> >     0.00 │      mov    $0x31,%esi
> >     0.00 │      mov    $0xffffffff817cd6e0,%rdi
> >     0.00 │      xor    %eax,%eax
> >     0.00 │      callq  ffffffff8104eab0<warn_slowpath_fmt>
> >     0.00 │      leaveq
> >     0.00 │      retq
> >     0.00 │ 65:  mov    %rdi,%rcx
> >     0.00 │      mov    $0xffffffff817cd780,%rdx
> >     0.00 │      mov    $0x3a,%esi
> >     0.00 │      mov    $0xffffffff817cd6e0,%rdi
> >     0.00 │      xor    %eax,%eax
> >     0.00 │      callq  ffffffff8104eab0<warn_slowpath_fmt>
> >     0.00 │      leaveq
> >     0.00 │      retq
> >
> >The infrastructure is there to provide formatters for any instruction,
> >like the one I'll do for call functions to elide the address.
> >
> >Signed-off-by: Arnaldo Carvalho de Melo<acme@redhat.com>
> >
> >----------------------------------------------------------------
> >Arnaldo Carvalho de Melo (13):
> >       perf annotate: Rename objdump_line to disasm_line
> >       perf annotate: Parse instruction
> >       perf annotate browser: Use the disasm_line instruction name and operand fields
> >       perf annotate: Disassembler instruction parsing
> >       perf annotate: Parse call targets earlier
> >       perf annotate: Introduce scnprintf ins_ops method
> >       perf annotate browser: Rename disasm_line_rb_node
> >       perf symbols: Introduce symbol__size method
> >       perf annotate browser: Hide non jump target addresses in offset mode
> >       perf annotate browser: Align jump labels
> >       perf annotate browser: Make lines more compact
> >       perf annotate browser: Use a vertical line as percentage separator
> >       perf annotate: Add missing jump variants
> >
> >  tools/perf/ui/browsers/annotate.c |  323 +++++++++++++++++++++----------------
> >  tools/perf/util/annotate.c        |  263 +++++++++++++++++++++++++-----
> >  tools/perf/util/annotate.h        |   32 +++-
> >  tools/perf/util/symbol.h          |    5 +
> >  tools/perf/util/util.c            |   10 ++
> >  tools/perf/util/util.h            |    2 +
> >  6 files changed, 446 insertions(+), 189 deletions(-)

Re: [GIT PULL 00/13] Annotation improvements (G+ edition)

[Linus Torvalds]

On Thu, Apr 19, 2012 at 1:33 PM, Arnaldo Carvalho de Melo
<acme@infradead.org> wrote:
>
> Annotate improvements
>
> Now the default annotate browser uses a much more compact format, implementing
> suggestions made made by several people, notably Linus.
>
> Here is part of the new __list_del_entry annotation:

Ok, so I did a quick test-run, and things are definitely more readable.

However, from a "where is the loop" angle, the one thing that is
actually fairly hard to visually see is the branches.

Sure, you can find them, but they are not nearly as easy to pick out
visually as the branch targets are, and I do wonder if it wouldn't
make sense to try to make them stand out a bit more.

What is visually important is
 (a) seeing that it's a branch
 (b) seeing the direction of the branch (to pick up on loops more easily).

I suspect that doing a whole arrow is just going to be a horrible
criss-crossing mess (although with some graphical UI it might not be
as bad as with the textual one), but I think even just doing it in
entirely local scope of the particular line that contains the branch
would be perfectly fine.

This, for example, is a snippet of a pretty simple function
(avc_lookup()) with a few branches and a few branch targets:

    2.08 │      test   %rdx,%rdx
    0.00 │      jne    38
    0.00 │      jmp    53
    5.13 │ 30:  mov    (%rdx),%rdx
    0.74 │      test   %rdx,%rdx
    0.00 │      je     53
    0.06 │ 38:  lea    -0x20(%rdx),%rax
   59.64 │      cmp    -0x20(%rdx),%edi
    0.40 │      jne    30
   17.36 │      cmp    -0x18(%rdx),%cx
    0.00 │      jne    30
    1.91 │      cmp    0x4(%rax),%esi
    0.00 │      jne    30
    0.22 │      test   %rax,%rax
    0.20 │      je     53
    1.68 │      leaveq
    1.90 │      retq
    0.00 │ 53:  incl   %gs:0xe214
    0.00 │      xor    %eax,%eax

and the branch targets are fairly easy to pick up, but seeing the
branches - and their directions - themselves takes much more parsing.
Ok, it's actually pretty simple above, because they have a faily
obvious pattern, but in bigger functions they really are harder to
pick out.

Now, what if the branch instruction was just followed with a simple
"arrow up/down" thing, and to make the distinction really simple to
see visually, make the indentation slightly different for the
back/forwards case, so that you don't have to really look at the
(crappy) arrow head to find possible loops (backwards jumps).

The above snippet would then become:

    2.08        test   %rdx,%rdx
    0.00        jne    38                  ----v
    0.00        jmp    53                  ----v
    5.13   30:  mov    (%rdx),%rdx
    0.74        test   %rdx,%rdx
    0.00        je     53                  ----v
    0.06   38:  lea    -0x20(%rdx),%rax
   59.64        cmp    -0x20(%rdx),%edi
    0.40        jne    30                      ----^
   17.36        cmp    -0x18(%rdx),%cx
    0.00        jne    30                      ----^
    1.91        cmp    0x4(%rax),%esi
    0.00        jne    30                      ----^
    0.22        test   %rax,%rax
    0.20        je     53                  ----v
    1.68        leaveq
    1.90        retq
    0.00   53:  incl   %gs:0xe214
    0.00        xor    %eax,%eax

where I think it's fairly easy to pick up the branches. Sure, once you
really need to connect them, you do need to compare the offsets (easy
in the above case, less obvious with big complex functions), but just
making it easy to *see* the backwards branches makes it easier to see
the loop, methinks.

I dunno. Just a suggestion. I definitely like the new format better
already, regardless of any arrows.

                   Linus

Re: [GIT PULL 00/13] Annotation improvements (G+ edition)

[Linus Torvalds]

On Thu, Apr 19, 2012 at 5:31 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Thu, Apr 19, 2012 at 1:33 PM, Arnaldo Carvalho de Melo
> (crappy) arrow head to find possible loops (backwards jumps).
>
> The above snippet would then become:
>
>    2.08        test   %rdx,%rdx
>    0.00        jne    38                  ----v
>    0.00        jmp    53                  ----v
>    5.13   30:  mov    (%rdx),%rdx
>    0.74        test   %rdx,%rdx
>    0.00        je     53                  ----v
>    0.06   38:  lea    -0x20(%rdx),%rax
>   59.64        cmp    -0x20(%rdx),%edi
>    0.40        jne    30                      ----^
>   17.36        cmp    -0x18(%rdx),%cx
>    0.00        jne    30                      ----^
>    1.91        cmp    0x4(%rax),%esi
>    0.00        jne    30                      ----^
>    0.22        test   %rax,%rax
>    0.20        je     53                  ----v
>    1.68        leaveq
>    1.90        retq
>    0.00   53:  incl   %gs:0xe214
>    0.00        xor    %eax,%eax

Side note: I'm not at all certain that the arrows should be at the end
of the line. It might be even better to show it at the head of the
line, ie something like


    2.08        test   %rdx,%rdx
    0.00 v      jne    38
    0.00 v      jmp    53
    5.13   30:  mov    (%rdx),%rdx
    0.74        test   %rdx,%rdx
    0.00 v      je     53
    0.06   38:  lea    -0x20(%rdx),%rax
   59.64        cmp    -0x20(%rdx),%edi
    0.40 ^--    jne    30
   17.36        cmp    -0x18(%rdx),%cx
    0.00 ^--    jne    30
    1.91        cmp    0x4(%rax),%esi
    0.00 ^--    jne    30
    0.22        test   %rax,%rax
    0.20 v      je     53
    1.68        leaveq
    1.90        retq
    0.00   53:  incl   %gs:0xe214
    0.00        xor    %eax,%eax

but then you obviously need to make it work right if a branch is also
a branch target.

Oh, and one other visual clue that could be useful: unconditional
branches (including "ret") that actually end the flow of code and
don't have a fallthrough to the next instruction would be nice to see
clearly. Even perhaps an empty line following them? Although maybe
that would be wasting precious vertical space..

                              Linus

Re: [GIT PULL 00/13] Annotation improvements (G+ edition)

[Arnaldo Carvalho de Melo]

Em Thu, Apr 19, 2012 at 05:31:04PM -0700, Linus Torvalds escreveu:
> On Thu, Apr 19, 2012 at 1:33 PM, Arnaldo Carvalho de Melo wrote:
> Ok, so I did a quick test-run, and things are definitely more readable.
> 
> However, from a "where is the loop" angle, the one thing that is
> actually fairly hard to visually see is the branches.
>
> Sure, you can find them, but they are not nearly as easy to pick out
> visually as the branch targets are, and I do wonder if it wouldn't
> make sense to try to make them stand out a bit more.
> 
> What is visually important is
>  (a) seeing that it's a branch
>  (b) seeing the direction of the branch (to pick up on loops more easily).
> 
> I suspect that doing a whole arrow is just going to be a horrible
> criss-crossing mess (although with some graphical UI it might not be
> as bad as with the textual one), but I think even just doing it in
> entirely local scope of the particular line that contains the branch
> would be perfectly fine.

You mean we could draw the arrow all the way from the branch to the
label when the cursor is on one of them (branch, label)?

It would avoid the criss-crossing mess, and with identing it would get
even easier to draw it, i.e. it would be a straight line with a tip just
below the branch and a "feather" just above the target label.
 
> This, for example, is a snippet of a pretty simple function
> (avc_lookup()) with a few branches and a few branch targets:
> 
>     2.08 │      test   %rdx,%rdx
>     0.00 │      jne    38
>     0.00 │      jmp    53
>     5.13 │ 30:  mov    (%rdx),%rdx
>     0.74 │      test   %rdx,%rdx
>     0.00 │      je     53
>     0.06 │ 38:  lea    -0x20(%rdx),%rax
>    59.64 │      cmp    -0x20(%rdx),%edi
>     0.40 │      jne    30
>    17.36 │      cmp    -0x18(%rdx),%cx
>     0.00 │      jne    30
>     1.91 │      cmp    0x4(%rax),%esi
>     0.00 │      jne    30
>     0.22 │      test   %rax,%rax
>     0.20 │      je     53
>     1.68 │      leaveq
>     1.90 │      retq
>     0.00 │ 53:  incl   %gs:0xe214
>     0.00 │      xor    %eax,%eax
> 
> and the branch targets are fairly easy to pick up, but seeing the
> branches - and their directions - themselves takes much more parsing.
> Ok, it's actually pretty simple above, because they have a faily
> obvious pattern, but in bigger functions they really are harder to
> pick out.
> 
> Now, what if the branch instruction was just followed with a simple
> "arrow up/down" thing, and to make the distinction really simple to
> see visually, make the indentation slightly different for the
> back/forwards case, so that you don't have to really look at the
> (crappy) arrow head to find possible loops (backwards jumps).
> 
> The above snippet would then become:
> 
>     2.08        test   %rdx,%rdx
>     0.00        jne    38                  ----v
>     0.00        jmp    53                  ----v
>     5.13   30:  mov    (%rdx),%rdx
>     0.74        test   %rdx,%rdx
>     0.00        je     53                  ----v
>     0.06   38:  lea    -0x20(%rdx),%rax
>    59.64        cmp    -0x20(%rdx),%edi
>     0.40        jne    30                      ----^
>    17.36        cmp    -0x18(%rdx),%cx
>     0.00        jne    30                      ----^
>     1.91        cmp    0x4(%rax),%esi
>     0.00        jne    30                      ----^
>     0.22        test   %rax,%rax
>     0.20        je     53                  ----v
>     1.68        leaveq
>     1.90        retq
>     0.00   53:  incl   %gs:0xe214
>     0.00        xor    %eax,%eax
> 
> where I think it's fairly easy to pick up the branches. Sure, once you
> really need to connect them, you do need to compare the offsets (easy
> in the above case, less obvious with big complex functions), but just
> making it easy to *see* the backwards branches makes it easier to see
> the loop, methinks.
> 
> I dunno. Just a suggestion. I definitely like the new format better
> already, regardless of any arrows.

Great, keep suggestions flowing. :-)

- Arnaldo

Re: [GIT PULL 00/13] Annotation improvements (G+ edition)

[Ingo Molnar]

* Arnaldo Carvalho de Melo <acme@infradead.org> wrote:

> Hi Ingo,
> 
> 	Please consider pulling,
> 
> - Arnaldo
> 
> The following changes since commit a385ec4f11bdcf81af094c03e2444ee9b7fad2e5:
> 
>   Merge tag 'v3.4-rc2' into perf/core (2012-04-13 09:57:10 +0200)
> 
> are available in the git repository at:
> 
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux perf/annotate
> 
> for you to fetch changes up to 3f862fd076275c442dfe295eddb5650a6e0aecd4:
> 
>   perf annotate: Add missing jump variants (2012-04-19 17:10:12 -0300)
> 
> ----------------------------------------------------------------
> Annotate improvements
> 
> Now the default annotate browser uses a much more compact format, implementing
> suggestions made made by several people, notably Linus.
> 
> Here is part of the new __list_del_entry annotation:
> 
> __list_del_entry
>     8.47 │      push   %rbp
>     8.47 │      mov    (%rdi),%rdx
>    20.34 │      mov    $0xdead000000100100,%rcx
>     3.39 │      mov    0x8(%rdi),%rax
>     0.00 │      mov    %rsp,%rbp
>     1.69 │      cmp    %rcx,%rdx
>     0.00 │      je     43
>     1.69 │      mov    $0xdead000000200200,%rcx
>     3.39 │      cmp    %rcx,%rax
>     0.00 │      je     a3
>     5.08 │      mov    (%rax),%r8
>    18.64 │      cmp    %r8,%rdi
>     0.00 │      jne    84
>     1.69 │      mov    0x8(%rdx),%r8
>    25.42 │      cmp    %r8,%rdi
>     0.00 │      jne    65
>     1.69 │      mov    %rax,0x8(%rdx)
>     0.00 │      mov    %rdx,(%rax)
>     0.00 │      leaveq
>     0.00 │      retq
>     0.00 │ 43:  mov    %rdx,%r8
>     0.00 │      mov    %rdi,%rcx
>     0.00 │      mov    $0xffffffff817cd6a8,%rdx
>     0.00 │      mov    $0x31,%esi
>     0.00 │      mov    $0xffffffff817cd6e0,%rdi
>     0.00 │      xor    %eax,%eax
>     0.00 │      callq  ffffffff8104eab0 <warn_slowpath_fmt>
>     0.00 │      leaveq
>     0.00 │      retq
>     0.00 │ 65:  mov    %rdi,%rcx
>     0.00 │      mov    $0xffffffff817cd780,%rdx
>     0.00 │      mov    $0x3a,%esi
>     0.00 │      mov    $0xffffffff817cd6e0,%rdi
>     0.00 │      xor    %eax,%eax
>     0.00 │      callq  ffffffff8104eab0 <warn_slowpath_fmt>
>     0.00 │      leaveq
>     0.00 │      retq
> 
> The infrastructure is there to provide formatters for any instruction,
> like the one I'll do for call functions to elide the address.
> 
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> ----------------------------------------------------------------
> Arnaldo Carvalho de Melo (13):
>       perf annotate: Rename objdump_line to disasm_line
>       perf annotate: Parse instruction
>       perf annotate browser: Use the disasm_line instruction name and operand fields
>       perf annotate: Disassembler instruction parsing
>       perf annotate: Parse call targets earlier
>       perf annotate: Introduce scnprintf ins_ops method
>       perf annotate browser: Rename disasm_line_rb_node
>       perf symbols: Introduce symbol__size method
>       perf annotate browser: Hide non jump target addresses in offset mode
>       perf annotate browser: Align jump labels
>       perf annotate browser: Make lines more compact
>       perf annotate browser: Use a vertical line as percentage separator
>       perf annotate: Add missing jump variants
> 
>  tools/perf/ui/browsers/annotate.c |  323 +++++++++++++++++++++----------------
>  tools/perf/util/annotate.c        |  263 +++++++++++++++++++++++++-----
>  tools/perf/util/annotate.h        |   32 +++-
>  tools/perf/util/symbol.h          |    5 +
>  tools/perf/util/util.c            |   10 ++
>  tools/perf/util/util.h            |    2 +
>  6 files changed, 446 insertions(+), 189 deletions(-)

The new output looks very (very!) nice - but it is still a bit 
fragile:

earth5:~/tip/tools/perf> ./perf record -a sleep 1
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.092 MB perf.data (~4008 samples) ]

earth5:~/tip/tools/perf> ./perf annotate _int_free
perf: Segmentation fault

The crash happens here:

Program received signal SIGSEGV, Segmentation fault.
0x0000000000486cda in annotate_browser__mark_jump_targets (size=<optimized out>, 
    browser=<optimized out>) at ui/browsers/annotate.c:704
704			bdlt->jump_target = true;

But other than that the new assembly output is awesome :-)

Thanks,

	Ingo

Re: [GIT PULL 00/13] Annotation improvements (G+ edition)

[Arnaldo Carvalho de Melo]

CCing Hagen, who also reported this crash on G+.

Em Wed, Apr 25, 2012 at 09:05:38AM +0200, Ingo Molnar escreveu:
> The new output looks very (very!) nice - but it is still a bit 
> fragile:

Glad you liked it :-)
 
> earth5:~/tip/tools/perf> ./perf record -a sleep 1
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.092 MB perf.data (~4008 samples) ]
> 
> earth5:~/tip/tools/perf> ./perf annotate _int_free
> perf: Segmentation fault
> 
> The crash happens here:
> 
> Program received signal SIGSEGV, Segmentation fault.
> 0x0000000000486cda in annotate_browser__mark_jump_targets (size=<optimized out>, 
>     browser=<optimized out>) at ui/browsers/annotate.c:704
> 704			bdlt->jump_target = true;
> 
> But other than that the new assembly output is awesome :-)

Humm...

683 static void annotate_browser__mark_jump_targets(struct annotate_browser *browser,
684							size_t size)
685	{
686		u64 offset;
687	
688		for (offset = 0; offset < size; ++offset) {
689			struct disasm_line *dl = browser->offsets[offset], *dlt;
690			struct browser_disasm_line *bdlt;
691	
692			if (!dl || !dl->ins || !ins__is_jump(dl->ins))
693				continue;
694	
695			if (dl->ops.target >= size) {
696				ui__error("jump to after symbol!\n"
697					  "size: %zx, jump target: %" PRIx64,
698					  size, dl->ops.target);
699				continue;
700			}
701	
702			dlt = browser->offsets[dl->ops.target];
703			bdlt = disasm_line__browser(dlt);
704			bdlt->jump_target = true;
705		}
706			
707	}

(gdb) p size
$5 = 2415
(gdb) p offset
$6 = 140
(gdb) p dl->ops.target
$7 = 143
(gdb) p browser->offsets[143]
$8 = (struct disasm_line *) 0x0
(gdb) p dl->name
$9 = 0x2363bd0 "je"
(gdb)

Really strange, the code assumed that at the jump target we would have an
assembly line, but only in the previous instruction offset we have a 'lock':

(gdb) p browser->offsets[144]
$10 = (struct disasm_line *) 0x0
(gdb) p browser->offsets[142]
$11 = (struct disasm_line *) 0x27bd620
(gdb) p browser->offsets[142]->name
$12 = 0x237a8a0 "lock"
(gdb)

I'll study this more, but for now I'll just check if there is a disasm_line
at dl->ops.target, i.e. a valid jump target.

- Arnaldo

Re: [GIT PULL 00/13] Annotation improvements (G+ edition)

[Arnaldo Carvalho de Melo]

After the bandaid, I see some strange constructs like (press 'o' to view
the original disassembly):

    0.00 ||  399f877dc6:v je     399f877dd5 <_int_free+0x735>
    0.00 ||  399f877dc8:  lock   decl (%r12)
    0.00 |+------877dcd:^ jne    399f87bd64 <_L_unlock_5659>
    0.00 |   399f877dd3:v jmp    399f877ddf <_int_free+0x73f> 

There is a bug above, the start_width for the arrow is bigger than it
should, will fix.

Pressing 'o' again this becomes:

    0.00 ||      v je     735
    0.00 ||        lock   decl (%r12)
    0.00 |+------^ jne    0
    0.00 |       v jmp    73f 

The 'jne 0' line, its a misparse of the <....> part, it ass-umed that a
+ was there all the time, which is not true, so I should instead do a
fixup when traversing the ->offsets array, looking for dl->target.ops =
0 and, since we know where the symbol starts at that point, fix it.

- Arnaldo