[PATCHSET 00/10] perf tools: Cleanups and bug fixes on sort keys

[PATCHSET 00/10] perf tools: Cleanups and bug fixes on sort keys

[Namhyung Kim]

Hi,

There's a bug report from Stefan Beller that gets segfault on using
some sort keys.  In addition to it, I had some TODO items related to
it, hence this patchset. :)

This is mostly simple few liners and more complex ones might come later.

Stephane, it'd be great if you check I'm not messed up something on
the branch stack side (or other area too).  Although I've tested basic
functionality there might be something that I missed.

You can also get this from following tree:

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

Thanks,
Namhyung


Cc: Stephane Eranian <eranian@google.com>
Cc: Namhyung Kim <namhyung.kim@lge.com>

Namhyung Kim (10):
  perf sort: Move misplaced sort entry functions
  perf sort: Get rid of unnecessary __maybe_unused
  perf sort: Fix --sort pid output
  perf sort: Align cpu column to right
  perf sort: Calculate parent column width too
  perf sort: Drop ip_[lr] arguments from _sort__sym_cmp()
  perf sort: Check return value of strdup()
  perf sort: Clean up sort__first_dimension setting
  perf sort: Separate out branch stack specific sort keys
  perf report: Update documentation for sort keys

 tools/perf/Documentation/perf-report.txt |  38 ++++-
 tools/perf/builtin-report.c              |   4 +-
 tools/perf/util/hist.c                   |   3 +
 tools/perf/util/sort.c                   | 230 ++++++++++++++++---------------
 tools/perf/util/sort.h                   |   8 +-
 5 files changed, 166 insertions(+), 117 deletions(-)

-- 
1.7.11.7

[PATCH 01/10] perf sort: Move misplaced sort entry functions

[Namhyung Kim]

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

Some functions are misplaced along with other entries.  Move them to a
right place so that it can be found together with related functions.

No functional change intended.

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

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index dbaded90312c..ce5d40456c07 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -97,6 +97,16 @@ static int hist_entry__comm_snprintf(struct hist_entry *self, char *bf,
 	return repsep_snprintf(bf, size, "%*s", width, self->thread->comm);
 }
 
+struct sort_entry sort_comm = {
+	.se_header	= "Command",
+	.se_cmp		= sort__comm_cmp,
+	.se_collapse	= sort__comm_collapse,
+	.se_snprintf	= hist_entry__comm_snprintf,
+	.se_width_idx	= HISTC_COMM,
+};
+
+/* --sort dso */
+
 static int64_t _sort__dso_cmp(struct map *map_l, struct map *map_r)
 {
 	struct dso *dso_l = map_l ? map_l->dso : NULL;
@@ -117,22 +127,38 @@ static int64_t _sort__dso_cmp(struct map *map_l, struct map *map_r)
 	return strcmp(dso_name_l, dso_name_r);
 }
 
-struct sort_entry sort_comm = {
-	.se_header	= "Command",
-	.se_cmp		= sort__comm_cmp,
-	.se_collapse	= sort__comm_collapse,
-	.se_snprintf	= hist_entry__comm_snprintf,
-	.se_width_idx	= HISTC_COMM,
-};
-
-/* --sort dso */
-
 static int64_t
 sort__dso_cmp(struct hist_entry *left, struct hist_entry *right)
 {
 	return _sort__dso_cmp(left->ms.map, right->ms.map);
 }
 
+static int _hist_entry__dso_snprintf(struct map *map, char *bf,
+				     size_t size, unsigned int width)
+{
+	if (map && map->dso) {
+		const char *dso_name = !verbose ? map->dso->short_name :
+			map->dso->long_name;
+		return repsep_snprintf(bf, size, "%-*s", width, dso_name);
+	}
+
+	return repsep_snprintf(bf, size, "%-*s", width, "[unknown]");
+}
+
+static int hist_entry__dso_snprintf(struct hist_entry *self, char *bf,
+				    size_t size, unsigned int width)
+{
+	return _hist_entry__dso_snprintf(self->ms.map, bf, size, width);
+}
+
+struct sort_entry sort_dso = {
+	.se_header	= "Shared Object",
+	.se_cmp		= sort__dso_cmp,
+	.se_snprintf	= hist_entry__dso_snprintf,
+	.se_width_idx	= HISTC_DSO,
+};
+
+/* --sort symbol */
 
 static int64_t _sort__sym_cmp(struct symbol *sym_l, struct symbol *sym_r,
 			      u64 ip_l, u64 ip_r)
@@ -149,22 +175,24 @@ static int64_t _sort__sym_cmp(struct symbol *sym_l, struct symbol *sym_r,
 	return (int64_t)(ip_r - ip_l);
 }
 
-static int _hist_entry__dso_snprintf(struct map *map, char *bf,
-				     size_t size, unsigned int width)
+static int64_t
+sort__sym_cmp(struct hist_entry *left, struct hist_entry *right)
 {
-	if (map && map->dso) {
-		const char *dso_name = !verbose ? map->dso->short_name :
-			map->dso->long_name;
-		return repsep_snprintf(bf, size, "%-*s", width, dso_name);
-	}
+	u64 ip_l, ip_r;
 
-	return repsep_snprintf(bf, size, "%-*s", width, "[unknown]");
-}
+	if (!left->ms.sym && !right->ms.sym)
+		return right->level - left->level;
 
-static int hist_entry__dso_snprintf(struct hist_entry *self, char *bf,
-				    size_t size, unsigned int width)
-{
-	return _hist_entry__dso_snprintf(self->ms.map, bf, size, width);
+	if (!left->ms.sym || !right->ms.sym)
+		return cmp_null(left->ms.sym, right->ms.sym);
+
+	if (left->ms.sym == right->ms.sym)
+		return 0;
+
+	ip_l = left->ms.sym->start;
+	ip_r = right->ms.sym->start;
+
+	return _sort__sym_cmp(left->ms.sym, right->ms.sym, ip_l, ip_r);
 }
 
 static int _hist_entry__sym_snprintf(struct map *map, struct symbol *sym,
@@ -195,14 +223,6 @@ static int _hist_entry__sym_snprintf(struct map *map, struct symbol *sym,
 	return ret;
 }
 
-
-struct sort_entry sort_dso = {
-	.se_header	= "Shared Object",
-	.se_cmp		= sort__dso_cmp,
-	.se_snprintf	= hist_entry__dso_snprintf,
-	.se_width_idx	= HISTC_DSO,
-};
-
 static int hist_entry__sym_snprintf(struct hist_entry *self, char *bf,
 				    size_t size,
 				    unsigned int width __maybe_unused)
@@ -211,27 +231,6 @@ static int hist_entry__sym_snprintf(struct hist_entry *self, char *bf,
 					 self->level, bf, size, width);
 }
 
-/* --sort symbol */
-static int64_t
-sort__sym_cmp(struct hist_entry *left, struct hist_entry *right)
-{
-	u64 ip_l, ip_r;
-
-	if (!left->ms.sym && !right->ms.sym)
-		return right->level - left->level;
-
-	if (!left->ms.sym || !right->ms.sym)
-		return cmp_null(left->ms.sym, right->ms.sym);
-
-	if (left->ms.sym == right->ms.sym)
-		return 0;
-
-	ip_l = left->ms.sym->start;
-	ip_r = right->ms.sym->start;
-
-	return _sort__sym_cmp(left->ms.sym, right->ms.sym, ip_l, ip_r);
-}
-
 struct sort_entry sort_sym = {
 	.se_header	= "Symbol",
 	.se_cmp		= sort__sym_cmp,
@@ -343,6 +342,8 @@ struct sort_entry sort_cpu = {
 	.se_width_idx	= HISTC_CPU,
 };
 
+/* sort keys for branch stacks */
+
 static int64_t
 sort__dso_from_cmp(struct hist_entry *left, struct hist_entry *right)
 {
@@ -357,13 +358,6 @@ static int hist_entry__dso_from_snprintf(struct hist_entry *self, char *bf,
 					 bf, size, width);
 }
 
-struct sort_entry sort_dso_from = {
-	.se_header	= "Source Shared Object",
-	.se_cmp		= sort__dso_from_cmp,
-	.se_snprintf	= hist_entry__dso_from_snprintf,
-	.se_width_idx	= HISTC_DSO_FROM,
-};
-
 static int64_t
 sort__dso_to_cmp(struct hist_entry *left, struct hist_entry *right)
 {
@@ -423,6 +417,13 @@ static int hist_entry__sym_to_snprintf(struct hist_entry *self, char *bf,
 
 }
 
+struct sort_entry sort_dso_from = {
+	.se_header	= "Source Shared Object",
+	.se_cmp		= sort__dso_from_cmp,
+	.se_snprintf	= hist_entry__dso_from_snprintf,
+	.se_width_idx	= HISTC_DSO_FROM,
+};
+
 struct sort_entry sort_dso_to = {
 	.se_header	= "Target Shared Object",
 	.se_cmp		= sort__dso_to_cmp,
-- 
1.7.11.7

[PATCH 02/10] perf sort: Get rid of unnecessary __maybe_unused

[Namhyung Kim]

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

Some functions have set __maybe_unused on its arguments that are used
actually.  Remove them.

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

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index ce5d40456c07..3c2836eeafb5 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -197,7 +197,7 @@ sort__sym_cmp(struct hist_entry *left, struct hist_entry *right)
 
 static int _hist_entry__sym_snprintf(struct map *map, struct symbol *sym,
 				     u64 ip, char level, char *bf, size_t size,
-				     unsigned int width __maybe_unused)
+				     unsigned int width)
 {
 	size_t ret = 0;
 
@@ -224,8 +224,7 @@ static int _hist_entry__sym_snprintf(struct map *map, struct symbol *sym,
 }
 
 static int hist_entry__sym_snprintf(struct hist_entry *self, char *bf,
-				    size_t size,
-				    unsigned int width __maybe_unused)
+				    size_t size, unsigned int width)
 {
 	return _hist_entry__sym_snprintf(self->ms.map, self->ms.sym, self->ip,
 					 self->level, bf, size, width);
@@ -398,8 +397,7 @@ sort__sym_to_cmp(struct hist_entry *left, struct hist_entry *right)
 }
 
 static int hist_entry__sym_from_snprintf(struct hist_entry *self, char *bf,
-					size_t size,
-					unsigned int width __maybe_unused)
+					 size_t size, unsigned int width)
 {
 	struct addr_map_symbol *from = &self->branch_info->from;
 	return _hist_entry__sym_snprintf(from->map, from->sym, from->addr,
@@ -408,8 +406,7 @@ static int hist_entry__sym_from_snprintf(struct hist_entry *self, char *bf,
 }
 
 static int hist_entry__sym_to_snprintf(struct hist_entry *self, char *bf,
-				       size_t size,
-				       unsigned int width __maybe_unused)
+				       size_t size, unsigned int width)
 {
 	struct addr_map_symbol *to = &self->branch_info->to;
 	return _hist_entry__sym_snprintf(to->map, to->sym, to->addr,
-- 
1.7.11.7

[PATCH 03/10] perf sort: Fix --sort pid output

[Namhyung Kim]

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

The "pid" sort key prints "Command: Pid" output but it's misaligned.
It's because of the offset of 6 was added to the column length during
the calculation in order to reserve an space for Pid part but it isn't
honored when printed.  The output before this patch was like this:

  # Overhead  Command:  Pid      Shared Object
  # ........  .............  .................
  #
      99.70%        noploop:17814  noploop
       0.29%        noploop:17814  [kernel.kallsyms]
       0.01%        noploop:17814  ld-2.15.so

Fix it by subtracting 6 for printing comm part.

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

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 3c2836eeafb5..8b6d70bd749b 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -60,7 +60,7 @@ sort__thread_cmp(struct hist_entry *left, struct hist_entry *right)
 static int hist_entry__thread_snprintf(struct hist_entry *self, char *bf,
 				       size_t size, unsigned int width)
 {
-	return repsep_snprintf(bf, size, "%*s:%5d", width,
+	return repsep_snprintf(bf, size, "%*s:%5d", width - 6,
 			      self->thread->comm ?: "", self->thread->pid);
 }
 
-- 
1.7.11.7

[PATCH 04/10] perf sort: Align cpu column to right

[Namhyung Kim]

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

Since cpu number is a natural number, it'd be more appropriate
aligning it to right.

Before:

  # Overhead  CPU      Command:  Pid          Shared Object
  # ........  ...  .................  .....................
  #
       8.91%  8    gnome-shell: 1497  perf-1497.map
       8.90%  7    gnome-shell: 1497  perf-1497.map
       8.86%  9    gnome-shell: 1497  perf-1497.map
       8.83%  6    gnome-shell: 1497  perf-1497.map
       8.81%  10   gnome-shell: 1497  perf-1497.map
       7.44%  5    gnome-shell: 1497  perf-1497.map
       6.20%  3    gnome-shell: 1497  perf-1497.map
       5.10%  0    gnome-shell: 1497  perf-1497.map

After:

  # Overhead  CPU      Command:  Pid          Shared Object
  # ........  ...  .................  .....................
  #
       8.91%    8  gnome-shell: 1497  perf-1497.map
       8.90%    7  gnome-shell: 1497  perf-1497.map
       8.86%    9  gnome-shell: 1497  perf-1497.map
       8.83%    6  gnome-shell: 1497  perf-1497.map
       8.81%   10  gnome-shell: 1497  perf-1497.map
       7.44%    5  gnome-shell: 1497  perf-1497.map
       6.20%    3  gnome-shell: 1497  perf-1497.map
       5.10%    0  gnome-shell: 1497  perf-1497.map

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

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 8b6d70bd749b..a36051b34901 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -331,7 +331,7 @@ sort__cpu_cmp(struct hist_entry *left, struct hist_entry *right)
 static int hist_entry__cpu_snprintf(struct hist_entry *self, char *bf,
 				       size_t size, unsigned int width)
 {
-	return repsep_snprintf(bf, size, "%-*d", width, self->cpu);
+	return repsep_snprintf(bf, size, "%*d", width, self->cpu);
 }
 
 struct sort_entry sort_cpu = {
-- 
1.7.11.7

[PATCH 05/10] perf sort: Calculate parent column width too

[Namhyung Kim]

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

When hists__calc_col_len() called, most of column length are refreshed
but it missed parent column.  So if the parent sort key was used along
with other keys rests will be misalinged since parent has no proper
column width.

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

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 965ebf948f2f..9485c7024f5b 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -82,6 +82,9 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
 		hists__new_col_len(hists, HISTC_DSO, len);
 	}
 
+	if (h->parent)
+		hists__new_col_len(hists, HISTC_PARENT, h->parent->namelen);
+
 	if (h->branch_info) {
 		int symlen;
 		/*
-- 
1.7.11.7

[PATCH 06/10] perf sort: Drop ip_[lr] arguments from _sort__sym_cmp()

[Namhyung Kim]

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

Current _sort__sym_cmp() function is used for comparing symbols
between two hist entries on symbol, symbol_from and symbol_to sort
keys.  Those functions pass addresses of symbols but it's meaningless
since it gets over-written inside of the _sort__sym_cmp function to a
start address of the symbol.  So just get rid of them.

This might cause a difference than prior output for branch stacks
since it seems not using start address of the symbol but branch
address.  However AFAICS it'd be same as it gets overwritten anyway.

Also remove redundant part of code in sort__sym_cmp().

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

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index a36051b34901..c02964cabdd0 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -160,9 +160,10 @@ struct sort_entry sort_dso = {
 
 /* --sort symbol */
 
-static int64_t _sort__sym_cmp(struct symbol *sym_l, struct symbol *sym_r,
-			      u64 ip_l, u64 ip_r)
+static int64_t _sort__sym_cmp(struct symbol *sym_l, struct symbol *sym_r)
 {
+	u64 ip_l, ip_r;
+
 	if (!sym_l || !sym_r)
 		return cmp_null(sym_l, sym_r);
 
@@ -178,21 +179,10 @@ static int64_t _sort__sym_cmp(struct symbol *sym_l, struct symbol *sym_r,
 static int64_t
 sort__sym_cmp(struct hist_entry *left, struct hist_entry *right)
 {
-	u64 ip_l, ip_r;
-
 	if (!left->ms.sym && !right->ms.sym)
 		return right->level - left->level;
 
-	if (!left->ms.sym || !right->ms.sym)
-		return cmp_null(left->ms.sym, right->ms.sym);
-
-	if (left->ms.sym == right->ms.sym)
-		return 0;
-
-	ip_l = left->ms.sym->start;
-	ip_r = right->ms.sym->start;
-
-	return _sort__sym_cmp(left->ms.sym, right->ms.sym, ip_l, ip_r);
+	return _sort__sym_cmp(left->ms.sym, right->ms.sym);
 }
 
 static int _hist_entry__sym_snprintf(struct map *map, struct symbol *sym,
@@ -380,8 +370,7 @@ sort__sym_from_cmp(struct hist_entry *left, struct hist_entry *right)
 	if (!from_l->sym && !from_r->sym)
 		return right->level - left->level;
 
-	return _sort__sym_cmp(from_l->sym, from_r->sym, from_l->addr,
-			     from_r->addr);
+	return _sort__sym_cmp(from_l->sym, from_r->sym);
 }
 
 static int64_t
@@ -393,7 +382,7 @@ sort__sym_to_cmp(struct hist_entry *left, struct hist_entry *right)
 	if (!to_l->sym && !to_r->sym)
 		return right->level - left->level;
 
-	return _sort__sym_cmp(to_l->sym, to_r->sym, to_l->addr, to_r->addr);
+	return _sort__sym_cmp(to_l->sym, to_r->sym);
 }
 
 static int hist_entry__sym_from_snprintf(struct hist_entry *self, char *bf,
-- 
1.7.11.7

[PATCH 07/10] perf sort: Check return value of strdup()

[Namhyung Kim]

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

When setup_sorting() is called, 'str' is passed to strtok_r() but it's
not checked to have a valid pointer.  As strtok_r() accepts NULL
pointer on a first argument and use the third argument in that case,
it can cause a trouble since our third argument, tmp, is not
initialized.

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

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index c02964cabdd0..9f827e25a2b5 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -552,6 +552,11 @@ void setup_sorting(const char * const usagestr[], const struct option *opts)
 {
 	char *tmp, *tok, *str = strdup(sort_order);
 
+	if (str == NULL) {
+		error("Not enough memory to setup sort keys");
+		exit(-1);
+	}
+
 	for (tok = strtok_r(str, ", ", &tmp);
 			tok; tok = strtok_r(NULL, ", ", &tmp)) {
 		if (sort_dimension__add(tok) < 0) {
-- 
1.7.11.7

[PATCH 08/10] perf sort: Clean up sort__first_dimension setting

[Namhyung Kim]

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

It doesn't need to compare to every sort key names since the index
already has the required information.

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

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 9f827e25a2b5..a5e732a03bda 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -515,30 +515,8 @@ int sort_dimension__add(const char *tok)
 		if (sd->entry->se_collapse)
 			sort__need_collapse = 1;
 
-		if (list_empty(&hist_entry__sort_list)) {
-			if (!strcmp(sd->name, "pid"))
-				sort__first_dimension = SORT_PID;
-			else if (!strcmp(sd->name, "comm"))
-				sort__first_dimension = SORT_COMM;
-			else if (!strcmp(sd->name, "dso"))
-				sort__first_dimension = SORT_DSO;
-			else if (!strcmp(sd->name, "symbol"))
-				sort__first_dimension = SORT_SYM;
-			else if (!strcmp(sd->name, "parent"))
-				sort__first_dimension = SORT_PARENT;
-			else if (!strcmp(sd->name, "cpu"))
-				sort__first_dimension = SORT_CPU;
-			else if (!strcmp(sd->name, "symbol_from"))
-				sort__first_dimension = SORT_SYM_FROM;
-			else if (!strcmp(sd->name, "symbol_to"))
-				sort__first_dimension = SORT_SYM_TO;
-			else if (!strcmp(sd->name, "dso_from"))
-				sort__first_dimension = SORT_DSO_FROM;
-			else if (!strcmp(sd->name, "dso_to"))
-				sort__first_dimension = SORT_DSO_TO;
-			else if (!strcmp(sd->name, "mispredict"))
-				sort__first_dimension = SORT_MISPREDICT;
-		}
+		if (list_empty(&hist_entry__sort_list))
+			sort__first_dimension = i;
 
 		list_add_tail(&sd->entry->list, &hist_entry__sort_list);
 		sd->taken = 1;
-- 
1.7.11.7

[PATCH 09/10] perf sort: Separate out branch stack specific sort keys

[Namhyung Kim]

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

Current perf report gets segmentation fault when a branch stack
specific sort key is provided by --sort option to a perf.data file
which contains no branch infomation.  It's because those sort keys
reference branch info of a hist entry unconditionally.  Maybe we can
change it checks whether such branch info is valid or not.  But if the
branch stacks are not recorded, it'd be nop.  Thus it'd be better to
make those keys are unselectable.

This patch separates those keys to a different dimension array, so
that if user passes such a key to a file which has no branch stack
will get following message rather than a segfault.

  Error: Invalid --sort key: `symbol_from'

Reported-by: Stefan Beller <stefanbeller@googlemail.com>
Cc: Stephane Eranian <eranian@google.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/sort.c | 64 ++++++++++++++++++++++++++++++++++++++++----------
 tools/perf/util/sort.h |  8 +++++--
 2 files changed, 58 insertions(+), 14 deletions(-)

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index a5e732a03bda..012a077fbce9 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -469,30 +469,40 @@ struct sort_dimension {
 
 #define DIM(d, n, func) [d] = { .name = n, .entry = &(func) }
 
-static struct sort_dimension sort_dimensions[] = {
+static struct sort_dimension common_sort_dimensions[] = {
 	DIM(SORT_PID, "pid", sort_thread),
 	DIM(SORT_COMM, "comm", sort_comm),
 	DIM(SORT_DSO, "dso", sort_dso),
-	DIM(SORT_DSO_FROM, "dso_from", sort_dso_from),
-	DIM(SORT_DSO_TO, "dso_to", sort_dso_to),
 	DIM(SORT_SYM, "symbol", sort_sym),
-	DIM(SORT_SYM_TO, "symbol_from", sort_sym_from),
-	DIM(SORT_SYM_FROM, "symbol_to", sort_sym_to),
 	DIM(SORT_PARENT, "parent", sort_parent),
 	DIM(SORT_CPU, "cpu", sort_cpu),
-	DIM(SORT_MISPREDICT, "mispredict", sort_mispredict),
 	DIM(SORT_SRCLINE, "srcline", sort_srcline),
 };
 
+#undef DIM
+
+#define DIM(d, n, func) [d - __SORT_BRANCH_STACK] = { .name = n, .entry = &(func) }
+
+static struct sort_dimension bstack_sort_dimensions[] = {
+	DIM(SORT_DSO_FROM, "dso_from", sort_dso_from),
+	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_MISPREDICT, "mispredict", sort_mispredict),
+};
+
+#undef DIM
+
 int sort_dimension__add(const char *tok)
 {
 	unsigned int i;
 
-	for (i = 0; i < ARRAY_SIZE(sort_dimensions); i++) {
-		struct sort_dimension *sd = &sort_dimensions[i];
+	for (i = 0; i < ARRAY_SIZE(common_sort_dimensions); i++) {
+		struct sort_dimension *sd = &common_sort_dimensions[i];
 
 		if (strncasecmp(tok, sd->name, strlen(tok)))
 			continue;
+
 		if (sd->entry == &sort_parent) {
 			int ret = regcomp(&parent_regex, parent_pattern, REG_EXTENDED);
 			if (ret) {
@@ -503,9 +513,7 @@ int sort_dimension__add(const char *tok)
 				return -EINVAL;
 			}
 			sort__has_parent = 1;
-		} else if (sd->entry == &sort_sym ||
-			   sd->entry == &sort_sym_from ||
-			   sd->entry == &sort_sym_to) {
+		} else if (sd->entry == &sort_sym) {
 			sort__has_sym = 1;
 		}
 
@@ -523,6 +531,34 @@ int sort_dimension__add(const char *tok)
 
 		return 0;
 	}
+
+	for (i = 0; i < ARRAY_SIZE(bstack_sort_dimensions); i++) {
+		struct sort_dimension *sd = &bstack_sort_dimensions[i];
+
+		if (strncasecmp(tok, sd->name, strlen(tok)))
+			continue;
+
+		if (sort__branch_mode != 1)
+			return -EINVAL;
+
+		if (sd->entry == &sort_sym_from || sd->entry == &sort_sym_to)
+			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_BRANCH_STACK;
+
+		list_add_tail(&sd->entry->list, &hist_entry__sort_list);
+		sd->taken = 1;
+
+		return 0;
+	}
+
 	return -ESRCH;
 }
 
@@ -537,7 +573,11 @@ void setup_sorting(const char * const usagestr[], const struct option *opts)
 
 	for (tok = strtok_r(str, ", ", &tmp);
 			tok; tok = strtok_r(NULL, ", ", &tmp)) {
-		if (sort_dimension__add(tok) < 0) {
+		int ret = sort_dimension__add(tok);
+		if (ret == -EINVAL) {
+			error("Invalid --sort key: `%s'", tok);
+			usage_with_options(usagestr, opts);
+		} else if (ret == -ESRCH) {
 			error("Unknown --sort key: `%s'", tok);
 			usage_with_options(usagestr, opts);
 		}
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index a1c0d56b6885..e994ad3e9897 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -122,18 +122,22 @@ static inline void hist_entry__add_pair(struct hist_entry *he,
 }
 
 enum sort_type {
+	/* common sort keys */
 	SORT_PID,
 	SORT_COMM,
 	SORT_DSO,
 	SORT_SYM,
 	SORT_PARENT,
 	SORT_CPU,
-	SORT_DSO_FROM,
+	SORT_SRCLINE,
+
+	/* branch stack specific sort keys */
+	__SORT_BRANCH_STACK,
+	SORT_DSO_FROM = __SORT_BRANCH_STACK,
 	SORT_DSO_TO,
 	SORT_SYM_FROM,
 	SORT_SYM_TO,
 	SORT_MISPREDICT,
-	SORT_SRCLINE,
 };
 
 /*
-- 
1.7.11.7

[PATCH 10/10] perf report: Update documentation for sort keys

[Namhyung Kim]

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

Add description of sort keys to the perf-report document and also add
missing cpu and srcline keys to the command line help string.

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

diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
index f4d91bebd59d..848a0dcb6dfd 100644
--- a/tools/perf/Documentation/perf-report.txt
+++ b/tools/perf/Documentation/perf-report.txt
@@ -57,11 +57,44 @@ OPTIONS
 
 -s::
 --sort=::
-	Sort by key(s): pid, comm, dso, symbol, parent, srcline.
+	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.
+
+	Each key has following meaning:
+
+	- comm: command (name) of the task which can be read via /proc/<pid>/comm
+	- pid: command and tid of the task
+	- dso: name of library or module executed at the time of sample
+	- symbol: name of function executed at the time of sample
+	- parent: name of function matched to the parent regex filter. Unmatched
+	entries are displayed as "[other]".
+	- 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.
+
+	By default, comm, dso and symbol keys are used.
+	(i.e. --sort comm,dso,symbol)
+
+	If --branch-stack option is used, following sort keys are also
+	available:
+	dso_from, dso_to, symbol_from, symbol_to, mispredict.
+
+	- dso_from: name of library or module branched from
+	- dso_to: name of library or module branched to
+	- symbol_from: name of function branched from
+	- symbol_to: name 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
+	and symbol_to, see '--branch-stack'.
 
 -p::
 --parent=<regex>::
-        regex filter to identify parent, see: '--sort parent'
+        A regex filter to identify parent. The parent is a caller of this
+	function and searched through the callchain, thus it requires callchain
+	information recorded. The pattern is in the exteneded regex format and
+	defaults to "\^sys_|^do_page_fault", see '--sort parent'.
 
 -x::
 --exclude-other::
@@ -74,7 +107,6 @@ OPTIONS
 
 -t::
 --field-separator=::
-
 	Use a special separator character and don't pad with spaces, replacing
 	all occurrences of this separator in symbol names (and other output)
 	with a '.' character, that thus it's the only non valid separator.
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 13cdf61c4f82..47a864478543 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -595,8 +595,8 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 	OPT_BOOLEAN(0, "stdio", &report.use_stdio,
 		    "Use the stdio interface"),
 	OPT_STRING('s', "sort", &sort_order, "key[,key2...]",
-		   "sort by key(s): pid, comm, dso, symbol, parent, dso_to,"
-		   " dso_from, symbol_to, symbol_from, mispredict"),
+		   "sort by key(s): pid, comm, dso, symbol, parent, cpu, srcline,"
+		   " dso_to, dso_from, symbol_to, symbol_from, mispredict"),
 	OPT_BOOLEAN(0, "showcpuutilization", &symbol_conf.show_cpu_utilization,
 		    "Show sample percentage for different cpu modes"),
 	OPT_STRING('p', "parent", &parent_pattern, "regex",
-- 
1.7.11.7

Re: [PATCHSET 00/10] perf tools: Cleanups and bug fixes on sort keys

[Jiri Olsa]

On Thu, Dec 27, 2012 at 06:11:37PM +0900, Namhyung Kim wrote:
> Hi,
> 
> There's a bug report from Stefan Beller that gets segfault on using
> some sort keys.  In addition to it, I had some TODO items related to
> it, hence this patchset. :)
> 
> This is mostly simple few liners and more complex ones might come later.
> 
> Stephane, it'd be great if you check I'm not messed up something on
> the branch stack side (or other area too).  Although I've tested basic
> functionality there might be something that I missed.
> 
> You can also get this from following tree:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git perf/sort-v1
> 
> Thanks,
> Namhyung
> 
> 
> Cc: Stephane Eranian <eranian@google.com>
> Cc: Namhyung Kim <namhyung.kim@lge.com>
> 
> Namhyung Kim (10):
>   perf sort: Move misplaced sort entry functions
>   perf sort: Get rid of unnecessary __maybe_unused
>   perf sort: Fix --sort pid output
>   perf sort: Align cpu column to right
>   perf sort: Calculate parent column width too
>   perf sort: Drop ip_[lr] arguments from _sort__sym_cmp()
>   perf sort: Check return value of strdup()
>   perf sort: Clean up sort__first_dimension setting
>   perf sort: Separate out branch stack specific sort keys
>   perf report: Update documentation for sort keys

I've tested all except for the branch related stuff, though they look ok as well

Acked-by: Jiri Olsa <jolsa@redhat.com>

for the the patchset

jirka

Re: [PATCH 06/10] perf sort: Drop ip_[lr] arguments from _sort__sym_cmp()

[Arnaldo Carvalho de Melo]

Stephane, can you take a look at this one and provide feedback?

- Arnaldo

Em Thu, Dec 27, 2012 at 06:11:43PM +0900, Namhyung Kim escreveu:
> From: Namhyung Kim <namhyung.kim@lge.com>
> 
> Current _sort__sym_cmp() function is used for comparing symbols
> between two hist entries on symbol, symbol_from and symbol_to sort
> keys.  Those functions pass addresses of symbols but it's meaningless
> since it gets over-written inside of the _sort__sym_cmp function to a
> start address of the symbol.  So just get rid of them.
> 
> This might cause a difference than prior output for branch stacks
> since it seems not using start address of the symbol but branch
> address.  However AFAICS it'd be same as it gets overwritten anyway.
> 
> Also remove redundant part of code in sort__sym_cmp().
> 
> Cc: Stephane Eranian <eranian@google.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/sort.c | 23 ++++++-----------------
>  1 file changed, 6 insertions(+), 17 deletions(-)
> 
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index a36051b34901..c02964cabdd0 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -160,9 +160,10 @@ struct sort_entry sort_dso = {
>  
>  /* --sort symbol */
>  
> -static int64_t _sort__sym_cmp(struct symbol *sym_l, struct symbol *sym_r,
> -			      u64 ip_l, u64 ip_r)
> +static int64_t _sort__sym_cmp(struct symbol *sym_l, struct symbol *sym_r)
>  {
> +	u64 ip_l, ip_r;
> +
>  	if (!sym_l || !sym_r)
>  		return cmp_null(sym_l, sym_r);
>  
> @@ -178,21 +179,10 @@ static int64_t _sort__sym_cmp(struct symbol *sym_l, struct symbol *sym_r,
>  static int64_t
>  sort__sym_cmp(struct hist_entry *left, struct hist_entry *right)
>  {
> -	u64 ip_l, ip_r;
> -
>  	if (!left->ms.sym && !right->ms.sym)
>  		return right->level - left->level;
>  
> -	if (!left->ms.sym || !right->ms.sym)
> -		return cmp_null(left->ms.sym, right->ms.sym);
> -
> -	if (left->ms.sym == right->ms.sym)
> -		return 0;
> -
> -	ip_l = left->ms.sym->start;
> -	ip_r = right->ms.sym->start;
> -
> -	return _sort__sym_cmp(left->ms.sym, right->ms.sym, ip_l, ip_r);
> +	return _sort__sym_cmp(left->ms.sym, right->ms.sym);
>  }
>  
>  static int _hist_entry__sym_snprintf(struct map *map, struct symbol *sym,
> @@ -380,8 +370,7 @@ sort__sym_from_cmp(struct hist_entry *left, struct hist_entry *right)
>  	if (!from_l->sym && !from_r->sym)
>  		return right->level - left->level;
>  
> -	return _sort__sym_cmp(from_l->sym, from_r->sym, from_l->addr,
> -			     from_r->addr);
> +	return _sort__sym_cmp(from_l->sym, from_r->sym);
>  }
>  
>  static int64_t
> @@ -393,7 +382,7 @@ sort__sym_to_cmp(struct hist_entry *left, struct hist_entry *right)
>  	if (!to_l->sym && !to_r->sym)
>  		return right->level - left->level;
>  
> -	return _sort__sym_cmp(to_l->sym, to_r->sym, to_l->addr, to_r->addr);
> +	return _sort__sym_cmp(to_l->sym, to_r->sym);
>  }
>  
>  static int hist_entry__sym_from_snprintf(struct hist_entry *self, char *bf,
> -- 
> 1.7.11.7

Re: [PATCH 07/10] perf sort: Check return value of strdup()

[Arnaldo Carvalho de Melo]

Em Thu, Dec 27, 2012 at 06:11:44PM +0900, Namhyung Kim escreveu:
> From: Namhyung Kim <namhyung.kim@lge.com>
> 
> When setup_sorting() is called, 'str' is passed to strtok_r() but it's
> not checked to have a valid pointer.  As strtok_r() accepts NULL
> pointer on a first argument and use the third argument in that case,
> it can cause a trouble since our third argument, tmp, is not
> initialized.

Ok, but calling exit from here? Better to check it at the callers and
propagate the error, letting things like TUI/gui exit routines to
execute.
 
> Cc: Stephane Eranian <eranian@google.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/sort.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index c02964cabdd0..9f827e25a2b5 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -552,6 +552,11 @@ void setup_sorting(const char * const usagestr[], const struct option *opts)
>  {
>  	char *tmp, *tok, *str = strdup(sort_order);
>  
> +	if (str == NULL) {
> +		error("Not enough memory to setup sort keys");
> +		exit(-1);
> +	}
> +
>  	for (tok = strtok_r(str, ", ", &tmp);
>  			tok; tok = strtok_r(NULL, ", ", &tmp)) {
>  		if (sort_dimension__add(tok) < 0) {
> -- 
> 1.7.11.7

Re: [PATCH 07/10] perf sort: Check return value of strdup()

[Namhyung Kim]

Hi Arnaldo,

2013-01-11 (금), 02:17 -0300, Arnaldo Carvalho de Melo:
> Em Thu, Dec 27, 2012 at 06:11:44PM +0900, Namhyung Kim escreveu:
> > From: Namhyung Kim <namhyung.kim@lge.com>
> > 
> > When setup_sorting() is called, 'str' is passed to strtok_r() but it's
> > not checked to have a valid pointer.  As strtok_r() accepts NULL
> > pointer on a first argument and use the third argument in that case,
> > it can cause a trouble since our third argument, tmp, is not
> > initialized.
> 
> Ok, but calling exit from here? Better to check it at the callers and
> propagate the error, letting things like TUI/gui exit routines to
> execute.

That's fine to me too, but currently setup_sorting() exits through
usage_with_options() if sort_dimension__add() fails and I followed the
convention here.  Making it to return will require changes to every
callsites of the function.

If you think it's better to change, I can do it as well.

Thanks,
Namhyung

Re: [PATCH 07/10] perf sort: Check return value of strdup()

[Arnaldo Carvalho de Melo]

Em Sun, Jan 13, 2013 at 05:43:37PM +0900, Namhyung Kim escreveu:
> Hi Arnaldo,
> 
> 2013-01-11 (금), 02:17 -0300, Arnaldo Carvalho de Melo:
> > Em Thu, Dec 27, 2012 at 06:11:44PM +0900, Namhyung Kim escreveu:
> > > From: Namhyung Kim <namhyung.kim@lge.com>
> > > 
> > > When setup_sorting() is called, 'str' is passed to strtok_r() but it's
> > > not checked to have a valid pointer.  As strtok_r() accepts NULL
> > > pointer on a first argument and use the third argument in that case,
> > > it can cause a trouble since our third argument, tmp, is not
> > > initialized.
> > 
> > Ok, but calling exit from here? Better to check it at the callers and
> > propagate the error, letting things like TUI/gui exit routines to
> > execute.
> 
> That's fine to me too, but currently setup_sorting() exits through
> usage_with_options() if sort_dimension__add() fails and I followed the
> convention here.  Making it to return will require changes to every
> callsites of the function.
> 
> If you think it's better to change, I can do it as well.

I think this is a good opportunity to do that :-)

I just grepped for 'setup_sorting' and we have some private ones in
builtin-kmem, for instance, that return a value and that value is
checked, etc. Good precendent :-)

- Arnaldo

[tip:perf/core] perf sort: Move misplaced sort entry functions

[tip-bot for Namhyung Kim]

Commit-ID:  14d1ac7429d104b09d65c72fd215e1cffd5c7eba
Gitweb:     http://git.kernel.org/tip/14d1ac7429d104b09d65c72fd215e1cffd5c7eba
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Thu, 27 Dec 2012 18:11:38 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 24 Jan 2013 16:40:20 -0300

perf sort: Move misplaced sort entry functions

Some functions are misplaced along with other entries.  Move them to a
right place so that it can be found together with related functions.

No functional change intended.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Jiri Olsa <jolsa@redhat.com>
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/1356599507-14226-2-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/sort.c | 119 +++++++++++++++++++++++++------------------------
 1 file changed, 60 insertions(+), 59 deletions(-)

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index dbaded9..ce5d404 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -97,6 +97,16 @@ static int hist_entry__comm_snprintf(struct hist_entry *self, char *bf,
 	return repsep_snprintf(bf, size, "%*s", width, self->thread->comm);
 }
 
+struct sort_entry sort_comm = {
+	.se_header	= "Command",
+	.se_cmp		= sort__comm_cmp,
+	.se_collapse	= sort__comm_collapse,
+	.se_snprintf	= hist_entry__comm_snprintf,
+	.se_width_idx	= HISTC_COMM,
+};
+
+/* --sort dso */
+
 static int64_t _sort__dso_cmp(struct map *map_l, struct map *map_r)
 {
 	struct dso *dso_l = map_l ? map_l->dso : NULL;
@@ -117,22 +127,38 @@ static int64_t _sort__dso_cmp(struct map *map_l, struct map *map_r)
 	return strcmp(dso_name_l, dso_name_r);
 }
 
-struct sort_entry sort_comm = {
-	.se_header	= "Command",
-	.se_cmp		= sort__comm_cmp,
-	.se_collapse	= sort__comm_collapse,
-	.se_snprintf	= hist_entry__comm_snprintf,
-	.se_width_idx	= HISTC_COMM,
-};
-
-/* --sort dso */
-
 static int64_t
 sort__dso_cmp(struct hist_entry *left, struct hist_entry *right)
 {
 	return _sort__dso_cmp(left->ms.map, right->ms.map);
 }
 
+static int _hist_entry__dso_snprintf(struct map *map, char *bf,
+				     size_t size, unsigned int width)
+{
+	if (map && map->dso) {
+		const char *dso_name = !verbose ? map->dso->short_name :
+			map->dso->long_name;
+		return repsep_snprintf(bf, size, "%-*s", width, dso_name);
+	}
+
+	return repsep_snprintf(bf, size, "%-*s", width, "[unknown]");
+}
+
+static int hist_entry__dso_snprintf(struct hist_entry *self, char *bf,
+				    size_t size, unsigned int width)
+{
+	return _hist_entry__dso_snprintf(self->ms.map, bf, size, width);
+}
+
+struct sort_entry sort_dso = {
+	.se_header	= "Shared Object",
+	.se_cmp		= sort__dso_cmp,
+	.se_snprintf	= hist_entry__dso_snprintf,
+	.se_width_idx	= HISTC_DSO,
+};
+
+/* --sort symbol */
 
 static int64_t _sort__sym_cmp(struct symbol *sym_l, struct symbol *sym_r,
 			      u64 ip_l, u64 ip_r)
@@ -149,22 +175,24 @@ static int64_t _sort__sym_cmp(struct symbol *sym_l, struct symbol *sym_r,
 	return (int64_t)(ip_r - ip_l);
 }
 
-static int _hist_entry__dso_snprintf(struct map *map, char *bf,
-				     size_t size, unsigned int width)
+static int64_t
+sort__sym_cmp(struct hist_entry *left, struct hist_entry *right)
 {
-	if (map && map->dso) {
-		const char *dso_name = !verbose ? map->dso->short_name :
-			map->dso->long_name;
-		return repsep_snprintf(bf, size, "%-*s", width, dso_name);
-	}
+	u64 ip_l, ip_r;
 
-	return repsep_snprintf(bf, size, "%-*s", width, "[unknown]");
-}
+	if (!left->ms.sym && !right->ms.sym)
+		return right->level - left->level;
 
-static int hist_entry__dso_snprintf(struct hist_entry *self, char *bf,
-				    size_t size, unsigned int width)
-{
-	return _hist_entry__dso_snprintf(self->ms.map, bf, size, width);
+	if (!left->ms.sym || !right->ms.sym)
+		return cmp_null(left->ms.sym, right->ms.sym);
+
+	if (left->ms.sym == right->ms.sym)
+		return 0;
+
+	ip_l = left->ms.sym->start;
+	ip_r = right->ms.sym->start;
+
+	return _sort__sym_cmp(left->ms.sym, right->ms.sym, ip_l, ip_r);
 }
 
 static int _hist_entry__sym_snprintf(struct map *map, struct symbol *sym,
@@ -195,14 +223,6 @@ static int _hist_entry__sym_snprintf(struct map *map, struct symbol *sym,
 	return ret;
 }
 
-
-struct sort_entry sort_dso = {
-	.se_header	= "Shared Object",
-	.se_cmp		= sort__dso_cmp,
-	.se_snprintf	= hist_entry__dso_snprintf,
-	.se_width_idx	= HISTC_DSO,
-};
-
 static int hist_entry__sym_snprintf(struct hist_entry *self, char *bf,
 				    size_t size,
 				    unsigned int width __maybe_unused)
@@ -211,27 +231,6 @@ static int hist_entry__sym_snprintf(struct hist_entry *self, char *bf,
 					 self->level, bf, size, width);
 }
 
-/* --sort symbol */
-static int64_t
-sort__sym_cmp(struct hist_entry *left, struct hist_entry *right)
-{
-	u64 ip_l, ip_r;
-
-	if (!left->ms.sym && !right->ms.sym)
-		return right->level - left->level;
-
-	if (!left->ms.sym || !right->ms.sym)
-		return cmp_null(left->ms.sym, right->ms.sym);
-
-	if (left->ms.sym == right->ms.sym)
-		return 0;
-
-	ip_l = left->ms.sym->start;
-	ip_r = right->ms.sym->start;
-
-	return _sort__sym_cmp(left->ms.sym, right->ms.sym, ip_l, ip_r);
-}
-
 struct sort_entry sort_sym = {
 	.se_header	= "Symbol",
 	.se_cmp		= sort__sym_cmp,
@@ -343,6 +342,8 @@ struct sort_entry sort_cpu = {
 	.se_width_idx	= HISTC_CPU,
 };
 
+/* sort keys for branch stacks */
+
 static int64_t
 sort__dso_from_cmp(struct hist_entry *left, struct hist_entry *right)
 {
@@ -357,13 +358,6 @@ static int hist_entry__dso_from_snprintf(struct hist_entry *self, char *bf,
 					 bf, size, width);
 }
 
-struct sort_entry sort_dso_from = {
-	.se_header	= "Source Shared Object",
-	.se_cmp		= sort__dso_from_cmp,
-	.se_snprintf	= hist_entry__dso_from_snprintf,
-	.se_width_idx	= HISTC_DSO_FROM,
-};
-
 static int64_t
 sort__dso_to_cmp(struct hist_entry *left, struct hist_entry *right)
 {
@@ -423,6 +417,13 @@ static int hist_entry__sym_to_snprintf(struct hist_entry *self, char *bf,
 
 }
 
+struct sort_entry sort_dso_from = {
+	.se_header	= "Source Shared Object",
+	.se_cmp		= sort__dso_from_cmp,
+	.se_snprintf	= hist_entry__dso_from_snprintf,
+	.se_width_idx	= HISTC_DSO_FROM,
+};
+
 struct sort_entry sort_dso_to = {
 	.se_header	= "Target Shared Object",
 	.se_cmp		= sort__dso_to_cmp,

[tip:perf/core] perf sort: Get rid of unnecessary __maybe_unused

[tip-bot for Namhyung Kim]

Commit-ID:  433555221b6c8bff4fee2d5ab84e7aea6b1f068e
Gitweb:     http://git.kernel.org/tip/433555221b6c8bff4fee2d5ab84e7aea6b1f068e
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Thu, 27 Dec 2012 18:11:39 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 24 Jan 2013 16:40:20 -0300

perf sort: Get rid of unnecessary __maybe_unused

Some functions have set __maybe_unused on its arguments that are used
actually.  Remove them.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Jiri Olsa <jolsa@redhat.com>
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/1356599507-14226-3-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/sort.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index ce5d404..3c2836e 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -197,7 +197,7 @@ sort__sym_cmp(struct hist_entry *left, struct hist_entry *right)
 
 static int _hist_entry__sym_snprintf(struct map *map, struct symbol *sym,
 				     u64 ip, char level, char *bf, size_t size,
-				     unsigned int width __maybe_unused)
+				     unsigned int width)
 {
 	size_t ret = 0;
 
@@ -224,8 +224,7 @@ static int _hist_entry__sym_snprintf(struct map *map, struct symbol *sym,
 }
 
 static int hist_entry__sym_snprintf(struct hist_entry *self, char *bf,
-				    size_t size,
-				    unsigned int width __maybe_unused)
+				    size_t size, unsigned int width)
 {
 	return _hist_entry__sym_snprintf(self->ms.map, self->ms.sym, self->ip,
 					 self->level, bf, size, width);
@@ -398,8 +397,7 @@ sort__sym_to_cmp(struct hist_entry *left, struct hist_entry *right)
 }
 
 static int hist_entry__sym_from_snprintf(struct hist_entry *self, char *bf,
-					size_t size,
-					unsigned int width __maybe_unused)
+					 size_t size, unsigned int width)
 {
 	struct addr_map_symbol *from = &self->branch_info->from;
 	return _hist_entry__sym_snprintf(from->map, from->sym, from->addr,
@@ -408,8 +406,7 @@ static int hist_entry__sym_from_snprintf(struct hist_entry *self, char *bf,
 }
 
 static int hist_entry__sym_to_snprintf(struct hist_entry *self, char *bf,
-				       size_t size,
-				       unsigned int width __maybe_unused)
+				       size_t size, unsigned int width)
 {
 	struct addr_map_symbol *to = &self->branch_info->to;
 	return _hist_entry__sym_snprintf(to->map, to->sym, to->addr,

[tip:perf/core] perf sort: Fix --sort pid output

[tip-bot for Namhyung Kim]

Commit-ID:  fb29a338b585ebcce793b8e4a6c62440c4574fa7
Gitweb:     http://git.kernel.org/tip/fb29a338b585ebcce793b8e4a6c62440c4574fa7
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Thu, 27 Dec 2012 18:11:40 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 24 Jan 2013 16:40:21 -0300

perf sort: Fix --sort pid output

The "pid" sort key prints "Command: Pid" output but it's misaligned.
It's because of the offset of 6 was added to the column length during
the calculation in order to reserve an space for Pid part but it isn't
honored when printed.  The output before this patch was like this:

  # Overhead  Command:  Pid      Shared Object
  # ........  .............  .................
  #
      99.70%        noploop:17814  noploop
       0.29%        noploop:17814  [kernel.kallsyms]
       0.01%        noploop:17814  ld-2.15.so

Fix it by subtracting 6 for printing comm part.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Jiri Olsa <jolsa@redhat.com>
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/1356599507-14226-4-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/sort.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 3c2836e..8b6d70b 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -60,7 +60,7 @@ sort__thread_cmp(struct hist_entry *left, struct hist_entry *right)
 static int hist_entry__thread_snprintf(struct hist_entry *self, char *bf,
 				       size_t size, unsigned int width)
 {
-	return repsep_snprintf(bf, size, "%*s:%5d", width,
+	return repsep_snprintf(bf, size, "%*s:%5d", width - 6,
 			      self->thread->comm ?: "", self->thread->pid);
 }
 

[tip:perf/core] perf sort: Align cpu column to right

[tip-bot for Namhyung Kim]

Commit-ID:  dccf180542c4e27621ea526769014dc97b9e5676
Gitweb:     http://git.kernel.org/tip/dccf180542c4e27621ea526769014dc97b9e5676
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Thu, 27 Dec 2012 18:11:41 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 24 Jan 2013 16:40:22 -0300

perf sort: Align cpu column to right

Since cpu number is a natural number, it'd be more appropriate
aligning it to right.

Before:

  # Overhead  CPU      Command:  Pid          Shared Object
  # ........  ...  .................  .....................
  #
       8.91%  8    gnome-shell: 1497  perf-1497.map
       8.90%  7    gnome-shell: 1497  perf-1497.map
       8.86%  9    gnome-shell: 1497  perf-1497.map
       8.83%  6    gnome-shell: 1497  perf-1497.map
       8.81%  10   gnome-shell: 1497  perf-1497.map
       7.44%  5    gnome-shell: 1497  perf-1497.map
       6.20%  3    gnome-shell: 1497  perf-1497.map
       5.10%  0    gnome-shell: 1497  perf-1497.map

After:

  # Overhead  CPU      Command:  Pid          Shared Object
  # ........  ...  .................  .....................
  #
       8.91%    8  gnome-shell: 1497  perf-1497.map
       8.90%    7  gnome-shell: 1497  perf-1497.map
       8.86%    9  gnome-shell: 1497  perf-1497.map
       8.83%    6  gnome-shell: 1497  perf-1497.map
       8.81%   10  gnome-shell: 1497  perf-1497.map
       7.44%    5  gnome-shell: 1497  perf-1497.map
       6.20%    3  gnome-shell: 1497  perf-1497.map
       5.10%    0  gnome-shell: 1497  perf-1497.map

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Jiri Olsa <jolsa@redhat.com>
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/1356599507-14226-5-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/sort.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 8b6d70b..a36051b 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -331,7 +331,7 @@ sort__cpu_cmp(struct hist_entry *left, struct hist_entry *right)
 static int hist_entry__cpu_snprintf(struct hist_entry *self, char *bf,
 				       size_t size, unsigned int width)
 {
-	return repsep_snprintf(bf, size, "%-*d", width, self->cpu);
+	return repsep_snprintf(bf, size, "%*d", width, self->cpu);
 }
 
 struct sort_entry sort_cpu = {

[tip:perf/core] perf sort: Calculate parent column width too

[tip-bot for Namhyung Kim]

Commit-ID:  cb993744554b0a5bd5c46239544cec9fd252a106
Gitweb:     http://git.kernel.org/tip/cb993744554b0a5bd5c46239544cec9fd252a106
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Thu, 27 Dec 2012 18:11:42 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 24 Jan 2013 16:40:24 -0300

perf sort: Calculate parent column width too

When hists__calc_col_len() called, most of column length are refreshed
but it missed parent column.  So if the parent sort key was used along
with other keys rests will be misalinged since parent has no proper
column width.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Jiri Olsa <jolsa@redhat.com>
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/1356599507-14226-6-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/hist.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 965ebf9..9485c70 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -82,6 +82,9 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
 		hists__new_col_len(hists, HISTC_DSO, len);
 	}
 
+	if (h->parent)
+		hists__new_col_len(hists, HISTC_PARENT, h->parent->namelen);
+
 	if (h->branch_info) {
 		int symlen;
 		/*

[tip:perf/core] perf sort: Clean up sort__first_dimension setting

[tip-bot for Namhyung Kim]

Commit-ID:  6f38cf25a6af5b21da1d52a94d9f56f5af2a215b
Gitweb:     http://git.kernel.org/tip/6f38cf25a6af5b21da1d52a94d9f56f5af2a215b
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Thu, 27 Dec 2012 18:11:45 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 24 Jan 2013 16:40:25 -0300

perf sort: Clean up sort__first_dimension setting

It doesn't need to compare to every sort key names since the index
already has the required information.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Jiri Olsa <jolsa@redhat.com>
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/1356599507-14226-9-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/sort.c | 26 ++------------------------
 1 file changed, 2 insertions(+), 24 deletions(-)

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index a36051b..39b1ab0 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -526,30 +526,8 @@ int sort_dimension__add(const char *tok)
 		if (sd->entry->se_collapse)
 			sort__need_collapse = 1;
 
-		if (list_empty(&hist_entry__sort_list)) {
-			if (!strcmp(sd->name, "pid"))
-				sort__first_dimension = SORT_PID;
-			else if (!strcmp(sd->name, "comm"))
-				sort__first_dimension = SORT_COMM;
-			else if (!strcmp(sd->name, "dso"))
-				sort__first_dimension = SORT_DSO;
-			else if (!strcmp(sd->name, "symbol"))
-				sort__first_dimension = SORT_SYM;
-			else if (!strcmp(sd->name, "parent"))
-				sort__first_dimension = SORT_PARENT;
-			else if (!strcmp(sd->name, "cpu"))
-				sort__first_dimension = SORT_CPU;
-			else if (!strcmp(sd->name, "symbol_from"))
-				sort__first_dimension = SORT_SYM_FROM;
-			else if (!strcmp(sd->name, "symbol_to"))
-				sort__first_dimension = SORT_SYM_TO;
-			else if (!strcmp(sd->name, "dso_from"))
-				sort__first_dimension = SORT_DSO_FROM;
-			else if (!strcmp(sd->name, "dso_to"))
-				sort__first_dimension = SORT_DSO_TO;
-			else if (!strcmp(sd->name, "mispredict"))
-				sort__first_dimension = SORT_MISPREDICT;
-		}
+		if (list_empty(&hist_entry__sort_list))
+			sort__first_dimension = i;
 
 		list_add_tail(&sd->entry->list, &hist_entry__sort_list);
 		sd->taken = 1;

[tip:perf/core] perf sort: Separate out branch stack specific sort keys

[tip-bot for Namhyung Kim]

Commit-ID:  fc5871ed0dcf8c76dd1b3b36ff0f70112d2f0e74
Gitweb:     http://git.kernel.org/tip/fc5871ed0dcf8c76dd1b3b36ff0f70112d2f0e74
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Thu, 27 Dec 2012 18:11:46 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 24 Jan 2013 16:40:26 -0300

perf sort: Separate out branch stack specific sort keys

Current perf report gets segmentation fault when a branch stack specific
sort key is provided by --sort option to a perf.data file which contains
no branch infomation.  It's because those sort keys reference branch
info of a hist entry unconditionally.  Maybe we can change it checks
whether such branch info is valid or not.  But if the branch stacks are
not recorded, it'd be nop.  Thus it'd be better to make those keys are
unselectable.

This patch separates those keys to a different dimension array, so that
if user passes such a key to a file which has no branch stack will get
following message rather than a segfault.

  Error: Invalid --sort key: `symbol_from'

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Reported-by: Stefan Beller <stefanbeller@googlemail.com>
Acked-by: Jiri Olsa <jolsa@redhat.com>
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/1356599507-14226-10-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/sort.c | 64 ++++++++++++++++++++++++++++++++++++++++----------
 tools/perf/util/sort.h |  8 +++++--
 2 files changed, 58 insertions(+), 14 deletions(-)

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 39b1ab0..7ad6239 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -480,30 +480,40 @@ struct sort_dimension {
 
 #define DIM(d, n, func) [d] = { .name = n, .entry = &(func) }
 
-static struct sort_dimension sort_dimensions[] = {
+static struct sort_dimension common_sort_dimensions[] = {
 	DIM(SORT_PID, "pid", sort_thread),
 	DIM(SORT_COMM, "comm", sort_comm),
 	DIM(SORT_DSO, "dso", sort_dso),
-	DIM(SORT_DSO_FROM, "dso_from", sort_dso_from),
-	DIM(SORT_DSO_TO, "dso_to", sort_dso_to),
 	DIM(SORT_SYM, "symbol", sort_sym),
-	DIM(SORT_SYM_TO, "symbol_from", sort_sym_from),
-	DIM(SORT_SYM_FROM, "symbol_to", sort_sym_to),
 	DIM(SORT_PARENT, "parent", sort_parent),
 	DIM(SORT_CPU, "cpu", sort_cpu),
-	DIM(SORT_MISPREDICT, "mispredict", sort_mispredict),
 	DIM(SORT_SRCLINE, "srcline", sort_srcline),
 };
 
+#undef DIM
+
+#define DIM(d, n, func) [d - __SORT_BRANCH_STACK] = { .name = n, .entry = &(func) }
+
+static struct sort_dimension bstack_sort_dimensions[] = {
+	DIM(SORT_DSO_FROM, "dso_from", sort_dso_from),
+	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_MISPREDICT, "mispredict", sort_mispredict),
+};
+
+#undef DIM
+
 int sort_dimension__add(const char *tok)
 {
 	unsigned int i;
 
-	for (i = 0; i < ARRAY_SIZE(sort_dimensions); i++) {
-		struct sort_dimension *sd = &sort_dimensions[i];
+	for (i = 0; i < ARRAY_SIZE(common_sort_dimensions); i++) {
+		struct sort_dimension *sd = &common_sort_dimensions[i];
 
 		if (strncasecmp(tok, sd->name, strlen(tok)))
 			continue;
+
 		if (sd->entry == &sort_parent) {
 			int ret = regcomp(&parent_regex, parent_pattern, REG_EXTENDED);
 			if (ret) {
@@ -514,9 +524,7 @@ int sort_dimension__add(const char *tok)
 				return -EINVAL;
 			}
 			sort__has_parent = 1;
-		} else if (sd->entry == &sort_sym ||
-			   sd->entry == &sort_sym_from ||
-			   sd->entry == &sort_sym_to) {
+		} else if (sd->entry == &sort_sym) {
 			sort__has_sym = 1;
 		}
 
@@ -534,6 +542,34 @@ int sort_dimension__add(const char *tok)
 
 		return 0;
 	}
+
+	for (i = 0; i < ARRAY_SIZE(bstack_sort_dimensions); i++) {
+		struct sort_dimension *sd = &bstack_sort_dimensions[i];
+
+		if (strncasecmp(tok, sd->name, strlen(tok)))
+			continue;
+
+		if (sort__branch_mode != 1)
+			return -EINVAL;
+
+		if (sd->entry == &sort_sym_from || sd->entry == &sort_sym_to)
+			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_BRANCH_STACK;
+
+		list_add_tail(&sd->entry->list, &hist_entry__sort_list);
+		sd->taken = 1;
+
+		return 0;
+	}
+
 	return -ESRCH;
 }
 
@@ -543,7 +579,11 @@ void setup_sorting(const char * const usagestr[], const struct option *opts)
 
 	for (tok = strtok_r(str, ", ", &tmp);
 			tok; tok = strtok_r(NULL, ", ", &tmp)) {
-		if (sort_dimension__add(tok) < 0) {
+		int ret = sort_dimension__add(tok);
+		if (ret == -EINVAL) {
+			error("Invalid --sort key: `%s'", tok);
+			usage_with_options(usagestr, opts);
+		} else if (ret == -ESRCH) {
 			error("Unknown --sort key: `%s'", tok);
 			usage_with_options(usagestr, opts);
 		}
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index a1c0d56..e994ad3e 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -122,18 +122,22 @@ static inline void hist_entry__add_pair(struct hist_entry *he,
 }
 
 enum sort_type {
+	/* common sort keys */
 	SORT_PID,
 	SORT_COMM,
 	SORT_DSO,
 	SORT_SYM,
 	SORT_PARENT,
 	SORT_CPU,
-	SORT_DSO_FROM,
+	SORT_SRCLINE,
+
+	/* branch stack specific sort keys */
+	__SORT_BRANCH_STACK,
+	SORT_DSO_FROM = __SORT_BRANCH_STACK,
 	SORT_DSO_TO,
 	SORT_SYM_FROM,
 	SORT_SYM_TO,
 	SORT_MISPREDICT,
-	SORT_SRCLINE,
 };
 
 /*

[tip:perf/core] perf report: Update documentation for sort keys

[tip-bot for Namhyung Kim]

Commit-ID:  9811360ec8b76a68599cb0629cebca026c93cfce
Gitweb:     http://git.kernel.org/tip/9811360ec8b76a68599cb0629cebca026c93cfce
Author:     Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Thu, 27 Dec 2012 18:11:47 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 24 Jan 2013 16:40:28 -0300

perf report: Update documentation for sort keys

Add description of sort keys to the perf-report document and also add
missing cpu and srcline keys to the command line help string.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Acked-by: Jiri Olsa <jolsa@redhat.com>
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/1356599507-14226-11-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Documentation/perf-report.txt | 38 +++++++++++++++++++++++++++++---
 tools/perf/builtin-report.c              |  4 ++--
 2 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
index f4d91be..848a0dc 100644
--- a/tools/perf/Documentation/perf-report.txt
+++ b/tools/perf/Documentation/perf-report.txt
@@ -57,11 +57,44 @@ OPTIONS
 
 -s::
 --sort=::
-	Sort by key(s): pid, comm, dso, symbol, parent, srcline.
+	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.
+
+	Each key has following meaning:
+
+	- comm: command (name) of the task which can be read via /proc/<pid>/comm
+	- pid: command and tid of the task
+	- dso: name of library or module executed at the time of sample
+	- symbol: name of function executed at the time of sample
+	- parent: name of function matched to the parent regex filter. Unmatched
+	entries are displayed as "[other]".
+	- 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.
+
+	By default, comm, dso and symbol keys are used.
+	(i.e. --sort comm,dso,symbol)
+
+	If --branch-stack option is used, following sort keys are also
+	available:
+	dso_from, dso_to, symbol_from, symbol_to, mispredict.
+
+	- dso_from: name of library or module branched from
+	- dso_to: name of library or module branched to
+	- symbol_from: name of function branched from
+	- symbol_to: name 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
+	and symbol_to, see '--branch-stack'.
 
 -p::
 --parent=<regex>::
-        regex filter to identify parent, see: '--sort parent'
+        A regex filter to identify parent. The parent is a caller of this
+	function and searched through the callchain, thus it requires callchain
+	information recorded. The pattern is in the exteneded regex format and
+	defaults to "\^sys_|^do_page_fault", see '--sort parent'.
 
 -x::
 --exclude-other::
@@ -74,7 +107,6 @@ OPTIONS
 
 -t::
 --field-separator=::
-
 	Use a special separator character and don't pad with spaces, replacing
 	all occurrences of this separator in symbol names (and other output)
 	with a '.' character, that thus it's the only non valid separator.
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 13cdf61..47a8644 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -595,8 +595,8 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 	OPT_BOOLEAN(0, "stdio", &report.use_stdio,
 		    "Use the stdio interface"),
 	OPT_STRING('s', "sort", &sort_order, "key[,key2...]",
-		   "sort by key(s): pid, comm, dso, symbol, parent, dso_to,"
-		   " dso_from, symbol_to, symbol_from, mispredict"),
+		   "sort by key(s): pid, comm, dso, symbol, parent, cpu, srcline,"
+		   " dso_to, dso_from, symbol_to, symbol_from, mispredict"),
 	OPT_BOOLEAN(0, "showcpuutilization", &symbol_conf.show_cpu_utilization,
 		    "Show sample percentage for different cpu modes"),
 	OPT_STRING('p', "parent", &parent_pattern, "regex",