From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756258AbbI2Izg (ORCPT ); Tue, 29 Sep 2015 04:55:36 -0400 Received: from mga03.intel.com ([134.134.136.65]:24225 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755951AbbI2Izc (ORCPT ); Tue, 29 Sep 2015 04:55:32 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.17,607,1437462000"; d="scan'208";a="799483491" Subject: Re: [PATCH 15/25] perf report: Make max_stack value allow for synthesized callchains To: Arnaldo Carvalho de Melo References: <1443186956-18718-1-git-send-email-adrian.hunter@intel.com> <1443186956-18718-16-git-send-email-adrian.hunter@intel.com> <20150928200309.GB9392@kernel.org> Cc: Jiri Olsa , linux-kernel@vger.kernel.org From: Adrian Hunter Organization: Intel Finland Oy, Registered Address: PL 281, 00181 Helsinki, Business Identity Code: 0357606 - 4, Domiciled in Helsinki Message-ID: <560A5155.4060105@intel.com> Date: Tue, 29 Sep 2015 11:52:37 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <20150928200309.GB9392@kernel.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 28/09/15 23:03, Arnaldo Carvalho de Melo wrote: > Em Fri, Sep 25, 2015 at 04:15:46PM +0300, Adrian Hunter escreveu: >> perf report has an option (--max-stack) to set the maximum stack depth >> when processing callchains. The option defaults to the hard-coded >> maximum definition PERF_MAX_STACK_DEPTH which is 127. The intention of >> the option is to allow the user to reduce the processing time by >> reducing the amount of the callchain that is processed. >> >> It is also possible, when processing instruction traces, to synthesize >> callchains. Synthesized callchains do not have the kernel size >> limitation and are whatever size the user requests, although validation >> presently prevents the user requested a value greater that 1024. The >> default value is 16. > > So, haven't checked the options, but one can possibly use both the way > itrace has to ask for a max stack size and also via --max-stack, right? Possibly, but it would not be a common paradigm. > > In that case we better emit a warning or plain state that one either > uses one way of setting the max stack or the other? max_stack was added as an optimization to reduce processing time, so people specifying --max-stack might get a increased processing time if combined with synthesized callchains, but otherwise no real harm. A warning seems like overkill. Could amend the documenation e.g. diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt index b941d5e07e28..ce499035e6d8 100644 --- a/tools/perf/Documentation/perf-report.txt +++ b/tools/perf/Documentation/perf-report.txt @@ -205,6 +205,8 @@ OPTIONS beyond the specified depth will be ignored. This is a trade-off between information loss and faster processing especially for workloads that can have a very long callchain stack. + Note that when using the --itrace option the synthesized callchain size + will override this value if the synthesized callchain size is bigger. Default: 127 > > I'm applying the patch, because it is unlikely that this gets specified, > but would be good to close this gap. > > - Arnaldo > >> To allow for synthesized callchains, make the max_stack value at least >> the same size as the synthesized callchain size. >> >> Signed-off-by: Adrian Hunter >> --- >> tools/perf/builtin-report.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c >> index e94e5c7155af..37c9f5125887 100644 >> --- a/tools/perf/builtin-report.c >> +++ b/tools/perf/builtin-report.c >> @@ -809,6 +809,10 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused) >> if (report.inverted_callchain) >> callchain_param.order = ORDER_CALLER; >> >> + if (itrace_synth_opts.callchain && >> + (int)itrace_synth_opts.callchain_sz > report.max_stack) >> + report.max_stack = itrace_synth_opts.callchain_sz; >> + >> if (!input_name || !strlen(input_name)) { >> if (!fstat(STDIN_FILENO, &st) && S_ISFIFO(st.st_mode)) >> input_name = "-"; >> -- >> 1.9.1 >