From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752814AbdGXDti (ORCPT ); Sun, 23 Jul 2017 23:49:38 -0400 Received: from mga01.intel.com ([192.55.52.88]:52599 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751997AbdGXDte (ORCPT ); Sun, 23 Jul 2017 23:49:34 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.40,404,1496127600"; d="scan'208";a="130941077" Subject: Re: [PATCH v2] perf report: Make --branch-history work without callgraphs(-g) option in perf record To: acme@kernel.org, jolsa@kernel.org, peterz@infradead.org, mingo@redhat.com, alexander.shishkin@linux.intel.com Cc: Linux-kernel@vger.kernel.org, ak@linux.intel.com, kan.liang@intel.com, yao.jin@intel.com References: <1494240182-28899-1-git-send-email-yao.jin@linux.intel.com> From: "Jin, Yao" Message-ID: <8d0dab1c-9cf8-ec28-bff1-6e9783e83a24@linux.intel.com> Date: Mon, 24 Jul 2017 11:49:30 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <1494240182-28899-1-git-send-email-yao.jin@linux.intel.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Arnaldo, Is this patch OK for merging? It's more than 2 months no more comments. Thanks Jin Yao On 5/8/2017 6:43 PM, Jin Yao wrote: > perf record -b -g > 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 > 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. > > Change log: > ---------- > v2: According to Milian Wolff's comment, change the obsolete error > message. Now the error message is: > > ┌─Error:─────────────────────────────────────┐ > │Selected -g or --branch-history. │ > │But no callchain or branch data. │ > │Did you call 'perf record' without -g or -b?│ > │ │ > │ │ > │Press any key... │ > └────────────────────────────────────────────┘ > > When passing the last parameter to hists__fprintf, > changes "|" to "||". > > hsts__fprintf(hists, !quiet, 0, 0, rep->min_percent, stdout, > symbol_conf.use_callchain || > symbol_conf.show_branchflag_count); > > Signed-off-by: Jin Yao > --- > tools/perf/builtin-report.c | 12 +++++++----- > tools/perf/util/callchain.c | 7 ++++--- > tools/perf/util/hist.c | 2 ++ > tools/perf/util/machine.c | 13 ++++++++++++- > 4 files changed, 25 insertions(+), 9 deletions(-) > > diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c > index 22478ff..4ceb2d2 100644 > --- a/tools/perf/builtin-report.c > +++ b/tools/perf/builtin-report.c > @@ -259,10 +259,11 @@ static int report__setup_sample_type(struct report *rep) > "'perf record' without -g?\n"); > return -EINVAL; > } > - if (symbol_conf.use_callchain) { > - ui__error("Selected -g or --branch-history but no " > - "callchain data. Did\n" > - "you call 'perf record' without -g?\n"); > + if (symbol_conf.use_callchain && > + !symbol_conf.show_branchflag_count) { > + ui__error("Selected -g or --branch-history.\n" > + "But no callchain or branch data.\n" > + "Did you call 'perf record' without -g or -b?\n"); > return -1; > } > } else if (!callchain_param.enabled && > @@ -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 d97e014..7b47080 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; > } >