* [PATCH 2/3] perf report: Enable itegrated annotation only if possible
2012-09-14 8:35 [PATCH 1/3] perf tools: Add sort__has_sym Namhyung Kim
@ 2012-09-14 8:35 ` Namhyung Kim
2012-09-19 15:16 ` [tip:perf/core] perf report: Enable integrated " tip-bot for Namhyung Kim
2012-09-14 8:35 ` [PATCH 3/3] perf ui/browser: Fix stale output of sorted result Namhyung Kim
2012-09-19 15:15 ` [tip:perf/core] perf tools: Add sort__has_sym tip-bot for Namhyung Kim
2 siblings, 1 reply; 6+ messages in thread
From: Namhyung Kim @ 2012-09-14 8:35 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, LKML, Namhyung Kim
From: Namhyung Kim <namhyung.kim@lge.com>
The integrated annotation feature is supported only in TUI mode. Also
it should be enabled with 'symbol' sort key otherwise resulting hist
entry doesn't need to have same symbol as of a sample so that it can
fail on hist_entry__inc_addr_samples with -ERANGE.
You can easily see it when start perf report TUI without symbol* sort
key. This patch fixes the problem.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/builtin-report.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 279155a47d1c..1da243dfbc3e 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -93,7 +93,7 @@ static int perf_report__add_branch_hist_entry(struct perf_tool *tool,
struct annotation *notes;
err = -ENOMEM;
bx = he->branch_info;
- if (bx->from.sym && use_browser > 0) {
+ if (bx->from.sym && use_browser == 1 && sort__has_sym) {
notes = symbol__annotation(bx->from.sym);
if (!notes->src
&& symbol__alloc_hist(bx->from.sym) < 0)
@@ -107,7 +107,7 @@ static int perf_report__add_branch_hist_entry(struct perf_tool *tool,
goto out;
}
- if (bx->to.sym && use_browser > 0) {
+ if (bx->to.sym && use_browser == 1 && sort__has_sym) {
notes = symbol__annotation(bx->to.sym);
if (!notes->src
&& symbol__alloc_hist(bx->to.sym) < 0)
@@ -162,7 +162,7 @@ static int perf_evsel__add_hist_entry(struct perf_evsel *evsel,
* so we don't allocated the extra space needed because the stdio
* code will not use it.
*/
- if (he->ms.sym != NULL && use_browser > 0) {
+ if (he->ms.sym != NULL && use_browser == 1 && sort__has_sym) {
struct annotation *notes = symbol__annotation(he->ms.sym);
assert(evsel != NULL);
@@ -694,12 +694,14 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
perf_hpp__init(false, false);
}
+ setup_sorting(report_usage, options);
+
/*
* Only in the newt browser we are doing integrated annotation,
* so don't allocate extra space that won't be used in the stdio
* implementation.
*/
- if (use_browser > 0) {
+ if (use_browser == 1 && sort__has_sym) {
symbol_conf.priv_size = sizeof(struct annotation);
report.annotate_init = symbol__annotate_init;
/*
@@ -722,8 +724,6 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
if (symbol__init() < 0)
goto error;
- setup_sorting(report_usage, options);
-
if (parent_pattern != default_parent_pattern) {
if (sort_dimension__add("parent") < 0)
goto error;
--
1.7.11.4
^ permalink raw reply [flat|nested] 6+ messages in thread* [tip:perf/core] perf report: Enable integrated annotation only if possible
2012-09-14 8:35 ` [PATCH 2/3] perf report: Enable itegrated annotation only if possible Namhyung Kim
@ 2012-09-19 15:16 ` tip-bot for Namhyung Kim
0 siblings, 0 replies; 6+ messages in thread
From: tip-bot for Namhyung Kim @ 2012-09-19 15:16 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, paulus, hpa, mingo, a.p.zijlstra,
namhyung.kim, namhyung, tglx
Commit-ID: 034a9265c289d5298bac7bfd824d3d5b9ec892b4
Gitweb: http://git.kernel.org/tip/034a9265c289d5298bac7bfd824d3d5b9ec892b4
Author: Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Fri, 14 Sep 2012 17:35:28 +0900
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 17 Sep 2012 13:09:43 -0300
perf report: Enable integrated annotation only if possible
The integrated annotation feature is supported only in TUI mode. Also
it should be enabled with 'symbol' sort key otherwise resulting hist
entry doesn't need to have same symbol as of a sample so that it can
fail on hist_entry__inc_addr_samples with -ERANGE.
You can easily see it when start perf report TUI without symbol* sort
key. This patch fixes the problem.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1347611729-16994-2-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-report.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 97b2e63..b6696dd 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -93,7 +93,7 @@ static int perf_report__add_branch_hist_entry(struct perf_tool *tool,
struct annotation *notes;
err = -ENOMEM;
bx = he->branch_info;
- if (bx->from.sym && use_browser > 0) {
+ if (bx->from.sym && use_browser == 1 && sort__has_sym) {
notes = symbol__annotation(bx->from.sym);
if (!notes->src
&& symbol__alloc_hist(bx->from.sym) < 0)
@@ -107,7 +107,7 @@ static int perf_report__add_branch_hist_entry(struct perf_tool *tool,
goto out;
}
- if (bx->to.sym && use_browser > 0) {
+ if (bx->to.sym && use_browser == 1 && sort__has_sym) {
notes = symbol__annotation(bx->to.sym);
if (!notes->src
&& symbol__alloc_hist(bx->to.sym) < 0)
@@ -162,7 +162,7 @@ static int perf_evsel__add_hist_entry(struct perf_evsel *evsel,
* so we don't allocated the extra space needed because the stdio
* code will not use it.
*/
- if (he->ms.sym != NULL && use_browser > 0) {
+ if (he->ms.sym != NULL && use_browser == 1 && sort__has_sym) {
struct annotation *notes = symbol__annotation(he->ms.sym);
assert(evsel != NULL);
@@ -692,12 +692,14 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
else
use_browser = 0;
+ setup_sorting(report_usage, options);
+
/*
* Only in the newt browser we are doing integrated annotation,
* so don't allocate extra space that won't be used in the stdio
* implementation.
*/
- if (use_browser > 0) {
+ if (use_browser == 1 && sort__has_sym) {
symbol_conf.priv_size = sizeof(struct annotation);
report.annotate_init = symbol__annotate_init;
/*
@@ -720,8 +722,6 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
if (symbol__init() < 0)
goto error;
- setup_sorting(report_usage, options);
-
if (parent_pattern != default_parent_pattern) {
if (sort_dimension__add("parent") < 0)
goto error;
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 3/3] perf ui/browser: Fix stale output of sorted result
2012-09-14 8:35 [PATCH 1/3] perf tools: Add sort__has_sym Namhyung Kim
2012-09-14 8:35 ` [PATCH 2/3] perf report: Enable itegrated annotation only if possible Namhyung Kim
@ 2012-09-14 8:35 ` Namhyung Kim
2012-09-25 5:03 ` Namhyung Kim
2012-09-19 15:15 ` [tip:perf/core] perf tools: Add sort__has_sym tip-bot for Namhyung Kim
2 siblings, 1 reply; 6+ messages in thread
From: Namhyung Kim @ 2012-09-14 8:35 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, LKML, Namhyung Kim
From: Namhyung Kim <namhyung.kim@lge.com>
The hist_entry__sort_snprintf() can return 0 if all of the sort keys
are elided. In this case a buffer which used for the function would
contain old message or a garbage and printed like below:
$ perf record -g -e cycles:u abc
$ perf --report -s comm -c abc
(...)
+ 100.00%100.00%
Fix it by checking return value.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/ui/browsers/hists.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index a21f40bebbac..75b3898ca651 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -664,8 +664,8 @@ static int hist_browser__show_entry(struct hist_browser *browser,
if (!browser->b.navkeypressed)
width += 1;
- hist_entry__sort_snprintf(entry, s, sizeof(s), browser->hists);
- slsmg_write_nstring(s, width);
+ if (hist_entry__sort_snprintf(entry, s, sizeof(s), browser->hists))
+ slsmg_write_nstring(s, width);
++row;
++printed;
} else
@@ -981,7 +981,6 @@ static int hist_browser__fprintf_entry(struct hist_browser *browser,
if (symbol_conf.use_callchain)
folded_sign = hist_entry__folded(he);
- hist_entry__sort_snprintf(he, s, sizeof(s), browser->hists);
percent = (he->period * 100.0) / browser->hists->stats.total_period;
if (symbol_conf.use_callchain)
@@ -995,7 +994,8 @@ static int hist_browser__fprintf_entry(struct hist_browser *browser,
if (symbol_conf.show_total_period)
printed += fprintf(fp, " %12" PRIu64, he->period);
- printed += fprintf(fp, "%s\n", rtrim(s));
+ if (hist_entry__sort_snprintf(he, s, sizeof(s), browser->hists))
+ printed += fprintf(fp, "%s\n", rtrim(s));
if (folded_sign == '-')
printed += hist_browser__fprintf_callchain(browser, &he->sorted_chain, 1, fp);
--
1.7.11.4
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] perf ui/browser: Fix stale output of sorted result
2012-09-14 8:35 ` [PATCH 3/3] perf ui/browser: Fix stale output of sorted result Namhyung Kim
@ 2012-09-25 5:03 ` Namhyung Kim
0 siblings, 0 replies; 6+ messages in thread
From: Namhyung Kim @ 2012-09-25 5:03 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, LKML, Namhyung Kim
Hi Arnaldo,
It seems it's not merged into your tree as I still can see this issue.
Would you consider applying?
Thanks,
Namhyung
On Fri, 14 Sep 2012 17:35:29 +0900, Namhyung Kim wrote:
> From: Namhyung Kim <namhyung.kim@lge.com>
>
> The hist_entry__sort_snprintf() can return 0 if all of the sort keys
> are elided. In this case a buffer which used for the function would
> contain old message or a garbage and printed like below:
>
> $ perf record -g -e cycles:u abc
> $ perf --report -s comm -c abc
> (...)
> + 100.00%100.00%
>
> Fix it by checking return value.
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
> tools/perf/ui/browsers/hists.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index a21f40bebbac..75b3898ca651 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -664,8 +664,8 @@ static int hist_browser__show_entry(struct hist_browser *browser,
> if (!browser->b.navkeypressed)
> width += 1;
>
> - hist_entry__sort_snprintf(entry, s, sizeof(s), browser->hists);
> - slsmg_write_nstring(s, width);
> + if (hist_entry__sort_snprintf(entry, s, sizeof(s), browser->hists))
> + slsmg_write_nstring(s, width);
> ++row;
> ++printed;
> } else
> @@ -981,7 +981,6 @@ static int hist_browser__fprintf_entry(struct hist_browser *browser,
> if (symbol_conf.use_callchain)
> folded_sign = hist_entry__folded(he);
>
> - hist_entry__sort_snprintf(he, s, sizeof(s), browser->hists);
> percent = (he->period * 100.0) / browser->hists->stats.total_period;
>
> if (symbol_conf.use_callchain)
> @@ -995,7 +994,8 @@ static int hist_browser__fprintf_entry(struct hist_browser *browser,
> if (symbol_conf.show_total_period)
> printed += fprintf(fp, " %12" PRIu64, he->period);
>
> - printed += fprintf(fp, "%s\n", rtrim(s));
> + if (hist_entry__sort_snprintf(he, s, sizeof(s), browser->hists))
> + printed += fprintf(fp, "%s\n", rtrim(s));
>
> if (folded_sign == '-')
> printed += hist_browser__fprintf_callchain(browser, &he->sorted_chain, 1, fp);
^ permalink raw reply [flat|nested] 6+ messages in thread
* [tip:perf/core] perf tools: Add sort__has_sym
2012-09-14 8:35 [PATCH 1/3] perf tools: Add sort__has_sym Namhyung Kim
2012-09-14 8:35 ` [PATCH 2/3] perf report: Enable itegrated annotation only if possible Namhyung Kim
2012-09-14 8:35 ` [PATCH 3/3] perf ui/browser: Fix stale output of sorted result Namhyung Kim
@ 2012-09-19 15:15 ` tip-bot for Namhyung Kim
2 siblings, 0 replies; 6+ messages in thread
From: tip-bot for Namhyung Kim @ 2012-09-19 15:15 UTC (permalink / raw)
To: linux-tip-commits
Cc: acme, linux-kernel, paulus, hpa, mingo, a.p.zijlstra,
namhyung.kim, namhyung, tglx
Commit-ID: 1af556406670f2076ea235ba8ba16da13d227e99
Gitweb: http://git.kernel.org/tip/1af556406670f2076ea235ba8ba16da13d227e99
Author: Namhyung Kim <namhyung.kim@lge.com>
AuthorDate: Fri, 14 Sep 2012 17:35:27 +0900
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 17 Sep 2012 13:08:59 -0300
perf tools: Add sort__has_sym
The sort__has_sym variable is for checking whether the sort_list
includes 'symbol' as a sort key. It will be used for later patch.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/1347611729-16994-1-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/sort.c | 5 +++++
tools/perf/util/sort.h | 1 +
2 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 0981bc7..b5b1b92 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -8,6 +8,7 @@ const char default_sort_order[] = "comm,dso,symbol";
const char *sort_order = default_sort_order;
int sort__need_collapse = 0;
int sort__has_parent = 0;
+int sort__has_sym = 0;
int sort__branch_mode = -1; /* -1 = means not set */
enum sort_type sort__first_dimension;
@@ -511,6 +512,10 @@ 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) {
+ sort__has_sym = 1;
}
if (sd->taken)
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index e459c98..12d6347 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -31,6 +31,7 @@ extern const char *parent_pattern;
extern const char default_sort_order[];
extern int sort__need_collapse;
extern int sort__has_parent;
+extern int sort__has_sym;
extern int sort__branch_mode;
extern struct sort_entry sort_comm;
extern struct sort_entry sort_dso;
^ permalink raw reply [flat|nested] 6+ messages in thread