Hello,
This is a second version of my sort key cleanup series. I Added
helper function for adding sort key and used unmap_ip() function for
printing symbol address as Jiri commented. But didn't change name of
the sort keys to 'ip'. If you really think I should change, I'm okay
to do it though. I also cleaned up sort__has_sym and sort key eliding
code in this version.
You can get it from 'perf/sort-v4' branch on my tree at:
git://git.kernel.org/pub/scm/linux/kernel/namhyung/linux-perf.git
Any comments are welcome, thanks.
Namhyung
Namhyung Kim (10):
perf sort: Factor out common code in sort_dimension__add()
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
perf sort: Cleanup sort__has_sym setting
perf top: Use sort__has_sym
perf hist browser: Use sort__has_sym
perf sort: Consolidate sort_entry__setup_elide()
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 | 6 +-
tools/perf/builtin-report.c | 28 +----
tools/perf/builtin-top.c | 19 +--
tools/perf/ui/browsers/hists.c | 9 +-
tools/perf/util/hist.c | 17 +--
tools/perf/util/hist.h | 5 +-
tools/perf/util/sort.c | 201 +++++++++++++++++++++++++------
tools/perf/util/sort.h | 25 ++--
tools/perf/util/top.h | 1 -
12 files changed, 218 insertions(+), 102 deletions(-)
--
1.7.11.7
From: Namhyung Kim <[email protected]>
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 <[email protected]>
Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=55561
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/hist.c | 2 ++
tools/perf/util/hist.h | 3 ++-
tools/perf/util/sort.c | 29 +++++++++++++++++++++++++++++
tools/perf/util/sort.h | 1 +
4 files changed, 34 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 1dbf16949250..5640a95b3575 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -342,6 +342,34 @@ 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)
+{
+ struct map *map = self->ms.map;
+ u64 addr = self->ip;
+
+ if (map)
+ addr = map->unmap_ip(map, self->ip);
+
+ return repsep_snprintf(bf, size, "%#*llu", width, addr);
+}
+
+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 +899,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
From: Namhyung Kim <[email protected]>
The same code was duplicate to places, factor them out to common
sort__setup_elide().
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/builtin-diff.c | 4 +---
tools/perf/builtin-report.c | 20 +-------------------
tools/perf/builtin-top.c | 4 +---
tools/perf/util/sort.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
tools/perf/util/sort.h | 3 +--
5 files changed, 47 insertions(+), 29 deletions(-)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 03b56c542bb6..316bf13e59c7 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -611,9 +611,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix __maybe_unused)
setup_pager();
- sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "dso", NULL);
- sort_entry__setup_elide(&sort_comm, symbol_conf.comm_list, "comm", NULL);
- sort_entry__setup_elide(&sort_sym, symbol_conf.sym_list, "symbol", NULL);
+ sort__setup_elide(NULL);
return __cmd_diff();
}
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index c95fd92fbd44..bff244fa4b5d 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -937,25 +937,7 @@ repeat:
report.symbol_filter_str = argv[0];
}
- sort_entry__setup_elide(&sort_comm, symbol_conf.comm_list, "comm", stdout);
-
- 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);
- sort_entry__setup_elide(&sort_sym_to, symbol_conf.sym_to_list, "sym_to", stdout);
- } else {
- if (report.mem_mode) {
- sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "symbol_daddr", stdout);
- sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "dso_daddr", stdout);
- sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "mem", stdout);
- sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "local_weight", stdout);
- sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "tlb", stdout);
- sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "snoop", stdout);
- }
- sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "dso", stdout);
- sort_entry__setup_elide(&sort_sym, symbol_conf.sym_list, "symbol", stdout);
- }
+ sort__setup_elide(stdout);
ret = __cmd_report(&report);
if (ret == K_SWITCH_INPUT_DATA) {
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 4aa504baaf0b..fe4acf568483 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1201,9 +1201,7 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
if (symbol__init() < 0)
return -1;
- sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "dso", stdout);
- sort_entry__setup_elide(&sort_comm, symbol_conf.comm_list, "comm", stdout);
- sort_entry__setup_elide(&sort_sym, symbol_conf.sym_list, "symbol", stdout);
+ sort__setup_elide(stdout);
get_term_dimensions(&top.winsize);
if (top.print_entries == 0) {
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 213831133e08..86ae94d8782e 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -1,5 +1,6 @@
#include "sort.h"
#include "hist.h"
+#include "symbol.h"
regex_t parent_regex;
const char default_parent_pattern[] = "^sys_|^do_page_fault";
@@ -1085,8 +1086,9 @@ int setup_sorting(void)
return ret;
}
-void sort_entry__setup_elide(struct sort_entry *self, struct strlist *list,
- const char *list_name, FILE *fp)
+static void sort_entry__setup_elide(struct sort_entry *self,
+ struct strlist *list,
+ const char *list_name, FILE *fp)
{
if (list && strlist__nr_entries(list) == 1) {
if (fp != NULL)
@@ -1095,3 +1097,42 @@ void sort_entry__setup_elide(struct sort_entry *self, struct strlist *list,
self->elide = true;
}
}
+
+void sort__setup_elide(FILE *output)
+{
+ sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list,
+ "dso", output);
+ sort_entry__setup_elide(&sort_comm, symbol_conf.comm_list,
+ "comm", output);
+ sort_entry__setup_elide(&sort_sym, symbol_conf.sym_list,
+ "symbol", output);
+
+ if (sort__mode == SORT_MODE__BRANCH) {
+ sort_entry__setup_elide(&sort_dso_from,
+ symbol_conf.dso_from_list,
+ "dso_from", output);
+ sort_entry__setup_elide(&sort_dso_to,
+ symbol_conf.dso_to_list,
+ "dso_to", output);
+ sort_entry__setup_elide(&sort_sym_from,
+ symbol_conf.sym_from_list,
+ "sym_from", output);
+ sort_entry__setup_elide(&sort_sym_to,
+ symbol_conf.sym_to_list,
+ "sym_to", output);
+ } else if (sort__mode == SORT_MODE__MEMORY) {
+ sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list,
+ "symbol_daddr", output);
+ sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list,
+ "dso_daddr", output);
+ sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list,
+ "mem", output);
+ sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list,
+ "local_weight", output);
+ sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list,
+ "tlb", output);
+ sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list,
+ "snoop", output);
+ }
+
+}
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 1f945a3b2e5b..c80aac4ae3a2 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -184,7 +184,6 @@ extern struct list_head hist_entry__sort_list;
int setup_sorting(void);
extern int sort_dimension__add(const char *);
-void sort_entry__setup_elide(struct sort_entry *self, struct strlist *list,
- const char *list_name, FILE *fp);
+void sort__setup_elide(FILE *fp);
#endif /* __PERF_SORT_H */
--
1.7.11.7
From: Namhyung Kim <[email protected]>
The TUI hist browser had a similar variable has_symbols for the same
purpose. Let's get rid of the duplication.
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/ui/browsers/hists.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index cad8e37f05d9..a4268cab1921 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -25,7 +25,6 @@ struct hist_browser {
struct map_symbol *selection;
int print_seq;
bool show_dso;
- bool has_symbols;
};
extern void hist_browser__init_hpp(void);
@@ -1155,10 +1154,6 @@ 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__mode == SORT_MODE__BRANCH)
- browser->has_symbols = sort_sym_from.list.next != NULL;
- else
- browser->has_symbols = sort_sym.list.next != NULL;
}
return browser;
@@ -1386,7 +1381,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
*/
goto out_free_stack;
case 'a':
- if (!browser->has_symbols) {
+ if (!sort__has_sym) {
ui_browser__warning(&browser->b, delay_secs * 2,
"Annotation is only available for symbolic views, "
"include \"sym*\" in --sort to use it.");
@@ -1485,7 +1480,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
continue;
}
- if (!browser->has_symbols)
+ if (!sort__has_sym)
goto add_exit_option;
if (sort__mode == SORT_MODE__BRANCH) {
--
1.7.11.7
From: Namhyung Kim <[email protected]>
perf top had a similar variable sort_has_symbols for the same purpose.
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/builtin-top.c | 12 +++---------
tools/perf/util/top.h | 1 -
2 files changed, 3 insertions(+), 10 deletions(-)
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 9aae3f518f39..4aa504baaf0b 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -794,7 +794,7 @@ static void perf_event__process_sample(struct perf_tool *tool,
return;
}
- if (top->sort_has_symbols)
+ if (sort__has_sym)
perf_top__record_precise_ip(top, he, evsel->idx, ip);
}
@@ -912,9 +912,9 @@ out_err:
return -1;
}
-static int perf_top__setup_sample_type(struct perf_top *top)
+static int perf_top__setup_sample_type(struct perf_top *top __maybe_unused)
{
- if (!top->sort_has_symbols) {
+ if (!sort__has_sym) {
if (symbol_conf.use_callchain) {
ui__error("Selected -g but \"sym\" not present in --sort/-s.");
return -EINVAL;
@@ -1205,12 +1205,6 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
sort_entry__setup_elide(&sort_comm, symbol_conf.comm_list, "comm", stdout);
sort_entry__setup_elide(&sort_sym, symbol_conf.sym_list, "symbol", stdout);
- /*
- * Avoid annotation data structures overhead when symbols aren't on the
- * sort list.
- */
- top.sort_has_symbols = sort_sym.list.next != NULL;
-
get_term_dimensions(&top.winsize);
if (top.print_entries == 0) {
struct sigaction act = {
diff --git a/tools/perf/util/top.h b/tools/perf/util/top.h
index 7ebf357dc9e1..f0a862539ba9 100644
--- a/tools/perf/util/top.h
+++ b/tools/perf/util/top.h
@@ -26,7 +26,6 @@ struct perf_top {
int print_entries, count_filter, delay_secs;
bool hide_kernel_symbols, hide_user_symbols, zero;
bool use_tui, use_stdio;
- bool sort_has_symbols;
bool kptr_restrict_warned;
bool vmlinux_warned;
bool dump_symtab;
--
1.7.11.7
From: Namhyung Kim <[email protected]>
They are set to constant length so no need to update every time.
Cc: Stephane Eranian <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
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
From: Namhyung Kim <[email protected]>
The sort__has_sym variable is set only if a symbol-related sort key
was added. Since branch stack and memory sort dimensions are
separated, it doesn't need to be checked from common dimension.
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/sort.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 7f66c822d8bd..213831133e08 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -1016,10 +1016,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 ||
- sd->entry == &sort_mem_daddr_sym) {
+ } else if (sd->entry == &sort_sym) {
sort__has_sym = 1;
}
--
1.7.11.7
From: Namhyung Kim <[email protected]>
Let's remove duplicate code.
Suggested-by: Jiri Olsa <[email protected]>
Cc: Stephane Eranian <[email protected]>
Cc: Andi Kleen <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/util/sort.c | 41 +++++++++++++++++------------------------
1 file changed, 17 insertions(+), 24 deletions(-)
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index a6ddad41d57a..a997955085eb 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -895,6 +895,21 @@ static struct sort_dimension bstack_sort_dimensions[] = {
#undef DIM
+static void __sort_dimension__add(struct sort_dimension *sd, enum sort_type idx)
+{
+ if (sd->taken)
+ return;
+
+ if (sd->entry->se_collapse)
+ sort__need_collapse = 1;
+
+ if (list_empty(&hist_entry__sort_list))
+ sort__first_dimension = idx;
+
+ list_add_tail(&sd->entry->list, &hist_entry__sort_list);
+ sd->taken = 1;
+}
+
int sort_dimension__add(const char *tok)
{
unsigned int i;
@@ -922,18 +937,7 @@ int sort_dimension__add(const char *tok)
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;
-
- list_add_tail(&sd->entry->list, &hist_entry__sort_list);
- sd->taken = 1;
-
+ __sort_dimension__add(sd, i);
return 0;
}
@@ -949,18 +953,7 @@ int sort_dimension__add(const char *tok)
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;
-
+ __sort_dimension__add(sd, i + __SORT_BRANCH_STACK);
return 0;
}
--
1.7.11.7
From: Namhyung Kim <[email protected]>
Update and add missing description of new sort keys.
Cc: Stephane Eranian <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
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
From: Namhyung Kim <[email protected]>
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 <[email protected]>
Cc: Stephane Eranian <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/builtin-report.c | 2 ++
tools/perf/util/sort.c | 39 +++++++++++++++++++++++++++++++--------
tools/perf/util/sort.h | 19 +++++++++++--------
3 files changed, 44 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 a997955085eb..1dbf16949250 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
+
static void __sort_dimension__add(struct sort_dimension *sd, enum sort_type idx)
{
if (sd->taken)
@@ -957,6 +964,22 @@ 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;
+
+ __sort_dimension__add(sd, i + __SORT_MEMORY_MODE);
+ 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
From: Namhyung Kim <[email protected]>
New addr_{to,from} sort keys provide a way to sort the entries by the
source/target symbol addresses.
Cc: Stephane Eranian <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
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 5640a95b3575..7f66c822d8bd 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -442,6 +442,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,
@@ -470,6 +504,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)
{
@@ -911,6 +959,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
What I expected was that the result was this:
perf report --sort addr | grep -v ^# | sort -k2 -n | less
And in hexadecimal, can you fix this?
- Arnaldo
Em Wed, Apr 03, 2013 at 09:26:12PM +0900, Namhyung Kim escreveu:
> 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,
Why move 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 1dbf16949250..5640a95b3575 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -342,6 +342,34 @@ 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)
> +{
> + struct map *map = self->ms.map;
> + u64 addr = self->ip;
> +
> + if (map)
> + addr = map->unmap_ip(map, self->ip);
> +
> + return repsep_snprintf(bf, size, "%#*llu", width, addr);
> +}
> +
> +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 +899,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
Em Wed, Apr 03, 2013 at 09:26:18PM +0900, Namhyung Kim escreveu:
> From: Namhyung Kim <[email protected]>
>
> The TUI hist browser had a similar variable has_symbols for the same
> purpose. Let's get rid of the duplication.
I'm ok with that, if it involves removing sort__has_sym, that is a
global variable, making it impossible to use different sort orders in
the same session, if we ever want to do that :-)
- Arnaldo
> Signed-off-by: Namhyung Kim <[email protected]>
> ---
> tools/perf/ui/browsers/hists.c | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index cad8e37f05d9..a4268cab1921 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -25,7 +25,6 @@ struct hist_browser {
> struct map_symbol *selection;
> int print_seq;
> bool show_dso;
> - bool has_symbols;
> };
>
> extern void hist_browser__init_hpp(void);
> @@ -1155,10 +1154,6 @@ 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__mode == SORT_MODE__BRANCH)
> - browser->has_symbols = sort_sym_from.list.next != NULL;
> - else
> - browser->has_symbols = sort_sym.list.next != NULL;
> }
>
> return browser;
> @@ -1386,7 +1381,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
> */
> goto out_free_stack;
> case 'a':
> - if (!browser->has_symbols) {
> + if (!sort__has_sym) {
> ui_browser__warning(&browser->b, delay_secs * 2,
> "Annotation is only available for symbolic views, "
> "include \"sym*\" in --sort to use it.");
> @@ -1485,7 +1480,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
> continue;
> }
>
> - if (!browser->has_symbols)
> + if (!sort__has_sym)
> goto add_exit_option;
>
> if (sort__mode == SORT_MODE__BRANCH) {
> --
> 1.7.11.7
Em Wed, Apr 03, 2013 at 09:26:19PM +0900, Namhyung Kim escreveu:
> From: Namhyung Kim <[email protected]>
>
> The same code was duplicate to places, factor them out to common
> sort__setup_elide().
Looks ok, applying after fixing up fuzzes due to this being at the end
of the patchseries. Things like this that are clear cleanups are best
positioned in the start of the patch series.
- Arnaldo
> Signed-off-by: Namhyung Kim <[email protected]>
> ---
> tools/perf/builtin-diff.c | 4 +---
> tools/perf/builtin-report.c | 20 +-------------------
> tools/perf/builtin-top.c | 4 +---
> tools/perf/util/sort.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
> tools/perf/util/sort.h | 3 +--
> 5 files changed, 47 insertions(+), 29 deletions(-)
>
> diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
> index 03b56c542bb6..316bf13e59c7 100644
> --- a/tools/perf/builtin-diff.c
> +++ b/tools/perf/builtin-diff.c
> @@ -611,9 +611,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix __maybe_unused)
>
> setup_pager();
>
> - sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "dso", NULL);
> - sort_entry__setup_elide(&sort_comm, symbol_conf.comm_list, "comm", NULL);
> - sort_entry__setup_elide(&sort_sym, symbol_conf.sym_list, "symbol", NULL);
> + sort__setup_elide(NULL);
>
> return __cmd_diff();
> }
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index c95fd92fbd44..bff244fa4b5d 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -937,25 +937,7 @@ repeat:
> report.symbol_filter_str = argv[0];
> }
>
> - sort_entry__setup_elide(&sort_comm, symbol_conf.comm_list, "comm", stdout);
> -
> - 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);
> - sort_entry__setup_elide(&sort_sym_to, symbol_conf.sym_to_list, "sym_to", stdout);
> - } else {
> - if (report.mem_mode) {
> - sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "symbol_daddr", stdout);
> - sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "dso_daddr", stdout);
> - sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "mem", stdout);
> - sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "local_weight", stdout);
> - sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "tlb", stdout);
> - sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "snoop", stdout);
> - }
> - sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "dso", stdout);
> - sort_entry__setup_elide(&sort_sym, symbol_conf.sym_list, "symbol", stdout);
> - }
> + sort__setup_elide(stdout);
>
> ret = __cmd_report(&report);
> if (ret == K_SWITCH_INPUT_DATA) {
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index 4aa504baaf0b..fe4acf568483 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -1201,9 +1201,7 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
> if (symbol__init() < 0)
> return -1;
>
> - sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "dso", stdout);
> - sort_entry__setup_elide(&sort_comm, symbol_conf.comm_list, "comm", stdout);
> - sort_entry__setup_elide(&sort_sym, symbol_conf.sym_list, "symbol", stdout);
> + sort__setup_elide(stdout);
>
> get_term_dimensions(&top.winsize);
> if (top.print_entries == 0) {
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index 213831133e08..86ae94d8782e 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -1,5 +1,6 @@
> #include "sort.h"
> #include "hist.h"
> +#include "symbol.h"
>
> regex_t parent_regex;
> const char default_parent_pattern[] = "^sys_|^do_page_fault";
> @@ -1085,8 +1086,9 @@ int setup_sorting(void)
> return ret;
> }
>
> -void sort_entry__setup_elide(struct sort_entry *self, struct strlist *list,
> - const char *list_name, FILE *fp)
> +static void sort_entry__setup_elide(struct sort_entry *self,
> + struct strlist *list,
> + const char *list_name, FILE *fp)
> {
> if (list && strlist__nr_entries(list) == 1) {
> if (fp != NULL)
> @@ -1095,3 +1097,42 @@ void sort_entry__setup_elide(struct sort_entry *self, struct strlist *list,
> self->elide = true;
> }
> }
> +
> +void sort__setup_elide(FILE *output)
> +{
> + sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list,
> + "dso", output);
> + sort_entry__setup_elide(&sort_comm, symbol_conf.comm_list,
> + "comm", output);
> + sort_entry__setup_elide(&sort_sym, symbol_conf.sym_list,
> + "symbol", output);
> +
> + if (sort__mode == SORT_MODE__BRANCH) {
> + sort_entry__setup_elide(&sort_dso_from,
> + symbol_conf.dso_from_list,
> + "dso_from", output);
> + sort_entry__setup_elide(&sort_dso_to,
> + symbol_conf.dso_to_list,
> + "dso_to", output);
> + sort_entry__setup_elide(&sort_sym_from,
> + symbol_conf.sym_from_list,
> + "sym_from", output);
> + sort_entry__setup_elide(&sort_sym_to,
> + symbol_conf.sym_to_list,
> + "sym_to", output);
> + } else if (sort__mode == SORT_MODE__MEMORY) {
> + sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list,
> + "symbol_daddr", output);
> + sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list,
> + "dso_daddr", output);
> + sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list,
> + "mem", output);
> + sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list,
> + "local_weight", output);
> + sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list,
> + "tlb", output);
> + sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list,
> + "snoop", output);
> + }
> +
> +}
> diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
> index 1f945a3b2e5b..c80aac4ae3a2 100644
> --- a/tools/perf/util/sort.h
> +++ b/tools/perf/util/sort.h
> @@ -184,7 +184,6 @@ extern struct list_head hist_entry__sort_list;
>
> int setup_sorting(void);
> extern int sort_dimension__add(const char *);
> -void sort_entry__setup_elide(struct sort_entry *self, struct strlist *list,
> - const char *list_name, FILE *fp);
> +void sort__setup_elide(FILE *fp);
>
> #endif /* __PERF_SORT_H */
> --
> 1.7.11.7
Hi Arnaldo,
On Wed, 3 Apr 2013 14:06:10 -0300, Arnaldo Carvalho de Melo wrote:
> What I expected was that the result was this:
>
> perf report --sort addr | grep -v ^# | sort -k2 -n | less
>
> And in hexadecimal, can you fix this?
Oops, it was a mistake in the last minute change, sorry. :(
>
> Em Wed, Apr 03, 2013 at 09:26:12PM +0900, Namhyung Kim escreveu:
>> 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,
>
> Why move SRCLINE?
Because it's in common dimension. I'd like to separate it to give a
consistent view.
Thanks,
Namhyung
>
>> + 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 1dbf16949250..5640a95b3575 100644
>> --- a/tools/perf/util/sort.c
>> +++ b/tools/perf/util/sort.c
>> @@ -342,6 +342,34 @@ 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)
>> +{
>> + struct map *map = self->ms.map;
>> + u64 addr = self->ip;
>> +
>> + if (map)
>> + addr = map->unmap_ip(map, self->ip);
>> +
>> + return repsep_snprintf(bf, size, "%#*llu", width, addr);
>> +}
>> +
>> +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 +899,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
On Wed, 3 Apr 2013 14:07:54 -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, Apr 03, 2013 at 09:26:18PM +0900, Namhyung Kim escreveu:
>> From: Namhyung Kim <[email protected]>
>>
>> The TUI hist browser had a similar variable has_symbols for the same
>> purpose. Let's get rid of the duplication.
>
> I'm ok with that, if it involves removing sort__has_sym, that is a
> global variable, making it impossible to use different sort orders in
> the same session, if we ever want to do that :-)
So you want to keep current code for a potential future change?
Anyway it seems current logic would fail if only "symbol_to" sort key
was used without "symbol_from" in branch mode.
Thanks,
Namhyung
>
>> Signed-off-by: Namhyung Kim <[email protected]>
>> ---
>> tools/perf/ui/browsers/hists.c | 9 ++-------
>> 1 file changed, 2 insertions(+), 7 deletions(-)
>>
>> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
>> index cad8e37f05d9..a4268cab1921 100644
>> --- a/tools/perf/ui/browsers/hists.c
>> +++ b/tools/perf/ui/browsers/hists.c
>> @@ -25,7 +25,6 @@ struct hist_browser {
>> struct map_symbol *selection;
>> int print_seq;
>> bool show_dso;
>> - bool has_symbols;
>> };
>>
>> extern void hist_browser__init_hpp(void);
>> @@ -1155,10 +1154,6 @@ 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__mode == SORT_MODE__BRANCH)
>> - browser->has_symbols = sort_sym_from.list.next != NULL;
>> - else
>> - browser->has_symbols = sort_sym.list.next != NULL;
>> }
>>
>> return browser;
>> @@ -1386,7 +1381,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
>> */
>> goto out_free_stack;
>> case 'a':
>> - if (!browser->has_symbols) {
>> + if (!sort__has_sym) {
>> ui_browser__warning(&browser->b, delay_secs * 2,
>> "Annotation is only available for symbolic views, "
>> "include \"sym*\" in --sort to use it.");
>> @@ -1485,7 +1480,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
>> continue;
>> }
>>
>> - if (!browser->has_symbols)
>> + if (!sort__has_sym)
>> goto add_exit_option;
>>
>> if (sort__mode == SORT_MODE__BRANCH) {
>> --
>> 1.7.11.7
On Wed, 3 Apr 2013 14:09:44 -0300, Arnaldo Carvalho de Melo wrote:
> Em Wed, Apr 03, 2013 at 09:26:19PM +0900, Namhyung Kim escreveu:
>> From: Namhyung Kim <[email protected]>
>>
>> The same code was duplicate to places, factor them out to common
>> sort__setup_elide().
>
> Looks ok, applying after fixing up fuzzes due to this being at the end
> of the patchseries. Things like this that are clear cleanups are best
> positioned in the start of the patch series.
Will do that next time!
Thanks,
Namhyung
Commit-ID: 2f532d09fa3a7eaf7cf1c23de9767eab8c8c0e7e
Gitweb: http://git.kernel.org/tip/2f532d09fa3a7eaf7cf1c23de9767eab8c8c0e7e
Author: Namhyung Kim <[email protected]>
AuthorDate: Wed, 3 Apr 2013 21:26:10 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 28 May 2013 16:23:53 +0300
perf sort: Factor out common code in sort_dimension__add()
Let's remove duplicate code.
Suggested-by: Jiri Olsa <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/sort.c | 41 +++++++++++++++++------------------------
1 file changed, 17 insertions(+), 24 deletions(-)
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index a6ddad4..a997955 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -895,6 +895,21 @@ static struct sort_dimension bstack_sort_dimensions[] = {
#undef DIM
+static void __sort_dimension__add(struct sort_dimension *sd, enum sort_type idx)
+{
+ if (sd->taken)
+ return;
+
+ if (sd->entry->se_collapse)
+ sort__need_collapse = 1;
+
+ if (list_empty(&hist_entry__sort_list))
+ sort__first_dimension = idx;
+
+ list_add_tail(&sd->entry->list, &hist_entry__sort_list);
+ sd->taken = 1;
+}
+
int sort_dimension__add(const char *tok)
{
unsigned int i;
@@ -922,18 +937,7 @@ int sort_dimension__add(const char *tok)
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;
-
- list_add_tail(&sd->entry->list, &hist_entry__sort_list);
- sd->taken = 1;
-
+ __sort_dimension__add(sd, i);
return 0;
}
@@ -949,18 +953,7 @@ int sort_dimension__add(const char *tok)
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;
-
+ __sort_dimension__add(sd, i + __SORT_BRANCH_STACK);
return 0;
}
Commit-ID: afab87b91f3f331d55664172dad8e476e6ffca9d
Gitweb: http://git.kernel.org/tip/afab87b91f3f331d55664172dad8e476e6ffca9d
Author: Namhyung Kim <[email protected]>
AuthorDate: Wed, 3 Apr 2013 21:26:11 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 28 May 2013 16:23:54 +0300
perf sort: Separate out memory-specific sort keys
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.
Signed-off-by: Namhyung Kim <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-report.c | 2 ++
tools/perf/util/sort.c | 39 +++++++++++++++++++++++++++++++--------
tools/perf/util/sort.h | 19 +++++++++++--------
3 files changed, 44 insertions(+), 16 deletions(-)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index c877982..669405c 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 a997955..1dbf169 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
+
static void __sort_dimension__add(struct sort_dimension *sd, enum sort_type idx)
{
if (sd->taken)
@@ -957,6 +964,22 @@ 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;
+
+ __sort_dimension__add(sd, i + __SORT_MEMORY_MODE);
+ return 0;
+ }
+
return -ESRCH;
}
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 39ff4b8..0232d47 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,
};
/*
Commit-ID: 08e71542fd0f4a0e30b4e3794329d63ae891e0c0
Gitweb: http://git.kernel.org/tip/08e71542fd0f4a0e30b4e3794329d63ae891e0c0
Author: Namhyung Kim <[email protected]>
AuthorDate: Wed, 3 Apr 2013 21:26:19 +0900
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 28 May 2013 16:23:54 +0300
perf sort: Consolidate sort_entry__setup_elide()
The same code was duplicate to places, factor them out to common
sort__setup_elide().
Signed-off-by: Namhyung Kim <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-diff.c | 4 +---
tools/perf/builtin-report.c | 20 +-------------------
tools/perf/builtin-top.c | 4 +---
tools/perf/util/sort.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
tools/perf/util/sort.h | 3 +--
5 files changed, 47 insertions(+), 29 deletions(-)
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 2d0462d..cabbea5 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -611,9 +611,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix __maybe_unused)
setup_pager();
- sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "dso", NULL);
- sort_entry__setup_elide(&sort_comm, symbol_conf.comm_list, "comm", NULL);
- sort_entry__setup_elide(&sort_sym, symbol_conf.sym_list, "symbol", NULL);
+ sort__setup_elide(NULL);
return __cmd_diff();
}
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 669405c..d45bf9b 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -937,25 +937,7 @@ repeat:
report.symbol_filter_str = argv[0];
}
- sort_entry__setup_elide(&sort_comm, symbol_conf.comm_list, "comm", stdout);
-
- 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);
- sort_entry__setup_elide(&sort_sym_to, symbol_conf.sym_to_list, "sym_to", stdout);
- } else {
- if (report.mem_mode) {
- sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "symbol_daddr", stdout);
- sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "dso_daddr", stdout);
- sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "mem", stdout);
- sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "local_weight", stdout);
- sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "tlb", stdout);
- sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "snoop", stdout);
- }
- sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "dso", stdout);
- sort_entry__setup_elide(&sort_sym, symbol_conf.sym_list, "symbol", stdout);
- }
+ sort__setup_elide(stdout);
ret = __cmd_report(&report);
if (ret == K_SWITCH_INPUT_DATA) {
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 67bdb9f..2eb272d 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1200,9 +1200,7 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
if (symbol__init() < 0)
return -1;
- sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "dso", stdout);
- sort_entry__setup_elide(&sort_comm, symbol_conf.comm_list, "comm", stdout);
- sort_entry__setup_elide(&sort_sym, symbol_conf.sym_list, "symbol", stdout);
+ sort__setup_elide(stdout);
/*
* Avoid annotation data structures overhead when symbols aren't on the
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 1dbf169..701ab1d 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -1,5 +1,6 @@
#include "sort.h"
#include "hist.h"
+#include "symbol.h"
regex_t parent_regex;
const char default_parent_pattern[] = "^sys_|^do_page_fault";
@@ -1009,8 +1010,9 @@ int setup_sorting(void)
return ret;
}
-void sort_entry__setup_elide(struct sort_entry *self, struct strlist *list,
- const char *list_name, FILE *fp)
+static void sort_entry__setup_elide(struct sort_entry *self,
+ struct strlist *list,
+ const char *list_name, FILE *fp)
{
if (list && strlist__nr_entries(list) == 1) {
if (fp != NULL)
@@ -1019,3 +1021,42 @@ void sort_entry__setup_elide(struct sort_entry *self, struct strlist *list,
self->elide = true;
}
}
+
+void sort__setup_elide(FILE *output)
+{
+ sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list,
+ "dso", output);
+ sort_entry__setup_elide(&sort_comm, symbol_conf.comm_list,
+ "comm", output);
+ sort_entry__setup_elide(&sort_sym, symbol_conf.sym_list,
+ "symbol", output);
+
+ if (sort__mode == SORT_MODE__BRANCH) {
+ sort_entry__setup_elide(&sort_dso_from,
+ symbol_conf.dso_from_list,
+ "dso_from", output);
+ sort_entry__setup_elide(&sort_dso_to,
+ symbol_conf.dso_to_list,
+ "dso_to", output);
+ sort_entry__setup_elide(&sort_sym_from,
+ symbol_conf.sym_from_list,
+ "sym_from", output);
+ sort_entry__setup_elide(&sort_sym_to,
+ symbol_conf.sym_to_list,
+ "sym_to", output);
+ } else if (sort__mode == SORT_MODE__MEMORY) {
+ sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list,
+ "symbol_daddr", output);
+ sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list,
+ "dso_daddr", output);
+ sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list,
+ "mem", output);
+ sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list,
+ "local_weight", output);
+ sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list,
+ "tlb", output);
+ sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list,
+ "snoop", output);
+ }
+
+}
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 0232d47..51f1b5a 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -181,7 +181,6 @@ extern struct list_head hist_entry__sort_list;
int setup_sorting(void);
extern int sort_dimension__add(const char *);
-void sort_entry__setup_elide(struct sort_entry *self, struct strlist *list,
- const char *list_name, FILE *fp);
+void sort__setup_elide(FILE *fp);
#endif /* __PERF_SORT_H */
On Fri, May 31, 2013 at 04:20:20AM -0700, tip-bot for Namhyung Kim wrote:
> perf sort: Separate out memory-specific sort keys
>
> 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.
So I was finally able to test with this patch, but
I found it completely breaks TSX weight abort profiling.
It uses weight (global/local), but it's not
running in memory mode.
I'll send a patch to move weight back into
the common keys.
-Andi
Hi Andi,
2013-06-25 AM 9:30, Andi Kleen wrote:
> On Fri, May 31, 2013 at 04:20:20AM -0700, tip-bot for Namhyung Kim wrote:
>> perf sort: Separate out memory-specific sort keys
>>
>> 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.
>
> So I was finally able to test with this patch, but
> I found it completely breaks TSX weight abort profiling.
>
> It uses weight (global/local), but it's not
> running in memory mode.
>
> I'll send a patch to move weight back into
> the common keys.
I'm not sure it should move to the common keys as normal perf session
won't have those. I guess you need to set up a couple of TSX-specific
sort keys like perf mem, if so what about moving the two weights to your
table?
Thanks,
Namhyung
> I'm not sure it should move to the common keys as normal perf
> session won't have those.
Why not? If I enable weight sampling i get weights perfectly
fine in any session using the right PEBS events.
> I guess you need to set up a couple of
> TSX-specific sort keys like perf mem, if so what about moving the
> two weights to your table?
There's no TSX table and no separate perf TSX session.
It's just some additional attributes.
-Andi
2013-06-25 AM 10:02, Andi Kleen wrote:
>> I'm not sure it should move to the common keys as normal perf
>> session won't have those.
>
> Why not? If I enable weight sampling i get weights perfectly
> fine in any session using the right PEBS events.
>
>> I guess you need to set up a couple of
>> TSX-specific sort keys like perf mem, if so what about moving the
>> two weights to your table?
>
> There's no TSX table and no separate perf TSX session.
> It's just some additional attributes.
Okay then, I have no objection for moving them. :)
Thanks,
Namhyung