[PATCHSET 0/7] perf annotate: Add --code-with-type option

[PATCHSET 0/7] perf annotate: Add --code-with-type option

[Namhyung Kim]

Hello,

There are roughly two modes in the perf annotate.  One is normal code
anootation and the other is data type annotation.  I'm proposing a new
way to combine the code and data annotation together.

With this option, "# data-type: <name> [offset (field)]" part will be
adeded when it found a data type for the given instruction.  This is
for every instruction in the function regardless whether it has a
sample or not.

This will be useful to understand function level data access patterns.
Currently it only works with --stdio, but I can add it to TUI later.

Here's an example output.

  $ perf annotate --code-with-type --stdio
  ...
 Percent |      Source code & Disassembly of elf for cpu/mem-loads,ldlat=30/P (4 samples, percent: local period)
  ----------------------------------------------------------------------------------------------------------------
           : 0                0xffffffff81bb7060 <__schedule>:
      0.00 :   ffffffff81bb7060:        pushq   %rbp              # data-type: (stack operation)
      0.00 :   ffffffff81bb7061:        movq    %rsp, %rbp
      0.00 :   ffffffff81bb7064:        pushq   %r15              # data-type: (stack operation)
      0.00 :   ffffffff81bb7066:        pushq   %r14              # data-type: (stack operation)
      0.00 :   ffffffff81bb7068:        pushq   %r13              # data-type: (stack operation)
      0.00 :   ffffffff81bb706a:        pushq   %r12              # data-type: (stack operation)
      0.00 :   ffffffff81bb706c:        pushq   %rbx              # data-type: (stack operation)
      0.00 :   ffffffff81bb706d:        movq    $0x33180, %rbx
      0.00 :   ffffffff81bb7074:        movq    %rbx, %r12
      0.00 :   ffffffff81bb7077:        subq    $0x38, %rsp
      0.00 :   ffffffff81bb707b:        movl    %edi, -0x5c(%rbp)         # data-type: unsigned int +0
      0.00 :   ffffffff81bb707e:        movq    %gs:0x28, %rax            # data-type: (stack canary)
      0.00 :   ffffffff81bb7087:        movq    %rax, -0x30(%rbp)         # data-type: (stack canary)
      0.00 :   ffffffff81bb708b:        xorl    %eax, %eax
      0.00 :   ffffffff81bb708d:        movq    $0x0, -0x58(%rbp)         # data-type: struct rq_flags +0 (flags)
      0.00 :   ffffffff81bb7095:        movq    $0x0, -0x50(%rbp)         # data-type: struct rq_flags +0x8 (clock_update_flags)
      0.00 :   ffffffff81bb709d:        callq   0xffffffff81baa100 <debug_smp_processor_id>               # data-type: (stack operation)
      0.00 :   ffffffff81bb70a2:        cltq
      0.00 :   ffffffff81bb70a4:        addq    -0x7dcf0500(,%rax,8), %r12                # data-type: long unsigned int[] +0
      0.00 :   ffffffff81bb70ac:        movq    0x960(%r12), %r13         # data-type: struct rq +0x960 (curr)
      0.00 :   ffffffff81bb70b4:        movq    0x20(%r13), %rax          # data-type: struct task_struct +0x20 (stack)
      0.00 :   ffffffff81bb70b8:        cmpq    $0x57ac6e9d, (%rax)
      0.00 :   ffffffff81bb70bf:        jne     0xffffffff81bb7bca <__schedule+0xb6a>
      0.00 :   ffffffff81bb70c5:        movl    %gs:0x7e47b87c(%rip), %eax  # 0x32948 <pcpu_hot+0x8>              # data-type: struct pcpu_hot +0x8 (preempt_count)
      0.00 :   ffffffff81bb70cc:        andl    $0x7fffffff, %eax
      0.00 :   ffffffff81bb70d1:        cmpl    $0x1, %eax
      0.00 :   ffffffff81bb70d4:        jne     0xffffffff81bb79de <__schedule+0x97e>
      0.00 :   ffffffff81bb70da:        nopl    (%rax,%rax)
      0.00 :   ffffffff81bb70df:        cmpl    $0x2, 0xe86b3a(%rip)  # 0xffffffff82a3dc20 <prof_on>              # data-type: int +0
      0.00 :   ffffffff81bb70e6:        movq    0x8(%rbp), %rsi
      0.00 :   ffffffff81bb70ea:        je      0xffffffff81bb7a0b <__schedule+0x9ab>
      0.00 :   ffffffff81bb70f0:        nopl    (%rax,%rax)
      0.00 :   ffffffff81bb70f5:        nop
      ...

The code is also available at 'perf/annotate-code-data-v1' branch in
git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git

Thanks,
Namhyung


Namhyung Kim (7):
  perf annotate-data: Add annotated_data_type__get_member_name()
  perf annotate: Remove unused len parameter from
    annotation_line__print()
  perf annotate: Pass annotation_options to annotation_line__print()
  perf annotate: Pass hist_entry to annotate functions
  perf annotate: Factor out __hist_entry__get_data_type()
  perf annotate: Implement code + data type annotation
  perf annotate: Add --code-with-type option.

 tools/perf/Documentation/perf-annotate.txt |   4 +
 tools/perf/builtin-annotate.c              |  19 +-
 tools/perf/builtin-top.c                   |   2 +-
 tools/perf/util/annotate-data.c            |  34 +++
 tools/perf/util/annotate-data.h            |   3 +
 tools/perf/util/annotate.c                 | 267 +++++++++++++--------
 tools/perf/util/annotate.h                 |   8 +-
 tools/perf/util/sort.c                     |  39 +--
 8 files changed, 236 insertions(+), 140 deletions(-)

-- 
2.48.1.711.g2feabab25a-goog

[PATCH 1/7] perf annotate-data: Add annotated_data_type__get_member_name()

[Namhyung Kim]

Factor out a function to get the name of member field at the given
offset.  This will be used in other places.

Also update the output of typeoff sort key a little bit.  As we know
that some special types like (stack operation), (stack canary) and
(unknown) won't have fields, skip printing the offset and field.

For example, the following change is expected.

  "(stack operation) +0 (no field)"   ==>   "(stack operation)"

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/annotate-data.c | 34 ++++++++++++++++++++++++++++
 tools/perf/util/annotate-data.h |  3 +++
 tools/perf/util/sort.c          | 39 ++++++---------------------------
 3 files changed, 44 insertions(+), 32 deletions(-)

diff --git a/tools/perf/util/annotate-data.c b/tools/perf/util/annotate-data.c
index eaf96fa80c830d05..1ef2edbc71d91a50 100644
--- a/tools/perf/util/annotate-data.c
+++ b/tools/perf/util/annotate-data.c
@@ -314,6 +314,40 @@ static void delete_members(struct annotated_member *member)
 	}
 }
 
+static int fill_member_name(char *buf, size_t sz, struct annotated_member *m,
+			    int offset, bool first)
+{
+	struct annotated_member *child;
+
+	if (list_empty(&m->children))
+		return 0;
+
+	list_for_each_entry(child, &m->children, node) {
+		int len;
+
+		if (offset < child->offset || offset >= child->offset + child->size)
+			continue;
+
+		/* It can have anonymous struct/union members */
+		if (child->var_name) {
+			len = scnprintf(buf, sz, "%s%s",
+					first ? "" : ".", child->var_name);
+			first = false;
+		} else {
+			len = 0;
+		}
+
+		return fill_member_name(buf + len, sz - len, child, offset, first) + len;
+	}
+	return 0;
+}
+
+int annotated_data_type__get_member_name(struct annotated_data_type *adt,
+					 char *buf, size_t sz, int member_offset)
+{
+	return fill_member_name(buf, sz, &adt->self, member_offset, /*first=*/true);
+}
+
 static struct annotated_data_type *dso__findnew_data_type(struct dso *dso,
 							  Dwarf_Die *type_die)
 {
diff --git a/tools/perf/util/annotate-data.h b/tools/perf/util/annotate-data.h
index 98c80b2268dde889..c9a1678a8a32518c 100644
--- a/tools/perf/util/annotate-data.h
+++ b/tools/perf/util/annotate-data.h
@@ -229,6 +229,9 @@ void global_var_type__tree_delete(struct rb_root *root);
 
 int hist_entry__annotate_data_tty(struct hist_entry *he, struct evsel *evsel);
 
+int annotated_data_type__get_member_name(struct annotated_data_type *adt,
+					 char *buf, size_t sz, int member_offset);
+
 bool has_reg_type(struct type_state *state, int reg);
 struct type_state_stack *findnew_stack_state(struct type_state *state,
 						int offset, u8 kind,
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 2b6023de7a53ae2e..6f7696b11b97a9f9 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -2403,44 +2403,19 @@ sort__typeoff_sort(struct hist_entry *left, struct hist_entry *right)
 	return left->mem_type_off - right->mem_type_off;
 }
 
-static void fill_member_name(char *buf, size_t sz, struct annotated_member *m,
-			     int offset, bool first)
-{
-	struct annotated_member *child;
-
-	if (list_empty(&m->children))
-		return;
-
-	list_for_each_entry(child, &m->children, node) {
-		if (child->offset <= offset && offset < child->offset + child->size) {
-			int len = 0;
-
-			/* It can have anonymous struct/union members */
-			if (child->var_name) {
-				len = scnprintf(buf, sz, "%s%s",
-						first ? "" : ".", child->var_name);
-				first = false;
-			}
-
-			fill_member_name(buf + len, sz - len, child, offset, first);
-			return;
-		}
-	}
-}
-
 static int hist_entry__typeoff_snprintf(struct hist_entry *he, char *bf,
 				     size_t size, unsigned int width __maybe_unused)
 {
 	struct annotated_data_type *he_type = he->mem_type;
 	char buf[4096];
 
-	buf[0] = '\0';
-	if (list_empty(&he_type->self.children))
-		snprintf(buf, sizeof(buf), "no field");
-	else
-		fill_member_name(buf, sizeof(buf), &he_type->self,
-				 he->mem_type_off, true);
-	buf[4095] = '\0';
+	if (he_type == &unknown_type || he_type == &stackop_type ||
+	    he_type == &canary_type)
+		return repsep_snprintf(bf, size, "%s", he_type->self.type_name);
+
+	if (!annotated_data_type__get_member_name(he_type, buf, sizeof(buf),
+						  he->mem_type_off))
+		scnprintf(buf, sizeof(buf), "no field");
 
 	return repsep_snprintf(bf, size, "%s +%#x (%s)", he_type->self.type_name,
 			       he->mem_type_off, buf);
-- 
2.48.1.711.g2feabab25a-goog

[PATCH 2/7] perf annotate: Remove unused len parameter from annotation_line__print()

[Namhyung Kim]

It's not used anywhere, let's get rid of it.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/annotate.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 31bb326b07a68e46..2e0f70b4872b475a 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -760,7 +760,7 @@ static int disasm_line__print(struct disasm_line *dl, u64 start, int addr_fmt_wi
 
 static int
 annotation_line__print(struct annotation_line *al, struct symbol *sym, u64 start,
-		       struct evsel *evsel, u64 len, int min_pcnt, int printed,
+		       struct evsel *evsel, int min_pcnt, int printed,
 		       int max_lines, struct annotation_line *queue, int addr_fmt_width,
 		       int percent_type)
 {
@@ -796,7 +796,7 @@ annotation_line__print(struct annotation_line *al, struct symbol *sym, u64 start
 			list_for_each_entry_from(queue, &notes->src->source, node) {
 				if (queue == al)
 					break;
-				annotation_line__print(queue, sym, start, evsel, len,
+				annotation_line__print(queue, sym, start, evsel,
 						       0, 0, 1, NULL, addr_fmt_width,
 						       percent_type);
 			}
@@ -1183,7 +1183,6 @@ int symbol__annotate_printf(struct map_symbol *ms, struct evsel *evsel)
 	int printed = 2, queue_len = 0, addr_fmt_width;
 	int more = 0;
 	bool context = opts->context;
-	u64 len;
 	int width = annotation__pcnt_width(notes);
 	int graph_dotted_len;
 	char buf[512];
@@ -1197,8 +1196,6 @@ int symbol__annotate_printf(struct map_symbol *ms, struct evsel *evsel)
 	else
 		d_filename = basename(filename);
 
-	len = symbol__size(sym);
-
 	if (evsel__is_group_event(evsel)) {
 		evsel__group_desc(evsel, buf, sizeof(buf));
 		evsel_name = buf;
@@ -1227,7 +1224,7 @@ int symbol__annotate_printf(struct map_symbol *ms, struct evsel *evsel)
 			queue_len = 0;
 		}
 
-		err = annotation_line__print(pos, sym, start, evsel, len,
+		err = annotation_line__print(pos, sym, start, evsel,
 					     opts->min_pcnt, printed, opts->max_lines,
 					     queue, addr_fmt_width, opts->percent_type);
 
-- 
2.48.1.711.g2feabab25a-goog

[PATCH 3/7] perf annotate: Pass annotation_options to annotation_line__print()

[Namhyung Kim]

The annotation_line__print() has many arguments.  But min_percent,
max_lines and percent_type are from struct annotaion_options.  So let's
pass a pointer to the option instead of passing them separately to
reduce the number of function arguments.

Actually it has a recursive call if 'queue' is set.  Add a new option
instance to pass different values for the case.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/annotate.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 2e0f70b4872b475a..469fcc945126f4f7 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -760,13 +760,14 @@ static int disasm_line__print(struct disasm_line *dl, u64 start, int addr_fmt_wi
 
 static int
 annotation_line__print(struct annotation_line *al, struct symbol *sym, u64 start,
-		       struct evsel *evsel, int min_pcnt, int printed,
-		       int max_lines, struct annotation_line *queue, int addr_fmt_width,
-		       int percent_type)
+		       struct evsel *evsel, struct annotation_options *opts,
+		       int printed, struct annotation_line *queue, int addr_fmt_width)
 {
 	struct disasm_line *dl = container_of(al, struct disasm_line, al);
 	struct annotation *notes = symbol__annotation(sym);
 	static const char *prev_line;
+	int max_lines = opts->max_lines;
+	int percent_type = opts->percent_type;
 
 	if (al->offset != -1) {
 		double max_percent = 0.0;
@@ -786,19 +787,25 @@ annotation_line__print(struct annotation_line *al, struct symbol *sym, u64 start
 		if (al->data_nr > nr_percent)
 			nr_percent = al->data_nr;
 
-		if (max_percent < min_pcnt)
+		if (max_percent < opts->min_pcnt)
 			return -1;
 
 		if (max_lines && printed >= max_lines)
 			return 1;
 
 		if (queue != NULL) {
+			struct annotation_options queue_opts = {
+				.max_lines = 1,
+				.percent_type = percent_type,
+			};
+
 			list_for_each_entry_from(queue, &notes->src->source, node) {
 				if (queue == al)
 					break;
 				annotation_line__print(queue, sym, start, evsel,
-						       0, 0, 1, NULL, addr_fmt_width,
-						       percent_type);
+						       &queue_opts, /*printed=*/0,
+						       /*queue=*/NULL,
+						       addr_fmt_width);
 			}
 		}
 
@@ -1225,8 +1232,7 @@ int symbol__annotate_printf(struct map_symbol *ms, struct evsel *evsel)
 		}
 
 		err = annotation_line__print(pos, sym, start, evsel,
-					     opts->min_pcnt, printed, opts->max_lines,
-					     queue, addr_fmt_width, opts->percent_type);
+					     opts, printed, queue, addr_fmt_width);
 
 		switch (err) {
 		case 0:
-- 
2.48.1.711.g2feabab25a-goog

[PATCH 4/7] perf annotate: Pass hist_entry to annotate functions

[Namhyung Kim]

It's a prepartion to support code annotation and data type
annotation at the same time.  Data type annotation needs more
information in the hist_entry so it needs to be passed deeper.

Also rename a function with the same name in the builtin-annotate.c
to hist_entry__stdio_annotate since it matches better to the command
line option.  And change the condition inside to be simpler.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-annotate.c | 10 +++++-----
 tools/perf/builtin-top.c      |  2 +-
 tools/perf/util/annotate.c    | 11 +++++++----
 tools/perf/util/annotate.h    |  7 +++----
 4 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 836ae0122dabd0ea..966950c38306d6ea 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -321,14 +321,14 @@ static int process_feature_event(struct perf_session *session,
 	return 0;
 }
 
-static int hist_entry__tty_annotate(struct hist_entry *he,
+static int hist_entry__stdio_annotate(struct hist_entry *he,
 				    struct evsel *evsel,
 				    struct perf_annotate *ann)
 {
-	if (!ann->use_stdio2)
-		return symbol__tty_annotate(&he->ms, evsel);
+	if (ann->use_stdio2)
+		return hist_entry__tty_annotate2(he, evsel);
 
-	return symbol__tty_annotate2(&he->ms, evsel);
+	return hist_entry__tty_annotate(he, evsel);
 }
 
 static void print_annotate_data_stat(struct annotated_data_stat *s)
@@ -541,7 +541,7 @@ static void hists__find_annotations(struct hists *hists,
 			if (next != NULL)
 				nd = next;
 		} else {
-			hist_entry__tty_annotate(he, evsel, ann);
+			hist_entry__stdio_annotate(he, evsel, ann);
 			nd = rb_next(nd);
 		}
 	}
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index e1115b8317a0fe3d..7395fdef718d432d 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -263,7 +263,7 @@ static void perf_top__show_details(struct perf_top *top)
 	printf("Showing %s for %s\n", evsel__name(top->sym_evsel), symbol->name);
 	printf("  Events  Pcnt (>=%d%%)\n", annotate_opts.min_pcnt);
 
-	more = symbol__annotate_printf(&he->ms, top->sym_evsel);
+	more = hist_entry__annotate_printf(he, top->sym_evsel);
 
 	if (top->evlist->enabled) {
 		if (top->zero)
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 469fcc945126f4f7..029e47a521caf1af 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1174,8 +1174,9 @@ static int annotated_source__addr_fmt_width(struct list_head *lines, u64 start)
 	return 0;
 }
 
-int symbol__annotate_printf(struct map_symbol *ms, struct evsel *evsel)
+int hist_entry__annotate_printf(struct hist_entry *he, struct evsel *evsel)
 {
+	struct map_symbol *ms = &he->ms;
 	struct map *map = ms->map;
 	struct symbol *sym = ms->sym;
 	struct dso *dso = map__dso(map);
@@ -1600,8 +1601,9 @@ static void symbol__calc_lines(struct map_symbol *ms, struct rb_root *root)
 	annotation__calc_lines(notes, ms, root);
 }
 
-int symbol__tty_annotate2(struct map_symbol *ms, struct evsel *evsel)
+int hist_entry__tty_annotate2(struct hist_entry *he, struct evsel *evsel)
 {
+	struct map_symbol *ms = &he->ms;
 	struct dso *dso = map__dso(ms->map);
 	struct symbol *sym = ms->sym;
 	struct rb_root source_line = RB_ROOT;
@@ -1635,8 +1637,9 @@ int symbol__tty_annotate2(struct map_symbol *ms, struct evsel *evsel)
 	return 0;
 }
 
-int symbol__tty_annotate(struct map_symbol *ms, struct evsel *evsel)
+int hist_entry__tty_annotate(struct hist_entry *he, struct evsel *evsel)
 {
+	struct map_symbol *ms = &he->ms;
 	struct dso *dso = map__dso(ms->map);
 	struct symbol *sym = ms->sym;
 	struct rb_root source_line = RB_ROOT;
@@ -1660,7 +1663,7 @@ int symbol__tty_annotate(struct map_symbol *ms, struct evsel *evsel)
 		print_summary(&source_line, dso__long_name(dso));
 	}
 
-	symbol__annotate_printf(ms, evsel);
+	hist_entry__annotate_printf(he, evsel);
 
 	annotated_source__purge(symbol__annotation(sym)->src);
 
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 98db1b88daf43e13..4fba7cde4968db74 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -455,7 +455,6 @@ enum symbol_disassemble_errno {
 
 int symbol__strerror_disassemble(struct map_symbol *ms, int errnum, char *buf, size_t buflen);
 
-int symbol__annotate_printf(struct map_symbol *ms, struct evsel *evsel);
 void symbol__annotate_zero_histogram(struct symbol *sym, struct evsel *evsel);
 void symbol__annotate_decay_histogram(struct symbol *sym, struct evsel *evsel);
 void annotated_source__purge(struct annotated_source *as);
@@ -464,9 +463,9 @@ int map_symbol__annotation_dump(struct map_symbol *ms, struct evsel *evsel);
 
 bool ui__has_annotation(void);
 
-int symbol__tty_annotate(struct map_symbol *ms, struct evsel *evsel);
-
-int symbol__tty_annotate2(struct map_symbol *ms, struct evsel *evsel);
+int hist_entry__annotate_printf(struct hist_entry *he, struct evsel *evsel);
+int hist_entry__tty_annotate(struct hist_entry *he, struct evsel *evsel);
+int hist_entry__tty_annotate2(struct hist_entry *he, struct evsel *evsel);
 
 #ifdef HAVE_SLANG_SUPPORT
 int symbol__tui_annotate(struct map_symbol *ms, struct evsel *evsel,
-- 
2.48.1.711.g2feabab25a-goog

[PATCH 5/7] perf annotate: Factor out __hist_entry__get_data_type()

[Namhyung Kim]

So that it can only handle a single disasm_linme and hopefully make the
code simpler.  This is also a preparation to be called from different
places later.

The NO_TYPE macro was added to distinguish when it failed or needs retry.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/annotate.c | 170 +++++++++++++++++++++----------------
 1 file changed, 95 insertions(+), 75 deletions(-)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 029e47a521caf1af..8a67340a64b94c39 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -87,6 +87,8 @@ struct annotated_data_type canary_type = {
 	},
 };
 
+#define NO_TYPE ((struct annotated_data_type *)-1UL)
+
 /* symbol histogram: key = offset << 16 | evsel->core.idx */
 static size_t sym_hist_hash(long key, void *ctx __maybe_unused)
 {
@@ -2649,6 +2651,92 @@ void debuginfo_cache__delete(void)
 	di_cache.dbg = NULL;
 }
 
+static struct annotated_data_type *
+__hist_entry__get_data_type(struct hist_entry *he, struct arch *arch,
+			    struct debuginfo *dbg, struct disasm_line *dl,
+			    int *type_offset)
+{
+	struct map_symbol *ms = &he->ms;
+	struct annotated_insn_loc loc;
+	struct annotated_op_loc *op_loc;
+	struct annotated_data_type *mem_type;
+	struct annotated_item_stat *istat;
+	int i;
+
+	istat = annotate_data_stat(&ann_insn_stat, dl->ins.name);
+	if (istat == NULL) {
+		ann_data_stat.no_insn++;
+		return NO_TYPE;
+	}
+
+	if (annotate_get_insn_location(arch, dl, &loc) < 0) {
+		ann_data_stat.no_insn_ops++;
+		istat->bad++;
+		return NO_TYPE;
+	}
+
+	if (is_stack_operation(arch, dl)) {
+		istat->good++;
+		*type_offset = 0;
+		return &stackop_type;
+	}
+
+	for_each_insn_op_loc(&loc, i, op_loc) {
+		struct data_loc_info dloc = {
+			.arch = arch,
+			.thread = he->thread,
+			.ms = ms,
+			.ip = ms->sym->start + dl->al.offset,
+			.cpumode = he->cpumode,
+			.op = op_loc,
+			.di = dbg,
+		};
+
+		if (!op_loc->mem_ref && op_loc->segment == INSN_SEG_NONE)
+			continue;
+
+		/* PC-relative addressing */
+		if (op_loc->reg1 == DWARF_REG_PC) {
+			dloc.var_addr = annotate_calc_pcrel(ms, dloc.ip,
+							    op_loc->offset, dl);
+		}
+
+		/* This CPU access in kernel - pretend PC-relative addressing */
+		if (dso__kernel(map__dso(ms->map)) && arch__is(arch, "x86") &&
+		    op_loc->segment == INSN_SEG_X86_GS && op_loc->imm) {
+			dloc.var_addr = op_loc->offset;
+			op_loc->reg1 = DWARF_REG_PC;
+		}
+
+		mem_type = find_data_type(&dloc);
+
+		if (mem_type == NULL && is_stack_canary(arch, op_loc)) {
+			istat->good++;
+			*type_offset = 0;
+			return &canary_type;
+		}
+
+		if (mem_type)
+			istat->good++;
+		else
+			istat->bad++;
+
+		if (symbol_conf.annotate_data_sample) {
+			struct evsel *evsel = hists_to_evsel(he->hists);
+
+			annotated_data_type__update_samples(mem_type, evsel,
+							    dloc.type_offset,
+							    he->stat.nr_events,
+							    he->stat.period);
+		}
+		*type_offset = dloc.type_offset;
+		return mem_type ?: NO_TYPE;
+	}
+
+	/* retry with a fused instruction */
+	return NULL;
+}
+
 /**
  * hist_entry__get_data_type - find data type for given hist entry
  * @he: hist entry
@@ -2664,12 +2752,9 @@ struct annotated_data_type *hist_entry__get_data_type(struct hist_entry *he)
 	struct evsel *evsel = hists_to_evsel(he->hists);
 	struct arch *arch;
 	struct disasm_line *dl;
-	struct annotated_insn_loc loc;
-	struct annotated_op_loc *op_loc;
 	struct annotated_data_type *mem_type;
 	struct annotated_item_stat *istat;
 	u64 ip = he->ip;
-	int i;
 
 	ann_data_stat.total++;
 
@@ -2721,77 +2806,10 @@ struct annotated_data_type *hist_entry__get_data_type(struct hist_entry *he)
 	}
 
 retry:
-	istat = annotate_data_stat(&ann_insn_stat, dl->ins.name);
-	if (istat == NULL) {
-		ann_data_stat.no_insn++;
-		return NULL;
-	}
-
-	if (annotate_get_insn_location(arch, dl, &loc) < 0) {
-		ann_data_stat.no_insn_ops++;
-		istat->bad++;
-		return NULL;
-	}
-
-	if (is_stack_operation(arch, dl)) {
-		istat->good++;
-		he->mem_type_off = 0;
-		return &stackop_type;
-	}
-
-	for_each_insn_op_loc(&loc, i, op_loc) {
-		struct data_loc_info dloc = {
-			.arch = arch,
-			.thread = he->thread,
-			.ms = ms,
-			/* Recalculate IP for LOCK prefix or insn fusion */
-			.ip = ms->sym->start + dl->al.offset,
-			.cpumode = he->cpumode,
-			.op = op_loc,
-			.di = di_cache.dbg,
-		};
-
-		if (!op_loc->mem_ref && op_loc->segment == INSN_SEG_NONE)
-			continue;
-
-		/* Recalculate IP because of LOCK prefix or insn fusion */
-		ip = ms->sym->start + dl->al.offset;
-
-		/* PC-relative addressing */
-		if (op_loc->reg1 == DWARF_REG_PC) {
-			dloc.var_addr = annotate_calc_pcrel(ms, dloc.ip,
-							    op_loc->offset, dl);
-		}
-
-		/* This CPU access in kernel - pretend PC-relative addressing */
-		if (dso__kernel(map__dso(ms->map)) && arch__is(arch, "x86") &&
-		    op_loc->segment == INSN_SEG_X86_GS && op_loc->imm) {
-			dloc.var_addr = op_loc->offset;
-			op_loc->reg1 = DWARF_REG_PC;
-		}
-
-		mem_type = find_data_type(&dloc);
-
-		if (mem_type == NULL && is_stack_canary(arch, op_loc)) {
-			istat->good++;
-			he->mem_type_off = 0;
-			return &canary_type;
-		}
-
-		if (mem_type)
-			istat->good++;
-		else
-			istat->bad++;
-
-		if (symbol_conf.annotate_data_sample) {
-			annotated_data_type__update_samples(mem_type, evsel,
-							    dloc.type_offset,
-							    he->stat.nr_events,
-							    he->stat.period);
-		}
-		he->mem_type_off = dloc.type_offset;
-		return mem_type;
-	}
+	mem_type = __hist_entry__get_data_type(he, arch, di_cache.dbg, dl,
+					       &he->mem_type_off);
+	if (mem_type)
+		return mem_type == NO_TYPE ? NULL : mem_type;
 
 	/*
 	 * Some instructions can be fused and the actual memory access came
@@ -2811,7 +2829,9 @@ struct annotated_data_type *hist_entry__get_data_type(struct hist_entry *he)
 	}
 
 	ann_data_stat.no_mem_ops++;
-	istat->bad++;
+	istat = annotate_data_stat(&ann_insn_stat, dl->ins.name);
+	if (istat)
+		istat->bad++;
 	return NULL;
 }
 
-- 
2.48.1.711.g2feabab25a-goog

[PATCH 6/7] perf annotate: Implement code + data type annotation

[Namhyung Kim]

Sometimes it's useful to see both instructions and their data type
together.  Let's extend the annotate code to use data type profiling
functions.

To make it easy to pass more argument, introduce a struct to carry
necessary information together.  Also add a new annotation_option called
'code_with_type' to control the behavior.  This is not enabled yet but
it'll be set later from the command line.

For simplicity, this is implemented for --stdio only.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/annotate.c | 73 ++++++++++++++++++++++++++++++--------
 tools/perf/util/annotate.h |  1 +
 2 files changed, 60 insertions(+), 14 deletions(-)

diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 8a67340a64b94c39..1e59b9e5339d393b 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -760,11 +760,26 @@ static int disasm_line__print(struct disasm_line *dl, u64 start, int addr_fmt_wi
 	return 0;
 }
 
+static struct annotated_data_type *
+__hist_entry__get_data_type(struct hist_entry *he, struct arch *arch,
+			    struct debuginfo *dbg, struct disasm_line *dl,
+			    int *type_offset);
+
+struct annotation_print_data {
+	struct hist_entry *he;
+	struct evsel *evsel;
+	struct arch *arch;
+	struct debuginfo *dbg;
+	u64 start;
+	int addr_fmt_width;
+};
+
 static int
-annotation_line__print(struct annotation_line *al, struct symbol *sym, u64 start,
-		       struct evsel *evsel, struct annotation_options *opts,
-		       int printed, struct annotation_line *queue, int addr_fmt_width)
+annotation_line__print(struct annotation_line *al, struct annotation_print_data *apd,
+		       struct annotation_options *opts, int printed,
+		       struct annotation_line *queue)
 {
+	struct symbol *sym = apd->he->ms.sym;
 	struct disasm_line *dl = container_of(al, struct disasm_line, al);
 	struct annotation *notes = symbol__annotation(sym);
 	static const char *prev_line;
@@ -804,10 +819,8 @@ annotation_line__print(struct annotation_line *al, struct symbol *sym, u64 start
 			list_for_each_entry_from(queue, &notes->src->source, node) {
 				if (queue == al)
 					break;
-				annotation_line__print(queue, sym, start, evsel,
-						       &queue_opts, /*printed=*/0,
-						       /*queue=*/NULL,
-						       addr_fmt_width);
+				annotation_line__print(queue, apd, &queue_opts,
+						       /*printed=*/0, /*queue=*/NULL);
 			}
 		}
 
@@ -832,7 +845,31 @@ annotation_line__print(struct annotation_line *al, struct symbol *sym, u64 start
 
 		printf(" : ");
 
-		disasm_line__print(dl, start, addr_fmt_width);
+		disasm_line__print(dl, apd->start, apd->addr_fmt_width);
+
+		if (opts->code_with_type && apd->dbg) {
+			struct annotated_data_type *data_type;
+			int offset = 0;
+
+			data_type = __hist_entry__get_data_type(apd->he, apd->arch,
+								apd->dbg, dl, &offset);
+			if (data_type && data_type != NO_TYPE) {
+				char buf[4096];
+
+				printf("\t\t# data-type: %s",
+				       data_type->self.type_name);
+
+				if (data_type != &stackop_type &&
+				    data_type != &canary_type)
+					printf(" +%#x", offset);
+
+				if (annotated_data_type__get_member_name(data_type,
+									 buf,
+									 sizeof(buf),
+									 offset))
+					printf(" (%s)", buf);
+			}
+		}
 
 		/*
 		 * Also color the filename and line if needed, with
@@ -858,7 +895,8 @@ annotation_line__print(struct annotation_line *al, struct symbol *sym, u64 start
 		if (!*al->line)
 			printf(" %*s:\n", width, " ");
 		else
-			printf(" %*s: %-*d %s\n", width, " ", addr_fmt_width, al->line_nr, al->line);
+			printf(" %*s: %-*d %s\n", width, " ", apd->addr_fmt_width,
+			       al->line_nr, al->line);
 	}
 
 	return 0;
@@ -1189,8 +1227,12 @@ int hist_entry__annotate_printf(struct hist_entry *he, struct evsel *evsel)
 	struct sym_hist *h = annotation__histogram(notes, evsel);
 	struct annotation_line *pos, *queue = NULL;
 	struct annotation_options *opts = &annotate_opts;
-	u64 start = map__rip_2objdump(map, sym->start);
-	int printed = 2, queue_len = 0, addr_fmt_width;
+	struct annotation_print_data apd = {
+		.he = he,
+		.evsel = evsel,
+		.start = map__rip_2objdump(map, sym->start),
+	};
+	int printed = 2, queue_len = 0;
 	int more = 0;
 	bool context = opts->context;
 	int width = annotation__pcnt_width(notes);
@@ -1224,7 +1266,10 @@ int hist_entry__annotate_printf(struct hist_entry *he, struct evsel *evsel)
 	if (verbose > 0)
 		symbol__annotate_hits(sym, evsel);
 
-	addr_fmt_width = annotated_source__addr_fmt_width(&notes->src->source, start);
+	apd.addr_fmt_width = annotated_source__addr_fmt_width(&notes->src->source,
+							      apd.start);
+	evsel__get_arch(evsel, &apd.arch);
+	apd.dbg = debuginfo__new(filename);
 
 	list_for_each_entry(pos, &notes->src->source, node) {
 		int err;
@@ -1234,8 +1279,7 @@ int hist_entry__annotate_printf(struct hist_entry *he, struct evsel *evsel)
 			queue_len = 0;
 		}
 
-		err = annotation_line__print(pos, sym, start, evsel,
-					     opts, printed, queue, addr_fmt_width);
+		err = annotation_line__print(pos, &apd, opts, printed, queue);
 
 		switch (err) {
 		case 0:
@@ -1266,6 +1310,7 @@ int hist_entry__annotate_printf(struct hist_entry *he, struct evsel *evsel)
 		}
 	}
 
+	debuginfo__delete(apd.dbg);
 	free(filename);
 
 	return more;
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 4fba7cde4968db74..03a7cc24058ff5a2 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -55,6 +55,7 @@ struct annotation_options {
 	     show_asm_raw,
 	     show_br_cntr,
 	     annotate_src,
+	     code_with_type,
 	     full_addr;
 	u8   offset_level;
 	u8   disassemblers[MAX_DISASSEMBLERS];
-- 
2.48.1.711.g2feabab25a-goog

[PATCH 7/7] perf annotate: Add --code-with-type option.

[Namhyung Kim]

This option is to show data type info in the regular (code) annotation.
It tries to find data type for each (memory) instruction in the
function.  It'd be useful to see function-level memory access pattern
and also to debug the data type profiling result.

The output would be added at the end of the line and have "# data-type:"
prefix.

For now, it only works with --stdio mode for simplicity.  I can work on
enabling it for TUI later.

  $ perf annotate --stdio --code-with-type
   Percent |      Source code & Disassembly of vmlinux for cpu/mem-loads/ppk (253 samples, percent: local period)
  ---------------------------------------------------------------------------------------------------------------
           : 0                0xffffffff81baa000 <check_preemption_disabled>:
      0.00 :   ffffffff81baa000:        pushq   %r12              # data-type: (stack operation)
      0.00 :   ffffffff81baa002:        pushq   %rbp              # data-type: (stack operation)
      0.00 :   ffffffff81baa003:        pushq   %rbx              # data-type: (stack operation)
      0.00 :   ffffffff81baa004:        subq    $0x8, %rsp
     18.00 :   ffffffff81baa008:        movl    %gs:0x7e48893d(%rip), %ebx  # 0x3294c <pcpu_hot+0xc>              # data-type: struct pcpu_hot +0xc (cpu_number)
     12.58 :   ffffffff81baa00f:        movl    %gs:0x7e488932(%rip), %eax  # 0x32948 <pcpu_hot+0x8>              # data-type: struct pcpu_hot +0x8 (preempt_count)
      0.00 :   ffffffff81baa016:        testl   $0x7fffffff, %eax
      0.00 :   ffffffff81baa01b:        je      0xffffffff81baa02c <check_preemption_disabled+0x2c>
      0.00 :   ffffffff81baa01d:        addq    $0x8, %rsp
      0.00 :   ffffffff81baa021:        movl    %ebx, %eax
     14.19 :   ffffffff81baa023:        popq    %rbx              # data-type: (stack operation)
     18.86 :   ffffffff81baa024:        popq    %rbp              # data-type: (stack operation)
     12.10 :   ffffffff81baa025:        popq    %r12              # data-type: (stack operation)
     17.78 :   ffffffff81baa027:        jmp     0xffffffff81bc1170 <__x86_return_thunk>
      6.49 :   ffffffff81baa02c:        callq   *0xc9139e(%rip)  # 0xffffffff8283b3d0 <pv_ops+0xf0>               # data-type: (stack operation)
      0.00 :   ffffffff81baa032:        testb   $0x2, %ah
      0.00 :   ffffffff81baa035:        je      0xffffffff81baa01d <check_preemption_disabled+0x1d>
      0.00 :   ffffffff81baa037:        movq    %rdi, %rbp
      0.00 :   ffffffff81baa03a:        movq    %gs:0x32940, %rax         # data-type: struct pcpu_hot +0 (current_task)
      0.00 :   ffffffff81baa043:        testb   $0x4, 0x2f(%rax)          # data-type: struct task_struct +0x2f (flags)
      0.00 :   ffffffff81baa047:        je      0xffffffff81baa052 <check_preemption_disabled+0x52>
      0.00 :   ffffffff81baa049:        cmpl    $0x1, 0x3d0(%rax)         # data-type: struct task_struct +0x3d0 (nr_cpus_allowed)
      0.00 :   ffffffff81baa050:        je      0xffffffff81baa01d <check_preemption_disabled+0x1d>
      0.00 :   ffffffff81baa052:        movq    %gs:0x32940, %r12         # data-type: struct pcpu_hot +0 (current_task)
      0.00 :   ffffffff81baa05b:        cmpw    $0x0, 0x7f0(%r12)         # data-type: struct task_struct +0x7f0 (migration_disabled)
      0.00 :   ffffffff81baa065:        movq    %rsi, (%rsp)
      0.00 :   ffffffff81baa069:        jne     0xffffffff81baa01d <check_preemption_disabled+0x1d>
      0.00 :   ffffffff81baa06b:        movl    0xe8dd13(%rip), %eax  # 0xffffffff82a37d84 <system_state>         # data-type: enum system_states +0
      0.00 :   ffffffff81baa071:        testl   %eax, %eax
      0.00 :   ffffffff81baa073:        je      0xffffffff81baa01d <check_preemption_disabled+0x1d>
      0.00 :   ffffffff81baa075:        incl    %gs:0x7e4888cc(%rip)  # 0x32948 <pcpu_hot+0x8>            # data-type: struct pcpu_hot +0x8 (preempt_count)
      0.00 :   ffffffff81baa07c:        movq    $-0x7e14a100, %rdi
      0.00 :   ffffffff81baa083:        callq   0xffffffff81148c40 <__printk_ratelimit>           # data-type: (stack operation)
      0.00 :   ffffffff81baa088:        testl   %eax, %eax
      0.00 :   ffffffff81baa08a:        je      0xffffffff81baa0d5 <check_preemption_disabled+0xd5>
      0.00 :   ffffffff81baa08c:        movl    0x958(%r12), %r9d         # data-type: struct task_struct +0x958 (pid)
      0.00 :   ffffffff81baa094:        movq    (%rsp), %rdx              # data-type: char* +0
      0.00 :   ffffffff81baa098:        movq    %rbp, %rsi
      0.00 :   ffffffff81baa09b:        leaq    0xb88(%r12), %r8          # data-type: struct task_struct +0xb88 (comm)
      0.00 :   ffffffff81baa0a3:        movl    %gs:0x7e48889e(%rip), %ecx  # 0x32948 <pcpu_hot+0x8>              # data-type: struct pcpu_hot +0x8 (preempt_count)
      0.00 :   ffffffff81baa0aa:        andl    $0x7fffffff, %ecx
      0.00 :   ffffffff81baa0b0:        movq    $-0x7dd3cdf0, %rdi
      0.00 :   ffffffff81baa0b7:        subl    $0x1, %ecx
      0.00 :   ffffffff81baa0ba:        callq   0xffffffff81149340 <_printk>              # data-type: (stack operation)
      0.00 :   ffffffff81baa0bf:        movq    0x20(%rsp), %rsi
      0.00 :   ffffffff81baa0c4:        movq    $-0x7ddb8c7e, %rdi
      0.00 :   ffffffff81baa0cb:        callq   0xffffffff81149340 <_printk>              # data-type: (stack operation)
      0.00 :   ffffffff81baa0d0:        callq   0xffffffff81b7ab60 <dump_stack>           # data-type: (stack operation)
      0.00 :   ffffffff81baa0d5:        decl    %gs:0x7e48886c(%rip)  # 0x32948 <pcpu_hot+0x8>            # data-type: struct pcpu_hot +0x8 (preempt_count)
      0.00 :   ffffffff81baa0dc:        jmp     0xffffffff81baa01d <check_preemption_disabled+0x1d>

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/Documentation/perf-annotate.txt | 4 ++++
 tools/perf/builtin-annotate.c              | 9 +++++++++
 2 files changed, 13 insertions(+)

diff --git a/tools/perf/Documentation/perf-annotate.txt b/tools/perf/Documentation/perf-annotate.txt
index 156c5f37b05141ca..46090c5b42b4762f 100644
--- a/tools/perf/Documentation/perf-annotate.txt
+++ b/tools/perf/Documentation/perf-annotate.txt
@@ -168,6 +168,10 @@ include::itrace.txt[]
 --skip-empty::
 	Do not display empty (or dummy) events.
 
+--code-with-type::
+	Show data type info in code annotation (for memory instructions only).
+	Currently it only works with --stdio option.
+
 
 SEE ALSO
 --------
diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 966950c38306d6ea..9833c2c82a2fee46 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -788,6 +788,8 @@ int cmd_annotate(int argc, const char **argv)
 		    "Show instruction stats for the data type annotation"),
 	OPT_BOOLEAN(0, "skip-empty", &symbol_conf.skip_empty,
 		    "Do not display empty (or dummy) events in the output"),
+	OPT_BOOLEAN(0, "code-with-type", &annotate_opts.code_with_type,
+		    "Show data type info in code annotation (memory instructions only)"),
 	OPT_END()
 	};
 	int ret;
@@ -913,6 +915,13 @@ int cmd_annotate(int argc, const char **argv)
 		annotate_opts.annotate_src = false;
 		symbol_conf.annotate_data_member = true;
 		symbol_conf.annotate_data_sample = true;
+	} else if (annotate_opts.code_with_type) {
+		symbol_conf.annotate_data_member = true;
+
+		if (!annotate.use_stdio) {
+			pr_err("--code-with-type only works with --stdio.\n");
+			goto out_delete;
+		}
 	}
 
 	setup_browser(true);
-- 
2.48.1.711.g2feabab25a-goog

Re: [PATCHSET 0/7] perf annotate: Add --code-with-type option

[Ian Rogers]

On Mon, Mar 3, 2025 at 9:38 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Hello,
>
> There are roughly two modes in the perf annotate.  One is normal code
> anootation and the other is data type annotation.  I'm proposing a new

nit: s/anootation/annotation/

> way to combine the code and data annotation together.
>
> With this option, "# data-type: <name> [offset (field)]" part will be
> adeded when it found a data type for the given instruction.  This is

nit: s/adeded/added/

> for every instruction in the function regardless whether it has a
> sample or not.
>
> This will be useful to understand function level data access patterns.
> Currently it only works with --stdio, but I can add it to TUI later.
>
> Here's an example output.

At some point I think we might as well have our own disassembler. The
annotations remind me of the relocation and other annotation output of
objdump.

>   $ perf annotate --code-with-type --stdio
>   ...
>  Percent |      Source code & Disassembly of elf for cpu/mem-loads,ldlat=30/P (4 samples, percent: local period)
>   ----------------------------------------------------------------------------------------------------------------
>            : 0                0xffffffff81bb7060 <__schedule>:
>       0.00 :   ffffffff81bb7060:        pushq   %rbp              # data-type: (stack operation)
>       0.00 :   ffffffff81bb7061:        movq    %rsp, %rbp
>       0.00 :   ffffffff81bb7064:        pushq   %r15              # data-type: (stack operation)
>       0.00 :   ffffffff81bb7066:        pushq   %r14              # data-type: (stack operation)
>       0.00 :   ffffffff81bb7068:        pushq   %r13              # data-type: (stack operation)
>       0.00 :   ffffffff81bb706a:        pushq   %r12              # data-type: (stack operation)
>       0.00 :   ffffffff81bb706c:        pushq   %rbx              # data-type: (stack operation)

data-type: looks a bit weird here. I wonder if the
.cfi_offset/.cfi_restore could  make this information richer, maybe:
       0.00 :   ffffffff81bb7060:        pushq   %rbp              #
*(frame-16) = caller rbp
       0.00 :   ffffffff81bb7061:        movq    %rsp, %rbp
       0.00 :   ffffffff81bb7064:        pushq   %r15              #
*(frame-24) = caller r15

>       0.00 :   ffffffff81bb706d:        movq    $0x33180, %rbx
>       0.00 :   ffffffff81bb7074:        movq    %rbx, %r12
>       0.00 :   ffffffff81bb7077:        subq    $0x38, %rsp
>       0.00 :   ffffffff81bb707b:        movl    %edi, -0x5c(%rbp)         # data-type: unsigned int +0
>       0.00 :   ffffffff81bb707e:        movq    %gs:0x28, %rax            # data-type: (stack canary)
>       0.00 :   ffffffff81bb7087:        movq    %rax, -0x30(%rbp)         # data-type: (stack canary)
>       0.00 :   ffffffff81bb708b:        xorl    %eax, %eax
>       0.00 :   ffffffff81bb708d:        movq    $0x0, -0x58(%rbp)         # data-type: struct rq_flags +0 (flags)
>       0.00 :   ffffffff81bb7095:        movq    $0x0, -0x50(%rbp)         # data-type: struct rq_flags +0x8 (clock_update_flags)
>       0.00 :   ffffffff81bb709d:        callq   0xffffffff81baa100 <debug_smp_processor_id>               # data-type: (stack operation)
>       0.00 :   ffffffff81bb70a2:        cltq
>       0.00 :   ffffffff81bb70a4:        addq    -0x7dcf0500(,%rax,8), %r12                # data-type: long unsigned int[] +0
>       0.00 :   ffffffff81bb70ac:        movq    0x960(%r12), %r13         # data-type: struct rq +0x960 (curr)
>       0.00 :   ffffffff81bb70b4:        movq    0x20(%r13), %rax          # data-type: struct task_struct +0x20 (stack)
>       0.00 :   ffffffff81bb70b8:        cmpq    $0x57ac6e9d, (%rax)
>       0.00 :   ffffffff81bb70bf:        jne     0xffffffff81bb7bca <__schedule+0xb6a>
>       0.00 :   ffffffff81bb70c5:        movl    %gs:0x7e47b87c(%rip), %eax  # 0x32948 <pcpu_hot+0x8>              # data-type: struct pcpu_hot +0x8 (preempt_count)

The variable name in the struct is really nice, it is a shame the same
information gets repeated a few times on the line.

Thanks,
Ian

>       0.00 :   ffffffff81bb70cc:        andl    $0x7fffffff, %eax
>       0.00 :   ffffffff81bb70d1:        cmpl    $0x1, %eax
>       0.00 :   ffffffff81bb70d4:        jne     0xffffffff81bb79de <__schedule+0x97e>
>       0.00 :   ffffffff81bb70da:        nopl    (%rax,%rax)
>       0.00 :   ffffffff81bb70df:        cmpl    $0x2, 0xe86b3a(%rip)  # 0xffffffff82a3dc20 <prof_on>              # data-type: int +0
>       0.00 :   ffffffff81bb70e6:        movq    0x8(%rbp), %rsi
>       0.00 :   ffffffff81bb70ea:        je      0xffffffff81bb7a0b <__schedule+0x9ab>
>       0.00 :   ffffffff81bb70f0:        nopl    (%rax,%rax)
>       0.00 :   ffffffff81bb70f5:        nop
>       ...
>
> The code is also available at 'perf/annotate-code-data-v1' branch in
> git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
>
> Thanks,
> Namhyung
>
>
> Namhyung Kim (7):
>   perf annotate-data: Add annotated_data_type__get_member_name()
>   perf annotate: Remove unused len parameter from
>     annotation_line__print()
>   perf annotate: Pass annotation_options to annotation_line__print()
>   perf annotate: Pass hist_entry to annotate functions
>   perf annotate: Factor out __hist_entry__get_data_type()
>   perf annotate: Implement code + data type annotation
>   perf annotate: Add --code-with-type option.
>
>  tools/perf/Documentation/perf-annotate.txt |   4 +
>  tools/perf/builtin-annotate.c              |  19 +-
>  tools/perf/builtin-top.c                   |   2 +-
>  tools/perf/util/annotate-data.c            |  34 +++
>  tools/perf/util/annotate-data.h            |   3 +
>  tools/perf/util/annotate.c                 | 267 +++++++++++++--------
>  tools/perf/util/annotate.h                 |   8 +-
>  tools/perf/util/sort.c                     |  39 +--
>  8 files changed, 236 insertions(+), 140 deletions(-)
>
> --
> 2.48.1.711.g2feabab25a-goog
>

Re: [PATCHSET 0/7] perf annotate: Add --code-with-type option

[Ian Rogers]

On Wed, Mar 5, 2025 at 3:27 PM Ian Rogers <irogers@google.com> wrote:
>
> On Mon, Mar 3, 2025 at 9:38 AM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > Hello,
> >
> > There are roughly two modes in the perf annotate.  One is normal code
> > anootation and the other is data type annotation.  I'm proposing a new
>
> nit: s/anootation/annotation/
>
> > way to combine the code and data annotation together.
> >
> > With this option, "# data-type: <name> [offset (field)]" part will be
> > adeded when it found a data type for the given instruction.  This is
>
> nit: s/adeded/added/
>
> > for every instruction in the function regardless whether it has a
> > sample or not.
> >
> > This will be useful to understand function level data access patterns.
> > Currently it only works with --stdio, but I can add it to TUI later.
> >
> > Here's an example output.
>
> At some point I think we might as well have our own disassembler. The
> annotations remind me of the relocation and other annotation output of
> objdump.
>
> >   $ perf annotate --code-with-type --stdio
> >   ...
> >  Percent |      Source code & Disassembly of elf for cpu/mem-loads,ldlat=30/P (4 samples, percent: local period)
> >   ----------------------------------------------------------------------------------------------------------------
> >            : 0                0xffffffff81bb7060 <__schedule>:
> >       0.00 :   ffffffff81bb7060:        pushq   %rbp              # data-type: (stack operation)
> >       0.00 :   ffffffff81bb7061:        movq    %rsp, %rbp
> >       0.00 :   ffffffff81bb7064:        pushq   %r15              # data-type: (stack operation)
> >       0.00 :   ffffffff81bb7066:        pushq   %r14              # data-type: (stack operation)
> >       0.00 :   ffffffff81bb7068:        pushq   %r13              # data-type: (stack operation)
> >       0.00 :   ffffffff81bb706a:        pushq   %r12              # data-type: (stack operation)
> >       0.00 :   ffffffff81bb706c:        pushq   %rbx              # data-type: (stack operation)
>
> data-type: looks a bit weird here. I wonder if the
> .cfi_offset/.cfi_restore could  make this information richer, maybe:
>        0.00 :   ffffffff81bb7060:        pushq   %rbp              #
> *(frame-16) = caller rbp
>        0.00 :   ffffffff81bb7061:        movq    %rsp, %rbp
>        0.00 :   ffffffff81bb7064:        pushq   %r15              #
> *(frame-24) = caller r15
>
> >       0.00 :   ffffffff81bb706d:        movq    $0x33180, %rbx
> >       0.00 :   ffffffff81bb7074:        movq    %rbx, %r12
> >       0.00 :   ffffffff81bb7077:        subq    $0x38, %rsp
> >       0.00 :   ffffffff81bb707b:        movl    %edi, -0x5c(%rbp)         # data-type: unsigned int +0
> >       0.00 :   ffffffff81bb707e:        movq    %gs:0x28, %rax            # data-type: (stack canary)
> >       0.00 :   ffffffff81bb7087:        movq    %rax, -0x30(%rbp)         # data-type: (stack canary)
> >       0.00 :   ffffffff81bb708b:        xorl    %eax, %eax
> >       0.00 :   ffffffff81bb708d:        movq    $0x0, -0x58(%rbp)         # data-type: struct rq_flags +0 (flags)
> >       0.00 :   ffffffff81bb7095:        movq    $0x0, -0x50(%rbp)         # data-type: struct rq_flags +0x8 (clock_update_flags)
> >       0.00 :   ffffffff81bb709d:        callq   0xffffffff81baa100 <debug_smp_processor_id>               # data-type: (stack operation)
> >       0.00 :   ffffffff81bb70a2:        cltq
> >       0.00 :   ffffffff81bb70a4:        addq    -0x7dcf0500(,%rax,8), %r12                # data-type: long unsigned int[] +0
> >       0.00 :   ffffffff81bb70ac:        movq    0x960(%r12), %r13         # data-type: struct rq +0x960 (curr)
> >       0.00 :   ffffffff81bb70b4:        movq    0x20(%r13), %rax          # data-type: struct task_struct +0x20 (stack)
> >       0.00 :   ffffffff81bb70b8:        cmpq    $0x57ac6e9d, (%rax)
> >       0.00 :   ffffffff81bb70bf:        jne     0xffffffff81bb7bca <__schedule+0xb6a>
> >       0.00 :   ffffffff81bb70c5:        movl    %gs:0x7e47b87c(%rip), %eax  # 0x32948 <pcpu_hot+0x8>              # data-type: struct pcpu_hot +0x8 (preempt_count)
>
> The variable name in the struct is really nice, it is a shame the same
> information gets repeated a few times on the line.

Outside of the typos in the cover letter, the rest of the patches lgtm.

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> >       0.00 :   ffffffff81bb70cc:        andl    $0x7fffffff, %eax
> >       0.00 :   ffffffff81bb70d1:        cmpl    $0x1, %eax
> >       0.00 :   ffffffff81bb70d4:        jne     0xffffffff81bb79de <__schedule+0x97e>
> >       0.00 :   ffffffff81bb70da:        nopl    (%rax,%rax)
> >       0.00 :   ffffffff81bb70df:        cmpl    $0x2, 0xe86b3a(%rip)  # 0xffffffff82a3dc20 <prof_on>              # data-type: int +0
> >       0.00 :   ffffffff81bb70e6:        movq    0x8(%rbp), %rsi
> >       0.00 :   ffffffff81bb70ea:        je      0xffffffff81bb7a0b <__schedule+0x9ab>
> >       0.00 :   ffffffff81bb70f0:        nopl    (%rax,%rax)
> >       0.00 :   ffffffff81bb70f5:        nop
> >       ...
> >
> > The code is also available at 'perf/annotate-code-data-v1' branch in
> > git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> >
> > Thanks,
> > Namhyung
> >
> >
> > Namhyung Kim (7):
> >   perf annotate-data: Add annotated_data_type__get_member_name()
> >   perf annotate: Remove unused len parameter from
> >     annotation_line__print()
> >   perf annotate: Pass annotation_options to annotation_line__print()
> >   perf annotate: Pass hist_entry to annotate functions
> >   perf annotate: Factor out __hist_entry__get_data_type()
> >   perf annotate: Implement code + data type annotation
> >   perf annotate: Add --code-with-type option.
> >
> >  tools/perf/Documentation/perf-annotate.txt |   4 +
> >  tools/perf/builtin-annotate.c              |  19 +-
> >  tools/perf/builtin-top.c                   |   2 +-
> >  tools/perf/util/annotate-data.c            |  34 +++
> >  tools/perf/util/annotate-data.h            |   3 +
> >  tools/perf/util/annotate.c                 | 267 +++++++++++++--------
> >  tools/perf/util/annotate.h                 |   8 +-
> >  tools/perf/util/sort.c                     |  39 +--
> >  8 files changed, 236 insertions(+), 140 deletions(-)
> >
> > --
> > 2.48.1.711.g2feabab25a-goog
> >

Re: [PATCHSET 0/7] perf annotate: Add --code-with-type option

[Namhyung Kim]

On Wed, Mar 05, 2025 at 03:31:45PM -0800, Ian Rogers wrote:
> On Wed, Mar 5, 2025 at 3:27 PM Ian Rogers <irogers@google.com> wrote:
> >
> > On Mon, Mar 3, 2025 at 9:38 AM Namhyung Kim <namhyung@kernel.org> wrote:
> > >
> > > Hello,
> > >
> > > There are roughly two modes in the perf annotate.  One is normal code
> > > anootation and the other is data type annotation.  I'm proposing a new
> >
> > nit: s/anootation/annotation/

Oops.

> >
> > > way to combine the code and data annotation together.
> > >
> > > With this option, "# data-type: <name> [offset (field)]" part will be
> > > adeded when it found a data type for the given instruction.  This is
> >
> > nit: s/adeded/added/

Thanks for pointing this out.

> >
> > > for every instruction in the function regardless whether it has a
> > > sample or not.
> > >
> > > This will be useful to understand function level data access patterns.
> > > Currently it only works with --stdio, but I can add it to TUI later.
> > >
> > > Here's an example output.
> >
> > At some point I think we might as well have our own disassembler. The
> > annotations remind me of the relocation and other annotation output of
> > objdump.
> >
> > >   $ perf annotate --code-with-type --stdio
> > >   ...
> > >  Percent |      Source code & Disassembly of elf for cpu/mem-loads,ldlat=30/P (4 samples, percent: local period)
> > >   ----------------------------------------------------------------------------------------------------------------
> > >            : 0                0xffffffff81bb7060 <__schedule>:
> > >       0.00 :   ffffffff81bb7060:        pushq   %rbp              # data-type: (stack operation)
> > >       0.00 :   ffffffff81bb7061:        movq    %rsp, %rbp
> > >       0.00 :   ffffffff81bb7064:        pushq   %r15              # data-type: (stack operation)
> > >       0.00 :   ffffffff81bb7066:        pushq   %r14              # data-type: (stack operation)
> > >       0.00 :   ffffffff81bb7068:        pushq   %r13              # data-type: (stack operation)
> > >       0.00 :   ffffffff81bb706a:        pushq   %r12              # data-type: (stack operation)
> > >       0.00 :   ffffffff81bb706c:        pushq   %rbx              # data-type: (stack operation)
> >
> > data-type: looks a bit weird here. I wonder if the
> > .cfi_offset/.cfi_restore could  make this information richer, maybe:
> >        0.00 :   ffffffff81bb7060:        pushq   %rbp              # *(frame-16) = caller rbp
> >        0.00 :   ffffffff81bb7061:        movq    %rsp, %rbp
> >        0.00 :   ffffffff81bb7064:        pushq   %r15              # *(frame-24) = caller r15

Probably possible, but it's also obvious from the instruction.
I didn't try to be smart here as we're interested in types.

> >
> > >       0.00 :   ffffffff81bb706d:        movq    $0x33180, %rbx
> > >       0.00 :   ffffffff81bb7074:        movq    %rbx, %r12
> > >       0.00 :   ffffffff81bb7077:        subq    $0x38, %rsp
> > >       0.00 :   ffffffff81bb707b:        movl    %edi, -0x5c(%rbp)         # data-type: unsigned int +0
> > >       0.00 :   ffffffff81bb707e:        movq    %gs:0x28, %rax            # data-type: (stack canary)
> > >       0.00 :   ffffffff81bb7087:        movq    %rax, -0x30(%rbp)         # data-type: (stack canary)
> > >       0.00 :   ffffffff81bb708b:        xorl    %eax, %eax
> > >       0.00 :   ffffffff81bb708d:        movq    $0x0, -0x58(%rbp)         # data-type: struct rq_flags +0 (flags)
> > >       0.00 :   ffffffff81bb7095:        movq    $0x0, -0x50(%rbp)         # data-type: struct rq_flags +0x8 (clock_update_flags)
> > >       0.00 :   ffffffff81bb709d:        callq   0xffffffff81baa100 <debug_smp_processor_id>               # data-type: (stack operation)
> > >       0.00 :   ffffffff81bb70a2:        cltq
> > >       0.00 :   ffffffff81bb70a4:        addq    -0x7dcf0500(,%rax,8), %r12                # data-type: long unsigned int[] +0
> > >       0.00 :   ffffffff81bb70ac:        movq    0x960(%r12), %r13         # data-type: struct rq +0x960 (curr)
> > >       0.00 :   ffffffff81bb70b4:        movq    0x20(%r13), %rax          # data-type: struct task_struct +0x20 (stack)
> > >       0.00 :   ffffffff81bb70b8:        cmpq    $0x57ac6e9d, (%rax)
> > >       0.00 :   ffffffff81bb70bf:        jne     0xffffffff81bb7bca <__schedule+0xb6a>
> > >       0.00 :   ffffffff81bb70c5:        movl    %gs:0x7e47b87c(%rip), %eax  # 0x32948 <pcpu_hot+0x8>              # data-type: struct pcpu_hot +0x8 (preempt_count)
> >
> > The variable name in the struct is really nice, it is a shame the same
> > information gets repeated a few times on the line.

I guess you meant 'pcpu_hot'.  But the assembly shows the name of the
global variable and the data type shows the name of struct.  It happened
to be the same in this case.

> 
> Outside of the typos in the cover letter, the rest of the patches lgtm.
> 
> Reviewed-by: Ian Rogers <irogers@google.com>

Thanks for your review!
Namhyung

> 
> > >       0.00 :   ffffffff81bb70cc:        andl    $0x7fffffff, %eax
> > >       0.00 :   ffffffff81bb70d1:        cmpl    $0x1, %eax
> > >       0.00 :   ffffffff81bb70d4:        jne     0xffffffff81bb79de <__schedule+0x97e>
> > >       0.00 :   ffffffff81bb70da:        nopl    (%rax,%rax)
> > >       0.00 :   ffffffff81bb70df:        cmpl    $0x2, 0xe86b3a(%rip)  # 0xffffffff82a3dc20 <prof_on>              # data-type: int +0
> > >       0.00 :   ffffffff81bb70e6:        movq    0x8(%rbp), %rsi
> > >       0.00 :   ffffffff81bb70ea:        je      0xffffffff81bb7a0b <__schedule+0x9ab>
> > >       0.00 :   ffffffff81bb70f0:        nopl    (%rax,%rax)
> > >       0.00 :   ffffffff81bb70f5:        nop
> > >       ...
> > >
> > > The code is also available at 'perf/annotate-code-data-v1' branch in
> > > git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> > >
> > > Thanks,
> > > Namhyung
> > >
> > >
> > > Namhyung Kim (7):
> > >   perf annotate-data: Add annotated_data_type__get_member_name()
> > >   perf annotate: Remove unused len parameter from
> > >     annotation_line__print()
> > >   perf annotate: Pass annotation_options to annotation_line__print()
> > >   perf annotate: Pass hist_entry to annotate functions
> > >   perf annotate: Factor out __hist_entry__get_data_type()
> > >   perf annotate: Implement code + data type annotation
> > >   perf annotate: Add --code-with-type option.
> > >
> > >  tools/perf/Documentation/perf-annotate.txt |   4 +
> > >  tools/perf/builtin-annotate.c              |  19 +-
> > >  tools/perf/builtin-top.c                   |   2 +-
> > >  tools/perf/util/annotate-data.c            |  34 +++
> > >  tools/perf/util/annotate-data.h            |   3 +
> > >  tools/perf/util/annotate.c                 | 267 +++++++++++++--------
> > >  tools/perf/util/annotate.h                 |   8 +-
> > >  tools/perf/util/sort.c                     |  39 +--
> > >  8 files changed, 236 insertions(+), 140 deletions(-)
> > >
> > > --
> > > 2.48.1.711.g2feabab25a-goog
> > >