From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755471AbdEEDK3 (ORCPT ); Thu, 4 May 2017 23:10:29 -0400 Received: from mga04.intel.com ([192.55.52.120]:27247 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755426AbdEEDK1 (ORCPT ); Thu, 4 May 2017 23:10:27 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.38,290,1491289200"; d="scan'208";a="1164853126" Subject: Re: [PATCH] perf report: Make --branch-history work without callgraphs(-g) option in perf record To: Milian Wolff Cc: acme@kernel.org, jolsa@kernel.org, peterz@infradead.org, mingo@redhat.com, alexander.shishkin@linux.intel.com, Linux-kernel@vger.kernel.org, ak@linux.intel.com, kan.liang@intel.com, yao.jin@intel.com References: <1493886697-28245-1-git-send-email-yao.jin@linux.intel.com> <21042314.DNPhWqAGjE@milian-kdab2> From: "Jin, Yao" Message-ID: Date: Fri, 5 May 2017 11:10:24 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.1.0 MIME-Version: 1.0 In-Reply-To: <21042314.DNPhWqAGjE@milian-kdab2> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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