* [PATCH] perf report: Make --branch-history work without callgraphs(-g) option in perf record
@ 2017-05-04 8:31 Jin Yao
2017-05-04 8:58 ` Milian Wolff
0 siblings, 1 reply; 3+ messages in thread
From: Jin Yao @ 2017-05-04 8:31 UTC (permalink / raw)
To: acme, jolsa, peterz, mingo, alexander.shishkin
Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao
perf record -b -g <command>
perf report --branch-history
This merges the LBRs with the callgraphs.
However it would be nice if it also works without callgraphs (-g)
set in perf record, so that only the LBRs are displayed.
But currently perf report errors in this case. For example,
perf record -b <command>
perf report --branch-history
Error:
Selected -g or --branch-history but no callchain data. Did
you call 'perf record' without -g?
This patch displays the LBRs only even if callgraphs(-g) is not
enabled in perf record.
Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
tools/perf/builtin-report.c | 6 ++++--
tools/perf/util/callchain.c | 7 ++++---
tools/perf/util/hist.c | 2 ++
tools/perf/util/machine.c | 13 ++++++++++++-
4 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 22478ff..7f82369 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -259,7 +259,8 @@ static int report__setup_sample_type(struct report *rep)
"'perf record' without -g?\n");
return -EINVAL;
}
- if (symbol_conf.use_callchain) {
+ if (symbol_conf.use_callchain &&
+ !symbol_conf.show_branchflag_count) {
ui__error("Selected -g or --branch-history but no "
"callchain data. Did\n"
"you call 'perf record' without -g?\n");
@@ -397,7 +398,8 @@ static int perf_evlist__tty_browse_hists(struct perf_evlist *evlist,
hists__fprintf_nr_sample_events(hists, rep, evname, stdout);
hists__fprintf(hists, !quiet, 0, 0, rep->min_percent, stdout,
- symbol_conf.use_callchain);
+ symbol_conf.use_callchain |
+ symbol_conf.show_branchflag_count);
fprintf(stdout, "\n\n");
}
diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index 81fc29a..08d3abf 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -993,11 +993,11 @@ int sample__resolve_callchain(struct perf_sample *sample,
struct perf_evsel *evsel, struct addr_location *al,
int max_stack)
{
- if (sample->callchain == NULL)
+ if (sample->callchain == NULL && !symbol_conf.show_branchflag_count)
return 0;
if (symbol_conf.use_callchain || symbol_conf.cumulate_callchain ||
- perf_hpp_list.parent) {
+ perf_hpp_list.parent || symbol_conf.show_branchflag_count) {
return thread__resolve_callchain(al->thread, cursor, evsel, sample,
parent, al, max_stack);
}
@@ -1006,7 +1006,8 @@ int sample__resolve_callchain(struct perf_sample *sample,
int hist_entry__append_callchain(struct hist_entry *he, struct perf_sample *sample)
{
- if (!symbol_conf.use_callchain || sample->callchain == NULL)
+ if ((!symbol_conf.use_callchain || sample->callchain == NULL) &&
+ !symbol_conf.show_branchflag_count)
return 0;
return callchain_append(he->callchain, &callchain_cursor, sample->period);
}
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index cf0186a..8b045a5 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -1762,6 +1762,8 @@ void perf_evsel__output_resort(struct perf_evsel *evsel, struct ui_progress *pro
else
use_callchain = symbol_conf.use_callchain;
+ use_callchain |= symbol_conf.show_branchflag_count;
+
output_resort(evsel__hists(evsel), prog, use_callchain, NULL);
}
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index a98f55a..792ddcd 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -1901,13 +1901,16 @@ static int thread__resolve_callchain_sample(struct thread *thread,
{
struct branch_stack *branch = sample->branch_stack;
struct ip_callchain *chain = sample->callchain;
- int chain_nr = chain->nr;
+ int chain_nr = 0;
u8 cpumode = PERF_RECORD_MISC_USER;
int i, j, err, nr_entries;
int skip_idx = -1;
int first_call = 0;
int nr_loop_iter;
+ if (chain)
+ chain_nr = chain->nr;
+
if (perf_evsel__has_branch_callstack(evsel)) {
err = resolve_lbr_callchain_sample(thread, cursor, sample, parent,
root_al, max_stack);
@@ -1945,6 +1948,10 @@ static int thread__resolve_callchain_sample(struct thread *thread,
for (i = 0; i < nr; i++) {
if (callchain_param.order == ORDER_CALLEE) {
be[i] = branch->entries[i];
+
+ if (chain == NULL)
+ continue;
+
/*
* Check for overlap into the callchain.
* The return address is one off compared to
@@ -1998,6 +2005,10 @@ static int thread__resolve_callchain_sample(struct thread *thread,
if (err)
return err;
}
+
+ if (chain_nr == 0)
+ return 0;
+
chain_nr -= nr;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] perf report: Make --branch-history work without callgraphs(-g) option in perf record
2017-05-04 8:31 [PATCH] perf report: Make --branch-history work without callgraphs(-g) option in perf record Jin Yao
@ 2017-05-04 8:58 ` Milian Wolff
2017-05-05 3:10 ` Jin, Yao
0 siblings, 1 reply; 3+ messages in thread
From: Milian Wolff @ 2017-05-04 8:58 UTC (permalink / raw)
To: Jin Yao
Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
kan.liang, yao.jin
On Thursday, May 4, 2017 10:31:37 AM CEST Jin Yao wrote:
> perf record -b -g <command>
> perf report --branch-history
>
> This merges the LBRs with the callgraphs.
>
> However it would be nice if it also works without callgraphs (-g)
> set in perf record, so that only the LBRs are displayed.
> But currently perf report errors in this case. For example,
>
> perf record -b <command>
> perf report --branch-history
>
> Error:
> Selected -g or --branch-history but no callchain data. Did
> you call 'perf record' without -g?
>
> This patch displays the LBRs only even if callgraphs(-g) is not
> enabled in perf record.
>
> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
> ---
> tools/perf/builtin-report.c | 6 ++++--
> tools/perf/util/callchain.c | 7 ++++---
> tools/perf/util/hist.c | 2 ++
> tools/perf/util/machine.c | 13 ++++++++++++-
> 4 files changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 22478ff..7f82369 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -259,7 +259,8 @@ static int report__setup_sample_type(struct report *rep)
> "'perf record' without -g?\n");
> return -EINVAL;
> }
> - if (symbol_conf.use_callchain) {
> + if (symbol_conf.use_callchain &&
> + !symbol_conf.show_branchflag_count) {
> ui__error("Selected -g or --branch-history but no "
> "callchain data. Did\n"
> "you call 'perf record' without -g?\n");
The `--branch-history` part of this error message is now stale, no?
> @@ -397,7 +398,8 @@ static int perf_evlist__tty_browse_hists(struct
> perf_evlist *evlist,
>
> hists__fprintf_nr_sample_events(hists, rep, evname, stdout);
> hists__fprintf(hists, !quiet, 0, 0, rep->min_percent, stdout,
> - symbol_conf.use_callchain);
> + symbol_conf.use_callchain |
> + symbol_conf.show_branchflag_count);
> fprintf(stdout, "\n\n");
> }
Not sure about the coding style here, but shouldn't this be a boolean or
operator here '||' - semantically? Functionality wise it will be the same, I
guess?
> diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
> index 81fc29a..08d3abf 100644
> --- a/tools/perf/util/callchain.c
> +++ b/tools/perf/util/callchain.c
> @@ -993,11 +993,11 @@ int sample__resolve_callchain(struct perf_sample
> *sample, struct perf_evsel *evsel, struct addr_location *al,
> int max_stack)
> {
> - if (sample->callchain == NULL)
> + if (sample->callchain == NULL && !symbol_conf.show_branchflag_count)
> return 0;
>
> if (symbol_conf.use_callchain || symbol_conf.cumulate_callchain ||
> - perf_hpp_list.parent) {
> + perf_hpp_list.parent || symbol_conf.show_branchflag_count) {
> return thread__resolve_callchain(al->thread, cursor, evsel, sample,
> parent, al, max_stack);
> }
> @@ -1006,7 +1006,8 @@ int sample__resolve_callchain(struct perf_sample
> *sample,
>
> int hist_entry__append_callchain(struct hist_entry *he, struct perf_sample
> *sample) {
> - if (!symbol_conf.use_callchain || sample->callchain == NULL)
> + if ((!symbol_conf.use_callchain || sample->callchain == NULL) &&
> + !symbol_conf.show_branchflag_count)
> return 0;
> return callchain_append(he->callchain, &callchain_cursor, sample->period);
> }
> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index cf0186a..8b045a5 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -1762,6 +1762,8 @@ void perf_evsel__output_resort(struct perf_evsel
> *evsel, struct ui_progress *pro else
> use_callchain = symbol_conf.use_callchain;
>
> + use_callchain |= symbol_conf.show_branchflag_count;
> +
> output_resort(evsel__hists(evsel), prog, use_callchain, NULL);
> }
>
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index a98f55a..792ddcd 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -1901,13 +1901,16 @@ static int thread__resolve_callchain_sample(struct
> thread *thread, {
> struct branch_stack *branch = sample->branch_stack;
> struct ip_callchain *chain = sample->callchain;
> - int chain_nr = chain->nr;
> + int chain_nr = 0;
> u8 cpumode = PERF_RECORD_MISC_USER;
> int i, j, err, nr_entries;
> int skip_idx = -1;
> int first_call = 0;
> int nr_loop_iter;
>
> + if (chain)
> + chain_nr = chain->nr;
> +
> if (perf_evsel__has_branch_callstack(evsel)) {
> err = resolve_lbr_callchain_sample(thread, cursor, sample, parent,
> root_al, max_stack);
> @@ -1945,6 +1948,10 @@ static int thread__resolve_callchain_sample(struct
> thread *thread, for (i = 0; i < nr; i++) {
> if (callchain_param.order == ORDER_CALLEE) {
> be[i] = branch->entries[i];
> +
> + if (chain == NULL)
> + continue;
> +
> /*
> * Check for overlap into the callchain.
> * The return address is one off compared to
> @@ -1998,6 +2005,10 @@ static int thread__resolve_callchain_sample(struct
> thread *thread, if (err)
> return err;
> }
> +
> + if (chain_nr == 0)
> + return 0;
> +
> chain_nr -= nr;
> }
--
Milian Wolff | milian.wolff@kdab.com | Software Engineer
KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt Experts
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] perf report: Make --branch-history work without callgraphs(-g) option in perf record
2017-05-04 8:58 ` Milian Wolff
@ 2017-05-05 3:10 ` Jin, Yao
0 siblings, 0 replies; 3+ messages in thread
From: Jin, Yao @ 2017-05-05 3:10 UTC (permalink / raw)
To: Milian Wolff
Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
kan.liang, yao.jin
SNIP
>> - if (symbol_conf.use_callchain) {
>> + if (symbol_conf.use_callchain &&
>> + !symbol_conf.show_branchflag_count) {
>> ui__error("Selected -g or --branch-history but no "
>> "callchain data. Did\n"
>> "you call 'perf record' without -g?\n");
> The `--branch-history` part of this error message is now stale, no?
Yes, the error message is obsolete. I will change it in next version.
>
>> @@ -397,7 +398,8 @@ static int perf_evlist__tty_browse_hists(struct
>> perf_evlist *evlist,
>>
>> hists__fprintf_nr_sample_events(hists, rep, evname, stdout);
>> hists__fprintf(hists, !quiet, 0, 0, rep->min_percent, stdout,
>> - symbol_conf.use_callchain);
>> + symbol_conf.use_callchain |
>> + symbol_conf.show_branchflag_count);
>> fprintf(stdout, "\n\n");
>> }
> Not sure about the coding style here, but shouldn't this be a boolean or
> operator here '||' - semantically? Functionality wise it will be the same, I
> guess?
Should be same. While I will follow your comments to change it to '||'.
I will wait other comments for some days and then send out the next version.
Thanks
Jin Yao
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-05-05 3:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-04 8:31 [PATCH] perf report: Make --branch-history work without callgraphs(-g) option in perf record Jin Yao
2017-05-04 8:58 ` Milian Wolff
2017-05-05 3:10 ` Jin, Yao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).