[PATCH 1/3] perf tools: Move callchain help messages to callchain.h

[PATCH 1/3] perf tools: Move callchain help messages to callchain.h

[Namhyung Kim]

These messages will be used by 'perf top' in the next patch.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Chandler Carruth <chandlerc@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-record.c |  8 +-------
 tools/perf/builtin-report.c | 10 +++++++---
 tools/perf/util/callchain.h | 12 ++++++++++++
 3 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 24ace2f318c1..1a117623d396 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1010,13 +1010,7 @@ static struct record record = {
 	},
 };
 
-#define CALLCHAIN_HELP "setup and enables call-graph (stack chain/backtrace) recording: "
-
-#ifdef HAVE_DWARF_UNWIND_SUPPORT
-const char record_callchain_help[] = CALLCHAIN_HELP "fp dwarf lbr";
-#else
-const char record_callchain_help[] = CALLCHAIN_HELP "fp lbr";
-#endif
+const char record_callchain_help[] = CALLCHAIN_RECORD_HELP;
 
 /*
  * XXX Will stay a global variable till we fix builtin-script.c to stop messing
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 3b23b25d1589..18a8c52d921e 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -625,6 +625,9 @@ parse_percent_limit(const struct option *opt, const char *str,
 	return 0;
 }
 
+const char report_callchain_help[] = "Display callchains using " CALLCHAIN_REPORT_HELP ". "
+				     "Default: graph,0.5,caller";
+
 int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 {
 	struct perf_session *session;
@@ -699,9 +702,10 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 		   "regex filter to identify parent, see: '--sort parent'"),
 	OPT_BOOLEAN('x', "exclude-other", &symbol_conf.exclude_other,
 		    "Only display entries with parent-match"),
-	OPT_CALLBACK_DEFAULT('g', "call-graph", &report, "output_type,min_percent[,print_limit],call_order[,branch]",
-		     "Display callchains using output_type (graph, flat, fractal, or none) , min percent threshold, optional print limit, callchain order, key (function or address), add branches. "
-		     "Default: graph,0.5,caller", &report_parse_callchain_opt, callchain_default_opt),
+	OPT_CALLBACK_DEFAULT('g', "call-graph", &report,
+			     "output_type,min_percent[,print_limit],call_order[,branch]",
+			     report_callchain_help, &report_parse_callchain_opt,
+			     callchain_default_opt),
 	OPT_BOOLEAN(0, "children", &symbol_conf.cumulate_callchain,
 		    "Accumulate callchains of children and show total overhead as well"),
 	OPT_INTEGER(0, "max-stack", &report.max_stack,
diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
index acee2b3cd801..c9e3a2e85a72 100644
--- a/tools/perf/util/callchain.h
+++ b/tools/perf/util/callchain.h
@@ -7,6 +7,18 @@
 #include "event.h"
 #include "symbol.h"
 
+#define CALLCHAIN_HELP "setup and enables call-graph (stack chain/backtrace) recording: "
+
+#ifdef HAVE_DWARF_UNWIND_SUPPORT
+#define CALLCHAIN_RECORD_HELP  CALLCHAIN_HELP "fp dwarf lbr"
+#else
+#define CALLCHAIN_RECORD_HELP  CALLCHAIN_HELP "fp lbr"
+#endif
+
+#define CALLCHAIN_REPORT_HELP  "output_type (graph, flat, fractal, or none), " \
+	"min percent threshold, optional print limit, callchain order, " \
+	"key (function or address), add branches"
+
 enum perf_call_graph_mode {
 	CALLCHAIN_NONE,
 	CALLCHAIN_FP,
-- 
2.6.0

[PATCH 2/3] perf top: Support call-graph display options also

[Namhyung Kim]

Currently 'perf top --call-graph' option is same as 'perf record'.  But
'perf top' also need to receive display options in 'perf report'.  To do
that, change parse_callchain_report_opt() to allow record options too.

Now perf top can receive display options like below:

  $ perf top --call-graph
    Error: option `call-graph' requires a value

   Usage: perf top [<options>]

        --call-graph
          <mode[,dump_size],output_type,min_percent[,print_limit],call_order[,branch]>
                     setup and enables call-graph (stack chain/backtrace)
                     recording: fp dwarf lbr, output_type (graph, flat,
		     fractal, or none), min percent threshold, optional
		     print limit, callchain order, key (function or
		     address), add branches

  $ perf top --call-graph callee,graph,fp

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Chandler Carruth <chandlerc@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/Documentation/perf-top.txt |  5 +++--
 tools/perf/builtin-top.c              | 26 ++++++++++++++++++-----
 tools/perf/util/callchain.c           | 40 ++++++++++++++++++++++++++++++++---
 tools/perf/util/callchain.h           |  1 +
 4 files changed, 62 insertions(+), 10 deletions(-)

diff --git a/tools/perf/Documentation/perf-top.txt b/tools/perf/Documentation/perf-top.txt
index f6a23eb294e7..556cec09bf50 100644
--- a/tools/perf/Documentation/perf-top.txt
+++ b/tools/perf/Documentation/perf-top.txt
@@ -160,9 +160,10 @@ Default is to monitor all CPUS.
 -g::
 	Enables call-graph (stack chain/backtrace) recording.
 
---call-graph::
+--call-graph [mode,type,min[,limit],order[,key][,branch]]::
 	Setup and enable call-graph (stack chain/backtrace) recording,
-	implies -g.
+	implies -g.  See `--call-graph` section in perf-record and
+	perf-report man pages for details.
 
 --children::
 	Accumulate callchain of children to parent entry so that then can
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 6f641fd68296..1de381d3f29f 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1053,8 +1053,22 @@ callchain_opt(const struct option *opt, const char *arg, int unset)
 static int
 parse_callchain_opt(const struct option *opt, const char *arg, int unset)
 {
-	symbol_conf.use_callchain = true;
-	return record_parse_callchain_opt(opt, arg, unset);
+	struct record_opts *record = (struct record_opts *)opt->value;
+
+	record->callgraph_set = true;
+	callchain_param.enabled = !unset;
+	callchain_param.record_mode = CALLCHAIN_FP;
+
+	/*
+	 * --no-call-graph
+	 */
+	if (unset) {
+		symbol_conf.use_callchain = false;
+		callchain_param.record_mode = CALLCHAIN_NONE;
+		return 0;
+	}
+
+	return parse_callchain_top_opt(arg);
 }
 
 static int perf_top_config(const char *var, const char *value, void *cb)
@@ -1079,6 +1093,8 @@ parse_percent_limit(const struct option *opt, const char *arg,
 	return 0;
 }
 
+const char top_callchain_help[] = CALLCHAIN_RECORD_HELP ", " CALLCHAIN_REPORT_HELP;
+
 int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
 {
 	char errbuf[BUFSIZ];
@@ -1154,11 +1170,11 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
 	OPT_BOOLEAN('n', "show-nr-samples", &symbol_conf.show_nr_samples,
 		    "Show a column with the number of samples"),
 	OPT_CALLBACK_NOOPT('g', NULL, &top.record_opts,
-			   NULL, "enables call-graph recording",
+			   NULL, "enables call-graph recording and display",
 			   &callchain_opt),
 	OPT_CALLBACK(0, "call-graph", &top.record_opts,
-		     "mode[,dump_size]", record_callchain_help,
-		     &parse_callchain_opt),
+		     "mode[,dump_size],output_type,min_percent[,print_limit],call_order[,branch]",
+		     top_callchain_help, &parse_callchain_opt),
 	OPT_BOOLEAN(0, "children", &symbol_conf.cumulate_callchain,
 		    "Accumulate callchains of children and show total overhead as well"),
 	OPT_INTEGER(0, "max-stack", &top.max_stack,
diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index 773fe13ce627..842be32899ee 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -77,12 +77,14 @@ static int parse_callchain_sort_key(const char *value)
 	return -1;
 }
 
-int
-parse_callchain_report_opt(const char *arg)
+static int
+__parse_callchain_report_opt(const char *arg, bool allow_record_opt)
 {
 	char *tok;
 	char *endptr;
 	bool minpcnt_set = false;
+	bool record_opt_set = false;
+	bool try_stack_size = false;
 
 	symbol_conf.use_callchain = true;
 
@@ -100,6 +102,28 @@ parse_callchain_report_opt(const char *arg)
 		    !parse_callchain_order(tok) ||
 		    !parse_callchain_sort_key(tok)) {
 			/* parsing ok - move on to the next */
+			try_stack_size = false;
+			goto next;
+		} else if (allow_record_opt && !record_opt_set) {
+			if (parse_callchain_record(tok, &callchain_param))
+				goto try_numbers;
+
+			/* assume that number followed by 'dwarf' is stack size */
+			if (callchain_param.record_mode == CALLCHAIN_DWARF)
+				try_stack_size = true;
+
+			record_opt_set = true;
+			goto next;
+		}
+
+try_numbers:
+		if (try_stack_size) {
+			unsigned long size = 0;
+
+			if (get_stack_size(tok, &size) < 0)
+				return -1;
+			callchain_param.dump_size = size;
+			try_stack_size = false;
 		} else if (!minpcnt_set) {
 			/* try to get the min percent */
 			callchain_param.min_percent = strtod(tok, &endptr);
@@ -112,7 +136,7 @@ parse_callchain_report_opt(const char *arg)
 			if (tok == endptr)
 				return -1;
 		}
-
+next:
 		arg = NULL;
 	}
 
@@ -123,6 +147,16 @@ parse_callchain_report_opt(const char *arg)
 	return 0;
 }
 
+int parse_callchain_report_opt(const char *arg)
+{
+	return __parse_callchain_report_opt(arg, false);
+}
+
+int parse_callchain_top_opt(const char *arg)
+{
+	return __parse_callchain_report_opt(arg, true);
+}
+
 int perf_callchain_config(const char *var, const char *value)
 {
 	char *endptr;
diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
index c9e3a2e85a72..836d59a001bc 100644
--- a/tools/perf/util/callchain.h
+++ b/tools/perf/util/callchain.h
@@ -192,6 +192,7 @@ extern const char record_callchain_help[];
 extern int parse_callchain_record(const char *arg, struct callchain_param *param);
 int parse_callchain_record_opt(const char *arg, struct callchain_param *param);
 int parse_callchain_report_opt(const char *arg);
+int parse_callchain_top_opt(const char *arg);
 int perf_callchain_config(const char *var, const char *value);
 
 static inline void callchain_cursor_snapshot(struct callchain_cursor *dest,
-- 
2.6.0

[RFC/PATCH 3/3] perf tools: Defaults to 'caller' callchain order only if --children is enabled

[Namhyung Kim]

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 caller order.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
Cc: Chandler Carruth <chandlerc@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 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 +-
 5 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 18a8c52d921e..545c51cef7f7 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -812,6 +812,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 1de381d3f29f..af849b1d7389 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1304,6 +1304,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 842be32899ee..735ad48e1858 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 836d59a001bc..aaf467c9ef2b 100644
--- a/tools/perf/util/callchain.h
+++ b/tools/perf/util/callchain.h
@@ -75,6 +75,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.6.0

Re: [RFC/PATCH 3/3] perf tools: Defaults to 'caller' callchain order only if --children is enabled

[Ingo Molnar]

* Namhyung Kim <namhyung@kernel.org> wrote:

> 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 caller order.
> 
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
> Cc: Chandler Carruth <chandlerc@gmail.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Stephane Eranian <eranian@google.com>
> Cc: Wang Nan <wangnan0@huawei.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Very nice fixes, thanks!

	Ingo

Re: [RFC/PATCH 3/3] perf tools: Defaults to 'caller' callchain order only if --children is enabled

[Namhyung Kim]

Hi Ingo,

On Thu, Oct 22, 2015 at 4:32 PM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Namhyung Kim <namhyung@kernel.org> wrote:
>
>> 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 caller order.

Oops, there's a typo:  s/caller order/callee order/ :)


>>
>> Cc: Adrian Hunter <adrian.hunter@intel.com>
>> Cc: Borislav Petkov <bp@suse.de>
>> Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
>> Cc: Chandler Carruth <chandlerc@gmail.com>
>> Cc: Frederic Weisbecker <fweisbec@gmail.com>
>> Cc: Stephane Eranian <eranian@google.com>
>> Cc: Wang Nan <wangnan0@huawei.com>
>> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
>
> Very nice fixes, thanks!

Thank you!
Namhyung

[RFC/PATCH RESEND 3/3] perf tools: Defaults to 'caller' callchain order only if --children is enabled

[Namhyung Kim]

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.

Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
Cc: Chandler Carruth <chandlerc@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
fix typo

 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 +-
 5 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 18a8c52d921e..545c51cef7f7 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -812,6 +812,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 1de381d3f29f..af849b1d7389 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1304,6 +1304,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 842be32899ee..735ad48e1858 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 836d59a001bc..aaf467c9ef2b 100644
--- a/tools/perf/util/callchain.h
+++ b/tools/perf/util/callchain.h
@@ -75,6 +75,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.6.0

Re: [PATCH 1/3] perf tools: Move callchain help messages to callchain.h

[Ingo Molnar]

* Namhyung Kim <namhyung@kernel.org> wrote:

> +#define CALLCHAIN_HELP "setup and enables call-graph (stack chain/backtrace) recording: "
> +
> +#ifdef HAVE_DWARF_UNWIND_SUPPORT
> +#define CALLCHAIN_RECORD_HELP  CALLCHAIN_HELP "fp dwarf lbr"
> +#else
> +#define CALLCHAIN_RECORD_HELP  CALLCHAIN_HELP "fp lbr"
> +#endif

nano-nit, could we structure such balanced #ifdefs the following way:

#ifdef HAVE_DWARF_UNWIND_SUPPORT
# define CALLCHAIN_RECORD_HELP CALLCHAIN_HELP "fp dwarf lbr"
#else
# define CALLCHAIN_RECORD_HELP CALLCHAIN_HELP "fp lbr"
#endif

makes the construct stand out a lot better visually.

I also had another look at the help text:

> output_type,min_percent[,print_limit],call_order[,branch]

> +#define CALLCHAIN_REPORT_HELP  "output_type (graph, flat, fractal, or none), " \
> +	"min percent threshold, optional print limit, callchain order, " \
> +	"key (function or address), add branches"

Btw., when I first read this message in the help text yesterday, I had to read the 
'min percent threshold' twice, to realize that the default 0.5 is in units of 
percentage - the wording wasn't entirely clear about that.

Also, I had to go into the code to decode the real meaning of all the other 
parameters. I'd have expected them to be more obvious from reading the help text.

Wording them the following way would have made things a lot more apparent to me:

	print_style,min_percent[,print_percent],call_order[,key]

	call chain tree printing style (graph|flat|fractal|none)
	minimum tree inclusion threshold (percent)
	printing threshold (percent)
	call chain order (caller|callee)
	key (function|address|branch)

Note that I extended the help text with new options not mentioned in the help text 
but present in the current code - such as the 'branch' key.

Also note that in the code I did not find any trace of the '[,branch]' and
'add branches' part present in the help text. What we have is a 'branch'
option in the (optional) key parameter.

I also made various edits to the help text to make it more consistent and more 
self-explanatory. I think we should also put the various options into a new line 
in the help screen, not the single line dump of text it is currently.

Btw., we also have a grammar problem with all things call chains: there's 800+ 
occurances of 'callchain' in the perf code, and less than 20 spellings of 'call 
chain'. But the latter is the correct variant: Google won't even let you search 
for 'callchain' by default and corrects it to 'call chain' automatically.

If you insist on searching for 'callchain', Google finds this number of hits:

  'code callchain':       54,200
  'code call chain': 141,000,000

I think it's pretty obvious what the dominant spelling is in the industry! ;-)

So we should probably rename all occurances of 'callchain' to 'call chain' or 
'call_chain'.

Thanks,

	Ingo

Re: [PATCH 2/3] perf top: Support call-graph display options also

[Ingo Molnar]

* Namhyung Kim <namhyung@kernel.org> wrote:

> Currently 'perf top --call-graph' option is same as 'perf record'.  But
> 'perf top' also need to receive display options in 'perf report'.  To do
> that, change parse_callchain_report_opt() to allow record options too.
> 
> Now perf top can receive display options like below:
> 
>   $ perf top --call-graph
>     Error: option `call-graph' requires a value
> 
>    Usage: perf top [<options>]
> 
>         --call-graph
>           <mode[,dump_size],output_type,min_percent[,print_limit],call_order[,branch]>
>                      setup and enables call-graph (stack chain/backtrace)
>                      recording: fp dwarf lbr, output_type (graph, flat,
> 		     fractal, or none), min percent threshold, optional
> 		     print limit, callchain order, key (function or
> 		     address), add branches

Yeah, so this fix is nice, and I think we should also do another patch to fix the 
help text output to be the following:

         --call-graph
           <record_mode[,record_size],print_style,limit[,print_limit],call_order[,sort_key]>

              Set up and enable call graph (call chain, stack backtrace) recording:

                 record_mode:   call graph recording mode (fp|dwarf|lbr)
                 record_size:   if rec_mode == dwarf, maximum depth of stack recording (bytes),
                                default: 8192 bytes.
                 print_style:   call graph printing style (graph|flat|fractal|none)
                 limit:         minimum call graph inclusion threshold (percent)
                 print_limit:   printing threshold (percent)
                 call_order:    call graph order (caller|callee)
                 sort_key:      sorting key (function|address|branch)

               Default: fp,graph,0.5,0.0,caller,function

Note that this text evolved a bit over what I sent in my previous mail.

Also note that I think we should sync up the perf config options to be the same as 
the option name shortcuts used in this help text.

Side note: to improve perf config usability, we should probably also recognize 
underscores in perf config entries, i.e. the following variants should both work:

	print_percent = 1
	print-percent = 1

Right now only the second one will match.

Thanks,

	Ingo

Re: [RFC/PATCH 3/3] perf tools: Defaults to 'caller' callchain order only if --children is enabled

[Brendan Gregg]

On Thu, Oct 22, 2015 at 12:38 AM, Namhyung Kim <namhyung@kernel.org> wrote:
> Hi Ingo,
>
> On Thu, Oct 22, 2015 at 4:32 PM, Ingo Molnar <mingo@kernel.org> wrote:
>>
>> * Namhyung Kim <namhyung@kernel.org> wrote:
>>
>>> 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 caller order.
>
> Oops, there's a typo:  s/caller order/callee order/ :)

Thanks, I was wondering about that (and I've made that typo myself).
Thanks for the patch!

Brendan

Re: [PATCH 1/3] perf tools: Move callchain help messages to callchain.h

[Namhyung Kim]

On Thu, Oct 22, 2015 at 5:02 PM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Namhyung Kim <namhyung@kernel.org> wrote:
>
>> +#define CALLCHAIN_HELP "setup and enables call-graph (stack chain/backtrace) recording: "
>> +
>> +#ifdef HAVE_DWARF_UNWIND_SUPPORT
>> +#define CALLCHAIN_RECORD_HELP  CALLCHAIN_HELP "fp dwarf lbr"
>> +#else
>> +#define CALLCHAIN_RECORD_HELP  CALLCHAIN_HELP "fp lbr"
>> +#endif
>
> nano-nit, could we structure such balanced #ifdefs the following way:
>
> #ifdef HAVE_DWARF_UNWIND_SUPPORT
> # define CALLCHAIN_RECORD_HELP CALLCHAIN_HELP "fp dwarf lbr"
> #else
> # define CALLCHAIN_RECORD_HELP CALLCHAIN_HELP "fp lbr"
> #endif
>
> makes the construct stand out a lot better visually.

OK

>
> I also had another look at the help text:
>
>> output_type,min_percent[,print_limit],call_order[,branch]
>
>> +#define CALLCHAIN_REPORT_HELP  "output_type (graph, flat, fractal, or none), " \
>> +     "min percent threshold, optional print limit, callchain order, " \
>> +     "key (function or address), add branches"
>
> Btw., when I first read this message in the help text yesterday, I had to read the
> 'min percent threshold' twice, to realize that the default 0.5 is in units of
> percentage - the wording wasn't entirely clear about that.

OK

>
> Also, I had to go into the code to decode the real meaning of all the other
> parameters. I'd have expected them to be more obvious from reading the help text.

Did you check the man page also?  I think we have (short) explanation
for each parameter and users should read it first to understand the
meaning.  But I agree that the help text should also be improved to
provide quick reference.


>
> Wording them the following way would have made things a lot more apparent to me:
>
>         print_style,min_percent[,print_percent],call_order[,key]
>
>         call chain tree printing style (graph|flat|fractal|none)
>         minimum tree inclusion threshold (percent)
>         printing threshold (percent)

Note that this 'printing threshold' is not percent.  It's to limit
number of callchain entries printed for each hist entry.  However it
works for --stdio only probably since it lacks interactive
collapse/expand feature.


>         call chain order (caller|callee)
>         key (function|address|branch)
>
> Note that I extended the help text with new options not mentioned in the help text
> but present in the current code - such as the 'branch' key.
>
> Also note that in the code I did not find any trace of the '[,branch]' and
> 'add branches' part present in the help text. What we have is a 'branch'
> option in the (optional) key parameter.

Looking at the document, it seems branch is not a key:

    branch can be:
    - branch: include last branch information in callgraph
    when available. Usually more convenient to use --branch-history
    for this.

Confusingly, it was checked in parse_callchain_sort_key() but does
nothing with the sort key IIUC.

>
> I also made various edits to the help text to make it more consistent and more
> self-explanatory. I think we should also put the various options into a new line
> in the help screen, not the single line dump of text it is currently.

OK

>
> Btw., we also have a grammar problem with all things call chains: there's 800+
> occurances of 'callchain' in the perf code, and less than 20 spellings of 'call
> chain'. But the latter is the correct variant: Google won't even let you search
> for 'callchain' by default and corrects it to 'call chain' automatically.
>
> If you insist on searching for 'callchain', Google finds this number of hits:
>
>   'code callchain':       54,200
>   'code call chain': 141,000,000
>
> I think it's pretty obvious what the dominant spelling is in the industry! ;-)
>
> So we should probably rename all occurances of 'callchain' to 'call chain' or
> 'call_chain'.

Not sure about this part.  Do you really think it's worth changing?

Thanks,
Namhyung

Re: [RFC/PATCH 3/3] perf tools: Defaults to 'caller' callchain order only if --children is enabled

[Frederic Weisbecker]

On Thu, Oct 22, 2015 at 03:28:50PM +0900, Namhyung Kim wrote:
> 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 caller order.
                                 ^^^^
                                 callee?

Thanks!

Re: [PATCH 2/3] perf top: Support call-graph display options also

[Namhyung Kim]

On Thu, Oct 22, 2015 at 5:23 PM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Namhyung Kim <namhyung@kernel.org> wrote:
>
>> Currently 'perf top --call-graph' option is same as 'perf record'.  But
>> 'perf top' also need to receive display options in 'perf report'.  To do
>> that, change parse_callchain_report_opt() to allow record options too.
>>
>> Now perf top can receive display options like below:
>>
>>   $ perf top --call-graph
>>     Error: option `call-graph' requires a value
>>
>>    Usage: perf top [<options>]
>>
>>         --call-graph
>>           <mode[,dump_size],output_type,min_percent[,print_limit],call_order[,branch]>
>>                      setup and enables call-graph (stack chain/backtrace)
>>                      recording: fp dwarf lbr, output_type (graph, flat,
>>                    fractal, or none), min percent threshold, optional
>>                    print limit, callchain order, key (function or
>>                    address), add branches
>
> Yeah, so this fix is nice, and I think we should also do another patch to fix the
> help text output to be the following:
>
>          --call-graph
>            <record_mode[,record_size],print_style,limit[,print_limit],call_order[,sort_key]>
>
>               Set up and enable call graph (call chain, stack backtrace) recording:
>
>                  record_mode:   call graph recording mode (fp|dwarf|lbr)
>                  record_size:   if rec_mode == dwarf, maximum depth of stack recording (bytes),
>                                 default: 8192 bytes.
>                  print_style:   call graph printing style (graph|flat|fractal|none)
>                  limit:         minimum call graph inclusion threshold (percent)
>                  print_limit:   printing threshold (percent)
>                  call_order:    call graph order (caller|callee)
>                  sort_key:      sorting key (function|address|branch)
>
>                Default: fp,graph,0.5,0.0,caller,function
>
> Note that this text evolved a bit over what I sent in my previous mail.

OK.  I'll cook a patch to improve help message like this as well as man page.


>
> Also note that I think we should sync up the perf config options to be the same as
> the option name shortcuts used in this help text.

Ah, good idea.

>
> Side note: to improve perf config usability, we should probably also recognize
> underscores in perf config entries, i.e. the following variants should both work:
>
>         print_percent = 1
>         print-percent = 1
>
> Right now only the second one will match.

Taeung, could you consider fixing this in your config patchset?

Thanks,
Namhyung

Re: [RFC/PATCH 3/3] perf tools: Defaults to 'caller' callchain order only if --children is enabled

[Frederic Weisbecker]

On Thu, Oct 22, 2015 at 04:38:44PM +0900, Namhyung Kim wrote:
> Hi Ingo,
> 
> On Thu, Oct 22, 2015 at 4:32 PM, Ingo Molnar <mingo@kernel.org> wrote:
> >
> > * Namhyung Kim <namhyung@kernel.org> wrote:
> >
> >> 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 caller order.
> 
> Oops, there's a typo:  s/caller order/callee order/ :)

Oops, didn't see that reply.

Thanks!

Acked-by: Frederic Weisbecker <fweisbec@gmail.com>

Re: [PATCH 2/3] perf top: Support call-graph display options also

[Taeung Song]

Hi, Namhyung and Ingo

> On Oct 22, 2015, at 9:20 PM, Namhyung Kim <namhyung@kernel.org> wrote:
> 
> On Thu, Oct 22, 2015 at 5:23 PM, Ingo Molnar <mingo@kernel.org> wrote:
>> 
>> * Namhyung Kim <namhyung@kernel.org> wrote:
>> 
>>> Currently 'perf top --call-graph' option is same as 'perf record'.  But
>>> 'perf top' also need to receive display options in 'perf report'.  To do
>>> that, change parse_callchain_report_opt() to allow record options too.
>>> 
>>> Now perf top can receive display options like below:
>>> 
>>>  $ perf top --call-graph
>>>    Error: option `call-graph' requires a value
>>> 
>>>   Usage: perf top [<options>]
>>> 
>>>        --call-graph
>>>          <mode[,dump_size],output_type,min_percent[,print_limit],call_order[,branch]>
>>>                     setup and enables call-graph (stack chain/backtrace)
>>>                     recording: fp dwarf lbr, output_type (graph, flat,
>>>                   fractal, or none), min percent threshold, optional
>>>                   print limit, callchain order, key (function or
>>>                   address), add branches
>> 
>> Yeah, so this fix is nice, and I think we should also do another patch to fix the
>> help text output to be the following:
>> 
>>         --call-graph
>>           <record_mode[,record_size],print_style,limit[,print_limit],call_order[,sort_key]>
>> 
>>              Set up and enable call graph (call chain, stack backtrace) recording:
>> 
>>                 record_mode:   call graph recording mode (fp|dwarf|lbr)
>>                 record_size:   if rec_mode == dwarf, maximum depth of stack recording (bytes),
>>                                default: 8192 bytes.
>>                 print_style:   call graph printing style (graph|flat|fractal|none)
>>                 limit:         minimum call graph inclusion threshold (percent)
>>                 print_limit:   printing threshold (percent)
>>                 call_order:    call graph order (caller|callee)
>>                 sort_key:      sorting key (function|address|branch)
>> 
>>               Default: fp,graph,0.5,0.0,caller,function
>> 
>> Note that this text evolved a bit over what I sent in my previous mail.
> 
> OK.  I'll cook a patch to improve help message like this as well as man page.
> 
> 
>> 
>> Also note that I think we should sync up the perf config options to be the same as
>> the option name shortcuts used in this help text.
> 
> Ah, good idea.
> 
>> 
>> Side note: to improve perf config usability, we should probably also recognize
>> underscores in perf config entries, i.e. the following variants should both work:
>> 
>>        print_percent = 1
>>        print-percent = 1
>> 
>> Right now only the second one will match.
> 
> Taeung, could you consider fixing this in your config patchiest?
> 

Sure, no problem.

I thought as below.
Even if one use a print_percent variable, it will be handled like print-percent.

Also if a user change print_percent variable by perf-config command,
print-percent variable will remain in perfconfig file.
(Because perf-config command will rewrite perfconfig file.)

Is anything wrong ?

Thanks,
Taeung

Re: [RFC/PATCH 3/3] perf tools: Defaults to 'caller' callchain order only if --children is enabled

[Arnaldo Carvalho de Melo]

Em Thu, Oct 22, 2015 at 02:49:11AM -0700, Brendan Gregg escreveu:
> On Thu, Oct 22, 2015 at 12:38 AM, Namhyung Kim <namhyung@kernel.org> wrote:
> > Hi Ingo,
> >
> > On Thu, Oct 22, 2015 at 4:32 PM, Ingo Molnar <mingo@kernel.org> wrote:
> >>
> >> * Namhyung Kim <namhyung@kernel.org> wrote:
> >>
> >>> 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 caller order.
> >
> > Oops, there's a typo:  s/caller order/callee order/ :)
> 
> Thanks, I was wondering about that (and I've made that typo myself).
> Thanks for the patch!

Brendan, can I take that as an Acked-by?
 
> Brendan

Re: [RFC/PATCH 3/3] perf tools: Defaults to 'caller' callchain order only if --children is enabled

[Arnaldo Carvalho de Melo]

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?

Also you forgot to fix the docs to mention this new default, I did it
and the resulting patch, still with GRAPH_ABS follows:

>From fd2dcde26d09b34fccc3c164d9fc36e662c21fc4 Mon Sep 17 00:00:00 2001
From: Namhyung Kim <namhyung@kernel.org>
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 <namhyung@kernel.org>
Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
Acked-by: Brendan Gregg <brendan.d.gregg@gmail.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Chandler Carruth <chandlerc@gmail.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
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 <acme@redhat.com>
---
 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

Re: [PATCH 1/3] perf tools: Move callchain help messages to callchain.h

[Arnaldo Carvalho de Melo]

Em Thu, Oct 22, 2015 at 09:13:49PM +0900, Namhyung Kim escreveu:
> On Thu, Oct 22, 2015 at 5:02 PM, Ingo Molnar <mingo@kernel.org> wrote:
> >
> > * Namhyung Kim <namhyung@kernel.org> wrote:
> >
> >> +#define CALLCHAIN_HELP "setup and enables call-graph (stack chain/backtrace) recording: "
> >> +
> >> +#ifdef HAVE_DWARF_UNWIND_SUPPORT
> >> +#define CALLCHAIN_RECORD_HELP  CALLCHAIN_HELP "fp dwarf lbr"
> >> +#else
> >> +#define CALLCHAIN_RECORD_HELP  CALLCHAIN_HELP "fp lbr"
> >> +#endif
> >
> > nano-nit, could we structure such balanced #ifdefs the following way:
> >
> > #ifdef HAVE_DWARF_UNWIND_SUPPORT
> > # define CALLCHAIN_RECORD_HELP CALLCHAIN_HELP "fp dwarf lbr"
> > #else
> > # define CALLCHAIN_RECORD_HELP CALLCHAIN_HELP "fp lbr"
> > #endif
> >
> > makes the construct stand out a lot better visually.
> 
> OK

Done.
 
> >
> > I also had another look at the help text:
> >
> >> output_type,min_percent[,print_limit],call_order[,branch]
> >
> >> +#define CALLCHAIN_REPORT_HELP  "output_type (graph, flat, fractal, or none), " \
> >> +     "min percent threshold, optional print limit, callchain order, " \
> >> +     "key (function or address), add branches"
> >
> > Btw., when I first read this message in the help text yesterday, I had to read the
> > 'min percent threshold' twice, to realize that the default 0.5 is in units of
> > percentage - the wording wasn't entirely clear about that.
> 
> OK
> 
> >
> > Also, I had to go into the code to decode the real meaning of all the other
> > parameters. I'd have expected them to be more obvious from reading the help text.
> 
> Did you check the man page also?  I think we have (short) explanation
> for each parameter and users should read it first to understand the
> meaning.  But I agree that the help text should also be improved to
> provide quick reference.
> 
> 
> >
> > Wording them the following way would have made things a lot more apparent to me:
> >
> >         print_style,min_percent[,print_percent],call_order[,key]
> >
> >         call chain tree printing style (graph|flat|fractal|none)
> >         minimum tree inclusion threshold (percent)
> >         printing threshold (percent)
> 
> Note that this 'printing threshold' is not percent.  It's to limit
> number of callchain entries printed for each hist entry.  However it
> works for --stdio only probably since it lacks interactive
> collapse/expand feature.
> 
> 
> >         call chain order (caller|callee)
> >         key (function|address|branch)
> >
> > Note that I extended the help text with new options not mentioned in the help text
> > but present in the current code - such as the 'branch' key.
> >
> > Also note that in the code I did not find any trace of the '[,branch]' and
> > 'add branches' part present in the help text. What we have is a 'branch'
> > option in the (optional) key parameter.
> 
> Looking at the document, it seems branch is not a key:
> 
>     branch can be:
>     - branch: include last branch information in callgraph
>     when available. Usually more convenient to use --branch-history
>     for this.
> 
> Confusingly, it was checked in parse_callchain_sort_key() but does
> nothing with the sort key IIUC.
> 
> >
> > I also made various edits to the help text to make it more consistent and more
> > self-explanatory. I think we should also put the various options into a new line
> > in the help screen, not the single line dump of text it is currently.
> 
> OK

These can come on a followup patch, improving the situation, right?
 
> >
> > Btw., we also have a grammar problem with all things call chains: there's 800+
> > occurances of 'callchain' in the perf code, and less than 20 spellings of 'call
> > chain'. But the latter is the correct variant: Google won't even let you search
> > for 'callchain' by default and corrects it to 'call chain' automatically.
> >
> > If you insist on searching for 'callchain', Google finds this number of hits:
> >
> >   'code callchain':       54,200
> >   'code call chain': 141,000,000
> >
> > I think it's pretty obvious what the dominant spelling is in the industry! ;-)
> >
> > So we should probably rename all occurances of 'callchain' to 'call chain' or
> > 'call_chain'.
> 
> Not sure about this part.  Do you really think it's worth changing?
> 
> Thanks,
> Namhyung

Re: [RFC/PATCH 3/3] perf tools: Defaults to 'caller' callchain order only if --children is enabled

[Namhyung Kim]

Hi Arnaldo,

On Thu, Oct 22, 2015 at 11:13 PM, Arnaldo Carvalho de Melo
<acme@kernel.org> 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 <namhyung@kernel.org>
> 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 <namhyung@kernel.org>
> Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
> Acked-by: Brendan Gregg <brendan.d.gregg@gmail.com>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: Chandler Carruth <chandlerc@gmail.com>
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Stephane Eranian <eranian@google.com>
> Cc: Wang Nan <wangnan0@huawei.com>
> 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 <acme@redhat.com>
> ---
>  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

Re: [PATCH 2/3] perf top: Support call-graph display options also

[Namhyung Kim]

On Thu, Oct 22, 2015 at 10:57 PM, Taeung Song <treeze.taeung@gmail.com> wrote:
> Hi, Namhyung and Ingo
>
>> On Oct 22, 2015, at 9:20 PM, Namhyung Kim <namhyung@kernel.org> wrote:
>>
>> On Thu, Oct 22, 2015 at 5:23 PM, Ingo Molnar <mingo@kernel.org> wrote:
>>>
>>> * Namhyung Kim <namhyung@kernel.org> wrote:
>>>
>>>> Currently 'perf top --call-graph' option is same as 'perf record'.  But
>>>> 'perf top' also need to receive display options in 'perf report'.  To do
>>>> that, change parse_callchain_report_opt() to allow record options too.
>>>>
>>>> Now perf top can receive display options like below:
>>>>
>>>>  $ perf top --call-graph
>>>>    Error: option `call-graph' requires a value
>>>>
>>>>   Usage: perf top [<options>]
>>>>
>>>>        --call-graph
>>>>          <mode[,dump_size],output_type,min_percent[,print_limit],call_order[,branch]>
>>>>                     setup and enables call-graph (stack chain/backtrace)
>>>>                     recording: fp dwarf lbr, output_type (graph, flat,
>>>>                   fractal, or none), min percent threshold, optional
>>>>                   print limit, callchain order, key (function or
>>>>                   address), add branches
>>>
>>> Yeah, so this fix is nice, and I think we should also do another patch to fix the
>>> help text output to be the following:
>>>
>>>         --call-graph
>>>           <record_mode[,record_size],print_style,limit[,print_limit],call_order[,sort_key]>
>>>
>>>              Set up and enable call graph (call chain, stack backtrace) recording:
>>>
>>>                 record_mode:   call graph recording mode (fp|dwarf|lbr)
>>>                 record_size:   if rec_mode == dwarf, maximum depth of stack recording (bytes),
>>>                                default: 8192 bytes.
>>>                 print_style:   call graph printing style (graph|flat|fractal|none)
>>>                 limit:         minimum call graph inclusion threshold (percent)
>>>                 print_limit:   printing threshold (percent)
>>>                 call_order:    call graph order (caller|callee)
>>>                 sort_key:      sorting key (function|address|branch)
>>>
>>>               Default: fp,graph,0.5,0.0,caller,function
>>>
>>> Note that this text evolved a bit over what I sent in my previous mail.
>>
>> OK.  I'll cook a patch to improve help message like this as well as man page.
>>
>>
>>>
>>> Also note that I think we should sync up the perf config options to be the same as
>>> the option name shortcuts used in this help text.
>>
>> Ah, good idea.
>>
>>>
>>> Side note: to improve perf config usability, we should probably also recognize
>>> underscores in perf config entries, i.e. the following variants should both work:
>>>
>>>        print_percent = 1
>>>        print-percent = 1
>>>
>>> Right now only the second one will match.
>>
>> Taeung, could you consider fixing this in your config patchiest?
>>
>
> Sure, no problem.
>
> I thought as below.
> Even if one use a print_percent variable, it will be handled like print-percent.
>
> Also if a user change print_percent variable by perf-config command,
> print-percent variable will remain in perfconfig file.
> (Because perf-config command will rewrite perfconfig file.)
>
> Is anything wrong ?

Nop, looks good to me!

Thanks,
Namhyung

Re: [RFC/PATCH 3/3] perf tools: Defaults to 'caller' callchain order only if --children is enabled

[Arnaldo Carvalho de Melo]

Em Thu, Oct 22, 2015 at 11:22:40PM +0900, Namhyung Kim escreveu:
> On Thu, Oct 22, 2015 at 11:13 PM, Arnaldo Carvalho de Melo <acme@kernel.org> 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,
> >> -     .order  = ORDER_CALLER,
> >> +     .order  = ORDER_CALLEE,

> > 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.

Ok, its just that I don't recall seeing the part of the discussion about
keeping the change from REL to ABS.
 
> > 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.

Ok, but the default as it stands after applying this patch is "graph,
calee", so the docs should be changed to reflect that, yes, its not just
that, we need to tell, in the --children doc, that it defaults to
"caller".

Will we also flip the default to --no-children? I would advocate that,
together with showing a info box telling the user about this change and
how to ask for it, including instructions on how to do that via
~/.perfconfig.

> 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.

Ok, its just that I thought you was already asleep and was wanting to
make progress on this 8-)

Will wait for your patches and instead try to fix the annotation bug
that leads 'perf report --tui -S some_symbol_name' to exit without
printing anything, that Ingo reported, only happens in --tui, because
--stdio doesn't collects annotation info...

- Arnaldo

Re: [RFC/PATCH 3/3] perf tools: Defaults to 'caller' callchain order only if --children is enabled

[Namhyung Kim]

On Thu, Oct 22, 2015 at 11:37 PM, Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
> Em Thu, Oct 22, 2015 at 11:22:40PM +0900, Namhyung Kim escreveu:
>> On Thu, Oct 22, 2015 at 11:13 PM, Arnaldo Carvalho de Melo <acme@kernel.org> 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,
>> >> -     .order  = ORDER_CALLER,
>> >> +     .order  = ORDER_CALLEE,
>
>> > 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.
>
> Ok, its just that I don't recall seeing the part of the discussion about
> keeping the change from REL to ABS.
>
>> > 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.
>
> Ok, but the default as it stands after applying this patch is "graph,
> calee", so the docs should be changed to reflect that, yes, its not just
> that, we need to tell, in the --children doc, that it defaults to
> "caller".
>
> Will we also flip the default to --no-children? I would advocate that,
> together with showing a info box telling the user about this change and
> how to ask for it, including instructions on how to do that via
> ~/.perfconfig.

Not sure.  It seems too late to do it. ;-)

>
>> 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.
>
> Ok, its just that I thought you was already asleep and was wanting to
> make progress on this 8-)

Yeah, it's late.  I'll go to bed soon.. :)

>
> Will wait for your patches and instead try to fix the annotation bug
> that leads 'perf report --tui -S some_symbol_name' to exit without
> printing anything, that Ingo reported, only happens in --tui, because
> --stdio doesn't collects annotation info...

I guess -S option works only for symbols that have self overhead..

Thanks,
Namhyung

Re: [RFC/PATCH 3/3] perf tools: Defaults to 'caller' callchain order only if --children is enabled

[Arnaldo Carvalho de Melo]

Em Fri, Oct 23, 2015 at 12:51:01AM +0900, Namhyung Kim escreveu:
> On Thu, Oct 22, 2015 at 11:37 PM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > Will we also flip the default to --no-children? I would advocate that,
> > together with showing a info box telling the user about this change and
> > how to ask for it, including instructions on how to do that via
> > ~/.perfconfig.
 
> Not sure.  It seems too late to do it. ;-)

Heh, well, at least a info box, to show just once, telling where to read
about --children mode, and how to disable it, I'll probably cook this
up.
 
> >> 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.

> > Ok, its just that I thought you was already asleep and was wanting to
> > make progress on this 8-)
 
> Yeah, it's late.  I'll go to bed soon.. :)

:-)
 
> > Will wait for your patches and instead try to fix the annotation bug
> > that leads 'perf report --tui -S some_symbol_name' to exit without
> > printing anything, that Ingo reported, only happens in --tui, because
> > --stdio doesn't collects annotation info...
 
> I guess -S option works only for symbols that have self overhead..

Sure, say:

  # perf record -a sleep 1
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.171 MB perf.data (33 samples) ]
  # perf script | head -3
    perf 15174 [000] 52772.188928:  1 cycles:pp: ffffffff8105f4b8 native_write_msr_safe (/lib/modules/4.2.0/build/vmlinux)
    perf 15174 [000] 52772.188934:  1 cycles:pp: ffffffff8105f4b8 native_write_msr_safe (/lib/modules/4.2.0/build/vmlinux)
   swapper     0 [001] 52772.188971:  1 cycles:pp: ffffffff8105f4b8 native_write_msr_safe (/lib/modules/4.2.0/build/vmlinux)
  # perf report -S native_write_msr_safe
  # time perf report -S native_write_msr_safe

  real	0m0.023s
  user	0m0.019s
  sys	0m0.004s
  #

I.e. 'perf report -S symbol_that_has_self_overhead' doesn't work in --tui mode,
works in --stdio mode:

  # perf report --stdio -S native_write_msr_safe  | grep %
    59.61%  swapper  [kernel.vmlinux]
     0.17%  sleep    [kernel.vmlinux]
     0.00%  perf     [kernel.vmlinux]
  # 

Somehow we're calling symbol__inc_addr_samples() with a sample that is outside
that symbol, it returns ERANGE and we exit early, not emitting any message.

That happens only in --tui mode, --stdio doesn't call any annotation routine,
since it is not integrated with 'perf annotate' because it is not interactive.

- Arnaldo

Re: [PATCH 1/3] perf tools: Move callchain help messages to callchain.h

[Arnaldo Carvalho de Melo]

Em Thu, Oct 22, 2015 at 03:28:48PM +0900, Namhyung Kim escreveu:
> These messages will be used by 'perf top' in the next patch.

Applied

Re: [PATCH 2/3] perf top: Support call-graph display options also

[Arnaldo Carvalho de Melo]

Em Thu, Oct 22, 2015 at 03:28:49PM +0900, Namhyung Kim escreveu:
> Currently 'perf top --call-graph' option is same as 'perf record'.  But
> 'perf top' also need to receive display options in 'perf report'.  To do
> that, change parse_callchain_report_opt() to allow record options too.
> 
> Now perf top can receive display options like below:

Applied.

Re: [RFC/PATCH RESEND 3/3] perf tools: Defaults to 'caller' callchain order only if --children is enabled

[Arnaldo Carvalho de Melo]

Em Thu, Oct 22, 2015 at 04:45:46PM +0900, Namhyung Kim escreveu:
> 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.

Applied.

[tip:perf/core] perf tools: Move callchain help messages to callchain.h

[tip-bot for Namhyung Kim]

Commit-ID:  21cf62847d29392e51c37460856d3c3c57769c5e
Gitweb:     http://git.kernel.org/tip/21cf62847d29392e51c37460856d3c3c57769c5e
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 22 Oct 2015 15:28:48 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 22 Oct 2015 15:39:51 -0300

perf tools: Move callchain help messages to callchain.h

These messages will be used by 'perf top' in the next patch.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
Cc: Chandler Carruth <chandlerc@gmail.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1445495330-25416-1-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-record.c |  8 +-------
 tools/perf/builtin-report.c | 10 +++++++---
 tools/perf/util/callchain.h | 12 ++++++++++++
 3 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 24ace2f..1a11762 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -1010,13 +1010,7 @@ static struct record record = {
 	},
 };
 
-#define CALLCHAIN_HELP "setup and enables call-graph (stack chain/backtrace) recording: "
-
-#ifdef HAVE_DWARF_UNWIND_SUPPORT
-const char record_callchain_help[] = CALLCHAIN_HELP "fp dwarf lbr";
-#else
-const char record_callchain_help[] = CALLCHAIN_HELP "fp lbr";
-#endif
+const char record_callchain_help[] = CALLCHAIN_RECORD_HELP;
 
 /*
  * XXX Will stay a global variable till we fix builtin-script.c to stop messing
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 3b23b25..18a8c52 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -625,6 +625,9 @@ parse_percent_limit(const struct option *opt, const char *str,
 	return 0;
 }
 
+const char report_callchain_help[] = "Display callchains using " CALLCHAIN_REPORT_HELP ". "
+				     "Default: graph,0.5,caller";
+
 int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 {
 	struct perf_session *session;
@@ -699,9 +702,10 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 		   "regex filter to identify parent, see: '--sort parent'"),
 	OPT_BOOLEAN('x', "exclude-other", &symbol_conf.exclude_other,
 		    "Only display entries with parent-match"),
-	OPT_CALLBACK_DEFAULT('g', "call-graph", &report, "output_type,min_percent[,print_limit],call_order[,branch]",
-		     "Display callchains using output_type (graph, flat, fractal, or none) , min percent threshold, optional print limit, callchain order, key (function or address), add branches. "
-		     "Default: graph,0.5,caller", &report_parse_callchain_opt, callchain_default_opt),
+	OPT_CALLBACK_DEFAULT('g', "call-graph", &report,
+			     "output_type,min_percent[,print_limit],call_order[,branch]",
+			     report_callchain_help, &report_parse_callchain_opt,
+			     callchain_default_opt),
 	OPT_BOOLEAN(0, "children", &symbol_conf.cumulate_callchain,
 		    "Accumulate callchains of children and show total overhead as well"),
 	OPT_INTEGER(0, "max-stack", &report.max_stack,
diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
index acee2b3..c9e3a2e 100644
--- a/tools/perf/util/callchain.h
+++ b/tools/perf/util/callchain.h
@@ -7,6 +7,18 @@
 #include "event.h"
 #include "symbol.h"
 
+#define CALLCHAIN_HELP "setup and enables call-graph (stack chain/backtrace) recording: "
+
+#ifdef HAVE_DWARF_UNWIND_SUPPORT
+#define CALLCHAIN_RECORD_HELP  CALLCHAIN_HELP "fp dwarf lbr"
+#else
+#define CALLCHAIN_RECORD_HELP  CALLCHAIN_HELP "fp lbr"
+#endif
+
+#define CALLCHAIN_REPORT_HELP  "output_type (graph, flat, fractal, or none), " \
+	"min percent threshold, optional print limit, callchain order, " \
+	"key (function or address), add branches"
+
 enum perf_call_graph_mode {
 	CALLCHAIN_NONE,
 	CALLCHAIN_FP,

[tip:perf/core] perf top: Support call-graph display options also

[tip-bot for Namhyung Kim]

Commit-ID:  a2c10d39af49b00514f7cc7b750757fcc2174f0c
Gitweb:     http://git.kernel.org/tip/a2c10d39af49b00514f7cc7b750757fcc2174f0c
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 22 Oct 2015 15:28:49 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 22 Oct 2015 15:40:02 -0300

perf top: Support call-graph display options also

Currently 'perf top --call-graph' option is same as 'perf record'.  But
'perf top' also need to receive display options in 'perf report'.  To do
that, change parse_callchain_report_opt() to allow record options too.

Now perf top can receive display options like below:

  $ perf top --call-graph
    Error: option `call-graph' requires a value

   Usage: perf top [<options>]

        --call-graph
          <mode[,dump_size],output_type,min_percent[,print_limit],call_order[,branch]>
                     setup and enables call-graph (stack chain/backtrace)
                     recording: fp dwarf lbr, output_type (graph, flat,
		     fractal, or none), min percent threshold, optional
		     print limit, callchain order, key (function or
		     address), add branches

  $ perf top --call-graph callee,graph,fp

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
Cc: Chandler Carruth <chandlerc@gmail.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1445495330-25416-2-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Documentation/perf-top.txt |  5 +++--
 tools/perf/builtin-top.c              | 26 ++++++++++++++++++-----
 tools/perf/util/callchain.c           | 40 ++++++++++++++++++++++++++++++++---
 tools/perf/util/callchain.h           |  1 +
 4 files changed, 62 insertions(+), 10 deletions(-)

diff --git a/tools/perf/Documentation/perf-top.txt b/tools/perf/Documentation/perf-top.txt
index f6a23eb..556cec0 100644
--- a/tools/perf/Documentation/perf-top.txt
+++ b/tools/perf/Documentation/perf-top.txt
@@ -160,9 +160,10 @@ Default is to monitor all CPUS.
 -g::
 	Enables call-graph (stack chain/backtrace) recording.
 
---call-graph::
+--call-graph [mode,type,min[,limit],order[,key][,branch]]::
 	Setup and enable call-graph (stack chain/backtrace) recording,
-	implies -g.
+	implies -g.  See `--call-graph` section in perf-record and
+	perf-report man pages for details.
 
 --children::
 	Accumulate callchain of children to parent entry so that then can
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 6f641fd..1de381d 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1053,8 +1053,22 @@ callchain_opt(const struct option *opt, const char *arg, int unset)
 static int
 parse_callchain_opt(const struct option *opt, const char *arg, int unset)
 {
-	symbol_conf.use_callchain = true;
-	return record_parse_callchain_opt(opt, arg, unset);
+	struct record_opts *record = (struct record_opts *)opt->value;
+
+	record->callgraph_set = true;
+	callchain_param.enabled = !unset;
+	callchain_param.record_mode = CALLCHAIN_FP;
+
+	/*
+	 * --no-call-graph
+	 */
+	if (unset) {
+		symbol_conf.use_callchain = false;
+		callchain_param.record_mode = CALLCHAIN_NONE;
+		return 0;
+	}
+
+	return parse_callchain_top_opt(arg);
 }
 
 static int perf_top_config(const char *var, const char *value, void *cb)
@@ -1079,6 +1093,8 @@ parse_percent_limit(const struct option *opt, const char *arg,
 	return 0;
 }
 
+const char top_callchain_help[] = CALLCHAIN_RECORD_HELP ", " CALLCHAIN_REPORT_HELP;
+
 int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
 {
 	char errbuf[BUFSIZ];
@@ -1154,11 +1170,11 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
 	OPT_BOOLEAN('n', "show-nr-samples", &symbol_conf.show_nr_samples,
 		    "Show a column with the number of samples"),
 	OPT_CALLBACK_NOOPT('g', NULL, &top.record_opts,
-			   NULL, "enables call-graph recording",
+			   NULL, "enables call-graph recording and display",
 			   &callchain_opt),
 	OPT_CALLBACK(0, "call-graph", &top.record_opts,
-		     "mode[,dump_size]", record_callchain_help,
-		     &parse_callchain_opt),
+		     "mode[,dump_size],output_type,min_percent[,print_limit],call_order[,branch]",
+		     top_callchain_help, &parse_callchain_opt),
 	OPT_BOOLEAN(0, "children", &symbol_conf.cumulate_callchain,
 		    "Accumulate callchains of children and show total overhead as well"),
 	OPT_INTEGER(0, "max-stack", &top.max_stack,
diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index 773fe13..842be32 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -77,12 +77,14 @@ static int parse_callchain_sort_key(const char *value)
 	return -1;
 }
 
-int
-parse_callchain_report_opt(const char *arg)
+static int
+__parse_callchain_report_opt(const char *arg, bool allow_record_opt)
 {
 	char *tok;
 	char *endptr;
 	bool minpcnt_set = false;
+	bool record_opt_set = false;
+	bool try_stack_size = false;
 
 	symbol_conf.use_callchain = true;
 
@@ -100,6 +102,28 @@ parse_callchain_report_opt(const char *arg)
 		    !parse_callchain_order(tok) ||
 		    !parse_callchain_sort_key(tok)) {
 			/* parsing ok - move on to the next */
+			try_stack_size = false;
+			goto next;
+		} else if (allow_record_opt && !record_opt_set) {
+			if (parse_callchain_record(tok, &callchain_param))
+				goto try_numbers;
+
+			/* assume that number followed by 'dwarf' is stack size */
+			if (callchain_param.record_mode == CALLCHAIN_DWARF)
+				try_stack_size = true;
+
+			record_opt_set = true;
+			goto next;
+		}
+
+try_numbers:
+		if (try_stack_size) {
+			unsigned long size = 0;
+
+			if (get_stack_size(tok, &size) < 0)
+				return -1;
+			callchain_param.dump_size = size;
+			try_stack_size = false;
 		} else if (!minpcnt_set) {
 			/* try to get the min percent */
 			callchain_param.min_percent = strtod(tok, &endptr);
@@ -112,7 +136,7 @@ parse_callchain_report_opt(const char *arg)
 			if (tok == endptr)
 				return -1;
 		}
-
+next:
 		arg = NULL;
 	}
 
@@ -123,6 +147,16 @@ parse_callchain_report_opt(const char *arg)
 	return 0;
 }
 
+int parse_callchain_report_opt(const char *arg)
+{
+	return __parse_callchain_report_opt(arg, false);
+}
+
+int parse_callchain_top_opt(const char *arg)
+{
+	return __parse_callchain_report_opt(arg, true);
+}
+
 int perf_callchain_config(const char *var, const char *value)
 {
 	char *endptr;
diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
index c9e3a2e..836d59a 100644
--- a/tools/perf/util/callchain.h
+++ b/tools/perf/util/callchain.h
@@ -192,6 +192,7 @@ extern const char record_callchain_help[];
 extern int parse_callchain_record(const char *arg, struct callchain_param *param);
 int parse_callchain_record_opt(const char *arg, struct callchain_param *param);
 int parse_callchain_report_opt(const char *arg);
+int parse_callchain_top_opt(const char *arg);
 int perf_callchain_config(const char *var, const char *value);
 
 static inline void callchain_cursor_snapshot(struct callchain_cursor *dest,

[tip:perf/core] perf tools: Defaults to 'caller' callchain order only if --children is enabled

[tip-bot for Namhyung Kim]

Commit-ID:  792aeafa8ed08e5e18fb66ab93b470f78e619f75
Gitweb:     http://git.kernel.org/tip/792aeafa8ed08e5e18fb66ab93b470f78e619f75
Author:     Namhyung Kim <namhyung@kernel.org>
AuthorDate: Thu, 22 Oct 2015 16:45:46 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 22 Oct 2015 15:40:11 -0300

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 <namhyung@kernel.org>
Acked-by: Brendan Gregg <brendan.d.gregg@gmail.com>
Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
Acked-by: Ingo Molnar <mingo@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Chandler Carruth <chandlerc@gmail.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1445499946-29817-1-git-send-email-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 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 +-
 5 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 18a8c52..545c51c 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -812,6 +812,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 1de381d..af849b1 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1304,6 +1304,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 842be32..735ad48 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 836d59a..aaf467c 100644
--- a/tools/perf/util/callchain.h
+++ b/tools/perf/util/callchain.h
@@ -75,6 +75,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 c1bf9ff..cd12c25 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
 };
 

Re: [PATCH 1/3] perf tools: Move callchain help messages to callchain.h

[Ingo Molnar]

* Namhyung Kim <namhyung@kernel.org> wrote:

> > Also, I had to go into the code to decode the real meaning of all the other 
> > parameters. I'd have expected them to be more obvious from reading the help 
> > text.
> 
> Did you check the man page also?  I think we have (short) explanation for each 
> parameter and users should read it first to understand the meaning.  But I agree 
> that the help text should also be improved to provide quick reference.

ah, no.

Is there a way to call up the manpage output for any given option, or so?

Such as:

   perf report -g help

Outputs the short-style help text you fixed:

 Usage: perf report [<options>]

    -g, --call-graph <print_type,threshold[,print_limit],order,sort_key[,branch]>
                          Display call graph (stack chain/backtrace):

                                print_type:     call graph printing style (graph|flat|fractal|none)
                                threshold:      minimum call graph inclusion threshold (<percent>)
                                print_limit:    maximum number of call graph entry (<number>)
                                order:          call graph order (caller|callee)
                                sort_key:       call graph sort key (function|address)
                                branch:         include last branch info to call graph (branch)

                                Default: graph,0.5,caller,function

and we could append one more line:

 # Suggestion: By typing 'perf report -g man' you can see the detailed manpage of option

OTOH the manpage is not easily parsed into per option sections, right? So we could 
bring up the whole manpage.

Btw., another usability detail I noticed yesterday is that when I typed 'perf 
report -h' I got so much output that I couldn't find the specific option I was 
looking for, because there's no apparent ordering of the output:

triton:~/tip> perf report -h 2>&1 | grep -e ' -[a-Z],'
    -i, --input <file>    input file name
    -v, --verbose         be more verbose (show symbol address, etc)
    -D, --dump-raw-trace  dump raw trace in ASCII
    -k, --vmlinux <file>  vmlinux pathname
    -f, --force           don't complain, do it
    -m, --modules         load module symbols - WARNING: use only with -k and LIVE kernel
    -n, --show-nr-samples
    -T, --threads         Show per-thread event counters
    -s, --sort <key[,key2...]>
    -F, --fields <key[,keys...]>
    -p, --parent <regex>  regex filter to identify parent, see: '--sort parent'
    -x, --exclude-other   Only display entries with parent-match
    -g, --call-graph <print_type,threshold[,print_limit],order,sort_key[,branch]>
    -G, --inverted        alias for inverted call graph
    -d, --dsos <dso[,dso...]>
    -c, --comms <comm[,comm...]>
    -S, --symbols <symbol[,symbol...]>
    -w, --column-widths <width[,width...]>
    -t, --field-separator <separator>
    -U, --hide-unresolved
    -C, --cpu <cpu>       list of cpus to profile
    -I, --show-info       Display extended information about perf.data file
    -M, --disassembler-style <disassembler style>
    -b, --branch-stack    use branch records for per branch histogram filling

Such (alphabetic) ordering would be easier to navigate:

    -b, --branch-stack    use branch records for per branch histogram filling
    -c, --comms <comm[,comm...]>
    -C, --cpu <cpu>       list of cpus to profile
    -d, --dsos <dso[,dso...]>
    -D, --dump-raw-trace  dump raw trace in ASCII
    -F, --fields <key[,keys...]>
    -f, --force           don't complain, do it
    -g, --call-graph <print_type,threshold[,print_limit],order,sort_key[,branch]>
    -G, --inverted        alias for inverted call graph
    -i, --input <file>    input file name
    -I, --show-info       Display extended information about perf.data file
    -k, --vmlinux <file>  vmlinux pathname
    -M, --disassembler-style <disassembler style>
    -m, --modules         load module symbols - WARNING: use only with -k and LIVE kernel
    -n, --show-nr-samples
    -p, --parent <regex>  regex filter to identify parent, see: '--sort parent'
    -s, --sort <key[,key2...]>
    -S, --symbols <symbol[,symbol...]>
    -t, --field-separator <separator>
    -T, --threads         Show per-thread event counters
    -U, --hide-unresolved
    -v, --verbose         be more verbose (show symbol address, etc)
    -w, --column-widths <width[,width...]>
    -x, --exclude-other   Only display entries with parent-match

Since perf is growing rapidly:

triton:~/tip> L_PREV=0; for v in v2.6.32 v2.6.33 v2.6.34 v2.6.35 v2.6.36 v2.6.37 \
v2.6.38 v2.6.39 v3.0 v3.1 v3.2 v3.3 v3.4 v3.5 v3.6 v3.7 v3.8 v3.9 v3.10 v3.11 \
v3.12 v3.13 v3.14 v3.15 v3.16 v3.17 v3.18 v3.19 v4.0 v4.1 v4.2 master; do printf \ 
"%-10s: " $v; rm -rf tools/perf/; git checkout -f $v >/dev/null 2>&1; L=$(find \
tools/perf/ -type f | xargs cat | wc -l); GROWTH=$((L-L_PREV)); printf \
"%6d  (+%6d) LOC\n" $L $GROWTH; L_PREV=$L; done

v2.6.32   :  27743  (+ 27743) LOC
v2.6.33   :  36676  (+  8933) LOC
v2.6.34   :  41350  (+  4674) LOC
v2.6.35   :  46579  (+  5229) LOC
v2.6.36   :  49377  (+  2798) LOC
v2.6.37   :  51030  (+  1653) LOC
v2.6.38   :  53017  (+  1987) LOC
v2.6.39   :  56141  (+  3124) LOC
v3.0      :  57423  (+  1282) LOC
v3.1      :  58824  (+  1401) LOC
v3.2      :  61403  (+  2579) LOC
v3.3      :  62790  (+  1387) LOC
v3.4      :  66509  (+  3719) LOC
v3.5      :  65967  (+  -542) LOC
v3.6      :  68273  (+  2306) LOC
v3.7      :  74301  (+  6028) LOC
v3.8      :  78281  (+  3980) LOC
v3.9      :  82289  (+  4008) LOC
v3.10     :  84195  (+  1906) LOC
v3.11     :  84536  (+   341) LOC
v3.12     :  90159  (+  5623) LOC
v3.13     :  95169  (+  5010) LOC
v3.14     :  97233  (+  2064) LOC
v3.15     :  98857  (+  1624) LOC
v3.16     : 103760  (+  4903) LOC
v3.17     : 106358  (+  2598) LOC
v3.18     : 109347  (+  2989) LOC
v3.19     : 112867  (+  3520) LOC
v4.0      : 113194  (+   327) LOC
v4.1      : 115992  (+  2798) LOC
v4.2      : 123404  (+  7412) LOC
master    : 143429  (+ 20025) LOC

which is good! ;-) - but managing all the various pieces of documentation will 
become a larger and larger challenge I suspect as time goes on, both for users
and for developers.

Thanks,

	Ingo

Re: [PATCH 1/3] perf tools: Move callchain help messages to callchain.h

[Arnaldo Carvalho de Melo]

Em Fri, Oct 23, 2015 at 11:59:32AM +0200, Ingo Molnar escreveu:
> Btw., another usability detail I noticed yesterday is that when I typed 'perf 
> report -h' I got so much output that I couldn't find the specific option I was 
> looking for, because there's no apparent ordering of the output:
> 
> triton:~/tip> perf report -h 2>&1 | grep -e ' -[a-Z],'
>     -i, --input <file>    input file name
>     -v, --verbose         be more verbose (show symbol address, etc)
>     -D, --dump-raw-trace  dump raw trace in ASCII
SNIP:
>     -C, --cpu <cpu>       list of cpus to profile
>     -I, --show-info       Display extended information about perf.data file
>     -M, --disassembler-style <disassembler style>
>     -b, --branch-stack    use branch records for per branch histogram filling
> 
> Such (alphabetic) ordering would be easier to navigate:
> 
>     -b, --branch-stack    use branch records for per branch histogram filling
>     -c, --comms <comm[,comm...]>
>     -C, --cpu <cpu>       list of cpus to profile

Try the patch below:


>From 182ec8d7c105e331884def4f98f56b4f876a4f1d Mon Sep 17 00:00:00 2001
From: Arnaldo Carvalho de Melo <acme@redhat.com>
Date: Fri, 23 Oct 2015 11:23:28 -0300
Subject: [PATCH 1/1] perf tools: Show tool command line options ordered

When asking for a listing of the options, be it using -h or when an
unknown option is passed, order it by one-letter options, then the ones
having just long names.

Suggested-by: Ingo Molnar <mingo@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/n/tip-41qh68t35n4ehrpsuazp1dx8@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/parse-options.c | 42 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/tools/perf/util/parse-options.c b/tools/perf/util/parse-options.c
index 8aa7922397a9..47fb1405df7e 100644
--- a/tools/perf/util/parse-options.c
+++ b/tools/perf/util/parse-options.c
@@ -2,6 +2,7 @@
 #include "parse-options.h"
 #include "cache.h"
 #include "header.h"
+#include <linux/string.h>
 
 #define OPT_SHORT 1
 #define OPT_UNSET 2
@@ -642,9 +643,44 @@ static void print_option_help(const struct option *opts, int full)
 	fprintf(stderr, "%*s%s\n", pad + USAGE_GAP, "", opts->help);
 }
 
+static int option__cmp(const void *va, const void *vb)
+{
+	const struct option *a = va, *b = vb;
+	int sa = tolower(a->short_name), sb = tolower(b->short_name), ret;
+
+	if (sa == 0)
+		sa = 'z' + 1;
+	if (sb == 0)
+		sb = 'z' + 1;
+
+	ret = sa - sb;
+
+	return ret ?: strcmp(a->long_name, b->long_name);
+}
+
+static struct option *options__order(const struct option *opts)
+{
+	int nr_opts = 0;
+	const struct option *o = opts;
+	struct option *ordered;
+
+	for (o = opts; o->type != OPTION_END; o++)
+		++nr_opts;
+
+	ordered = memdup(opts, sizeof(*o) * (nr_opts + 1));
+	if (ordered == NULL)
+		goto out;
+
+	qsort(ordered, nr_opts, sizeof(*o), option__cmp);
+out:
+	return ordered;
+}
+
 int usage_with_options_internal(const char * const *usagestr,
 				const struct option *opts, int full)
 {
+	struct option *ordered;
+
 	if (!usagestr)
 		return PARSE_OPT_HELP;
 
@@ -661,11 +697,17 @@ int usage_with_options_internal(const char * const *usagestr,
 	if (opts->type != OPTION_GROUP)
 		fputc('\n', stderr);
 
+	ordered = options__order(opts);
+	if (ordered)
+		opts = ordered;
+
 	for (  ; opts->type != OPTION_END; opts++)
 		print_option_help(opts, full);
 
 	fputc('\n', stderr);
 
+	free(ordered);
+
 	return PARSE_OPT_HELP;
 }
 
-- 
2.1.0

Re: [PATCH 1/3] perf tools: Move callchain help messages to callchain.h

[Arnaldo Carvalho de Melo]

Em Fri, Oct 23, 2015 at 11:27:15AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Fri, Oct 23, 2015 at 11:59:32AM +0200, Ingo Molnar escreveu:
> > Btw., another usability detail I noticed yesterday is that when I typed 'perf 
> > report -h' I got so much output that I couldn't find the specific option I was 
> > looking for, because there's no apparent ordering of the output:
> > 
> > triton:~/tip> perf report -h 2>&1 | grep -e ' -[a-Z],'
> >     -i, --input <file>    input file name
> >     -v, --verbose         be more verbose (show symbol address, etc)
> >     -D, --dump-raw-trace  dump raw trace in ASCII
> SNIP:
> >     -C, --cpu <cpu>       list of cpus to profile
> >     -I, --show-info       Display extended information about perf.data file
> >     -M, --disassembler-style <disassembler style>
> >     -b, --branch-stack    use branch records for per branch histogram filling
> > 
> > Such (alphabetic) ordering would be easier to navigate:
> > 
> >     -b, --branch-stack    use branch records for per branch histogram filling
> >     -c, --comms <comm[,comm...]>
> >     -C, --cpu <cpu>       list of cpus to profile
> 
> Try the patch below:
> 

I fixed a case where long options could be NULL and implemented this
other patch based on another suggestion you made, which I'll have to fix
to check if opt->long_name is NULL ;-)

>From 648b9931e6bccf1d1e1d14474d56b4b687e5fb71 Mon Sep 17 00:00:00 2001
From: Arnaldo Carvalho de Melo <acme@redhat.com>
Date: Fri, 23 Oct 2015 13:27:39 -0300
Subject: [PATCH 1/1] perf tools: Provide help for subset of options

Some tools have a lot of options, so, providing a way to show help just
for some of them may come handy:

  $ perf report -h --tui

   Usage: perf report [<options>]

        --tui             Use the TUI interface

  $ perf report -h --tui --showcpuutilization -b -c

   Usage: perf report [<options>]

    -b, --branch-stack    use branch records for per branch histogram filling
    -c, --comms <comm[,comm...]>
                          only consider symbols in these comms
        --showcpuutilization
                          Show sample percentage for different cpu modes
        --tui             Use the TUI interface

  $

Using it with perf bash completion is also handy, just make sure you
source the needed file:

  $ . ~/git/linux/tools/perf/perf-completion.sh

Then press tab/tab after -- to see a list of options, put them after -h
and only the options chosen will have its help presented:

  $ perf report -h --
  --asm-raw              --demangle-kernel      --group
  --kallsyms             --pretty               --stdio
  --branch-history       --disassembler-style   --gtk
  --max-stack            --showcpuutilization   --symbol-filter
  --branch-stack         --dsos                 --header
  --mem-mode             --show-info            --symbols
  --call-graph           --dump-raw-trace       --header-only
  --modules              --show-nr-samples      --symfs
  --children             --exclude-other        --hide-unresolved
  --objdump              --show-ref-call-graph  --threads
  --column-widths        --fields               --ignore-callees
  --parent               --show-total-period    --tid
  --comms                --field-separator      --input
  --percentage           --socket-filter        --tui
  --cpu                  --force                --inverted
  --percent-limit        --sort                 --verbose
  --demangle             --full-source-path     --itrace
  --pid                  --source               --vmlinux
  $ perf report -h --socket-filter

   Usage: perf report [<options>]

      --socket-filter <n>
                  only show processor socket that match with this filter

Suggested-by: Ingo Molnar <mingo@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Brendan Gregg <brendan.d.gregg@gmail.com>
: Chandler Carruth <chandlerc@gmail.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/n/tip-83mcdd3wj0379jcgea8w0fxa@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/parse-options.c | 42 ++++++++++++++++++++++++++++++++---------
 1 file changed, 33 insertions(+), 9 deletions(-)

diff --git a/tools/perf/util/parse-options.c b/tools/perf/util/parse-options.c
index fb26532d67c3..47ffa90d1ed4 100644
--- a/tools/perf/util/parse-options.c
+++ b/tools/perf/util/parse-options.c
@@ -373,7 +373,8 @@ void parse_options_start(struct parse_opt_ctx_t *ctx,
 }
 
 static int usage_with_options_internal(const char * const *,
-				       const struct option *, int);
+				       const struct option *, int,
+				       struct parse_opt_ctx_t *);
 
 int parse_options_step(struct parse_opt_ctx_t *ctx,
 		       const struct option *options,
@@ -397,8 +398,9 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 
 		if (arg[1] != '-') {
 			ctx->opt = ++arg;
-			if (internal_help && *ctx->opt == 'h')
-				return usage_with_options_internal(usagestr, options, 0);
+			if (internal_help && *ctx->opt == 'h') {
+				return usage_with_options_internal(usagestr, options, 0, ctx);
+			}
 			switch (parse_short_opt(ctx, options)) {
 			case -1:
 				return parse_options_usage(usagestr, options, arg, 1);
@@ -413,7 +415,7 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 				check_typos(arg, options);
 			while (ctx->opt) {
 				if (internal_help && *ctx->opt == 'h')
-					return usage_with_options_internal(usagestr, options, 0);
+					return usage_with_options_internal(usagestr, options, 0, ctx);
 				arg = ctx->opt;
 				switch (parse_short_opt(ctx, options)) {
 				case -1:
@@ -446,9 +448,9 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 
 		arg += 2;
 		if (internal_help && !strcmp(arg, "help-all"))
-			return usage_with_options_internal(usagestr, options, 1);
+			return usage_with_options_internal(usagestr, options, 1, ctx);
 		if (internal_help && !strcmp(arg, "help"))
-			return usage_with_options_internal(usagestr, options, 0);
+			return usage_with_options_internal(usagestr, options, 0, ctx);
 		if (!strcmp(arg, "list-opts"))
 			return PARSE_OPT_LIST_OPTS;
 		if (!strcmp(arg, "list-cmds"))
@@ -682,8 +684,27 @@ out:
 	return ordered;
 }
 
+static bool option__in_argv(const struct option *opt, const struct parse_opt_ctx_t *ctx)
+{
+	int i;
+
+	for (i = 1; i < ctx->argc; ++i) {
+		const char *arg = ctx->argv[i];
+
+		if (arg[0] != '-')
+			continue;
+
+		if (arg[1] == opt->short_name ||
+		    (arg[1] == '-' && strcmp(opt->long_name, arg + 2) == 0))
+			return true;
+	}
+
+	return false;
+}
+
 int usage_with_options_internal(const char * const *usagestr,
-				const struct option *opts, int full)
+				const struct option *opts, int full,
+				struct parse_opt_ctx_t *ctx)
 {
 	struct option *ordered;
 
@@ -707,8 +728,11 @@ int usage_with_options_internal(const char * const *usagestr,
 	if (ordered)
 		opts = ordered;
 
-	for (  ; opts->type != OPTION_END; opts++)
+	for (  ; opts->type != OPTION_END; opts++) {
+		if (ctx && ctx->argc > 1 && !option__in_argv(opts, ctx))
+			continue;
 		print_option_help(opts, full);
+	}
 
 	fputc('\n', stderr);
 
@@ -721,7 +745,7 @@ void usage_with_options(const char * const *usagestr,
 			const struct option *opts)
 {
 	exit_browser(false);
-	usage_with_options_internal(usagestr, opts, 0);
+	usage_with_options_internal(usagestr, opts, 0, NULL);
 	exit(129);
 }
 
-- 
2.1.0

Re: [RFC/PATCH 3/3] perf tools: Defaults to 'caller' callchain order only if --children is enabled

[Brendan Gregg]

On Thu, Oct 22, 2015 at 7:03 AM, Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
> Em Thu, Oct 22, 2015 at 02:49:11AM -0700, Brendan Gregg escreveu:
>> On Thu, Oct 22, 2015 at 12:38 AM, Namhyung Kim <namhyung@kernel.org> wrote:
>> > Hi Ingo,
>> >
>> > On Thu, Oct 22, 2015 at 4:32 PM, Ingo Molnar <mingo@kernel.org> wrote:
>> >>
>> >> * Namhyung Kim <namhyung@kernel.org> wrote:
>> >>
>> >>> 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 caller order.
>> >
>> > Oops, there's a typo:  s/caller order/callee order/ :)
>>
>> Thanks, I was wondering about that (and I've made that typo myself).
>> Thanks for the patch!
>
> Brendan, can I take that as an Acked-by?

Belated, yes!

Brendan