From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753846AbbI3IqV (ORCPT ); Wed, 30 Sep 2015 04:46:21 -0400 Received: from mga14.intel.com ([192.55.52.115]:39427 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751566AbbI3IqF (ORCPT ); Wed, 30 Sep 2015 04:46:05 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.17,611,1437462000"; d="scan'208";a="571155614" 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> <560A5155.4060105@intel.com> <20150929155104.GD1944@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: <560BA09E.8070600@intel.com> Date: Wed, 30 Sep 2015 11:43:10 +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: <20150929155104.GD1944@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 29/09/15 18:51, Arnaldo Carvalho de Melo wrote: > Em Tue, Sep 29, 2015 at 11:52:37AM +0300, Adrian Hunter escreveu: >> 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. > > Adding the doc part helps, but actually telling that what they are > trying to do is not possible, even for unlikely scenarios like this, > seems cleaner, but no biggie. > > I'll add the patch below with your s-o-b, ack? Yes thank you. > > - Arnaldo > >> >> 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 >>> >