From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757959AbbJVOXF (ORCPT ); Thu, 22 Oct 2015 10:23:05 -0400 Received: from mail-io0-f181.google.com ([209.85.223.181]:34177 "EHLO mail-io0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757831AbbJVOXB (ORCPT ); Thu, 22 Oct 2015 10:23:01 -0400 MIME-Version: 1.0 In-Reply-To: <20151022141324.GA2455@kernel.org> References: <1445495330-25416-1-git-send-email-namhyung@kernel.org> <1445495330-25416-3-git-send-email-namhyung@kernel.org> <20151022141324.GA2455@kernel.org> From: Namhyung Kim Date: Thu, 22 Oct 2015 23:22:40 +0900 X-Google-Sender-Auth: hE7Ruez27-qNNr01xpl9EpOJQT0 Message-ID: Subject: Re: [RFC/PATCH 3/3] perf tools: Defaults to 'caller' callchain order only if --children is enabled To: Arnaldo Carvalho de Melo Cc: Ingo Molnar , Peter Zijlstra , Jiri Olsa , LKML , David Ahern , Adrian Hunter , Borislav Petkov , Brendan Gregg , Chandler Carruth , Frederic Weisbecker , Stephane Eranian , Wang Nan Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Arnaldo, On Thu, Oct 22, 2015 at 11:13 PM, Arnaldo Carvalho de Melo wrote: > Em Thu, Oct 22, 2015 at 03:28:50PM +0900, Namhyung Kim escreveu: >> +++ b/tools/perf/util/util.c >> @@ -19,7 +19,7 @@ >> struct callchain_param callchain_param = { >> .mode = CHAIN_GRAPH_ABS, >> .min_percent = 0.5, >> - .order = ORDER_CALLER, >> + .order = ORDER_CALLEE, >> .key = CCKEY_FUNCTION >> }; > > So, this isn't a revert of the previous default change, i.e. previously > we used CHAIN_GRAPH_REL, should we keep CHAIN_GRAPH_ABS + callee? Yes, it's not a revert. I intentionally didn't change the print type. > > Also you forgot to fix the docs to mention this new default, I did it > and the resulting patch, still with GRAPH_ABS follows: This is not simple. As Brendan said, --children is default on, so users will see 'caller' ordering that's why I didn't change the doc. But I should mention it'll use 'callee' ordering when --no-chlidren. I'm about to send doc fix patch Ingo requested, so please leave this patch as is. Thanks, Namhyung > > From fd2dcde26d09b34fccc3c164d9fc36e662c21fc4 Mon Sep 17 00:00:00 2001 > From: Namhyung Kim > Date: Thu, 22 Oct 2015 16:45:46 +0900 > Subject: [PATCH 1/1] perf tools: Defaults to 'caller' callchain order only if > --children is enabled > > The caller callchain order is useful with --children option since it can > show 'overview' style output, but other commands which don't use > --children feature like 'perf script' or even 'perf report/top' without > --children are better to keep callee order. > > Signed-off-by: Namhyung Kim > Acked-by: Frederic Weisbecker > Acked-by: Brendan Gregg > Cc: Adrian Hunter > Cc: Borislav Petkov > Cc: Chandler Carruth > Cc: David Ahern > Cc: Jiri Olsa > Cc: Peter Zijlstra > Cc: Stephane Eranian > Cc: Wang Nan > Link: http://lkml.kernel.org/r/1445499946-29817-1-git-send-email-namhyung@kernel.org > [ Update 'perf report' man page ] > Signed-off-by: Arnaldo Carvalho de Melo > --- > tools/perf/Documentation/perf-report.txt | 2 +- > tools/perf/builtin-report.c | 2 ++ > tools/perf/builtin-top.c | 3 +++ > tools/perf/util/callchain.c | 2 ++ > tools/perf/util/callchain.h | 1 + > tools/perf/util/util.c | 2 +- > 6 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt > index e4fdeeb51123..4ae1621f1def 100644 > --- a/tools/perf/Documentation/perf-report.txt > +++ b/tools/perf/Documentation/perf-report.txt > @@ -192,7 +192,7 @@ OPTIONS > when available. Usually more convenient to use --branch-history > for this. > > - Default: graph,0.5,caller > + Default: graph,0.5,callee > > --children:: > Accumulate callchain of children to parent entry so that then can > diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c > index 3b23b25d1589..a554906a3e03 100644 > --- a/tools/perf/builtin-report.c > +++ b/tools/perf/builtin-report.c > @@ -808,6 +808,8 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused) > > if (report.inverted_callchain) > callchain_param.order = ORDER_CALLER; > + if (symbol_conf.cumulate_callchain && !callchain_param.order_set) > + callchain_param.order = ORDER_CALLER; > > if (itrace_synth_opts.callchain && > (int)itrace_synth_opts.callchain_sz > report.max_stack) > diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c > index 6f641fd68296..be42e6eb6805 100644 > --- a/tools/perf/builtin-top.c > +++ b/tools/perf/builtin-top.c > @@ -1288,6 +1288,9 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused) > perf_hpp__cancel_cumulate(); > } > > + if (symbol_conf.cumulate_callchain && !callchain_param.order_set) > + callchain_param.order = ORDER_CALLER; > + > symbol_conf.priv_size = sizeof(struct annotation); > > symbol_conf.try_vmlinux_path = (symbol_conf.vmlinux_name == NULL); > diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c > index 773fe13ce627..a8ed80f77ff3 100644 > --- a/tools/perf/util/callchain.c > +++ b/tools/perf/util/callchain.c > @@ -51,10 +51,12 @@ static int parse_callchain_order(const char *value) > { > if (!strncmp(value, "caller", strlen(value))) { > callchain_param.order = ORDER_CALLER; > + callchain_param.order_set = true; > return 0; > } > if (!strncmp(value, "callee", strlen(value))) { > callchain_param.order = ORDER_CALLEE; > + callchain_param.order_set = true; > return 0; > } > return -1; > diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h > index acee2b3cd801..0e96496567c5 100644 > --- a/tools/perf/util/callchain.h > +++ b/tools/perf/util/callchain.h > @@ -63,6 +63,7 @@ struct callchain_param { > double min_percent; > sort_chain_func_t sort; > enum chain_order order; > + bool order_set; > enum chain_key key; > bool branch_callstack; > }; > diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c > index c1bf9ff210b0..cd12c25e4ea4 100644 > --- a/tools/perf/util/util.c > +++ b/tools/perf/util/util.c > @@ -19,7 +19,7 @@ > struct callchain_param callchain_param = { > .mode = CHAIN_GRAPH_ABS, > .min_percent = 0.5, > - .order = ORDER_CALLER, > + .order = ORDER_CALLEE, > .key = CCKEY_FUNCTION > }; > > -- > 2.1.0 > -- Thanks, Namhyung