[PATCHSET 0/9] perf tools: Bug fix and cleanup for sort keys

[PATCHSET 0/9] perf tools: Bug fix and cleanup for sort keys

[Namhyung Kim]

Hi,

This is a couple of bugfix, cleanup and enhancement for sort keys.

When I rebased previous --sort addr* patches, I found that separating
sort mode might be helpful to users so I did the cleanup.  But during
the test I, in turn, found a bug in freeing the branch info of a hist
entry.  And the mem info patch is only compile-tested - it'd be great
if Stephane could take a look and help to test.

You can get it from 'perf/sort-v3' branch on my tree at:

  git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git


Any comments are welcome, thanks.
Namhyung


Namhyung Kim (9):
  perf hists: Fix an invalid memory free on he->branch_info
  perf hists: Free unused mem info of a matched hist entry
  perf report: Fix alignment of symbol column when -v is given
  perf sort: Introduce sort__mode variable
  perf sort: Separate out memory-specific sort keys
  perf sort: Add 'addr' sort key
  perf sort: Add 'addr_to/from' sort key
  perf sort: Update documentation for sort keys
  perf hists: Move column length setting code

 tools/perf/Documentation/perf-diff.txt   |   2 +-
 tools/perf/Documentation/perf-report.txt |   5 +-
 tools/perf/Documentation/perf-top.txt    |   2 +-
 tools/perf/builtin-diff.c                |   2 +-
 tools/perf/builtin-report.c              |  40 ++++++----
 tools/perf/builtin-top.c                 |   3 +-
 tools/perf/ui/browsers/hists.c           |   4 +-
 tools/perf/util/hist.c                   |  63 ++++++++++-----
 tools/perf/util/hist.h                   |   5 +-
 tools/perf/util/sort.c                   | 129 ++++++++++++++++++++++++++++---
 tools/perf/util/sort.h                   |  30 ++++---
 11 files changed, 223 insertions(+), 62 deletions(-)

-- 
1.7.11.7

[PATCH 1/9] perf hists: Fix an invalid memory free on he->branch_info

[Namhyung Kim]

From: Namhyung Kim <namhyung.kim@lge.com>

The branch info was allocated for the whole stack and passed matching
hist entry for each level during processing samples.  Thus when a hist
entry tries to free its branch info like in hists__collapse_insert_entry
it'll face following error.

  *** glibc detected *** perf: munmap_chunk(): invalid pointer: 0x00000000014e9d20 ***
  ======= Backtrace: =========
  /lib64/libc.so.6[0x387d47ae16]
  perf[0x4923bd]
  perf(cmd_report+0xd68)[0x432a08]
  perf[0x41a663]
  perf(main+0x58f)[0x419eaf]
  /lib64/libc.so.6(__libc_start_main+0xf5)[0x387d421735]
  perf[0x419f95]

Fix it by allocating and copying branch info for each new hist entry.

Cc: Stephane Eranian <eranian@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-report.c |  9 ++++++---
 tools/perf/util/hist.c      | 14 ++++++++++++++
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index bd0ca81eeaca..d9f2de3e81fe 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -187,6 +187,9 @@ static int perf_report__add_branch_hist_entry(struct perf_tool *tool,
 	for (i = 0; i < sample->branch_stack->nr; i++) {
 		if (rep->hide_unresolved && !(bi[i].from.sym && bi[i].to.sym))
 			continue;
+
+		err = -ENOMEM;
+
 		/*
 		 * The report shows the percentage of total branches captured
 		 * and not events sampled. Thus we use a pseudo period of 1.
@@ -195,7 +198,6 @@ static int perf_report__add_branch_hist_entry(struct perf_tool *tool,
 				&bi[i], 1, 1);
 		if (he) {
 			struct annotation *notes;
-			err = -ENOMEM;
 			bx = he->branch_info;
 			if (bx->from.sym && use_browser == 1 && sort__has_sym) {
 				notes = symbol__annotation(bx->from.sym);
@@ -226,11 +228,12 @@ static int perf_report__add_branch_hist_entry(struct perf_tool *tool,
 			}
 			evsel->hists.stats.total_period += 1;
 			hists__inc_nr_events(&evsel->hists, PERF_RECORD_SAMPLE);
-			err = 0;
 		} else
-			return -ENOMEM;
+			goto out;
 	}
+	err = 0;
 out:
+	free(bi);
 	return err;
 }
 
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 6b32721f829a..9438d576459d 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -292,6 +292,20 @@ static struct hist_entry *hist_entry__new(struct hist_entry *template)
 			he->ms.map->referenced = true;
 
 		if (he->branch_info) {
+			/*
+			 * This branch info is (a part of) allocated from
+			 * machine__resolve_bstack() and will be freed after
+			 * adding new entries.  So we need to save a copy.
+			 */
+			he->branch_info = malloc(sizeof(*he->branch_info));
+			if (he->branch_info == NULL) {
+				free(he);
+				return NULL;
+			}
+
+			memcpy(he->branch_info, template->branch_info,
+			       sizeof(*he->branch_info));
+
 			if (he->branch_info->from.map)
 				he->branch_info->from.map->referenced = true;
 			if (he->branch_info->to.map)
-- 
1.7.11.7

[PATCH 2/9] perf hists: Free unused mem info of a matched hist entry

[Namhyung Kim]

From: Namhyung Kim <namhyung.kim@lge.com>

The mem info is shared between matched entries so one should be freed.

Cc: Stephane Eranian <eranian@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/hist.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 9438d576459d..514fc0470e38 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -374,6 +374,12 @@ static struct hist_entry *add_hist_entry(struct hists *hists,
 		if (!cmp) {
 			he_stat__add_period(&he->stat, period, weight);
 
+			/*
+			 * This mem info was allocated from machine__resolve_mem
+			 * and will not be used anymore.
+			 */
+			free(entry->mem_info);
+
 			/* If the map of an existing hist_entry has
 			 * become out-of-date due to an exec() or
 			 * similar, update it.  Otherwise we will
-- 
1.7.11.7

[PATCH 3/9] perf report: Fix alignment of symbol column when -v is given

[Namhyung Kim]

From: Namhyung Kim <namhyung.kim@lge.com>

When -v option is given, the symbol sort key prints its address also
but it wasn't properly aligned since hists__calc_col_len() misses the
additional part.  Also it missed 2 spaces for 0x prefix when printing.

  $ perf report --stdio -v -s sym
  # Samples: 133  of event 'cycles'
  # Event count (approx.): 50536717
  #
  # Overhead                          Symbol
  # ........  ..............................
  #
      12.20%  0xffffffff81384c50 v [k] intel_idle
       7.62%  0xffffffff8170976a v [k] ftrace_caller
       7.02%  0x2d986d         B [.] 0x00000000002d986d

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/hist.c | 26 +++++++++++++++-----------
 tools/perf/util/sort.c |  2 +-
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 514fc0470e38..72b4eec820c3 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -70,9 +70,17 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
 	int symlen;
 	u16 len;
 
-	if (h->ms.sym)
-		hists__new_col_len(hists, HISTC_SYMBOL, h->ms.sym->namelen + 4);
-	else {
+	/*
+	 * +4 accounts for '[x] ' priv level info
+	 * +2 accounts for 0x prefix on raw addresses
+	 * +3 accounts for ' y ' symtab origin info
+	 */
+	if (h->ms.sym) {
+		symlen = h->ms.sym->namelen + 4;
+		if (verbose)
+			symlen += BITS_PER_LONG / 4 + 2 + 3;
+		hists__new_col_len(hists, HISTC_SYMBOL, symlen);
+	} else {
 		symlen = unresolved_col_width + 4 + 2;
 		hists__new_col_len(hists, HISTC_SYMBOL, symlen);
 		hists__set_unres_dso_col_len(hists, HISTC_DSO);
@@ -91,12 +99,10 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
 		hists__new_col_len(hists, HISTC_PARENT, h->parent->namelen);
 
 	if (h->branch_info) {
-		/*
-		 * +4 accounts for '[x] ' priv level info
-		 * +2 account of 0x prefix on raw addresses
-		 */
 		if (h->branch_info->from.sym) {
 			symlen = (int)h->branch_info->from.sym->namelen + 4;
+			if (verbose)
+				symlen += BITS_PER_LONG / 4 + 2 + 3;
 			hists__new_col_len(hists, HISTC_SYMBOL_FROM, symlen);
 
 			symlen = dso__name_len(h->branch_info->from.map->dso);
@@ -109,6 +115,8 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
 
 		if (h->branch_info->to.sym) {
 			symlen = (int)h->branch_info->to.sym->namelen + 4;
+			if (verbose)
+				symlen += BITS_PER_LONG / 4 + 2 + 3;
 			hists__new_col_len(hists, HISTC_SYMBOL_TO, symlen);
 
 			symlen = dso__name_len(h->branch_info->to.map->dso);
@@ -121,10 +129,6 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
 	}
 
 	if (h->mem_info) {
-		/*
-		 * +4 accounts for '[x] ' priv level info
-		 * +2 account of 0x prefix on raw addresses
-		 */
 		if (h->mem_info->daddr.sym) {
 			symlen = (int)h->mem_info->daddr.sym->namelen + 4
 			       + unresolved_col_width + 2;
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 5f52d492590c..16d5e38befe5 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -194,7 +194,7 @@ static int _hist_entry__sym_snprintf(struct map *map, struct symbol *sym,
 	if (verbose) {
 		char o = map ? dso__symtab_origin(map->dso) : '!';
 		ret += repsep_snprintf(bf, size, "%-#*llx %c ",
-				       BITS_PER_LONG / 4, ip, o);
+				       BITS_PER_LONG / 4 + 2, ip, o);
 	}
 
 	ret += repsep_snprintf(bf + ret, size - ret, "[%c] ", level);
-- 
1.7.11.7

[PATCH 4/9] perf sort: Introduce sort__mode variable

[Namhyung Kim]

From: Namhyung Kim <namhyung.kim@lge.com>

It's used for determining current sort mode which can be one of
NORMAL, BRANCH and new MEMORY.

Cc: Stephane Eranian <eranian@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-report.c    | 23 +++++++++++++----------
 tools/perf/ui/browsers/hists.c |  4 ++--
 tools/perf/util/sort.c         |  4 ++--
 tools/perf/util/sort.h         |  8 +++++++-
 4 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index d9f2de3e81fe..c877982a64d3 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -311,7 +311,7 @@ static int process_sample_event(struct perf_tool *tool,
 	if (rep->cpu_list && !test_bit(sample->cpu, rep->cpu_bitmap))
 		return 0;
 
-	if (sort__branch_mode == 1) {
+	if (sort__mode == SORT_MODE__BRANCH) {
 		if (perf_report__add_branch_hist_entry(tool, &al, sample,
 						       evsel, machine)) {
 			pr_debug("problem adding lbr entry, skipping event\n");
@@ -387,7 +387,7 @@ static int perf_report__setup_sample_type(struct perf_report *rep)
 			}
 	}
 
-	if (sort__branch_mode == 1) {
+	if (sort__mode == SORT_MODE__BRANCH) {
 		if (!self->fd_pipe &&
 		    !(sample_type & PERF_SAMPLE_BRANCH_STACK)) {
 			ui__error("Selected -b but no branch data. "
@@ -694,7 +694,9 @@ static int
 parse_branch_mode(const struct option *opt __maybe_unused,
 		  const char *str __maybe_unused, int unset)
 {
-	sort__branch_mode = !unset;
+	int *branch_mode = opt->value;
+
+	*branch_mode = !unset;
 	return 0;
 }
 
@@ -703,6 +705,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 	struct perf_session *session;
 	struct stat st;
 	bool has_br_stack = false;
+	int branch_mode = -1;
 	int ret = -1;
 	char callchain_default_opt[] = "fractal,0.5,callee";
 	const char * const report_usage[] = {
@@ -799,7 +802,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 		    "Show a column with the sum of periods"),
 	OPT_BOOLEAN(0, "group", &symbol_conf.event_group,
 		    "Show event group information together"),
-	OPT_CALLBACK_NOOPT('b', "branch-stack", &sort__branch_mode, "",
+	OPT_CALLBACK_NOOPT('b', "branch-stack", &branch_mode, "",
 		    "use branch records for histogram filling", parse_branch_mode),
 	OPT_STRING(0, "objdump", &objdump_path, "path",
 		   "objdump binary to use for disassembly and annotations"),
@@ -849,11 +852,11 @@ repeat:
 	has_br_stack = perf_header__has_feat(&session->header,
 					     HEADER_BRANCH_STACK);
 
-	if (sort__branch_mode == -1 && has_br_stack)
-		sort__branch_mode = 1;
+	if (branch_mode == -1 && has_br_stack)
+		sort__mode = SORT_MODE__BRANCH;
 
-	/* sort__branch_mode could be 0 if --no-branch-stack */
-	if (sort__branch_mode == 1) {
+	/* sort__mode could be NORMAL if --no-branch-stack */
+	if (sort__mode == SORT_MODE__BRANCH) {
 		/*
 		 * if no sort_order is provided, then specify
 		 * branch-mode specific order
@@ -864,7 +867,7 @@ repeat:
 
 	}
 	if (report.mem_mode) {
-		if (sort__branch_mode == 1) {
+		if (sort__mode == SORT_MODE__BRANCH) {
 			fprintf(stderr, "branch and mem mode incompatible\n");
 			goto error;
 		}
@@ -934,7 +937,7 @@ repeat:
 
 	sort_entry__setup_elide(&sort_comm, symbol_conf.comm_list, "comm", stdout);
 
-	if (sort__branch_mode == 1) {
+	if (sort__mode == SORT_MODE__BRANCH) {
 		sort_entry__setup_elide(&sort_dso_from, symbol_conf.dso_from_list, "dso_from", stdout);
 		sort_entry__setup_elide(&sort_dso_to, symbol_conf.dso_to_list, "dso_to", stdout);
 		sort_entry__setup_elide(&sort_sym_from, symbol_conf.sym_from_list, "sym_from", stdout);
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 0cada654d387..cea7ef969cbc 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1155,7 +1155,7 @@ static struct hist_browser *hist_browser__new(struct hists *hists)
 		browser->b.refresh = hist_browser__refresh;
 		browser->b.seek = ui_browser__hists_seek;
 		browser->b.use_navkeypressed = true;
-		if (sort__branch_mode == 1)
+		if (sort__mode == SORT_MODE__BRANCH)
 			browser->has_symbols = sort_sym_from.list.next != NULL;
 		else
 			browser->has_symbols = sort_sym.list.next != NULL;
@@ -1487,7 +1487,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 		if (!browser->has_symbols)
 			goto add_exit_option;
 
-		if (sort__branch_mode == 1) {
+		if (sort__mode == SORT_MODE__BRANCH) {
 			bi = browser->he_selection->branch_info;
 			if (browser->selection != NULL &&
 			    bi &&
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 16d5e38befe5..a6ddad41d57a 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -9,7 +9,7 @@ const char	*sort_order = default_sort_order;
 int		sort__need_collapse = 0;
 int		sort__has_parent = 0;
 int		sort__has_sym = 0;
-int		sort__branch_mode = -1; /* -1 = means not set */
+enum sort_mode	sort__mode = SORT_MODE__NORMAL;
 
 enum sort_type	sort__first_dimension;
 
@@ -943,7 +943,7 @@ int sort_dimension__add(const char *tok)
 		if (strncasecmp(tok, sd->name, strlen(tok)))
 			continue;
 
-		if (sort__branch_mode != 1)
+		if (sort__mode != SORT_MODE__BRANCH)
 			return -EINVAL;
 
 		if (sd->entry == &sort_sym_from || sd->entry == &sort_sym_to)
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index f24bdf64238c..39ff4b86ae84 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -32,7 +32,7 @@ extern const char default_sort_order[];
 extern int sort__need_collapse;
 extern int sort__has_parent;
 extern int sort__has_sym;
-extern int sort__branch_mode;
+extern enum sort_mode sort__mode;
 extern struct sort_entry sort_comm;
 extern struct sort_entry sort_dso;
 extern struct sort_entry sort_sym;
@@ -123,6 +123,12 @@ static inline void hist_entry__add_pair(struct hist_entry *he,
 	list_add_tail(&he->pairs.head, &pair->pairs.node);
 }
 
+enum sort_mode {
+	SORT_MODE__NORMAL,
+	SORT_MODE__BRANCH,
+	SORT_MODE__MEMORY,
+};
+
 enum sort_type {
 	/* common sort keys */
 	SORT_PID,
-- 
1.7.11.7

[PATCH 5/9] perf sort: Separate out memory-specific sort keys

[Namhyung Kim]

From: Namhyung Kim <namhyung.kim@lge.com>

Since they're used only for perf mem, separate out them to a different
dimension so that normal user cannot access them by any chance.

For global/local weights, I'm not entirely sure to place them into the
memory dimension.  But it's the only user at this time.

Cc: Andi Kleen <andi@firstfloor.org>
Cc: Stephane Eranian <eranian@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-report.c |  2 ++
 tools/perf/util/sort.c      | 50 +++++++++++++++++++++++++++++++++++++--------
 tools/perf/util/sort.h      | 19 +++++++++--------
 3 files changed, 55 insertions(+), 16 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index c877982a64d3..669405c9b8a2 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -871,6 +871,8 @@ repeat:
 			fprintf(stderr, "branch and mem mode incompatible\n");
 			goto error;
 		}
+		sort__mode = SORT_MODE__MEMORY;
+
 		/*
 		 * if no sort_order is provided, then specify
 		 * branch-mode specific order
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index a6ddad41d57a..c19bf213cc86 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -871,14 +871,6 @@ static struct sort_dimension common_sort_dimensions[] = {
 	DIM(SORT_PARENT, "parent", sort_parent),
 	DIM(SORT_CPU, "cpu", sort_cpu),
 	DIM(SORT_SRCLINE, "srcline", sort_srcline),
-	DIM(SORT_LOCAL_WEIGHT, "local_weight", sort_local_weight),
-	DIM(SORT_GLOBAL_WEIGHT, "weight", sort_global_weight),
-	DIM(SORT_MEM_DADDR_SYMBOL, "symbol_daddr", sort_mem_daddr_sym),
-	DIM(SORT_MEM_DADDR_DSO, "dso_daddr", sort_mem_daddr_dso),
-	DIM(SORT_MEM_LOCKED, "locked", sort_mem_locked),
-	DIM(SORT_MEM_TLB, "tlb", sort_mem_tlb),
-	DIM(SORT_MEM_LVL, "mem", sort_mem_lvl),
-	DIM(SORT_MEM_SNOOP, "snoop", sort_mem_snoop),
 };
 
 #undef DIM
@@ -895,6 +887,21 @@ static struct sort_dimension bstack_sort_dimensions[] = {
 
 #undef DIM
 
+#define DIM(d, n, func) [d - __SORT_MEMORY_MODE] = { .name = n, .entry = &(func) }
+
+static struct sort_dimension memory_sort_dimensions[] = {
+	DIM(SORT_LOCAL_WEIGHT, "local_weight", sort_local_weight),
+	DIM(SORT_GLOBAL_WEIGHT, "weight", sort_global_weight),
+	DIM(SORT_MEM_DADDR_SYMBOL, "symbol_daddr", sort_mem_daddr_sym),
+	DIM(SORT_MEM_DADDR_DSO, "dso_daddr", sort_mem_daddr_dso),
+	DIM(SORT_MEM_LOCKED, "locked", sort_mem_locked),
+	DIM(SORT_MEM_TLB, "tlb", sort_mem_tlb),
+	DIM(SORT_MEM_LVL, "mem", sort_mem_lvl),
+	DIM(SORT_MEM_SNOOP, "snoop", sort_mem_snoop),
+};
+
+#undef DIM
+
 int sort_dimension__add(const char *tok)
 {
 	unsigned int i;
@@ -964,6 +971,33 @@ int sort_dimension__add(const char *tok)
 		return 0;
 	}
 
+	for (i = 0; i < ARRAY_SIZE(memory_sort_dimensions); i++) {
+		struct sort_dimension *sd = &memory_sort_dimensions[i];
+
+		if (strncasecmp(tok, sd->name, strlen(tok)))
+			continue;
+
+		if (sort__mode != SORT_MODE__MEMORY)
+			return -EINVAL;
+
+		if (sd->entry == &sort_mem_daddr_sym)
+			sort__has_sym = 1;
+
+		if (sd->taken)
+			return 0;
+
+		if (sd->entry->se_collapse)
+			sort__need_collapse = 1;
+
+		if (list_empty(&hist_entry__sort_list))
+			sort__first_dimension = i + __SORT_MEMORY_MODE;
+
+		list_add_tail(&sd->entry->list, &hist_entry__sort_list);
+		sd->taken = 1;
+
+		return 0;
+	}
+
 	return -ESRCH;
 }
 
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 39ff4b86ae84..0232d476da87 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -138,14 +138,6 @@ enum sort_type {
 	SORT_PARENT,
 	SORT_CPU,
 	SORT_SRCLINE,
-	SORT_LOCAL_WEIGHT,
-	SORT_GLOBAL_WEIGHT,
-	SORT_MEM_DADDR_SYMBOL,
-	SORT_MEM_DADDR_DSO,
-	SORT_MEM_LOCKED,
-	SORT_MEM_TLB,
-	SORT_MEM_LVL,
-	SORT_MEM_SNOOP,
 
 	/* branch stack specific sort keys */
 	__SORT_BRANCH_STACK,
@@ -154,6 +146,17 @@ enum sort_type {
 	SORT_SYM_FROM,
 	SORT_SYM_TO,
 	SORT_MISPREDICT,
+
+	/* memory mode specific sort keys */
+	__SORT_MEMORY_MODE,
+	SORT_LOCAL_WEIGHT = __SORT_MEMORY_MODE,
+	SORT_GLOBAL_WEIGHT,
+	SORT_MEM_DADDR_SYMBOL,
+	SORT_MEM_DADDR_DSO,
+	SORT_MEM_LOCKED,
+	SORT_MEM_TLB,
+	SORT_MEM_LVL,
+	SORT_MEM_SNOOP,
 };
 
 /*
-- 
1.7.11.7

[PATCH 6/9] perf sort: Add 'addr' sort key

[Namhyung Kim]

From: Namhyung Kim <namhyung.kim@lge.com>

New addr sort key provides a way to sort the entries by the symbol
addresses.  It can be helpful to figure out symbol resolution problem
when a dso cannot do it properly as well as finding hotpath in a dso
and/or a function.

Suggested-by: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=55561
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/hist.c |  2 ++
 tools/perf/util/hist.h |  3 ++-
 tools/perf/util/sort.c | 23 +++++++++++++++++++++++
 tools/perf/util/sort.h |  1 +
 4 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 72b4eec820c3..c098d6ebab1f 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -52,6 +52,8 @@ void hists__reset_col_len(struct hists *hists)
 
 	for (col = 0; col < HISTC_NR_COLS; ++col)
 		hists__set_col_len(hists, col, 0);
+
+	hists__set_col_len(hists, HISTC_ADDR, BITS_PER_LONG / 4 + 2);
 }
 
 static void hists__set_unres_dso_col_len(struct hists *hists, int dso)
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 14c2fe20aa62..9599f805828f 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -43,12 +43,13 @@ enum hist_column {
 	HISTC_COMM,
 	HISTC_PARENT,
 	HISTC_CPU,
+	HISTC_SRCLINE,
+	HISTC_ADDR,
 	HISTC_MISPREDICT,
 	HISTC_SYMBOL_FROM,
 	HISTC_SYMBOL_TO,
 	HISTC_DSO_FROM,
 	HISTC_DSO_TO,
-	HISTC_SRCLINE,
 	HISTC_LOCAL_WEIGHT,
 	HISTC_GLOBAL_WEIGHT,
 	HISTC_MEM_DADDR_SYMBOL,
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index c19bf213cc86..fecde9347cbd 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -342,6 +342,28 @@ struct sort_entry sort_cpu = {
 	.se_width_idx	= HISTC_CPU,
 };
 
+/* --sort addr */
+
+static int64_t
+sort__addr_cmp(struct hist_entry *left, struct hist_entry *right)
+{
+	return right->ip - left->ip;
+}
+
+static int hist_entry__addr_snprintf(struct hist_entry *self, char *bf,
+				     size_t size, unsigned int width)
+{
+	return repsep_snprintf(bf, size, "%#*"PRIx64, width, (uint64_t)self->ip);
+}
+
+struct sort_entry sort_addr = {
+	.se_header      = "Address",
+	.se_cmp	        = sort__addr_cmp,
+	.se_snprintf    = hist_entry__addr_snprintf,
+	.se_width_idx	= HISTC_ADDR,
+};
+
+
 /* sort keys for branch stacks */
 
 static int64_t
@@ -871,6 +893,7 @@ static struct sort_dimension common_sort_dimensions[] = {
 	DIM(SORT_PARENT, "parent", sort_parent),
 	DIM(SORT_CPU, "cpu", sort_cpu),
 	DIM(SORT_SRCLINE, "srcline", sort_srcline),
+	DIM(SORT_ADDR, "addr", sort_addr),
 };
 
 #undef DIM
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 0232d476da87..0815e344f38c 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -138,6 +138,7 @@ enum sort_type {
 	SORT_PARENT,
 	SORT_CPU,
 	SORT_SRCLINE,
+	SORT_ADDR,
 
 	/* branch stack specific sort keys */
 	__SORT_BRANCH_STACK,
-- 
1.7.11.7

[PATCH 7/9] perf sort: Add 'addr_to/from' sort key

[Namhyung Kim]

From: Namhyung Kim <namhyung.kim@lge.com>

New addr_{to,from} sort keys provide a way to sort the entries by the
source/target symbol addresses.

Cc: Stephane Eranian <eranian@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/hist.c |  2 ++
 tools/perf/util/hist.h |  2 ++
 tools/perf/util/sort.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/sort.h |  2 ++
 4 files changed, 56 insertions(+)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index c098d6ebab1f..1fb1535940f8 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -54,6 +54,8 @@ void hists__reset_col_len(struct hists *hists)
 		hists__set_col_len(hists, col, 0);
 
 	hists__set_col_len(hists, HISTC_ADDR, BITS_PER_LONG / 4 + 2);
+	hists__set_col_len(hists, HISTC_ADDR_FROM, BITS_PER_LONG / 4 + 2);
+	hists__set_col_len(hists, HISTC_ADDR_TO, BITS_PER_LONG / 4 + 2);
 }
 
 static void hists__set_unres_dso_col_len(struct hists *hists, int dso)
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 9599f805828f..2640fcc566e9 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -50,6 +50,8 @@ enum hist_column {
 	HISTC_SYMBOL_TO,
 	HISTC_DSO_FROM,
 	HISTC_DSO_TO,
+	HISTC_ADDR_FROM,
+	HISTC_ADDR_TO,
 	HISTC_LOCAL_WEIGHT,
 	HISTC_GLOBAL_WEIGHT,
 	HISTC_MEM_DADDR_SYMBOL,
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index fecde9347cbd..19f360b6e698 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -436,6 +436,40 @@ static int hist_entry__sym_to_snprintf(struct hist_entry *self, char *bf,
 
 }
 
+static int64_t
+sort__addr_from_cmp(struct hist_entry *left, struct hist_entry *right)
+{
+	struct addr_map_symbol *from_l = &left->branch_info->from;
+	struct addr_map_symbol *from_r = &right->branch_info->from;
+
+	return from_r->addr - from_l->addr;
+}
+
+static int hist_entry__addr_from_snprintf(struct hist_entry *self, char *bf,
+					  size_t size, unsigned int width)
+{
+	struct addr_map_symbol *from = &self->branch_info->from;
+	return repsep_snprintf(bf, size, "%#*"PRIx64, width,
+					(uint64_t)from->addr);
+}
+
+static int64_t
+sort__addr_to_cmp(struct hist_entry *left, struct hist_entry *right)
+{
+	struct addr_map_symbol *to_l = &left->branch_info->to;
+	struct addr_map_symbol *to_r = &right->branch_info->to;
+
+	return to_r->addr - to_l->addr;
+}
+
+static int hist_entry__addr_to_snprintf(struct hist_entry *self, char *bf,
+					size_t size, unsigned int width)
+{
+	struct addr_map_symbol *to = &self->branch_info->to;
+	return repsep_snprintf(bf, size, "%#*"PRIx64, width,
+					(uint64_t)to->addr);
+}
+
 struct sort_entry sort_dso_from = {
 	.se_header	= "Source Shared Object",
 	.se_cmp		= sort__dso_from_cmp,
@@ -464,6 +498,20 @@ struct sort_entry sort_sym_to = {
 	.se_width_idx	= HISTC_SYMBOL_TO,
 };
 
+struct sort_entry sort_addr_from = {
+	.se_header	= "Source Address",
+	.se_cmp		= sort__addr_from_cmp,
+	.se_snprintf	= hist_entry__addr_from_snprintf,
+	.se_width_idx	= HISTC_ADDR_FROM,
+};
+
+struct sort_entry sort_addr_to = {
+	.se_header	= "Target Address",
+	.se_cmp		= sort__addr_to_cmp,
+	.se_snprintf	= hist_entry__addr_to_snprintf,
+	.se_width_idx	= HISTC_ADDR_TO,
+};
+
 static int64_t
 sort__mispredict_cmp(struct hist_entry *left, struct hist_entry *right)
 {
@@ -905,6 +953,8 @@ static struct sort_dimension bstack_sort_dimensions[] = {
 	DIM(SORT_DSO_TO, "dso_to", sort_dso_to),
 	DIM(SORT_SYM_FROM, "symbol_from", sort_sym_from),
 	DIM(SORT_SYM_TO, "symbol_to", sort_sym_to),
+	DIM(SORT_ADDR_FROM, "addr_from", sort_addr_from),
+	DIM(SORT_ADDR_TO, "addr_to", sort_addr_to),
 	DIM(SORT_MISPREDICT, "mispredict", sort_mispredict),
 };
 
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 0815e344f38c..1f945a3b2e5b 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -146,6 +146,8 @@ enum sort_type {
 	SORT_DSO_TO,
 	SORT_SYM_FROM,
 	SORT_SYM_TO,
+	SORT_ADDR_FROM,
+	SORT_ADDR_TO,
 	SORT_MISPREDICT,
 
 	/* memory mode specific sort keys */
-- 
1.7.11.7

[PATCH 8/9] perf sort: Update documentation for sort keys

[Namhyung Kim]

From: Namhyung Kim <namhyung.kim@lge.com>

Update and add missing description of new sort keys.

Cc: Stephane Eranian <eranian@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/Documentation/perf-diff.txt   | 2 +-
 tools/perf/Documentation/perf-report.txt | 5 ++++-
 tools/perf/Documentation/perf-top.txt    | 2 +-
 tools/perf/builtin-diff.c                | 2 +-
 tools/perf/builtin-report.c              | 6 +++---
 tools/perf/builtin-top.c                 | 3 ++-
 6 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/tools/perf/Documentation/perf-diff.txt b/tools/perf/Documentation/perf-diff.txt
index 5b3123d5721f..f74ec064db0e 100644
--- a/tools/perf/Documentation/perf-diff.txt
+++ b/tools/perf/Documentation/perf-diff.txt
@@ -47,7 +47,7 @@ OPTIONS
 
 -s::
 --sort=::
-	Sort by key(s): pid, comm, dso, symbol.
+	Sort by key(s): pid, comm, dso, symbol, parent, cpu, srcline, addr.
 
 -t::
 --field-separator=::
diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
index 7d5f4f38aa52..54acc51995fc 100644
--- a/tools/perf/Documentation/perf-report.txt
+++ b/tools/perf/Documentation/perf-report.txt
@@ -59,7 +59,7 @@ OPTIONS
 --sort=::
 	Sort histogram entries by given key(s) - multiple keys can be specified
 	in CSV format.  Following sort keys are available:
-	pid, comm, dso, symbol, parent, cpu, srcline, weight, local_weight.
+	pid, comm, dso, symbol, parent, cpu, srcline, addr.
 
 	Each key has following meaning:
 
@@ -72,6 +72,7 @@ OPTIONS
 	- cpu: cpu number the task ran at the time of sample
 	- srcline: filename and line number executed at the time of sample.  The
 	DWARF debuggin info must be provided.
+	- addr: address of function executed at the time of sample
 
 	By default, comm, dso and symbol keys are used.
 	(i.e. --sort comm,dso,symbol)
@@ -84,6 +85,8 @@ OPTIONS
 	- dso_to: name of library or module branched to
 	- symbol_from: name of function branched from
 	- symbol_to: name of function branched to
+	- addr_from: address of function branched from
+	- addr_to: address of function branched to
 	- mispredict: "N" for predicted branch, "Y" for mispredicted branch
 
 	And default sort keys are changed to comm, dso_from, symbol_from, dso_to
diff --git a/tools/perf/Documentation/perf-top.txt b/tools/perf/Documentation/perf-top.txt
index 9f1a2fe54757..2b45e799c4be 100644
--- a/tools/perf/Documentation/perf-top.txt
+++ b/tools/perf/Documentation/perf-top.txt
@@ -112,7 +112,7 @@ Default is to monitor all CPUS.
 
 -s::
 --sort::
-	Sort by key(s): pid, comm, dso, symbol, parent, srcline, weight, local_weight.
+	Sort by key(s): pid, comm, dso, symbol, parent, srcline, cpu, addr.
 
 -n::
 --show-nr-samples::
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 2d0462d89a97..03b56c542bb6 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -542,7 +542,7 @@ static const struct option options[] = {
 	OPT_STRING('S', "symbols", &symbol_conf.sym_list_str, "symbol[,symbol...]",
 		   "only consider these symbols"),
 	OPT_STRING('s', "sort", &sort_order, "key[,key2...]",
-		   "sort by key(s): pid, comm, dso, symbol, parent"),
+		   "sort by key(s): pid, comm, dso, symbol, parent, cpu, srcline, addr"),
 	OPT_STRING('t', "field-separator", &symbol_conf.field_sep, "separator",
 		   "separator for columns, no spaces will be added between "
 		   "columns '.' is reserved."),
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 669405c9b8a2..c95fd92fbd44 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -756,9 +756,9 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 		    "Use the stdio interface"),
 	OPT_STRING('s', "sort", &sort_order, "key[,key2...]",
 		   "sort by key(s): pid, comm, dso, symbol, parent, cpu, srcline,"
-		   " dso_to, dso_from, symbol_to, symbol_from, mispredict,"
-		   " weight, local_weight, mem, symbol_daddr, dso_daddr, tlb, "
-		   "snoop, locked"),
+		   " addr, dso_to, dso_from, symbol_to, symbol_from, addr_to,"
+		   " addr_from, mispredict, weight, local_weight, mem,"
+		   " symbol_daddr, dso_daddr, tlb, snoop, locked"),
 	OPT_BOOLEAN(0, "showcpuutilization", &symbol_conf.show_cpu_utilization,
 		    "Show sample percentage for different cpu modes"),
 	OPT_STRING('p', "parent", &parent_pattern, "regex",
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 67bdb9f14ad6..9aae3f518f39 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1089,7 +1089,8 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
 	OPT_INCR('v', "verbose", &verbose,
 		    "be more verbose (show counter open errors, etc)"),
 	OPT_STRING('s', "sort", &sort_order, "key[,key2...]",
-		   "sort by key(s): pid, comm, dso, symbol, parent, weight, local_weight"),
+		   "sort by key(s): pid, comm, dso, symbol, parent, cpu,"
+		   " srcline, addr"),
 	OPT_BOOLEAN('n', "show-nr-samples", &symbol_conf.show_nr_samples,
 		    "Show a column with the number of samples"),
 	OPT_CALLBACK_DEFAULT('G', "call-graph", &top.record_opts,
-- 
1.7.11.7

[PATCH 9/9] perf hists: Move column length setting code

[Namhyung Kim]

From: Namhyung Kim <namhyung.kim@lge.com>

They are set to constant length so no need to update every time.

Cc: Stephane Eranian <eranian@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/hist.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 1fb1535940f8..e144aefc76e6 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -56,6 +56,12 @@ void hists__reset_col_len(struct hists *hists)
 	hists__set_col_len(hists, HISTC_ADDR, BITS_PER_LONG / 4 + 2);
 	hists__set_col_len(hists, HISTC_ADDR_FROM, BITS_PER_LONG / 4 + 2);
 	hists__set_col_len(hists, HISTC_ADDR_TO, BITS_PER_LONG / 4 + 2);
+	hists__set_col_len(hists, HISTC_LOCAL_WEIGHT, 12);
+	hists__set_col_len(hists, HISTC_GLOBAL_WEIGHT, 12);
+	hists__set_col_len(hists, HISTC_MEM_LOCKED, 6);
+	hists__set_col_len(hists, HISTC_MEM_TLB, 22);
+	hists__set_col_len(hists, HISTC_MEM_SNOOP, 12);
+	hists__set_col_len(hists, HISTC_MEM_LVL, 21 + 3);
 }
 
 static void hists__set_unres_dso_col_len(struct hists *hists, int dso)
@@ -156,13 +162,6 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
 		hists__new_col_len(hists, HISTC_MEM_DADDR_SYMBOL, symlen);
 		hists__set_unres_dso_col_len(hists, HISTC_MEM_DADDR_DSO);
 	}
-
-	hists__new_col_len(hists, HISTC_MEM_LOCKED, 6);
-	hists__new_col_len(hists, HISTC_MEM_TLB, 22);
-	hists__new_col_len(hists, HISTC_MEM_SNOOP, 12);
-	hists__new_col_len(hists, HISTC_MEM_LVL, 21 + 3);
-	hists__new_col_len(hists, HISTC_LOCAL_WEIGHT, 12);
-	hists__new_col_len(hists, HISTC_GLOBAL_WEIGHT, 12);
 }
 
 void hists__output_recalc_col_len(struct hists *hists, int max_rows)
-- 
1.7.11.7

Re: [PATCH 5/9] perf sort: Separate out memory-specific sort keys

[Jiri Olsa]

On Mon, Apr 01, 2013 at 08:35:21PM +0900, Namhyung Kim wrote:

SNIP

> +
>  int sort_dimension__add(const char *tok)
>  {
>  	unsigned int i;
> @@ -964,6 +971,33 @@ int sort_dimension__add(const char *tok)
>  		return 0;
>  	}
>  
> +	for (i = 0; i < ARRAY_SIZE(memory_sort_dimensions); i++) {
> +		struct sort_dimension *sd = &memory_sort_dimensions[i];
> +
> +		if (strncasecmp(tok, sd->name, strlen(tok)))
> +			continue;
> +
> +		if (sort__mode != SORT_MODE__MEMORY)
> +			return -EINVAL;
> +
> +		if (sd->entry == &sort_mem_daddr_sym)
> +			sort__has_sym = 1;
> +
> +		if (sd->taken)
> +			return 0;
> +
> +		if (sd->entry->se_collapse)
> +			sort__need_collapse = 1;
> +
> +		if (list_empty(&hist_entry__sort_list))
> +			sort__first_dimension = i + __SORT_MEMORY_MODE;
> +
> +		list_add_tail(&sd->entry->list, &hist_entry__sort_list);
> +		sd->taken = 1;
> +
> +		return 0;

nice idea, I wonder some of the above code be put into function
and used also in previous loop, no big deal though

jirka

Re: [PATCH 6/9] perf sort: Add 'addr' sort key

[Jiri Olsa]

On Mon, Apr 01, 2013 at 08:35:22PM +0900, Namhyung Kim wrote:
> From: Namhyung Kim <namhyung.kim@lge.com>
> 
> New addr sort key provides a way to sort the entries by the symbol
> addresses.  It can be helpful to figure out symbol resolution problem
> when a dso cannot do it properly as well as finding hotpath in a dso
> and/or a function.

maybe it's just the recent mem profiling patches, but wouldn't
it be better to use 'ip' instead of 'addr'?

also it's following code getting the data:
...
        if (sample_type & PERF_SAMPLE_IP)
                data->ip = perf_instruction_pointer(regs);
...

same for the "perf sort: Add 'addr_to/from' sort key" patch

jirka

Re: [PATCH 5/9] perf sort: Separate out memory-specific sort keys

[Namhyung Kim]

On Mon, 1 Apr 2013 22:29:37 +0200, Jiri Olsa wrote:
> On Mon, Apr 01, 2013 at 08:35:21PM +0900, Namhyung Kim wrote:
>
> SNIP
>
>> +
>>  int sort_dimension__add(const char *tok)
>>  {
>>  	unsigned int i;
>> @@ -964,6 +971,33 @@ int sort_dimension__add(const char *tok)
>>  		return 0;
>>  	}
>>  
>> +	for (i = 0; i < ARRAY_SIZE(memory_sort_dimensions); i++) {
>> +		struct sort_dimension *sd = &memory_sort_dimensions[i];
>> +
>> +		if (strncasecmp(tok, sd->name, strlen(tok)))
>> +			continue;
>> +
>> +		if (sort__mode != SORT_MODE__MEMORY)
>> +			return -EINVAL;
>> +
>> +		if (sd->entry == &sort_mem_daddr_sym)
>> +			sort__has_sym = 1;
>> +
>> +		if (sd->taken)
>> +			return 0;
>> +
>> +		if (sd->entry->se_collapse)
>> +			sort__need_collapse = 1;
>> +
>> +		if (list_empty(&hist_entry__sort_list))
>> +			sort__first_dimension = i + __SORT_MEMORY_MODE;
>> +
>> +		list_add_tail(&sd->entry->list, &hist_entry__sort_list);
>> +		sd->taken = 1;
>> +
>> +		return 0;
>
> nice idea, I wonder some of the above code be put into function
> and used also in previous loop, no big deal though

Hmm.. okay.  I'll try. :)

Thanks,
Namhyung

Re: [PATCH 6/9] perf sort: Add 'addr' sort key

[Namhyung Kim]

On Mon, 1 Apr 2013 22:40:23 +0200, Jiri Olsa wrote:
> On Mon, Apr 01, 2013 at 08:35:22PM +0900, Namhyung Kim wrote:
>> From: Namhyung Kim <namhyung.kim@lge.com>
>> 
>> New addr sort key provides a way to sort the entries by the symbol
>> addresses.  It can be helpful to figure out symbol resolution problem
>> when a dso cannot do it properly as well as finding hotpath in a dso
>> and/or a function.
>
> maybe it's just the recent mem profiling patches, but wouldn't
> it be better to use 'ip' instead of 'addr'?
>
> also it's following code getting the data:
> ...
>         if (sample_type & PERF_SAMPLE_IP)
>                 data->ip = perf_instruction_pointer(regs);
> ...
>
> same for the "perf sort: Add 'addr_to/from' sort key" patch

I'm not sure I understand what you mean exactly.

I used hist_entry->ip but it was set by al->addr which was converted
from the original sample ip to a relative ip by map->map_ip().

I can change it to use ->unmap_ip() before printing.  Is that your
concern?

And it seems that addr in branch info is original ip, Stephane?

Thanks,
Namhyung

Re: [PATCH 6/9] perf sort: Add 'addr' sort key

[Jiri Olsa]

On Tue, Apr 02, 2013 at 11:15:23AM +0900, Namhyung Kim wrote:
> On Mon, 1 Apr 2013 22:40:23 +0200, Jiri Olsa wrote:
> > On Mon, Apr 01, 2013 at 08:35:22PM +0900, Namhyung Kim wrote:
> >> From: Namhyung Kim <namhyung.kim@lge.com>
> >> 
> >> New addr sort key provides a way to sort the entries by the symbol
> >> addresses.  It can be helpful to figure out symbol resolution problem
> >> when a dso cannot do it properly as well as finding hotpath in a dso
> >> and/or a function.
> >
> > maybe it's just the recent mem profiling patches, but wouldn't
> > it be better to use 'ip' instead of 'addr'?
> >
> > also it's following code getting the data:
> > ...
> >         if (sample_type & PERF_SAMPLE_IP)
> >                 data->ip = perf_instruction_pointer(regs);
> > ...
> >
> > same for the "perf sort: Add 'addr_to/from' sort key" patch
> 
> I'm not sure I understand what you mean exactly.
> 
> I used hist_entry->ip but it was set by al->addr which was converted
> from the original sample ip to a relative ip by map->map_ip().
> 
> I can change it to use ->unmap_ip() before printing.  Is that your
> concern?

what I meant was the 'addr' name itself for -s option, like use:
  '-s ip' instead of '-s addr'

I'm fine with the implementation

jirka

> 
> And it seems that addr in branch info is original ip, Stephane?
> 
> Thanks,
> Namhyung

Re: [PATCHSET 0/9] perf tools: Bug fix and cleanup for sort keys

[Arnaldo Carvalho de Melo]

Em Mon, Apr 01, 2013 at 08:35:16PM +0900, Namhyung Kim escreveu:
> This is a couple of bugfix, cleanup and enhancement for sort keys.

Applied 1-4.

- Arnaldo

Re: [PATCH 6/9] perf sort: Add 'addr' sort key

[Namhyung Kim]

Hi Jiri,

On Tue, 2 Apr 2013 10:40:14 +0200, Jiri Olsa wrote:
> On Tue, Apr 02, 2013 at 11:15:23AM +0900, Namhyung Kim wrote:
>> On Mon, 1 Apr 2013 22:40:23 +0200, Jiri Olsa wrote:
>> > On Mon, Apr 01, 2013 at 08:35:22PM +0900, Namhyung Kim wrote:
>> >> From: Namhyung Kim <namhyung.kim@lge.com>
>> >> 
>> >> New addr sort key provides a way to sort the entries by the symbol
>> >> addresses.  It can be helpful to figure out symbol resolution problem
>> >> when a dso cannot do it properly as well as finding hotpath in a dso
>> >> and/or a function.
>> >
>> > maybe it's just the recent mem profiling patches, but wouldn't
>> > it be better to use 'ip' instead of 'addr'?
>> >
>> > also it's following code getting the data:
>> > ...
>> >         if (sample_type & PERF_SAMPLE_IP)
>> >                 data->ip = perf_instruction_pointer(regs);
>> > ...
>> >
>> > same for the "perf sort: Add 'addr_to/from' sort key" patch
>> 
>> I'm not sure I understand what you mean exactly.
>> 
>> I used hist_entry->ip but it was set by al->addr which was converted
>> from the original sample ip to a relative ip by map->map_ip().
>> 
>> I can change it to use ->unmap_ip() before printing.  Is that your
>> concern?
>
> what I meant was the 'addr' name itself for -s option, like use:
>   '-s ip' instead of '-s addr'

Well, I don't know what's better.  But the 'addr' looks more natural to
me and it's requested originally.  And the 'ip' can have multiple
meaning. :)

Thanks,
Namhyung

[tip:perf/core] perf hists: Fix an invalid memory free on he-> branch_info

[tip-bot for Namhyung Kim]

Commit-ID:  26353a61b977e57b58dd3555bc0422fea46c5ad6
Gitweb:     http://git.kernel.org/tip/26353a61b977e57b58dd3555bc0422fea46c5ad6
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Mon, 1 Apr 2013 20:35:17 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 28 May 2013 16:23:52 +0300

perf hists: Fix an invalid memory free on he->branch_info

The branch info was allocated for the whole stack and passed matching
hist entry for each level during processing samples.  Thus when a hist
entry tries to free its branch info like in hists__collapse_insert_entry
it'll face following error.

  *** glibc detected *** perf: munmap_chunk(): invalid pointer: 0x00000000014e9d20 ***
  ======= Backtrace: =========
  /lib64/libc.so.6[0x387d47ae16]
  perf[0x4923bd]
  perf(cmd_report+0xd68)[0x432a08]
  perf[0x41a663]
  perf(main+0x58f)[0x419eaf]
  /lib64/libc.so.6(__libc_start_main+0xf5)[0x387d421735]
  perf[0x419f95]

Fix it by allocating and copying branch info for each new hist entry.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1364816125-12212-2-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-report.c |  9 ++++++---
 tools/perf/util/hist.c      | 14 ++++++++++++++
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index bd0ca81..d9f2de3 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -187,6 +187,9 @@ static int perf_report__add_branch_hist_entry(struct perf_tool *tool,
 	for (i = 0; i < sample->branch_stack->nr; i++) {
 		if (rep->hide_unresolved && !(bi[i].from.sym && bi[i].to.sym))
 			continue;
+
+		err = -ENOMEM;
+
 		/*
 		 * The report shows the percentage of total branches captured
 		 * and not events sampled. Thus we use a pseudo period of 1.
@@ -195,7 +198,6 @@ static int perf_report__add_branch_hist_entry(struct perf_tool *tool,
 				&bi[i], 1, 1);
 		if (he) {
 			struct annotation *notes;
-			err = -ENOMEM;
 			bx = he->branch_info;
 			if (bx->from.sym && use_browser == 1 && sort__has_sym) {
 				notes = symbol__annotation(bx->from.sym);
@@ -226,11 +228,12 @@ static int perf_report__add_branch_hist_entry(struct perf_tool *tool,
 			}
 			evsel->hists.stats.total_period += 1;
 			hists__inc_nr_events(&evsel->hists, PERF_RECORD_SAMPLE);
-			err = 0;
 		} else
-			return -ENOMEM;
+			goto out;
 	}
+	err = 0;
 out:
+	free(bi);
 	return err;
 }
 
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 6b32721..9438d57 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -292,6 +292,20 @@ static struct hist_entry *hist_entry__new(struct hist_entry *template)
 			he->ms.map->referenced = true;
 
 		if (he->branch_info) {
+			/*
+			 * This branch info is (a part of) allocated from
+			 * machine__resolve_bstack() and will be freed after
+			 * adding new entries.  So we need to save a copy.
+			 */
+			he->branch_info = malloc(sizeof(*he->branch_info));
+			if (he->branch_info == NULL) {
+				free(he);
+				return NULL;
+			}
+
+			memcpy(he->branch_info, template->branch_info,
+			       sizeof(*he->branch_info));
+
 			if (he->branch_info->from.map)
 				he->branch_info->from.map->referenced = true;
 			if (he->branch_info->to.map)

[tip:perf/core] perf hists: Free unused mem info of a matched hist entry

[tip-bot for Namhyung Kim]

Commit-ID:  ceb2acbc2c1387c8785b3c98b482f5a2b89447c3
Gitweb:     http://git.kernel.org/tip/ceb2acbc2c1387c8785b3c98b482f5a2b89447c3
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Mon, 1 Apr 2013 20:35:18 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 28 May 2013 16:23:52 +0300

perf hists: Free unused mem info of a matched hist entry

The mem info is shared between matched entries so one should be freed.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1364816125-12212-3-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/hist.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 9438d57..514fc04 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -374,6 +374,12 @@ static struct hist_entry *add_hist_entry(struct hists *hists,
 		if (!cmp) {
 			he_stat__add_period(&he->stat, period, weight);
 
+			/*
+			 * This mem info was allocated from machine__resolve_mem
+			 * and will not be used anymore.
+			 */
+			free(entry->mem_info);
+
 			/* If the map of an existing hist_entry has
 			 * become out-of-date due to an exec() or
 			 * similar, update it.  Otherwise we will

[tip:perf/core] perf report: Fix alignment of symbol column when -v is given

[tip-bot for Namhyung Kim]

Commit-ID:  ded19d57a621e92a27a05972949ad3230f84d0b0
Gitweb:     http://git.kernel.org/tip/ded19d57a621e92a27a05972949ad3230f84d0b0
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Mon, 1 Apr 2013 20:35:19 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 28 May 2013 16:23:53 +0300

perf report: Fix alignment of symbol column when -v is given

When -v option is given, the symbol sort key prints its address also but
it wasn't properly aligned since hists__calc_col_len() misses the
additional part.  Also it missed 2 spaces for 0x prefix when printing.

  $ perf report --stdio -v -s sym
  # Samples: 133  of event 'cycles'
  # Event count (approx.): 50536717
  #
  # Overhead                          Symbol
  # ........  ..............................
  #
      12.20%  0xffffffff81384c50 v [k] intel_idle
       7.62%  0xffffffff8170976a v [k] ftrace_caller
       7.02%  0x2d986d         B [.] 0x00000000002d986d

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1364816125-12212-4-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/hist.c | 26 +++++++++++++++-----------
 tools/perf/util/sort.c |  2 +-
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 514fc04..72b4eec 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -70,9 +70,17 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
 	int symlen;
 	u16 len;
 
-	if (h->ms.sym)
-		hists__new_col_len(hists, HISTC_SYMBOL, h->ms.sym->namelen + 4);
-	else {
+	/*
+	 * +4 accounts for '[x] ' priv level info
+	 * +2 accounts for 0x prefix on raw addresses
+	 * +3 accounts for ' y ' symtab origin info
+	 */
+	if (h->ms.sym) {
+		symlen = h->ms.sym->namelen + 4;
+		if (verbose)
+			symlen += BITS_PER_LONG / 4 + 2 + 3;
+		hists__new_col_len(hists, HISTC_SYMBOL, symlen);
+	} else {
 		symlen = unresolved_col_width + 4 + 2;
 		hists__new_col_len(hists, HISTC_SYMBOL, symlen);
 		hists__set_unres_dso_col_len(hists, HISTC_DSO);
@@ -91,12 +99,10 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
 		hists__new_col_len(hists, HISTC_PARENT, h->parent->namelen);
 
 	if (h->branch_info) {
-		/*
-		 * +4 accounts for '[x] ' priv level info
-		 * +2 account of 0x prefix on raw addresses
-		 */
 		if (h->branch_info->from.sym) {
 			symlen = (int)h->branch_info->from.sym->namelen + 4;
+			if (verbose)
+				symlen += BITS_PER_LONG / 4 + 2 + 3;
 			hists__new_col_len(hists, HISTC_SYMBOL_FROM, symlen);
 
 			symlen = dso__name_len(h->branch_info->from.map->dso);
@@ -109,6 +115,8 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
 
 		if (h->branch_info->to.sym) {
 			symlen = (int)h->branch_info->to.sym->namelen + 4;
+			if (verbose)
+				symlen += BITS_PER_LONG / 4 + 2 + 3;
 			hists__new_col_len(hists, HISTC_SYMBOL_TO, symlen);
 
 			symlen = dso__name_len(h->branch_info->to.map->dso);
@@ -121,10 +129,6 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
 	}
 
 	if (h->mem_info) {
-		/*
-		 * +4 accounts for '[x] ' priv level info
-		 * +2 account of 0x prefix on raw addresses
-		 */
 		if (h->mem_info->daddr.sym) {
 			symlen = (int)h->mem_info->daddr.sym->namelen + 4
 			       + unresolved_col_width + 2;
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 5f52d49..16d5e38 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -194,7 +194,7 @@ static int _hist_entry__sym_snprintf(struct map *map, struct symbol *sym,
 	if (verbose) {
 		char o = map ? dso__symtab_origin(map->dso) : '!';
 		ret += repsep_snprintf(bf, size, "%-#*llx %c ",
-				       BITS_PER_LONG / 4, ip, o);
+				       BITS_PER_LONG / 4 + 2, ip, o);
 	}
 
 	ret += repsep_snprintf(bf + ret, size - ret, "[%c] ", level);

[tip:perf/core] perf sort: Introduce sort__mode variable

[tip-bot for Namhyung Kim]

Commit-ID:  55369fc179b0572d0b4a06a9be1d2779b3ac22e0
Gitweb:     http://git.kernel.org/tip/55369fc179b0572d0b4a06a9be1d2779b3ac22e0
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Mon, 1 Apr 2013 20:35:20 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 28 May 2013 16:23:53 +0300

perf sort: Introduce sort__mode variable

It's used for determining current sort mode which can be one of
NORMAL, BRANCH and new MEMORY.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/r/1364816125-12212-5-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-report.c    | 23 +++++++++++++----------
 tools/perf/ui/browsers/hists.c |  4 ++--
 tools/perf/util/sort.c         |  4 ++--
 tools/perf/util/sort.h         |  8 +++++++-
 4 files changed, 24 insertions(+), 15 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index d9f2de3..c877982 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -311,7 +311,7 @@ static int process_sample_event(struct perf_tool *tool,
 	if (rep->cpu_list && !test_bit(sample->cpu, rep->cpu_bitmap))
 		return 0;
 
-	if (sort__branch_mode == 1) {
+	if (sort__mode == SORT_MODE__BRANCH) {
 		if (perf_report__add_branch_hist_entry(tool, &al, sample,
 						       evsel, machine)) {
 			pr_debug("problem adding lbr entry, skipping event\n");
@@ -387,7 +387,7 @@ static int perf_report__setup_sample_type(struct perf_report *rep)
 			}
 	}
 
-	if (sort__branch_mode == 1) {
+	if (sort__mode == SORT_MODE__BRANCH) {
 		if (!self->fd_pipe &&
 		    !(sample_type & PERF_SAMPLE_BRANCH_STACK)) {
 			ui__error("Selected -b but no branch data. "
@@ -694,7 +694,9 @@ static int
 parse_branch_mode(const struct option *opt __maybe_unused,
 		  const char *str __maybe_unused, int unset)
 {
-	sort__branch_mode = !unset;
+	int *branch_mode = opt->value;
+
+	*branch_mode = !unset;
 	return 0;
 }
 
@@ -703,6 +705,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 	struct perf_session *session;
 	struct stat st;
 	bool has_br_stack = false;
+	int branch_mode = -1;
 	int ret = -1;
 	char callchain_default_opt[] = "fractal,0.5,callee";
 	const char * const report_usage[] = {
@@ -799,7 +802,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 		    "Show a column with the sum of periods"),
 	OPT_BOOLEAN(0, "group", &symbol_conf.event_group,
 		    "Show event group information together"),
-	OPT_CALLBACK_NOOPT('b', "branch-stack", &sort__branch_mode, "",
+	OPT_CALLBACK_NOOPT('b', "branch-stack", &branch_mode, "",
 		    "use branch records for histogram filling", parse_branch_mode),
 	OPT_STRING(0, "objdump", &objdump_path, "path",
 		   "objdump binary to use for disassembly and annotations"),
@@ -849,11 +852,11 @@ repeat:
 	has_br_stack = perf_header__has_feat(&session->header,
 					     HEADER_BRANCH_STACK);
 
-	if (sort__branch_mode == -1 && has_br_stack)
-		sort__branch_mode = 1;
+	if (branch_mode == -1 && has_br_stack)
+		sort__mode = SORT_MODE__BRANCH;
 
-	/* sort__branch_mode could be 0 if --no-branch-stack */
-	if (sort__branch_mode == 1) {
+	/* sort__mode could be NORMAL if --no-branch-stack */
+	if (sort__mode == SORT_MODE__BRANCH) {
 		/*
 		 * if no sort_order is provided, then specify
 		 * branch-mode specific order
@@ -864,7 +867,7 @@ repeat:
 
 	}
 	if (report.mem_mode) {
-		if (sort__branch_mode == 1) {
+		if (sort__mode == SORT_MODE__BRANCH) {
 			fprintf(stderr, "branch and mem mode incompatible\n");
 			goto error;
 		}
@@ -934,7 +937,7 @@ repeat:
 
 	sort_entry__setup_elide(&sort_comm, symbol_conf.comm_list, "comm", stdout);
 
-	if (sort__branch_mode == 1) {
+	if (sort__mode == SORT_MODE__BRANCH) {
 		sort_entry__setup_elide(&sort_dso_from, symbol_conf.dso_from_list, "dso_from", stdout);
 		sort_entry__setup_elide(&sort_dso_to, symbol_conf.dso_to_list, "dso_to", stdout);
 		sort_entry__setup_elide(&sort_sym_from, symbol_conf.sym_from_list, "sym_from", stdout);
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index d88a2d0..cad8e37 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -1155,7 +1155,7 @@ static struct hist_browser *hist_browser__new(struct hists *hists)
 		browser->b.refresh = hist_browser__refresh;
 		browser->b.seek = ui_browser__hists_seek;
 		browser->b.use_navkeypressed = true;
-		if (sort__branch_mode == 1)
+		if (sort__mode == SORT_MODE__BRANCH)
 			browser->has_symbols = sort_sym_from.list.next != NULL;
 		else
 			browser->has_symbols = sort_sym.list.next != NULL;
@@ -1488,7 +1488,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 		if (!browser->has_symbols)
 			goto add_exit_option;
 
-		if (sort__branch_mode == 1) {
+		if (sort__mode == SORT_MODE__BRANCH) {
 			bi = browser->he_selection->branch_info;
 			if (browser->selection != NULL &&
 			    bi &&
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 16d5e38..a6ddad4 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -9,7 +9,7 @@ const char	*sort_order = default_sort_order;
 int		sort__need_collapse = 0;
 int		sort__has_parent = 0;
 int		sort__has_sym = 0;
-int		sort__branch_mode = -1; /* -1 = means not set */
+enum sort_mode	sort__mode = SORT_MODE__NORMAL;
 
 enum sort_type	sort__first_dimension;
 
@@ -943,7 +943,7 @@ int sort_dimension__add(const char *tok)
 		if (strncasecmp(tok, sd->name, strlen(tok)))
 			continue;
 
-		if (sort__branch_mode != 1)
+		if (sort__mode != SORT_MODE__BRANCH)
 			return -EINVAL;
 
 		if (sd->entry == &sort_sym_from || sd->entry == &sort_sym_to)
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index f24bdf6..39ff4b8 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -32,7 +32,7 @@ extern const char default_sort_order[];
 extern int sort__need_collapse;
 extern int sort__has_parent;
 extern int sort__has_sym;
-extern int sort__branch_mode;
+extern enum sort_mode sort__mode;
 extern struct sort_entry sort_comm;
 extern struct sort_entry sort_dso;
 extern struct sort_entry sort_sym;
@@ -123,6 +123,12 @@ static inline void hist_entry__add_pair(struct hist_entry *he,
 	list_add_tail(&he->pairs.head, &pair->pairs.node);
 }
 
+enum sort_mode {
+	SORT_MODE__NORMAL,
+	SORT_MODE__BRANCH,
+	SORT_MODE__MEMORY,
+};
+
 enum sort_type {
 	/* common sort keys */
 	SORT_PID,