perf tools: Move callchain help messages to callchain.h

---
 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(-)

tools/perf/Documentation/perf-top.txt +3 -2

index f6a23eb294e7..556cec09bf50 100644
@@ -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

tools/perf/builtin-top.c +21 -5

index 6f641fd68296..1de381d3f29f 100644
@@ -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,

tools/perf/util/callchain.c +37 -3

index 773fe13ce627..842be32899ee 100644
@@ -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;

tools/perf/util/callchain.h +1 -0

index c9e3a2e85a72..836d59a001bc 100644
@@ -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,

Signature · 1 line

2.6.0

Ingo Molnar

2015-10-22 08:23 UTC #2 lore raw

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

Quoted 11 lines · 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>]

Quoted 7 lines

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

Namhyung Kim

author maintainer

2015-10-22 12:21 UTC #3 lore raw

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

Quoted 39 lines · Ingo Molnar

Quoted 11 lines · 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>]
Quoted 7 lines
--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)

Quoted 3 lines

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.

Quoted 3 lines

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.

Quoted 8 lines

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

Taeung Song

2015-10-22 13:57 UTC #4 lore raw

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

Hi, Namhyung and Ingo

Quoted 59 lines · Namhyung Kim

Quoted 39 lines · Ingo Molnar
Quoted 11 lines · 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>]
Quoted 7 lines
--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)
Quoted 3 lines
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.
Quoted 3 lines
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.
Quoted 4 lines
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:

Quoted 7 lines

Quoted 4 lines
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

Namhyung Kim

author maintainer

2015-10-22 14:34 UTC #5 lore raw

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

Quoted 71 lines · Taeung Song

Hi, Namhyung and Ingo
Quoted 59 lines · Namhyung Kim
Quoted 39 lines · Ingo Molnar
Quoted 11 lines · 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>]
Quoted 7 lines
--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)
Quoted 3 lines
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.
Quoted 3 lines
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.
Quoted 4 lines
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:
Quoted 7 lines
Quoted 4 lines
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.

Quoted 8 lines

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

Arnaldo Carvalho de Melo

maintainer Applied

2015-10-22 18:38 UTC #6 lore raw

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

Em Thu, Oct 22, 2015 at 03:28:49PM +0900, Namhyung Kim escreveu:

Quoted 5 lines

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.

tip-bot for Namhyung Kim

merge bot

2015-10-23 08:31 UTC #7 lore raw

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

perf top: Support call-graph display options also

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

perf top: Support call-graph display options also

Now perf top can receive display options like below:

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

Usage: perf top [<options>]

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

Cc Adrian Hunter <[email protected]>

Cc Borislav Petkov <[email protected]>

Cc Brendan Gregg <[email protected]>

Cc Chandler Carruth <[email protected]>

Cc David Ahern <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc Jiri Olsa <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Stephane Eranian <[email protected]>

Cc Wang Nan <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 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(-)

tools/perf/Documentation/perf-top.txt +3 -2

index f6a23eb..556cec0 100644
@@ -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

tools/perf/builtin-top.c +21 -5

index 6f641fd..1de381d 100644
@@ -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,

tools/perf/util/callchain.c +37 -3

index 773fe13..842be32 100644
@@ -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;

tools/perf/util/callchain.h +1 -0

index c9e3a2e..836d59a 100644
@@ -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,

Namhyung Kim

author maintainer

2015-10-22 06:29 UTC #8 lore raw

PATCH 3/3 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 caller order.

Cc Adrian Hunter <[email protected]>

Cc Borislav Petkov <[email protected]>

Cc Brendan Gregg <[email protected]>

Cc Chandler Carruth <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc Stephane Eranian <[email protected]>

Cc Wang Nan <[email protected]>

---
 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(-)

tools/perf/builtin-report.c +2 -0

index 18a8c52d921e..545c51cef7f7 100644
@@ -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)

tools/perf/builtin-top.c +3 -0

index 1de381d3f29f..af849b1d7389 100644
@@ -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);

tools/perf/util/callchain.c +2 -0

index 842be32899ee..735ad48e1858 100644
@@ -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;

tools/perf/util/callchain.h +1 -0

index 836d59a001bc..aaf467c9ef2b 100644
@@ -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;
 };

tools/perf/util/util.c +1 -1

index c1bf9ff210b0..cd12c25e4ea4 100644
@@ -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
 };

Signature · 1 line

2.6.0

Ingo Molnar

2015-10-22 07:32 UTC #9 lore raw

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

Quoted 5 lines · 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.

Quoted 8 lines

Cc Adrian Hunter <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Brendan Gregg <[email protected]>
Cc Chandler Carruth <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Stephane Eranian <[email protected]>
Cc Wang Nan <[email protected]>
Signed-off-by Namhyung Kim <[email protected]>

Very nice fixes, thanks!

Ingo

Namhyung Kim

author maintainer

2015-10-22 07:39 UTC #10 lore raw

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

Hi Ingo,

Quoted 7 lines · Ingo Molnar

Quoted 4 lines · 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.

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

Quoted 11 lines

Quoted 9 lines
Cc Adrian Hunter <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Brendan Gregg <[email protected]>
Cc Chandler Carruth <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Stephane Eranian <[email protected]>
Cc Wang Nan <[email protected]>
Signed-off-by Namhyung Kim <[email protected]>
Very nice fixes, thanks!

Thank you!
Namhyung

Brendan Gregg

2015-10-22 09:49 UTC #11 lore raw

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

Quoted 4 lines · Namhyung Kim

Hi Ingo,
Quoted 1 line · Ingo Molnar

Quoted 8 lines

Quoted 6 lines · Namhyung Kim
Quoted 5 lines
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

Arnaldo Carvalho de Melo

maintainer

2015-10-22 14:03 UTC #12 lore raw

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

Em Thu, Oct 22, 2015 at 02:49:11AM -0700, Brendan Gregg escreveu:

Quoted 12 lines · Namhyung Kim

Quoted 11 lines
Hi Ingo,
Quoted 7 lines · Ingo Molnar
Quoted 4 lines · 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.

Quoted 4 lines

Quoted 1 line
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?

Quoted 1 line

Brendan

Brendan Gregg

2015-11-03 00:00 UTC #13 lore raw

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

Quoted 13 lines · Arnaldo Carvalho de Melo

Em Thu, Oct 22, 2015 at 02:49:11AM -0700, Brendan Gregg escreveu:
Quoted 12 lines · Namhyung Kim
Quoted 11 lines
Hi Ingo,
Quoted 7 lines · Ingo Molnar
Quoted 4 lines · 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.

Quoted 6 lines

Quoted 4 lines
Quoted 1 line
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

Frederic Weisbecker

Acked-by

2015-10-22 12:21 UTC #14 lore raw

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

Quoted 4 lines · Namhyung Kim

Hi Ingo,
Quoted 1 line · Ingo Molnar

Quoted 8 lines

Quoted 6 lines · Namhyung Kim
Quoted 5 lines
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!

Namhyung Kim

author maintainer

2015-10-22 07:45 UTC #15 lore raw

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

Cc Adrian Hunter <[email protected]>

Cc Borislav Petkov <[email protected]>

Cc Brendan Gregg <[email protected]>

Cc Chandler Carruth <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc Stephane Eranian <[email protected]>

Cc Wang Nan <[email protected]>

---
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(-)

tools/perf/builtin-report.c +2 -0

index 18a8c52d921e..545c51cef7f7 100644
@@ -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)

tools/perf/builtin-top.c +3 -0

index 1de381d3f29f..af849b1d7389 100644
@@ -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);

tools/perf/util/callchain.c +2 -0

index 842be32899ee..735ad48e1858 100644
@@ -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;

tools/perf/util/callchain.h +1 -0

index 836d59a001bc..aaf467c9ef2b 100644
@@ -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;
 };

tools/perf/util/util.c +1 -1

index c1bf9ff210b0..cd12c25e4ea4 100644
@@ -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
 };

Signature · 1 line

2.6.0

Arnaldo Carvalho de Melo

maintainer Applied

2015-10-22 18:38 UTC #16 lore raw

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

Em Thu, Oct 22, 2015 at 04:45:46PM +0900, Namhyung Kim escreveu:

Quoted 4 lines

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-bot for Namhyung Kim

merge bot

2015-10-23 08:32 UTC #17 lore raw

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

perf tools: Defaults to 'caller' callchain order only if --children is enabled

Commit-ID:  792aeafa8ed08e5e18fb66ab93b470f78e619f75
Gitweb:     http://git.kernel.org/tip/792aeafa8ed08e5e18fb66ab93b470f78e619f75
Author:     Namhyung Kim <[email protected]>
AuthorDate: Thu, 22 Oct 2015 16:45:46 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
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.

Acked-by Brendan Gregg <[email protected]>

Acked-by Ingo Molnar <[email protected]>

Cc Adrian Hunter <[email protected]>

Cc Borislav Petkov <[email protected]>

Cc Chandler Carruth <[email protected]>

Cc David Ahern <[email protected]>

Cc Jiri Olsa <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Stephane Eranian <[email protected]>

Cc Wang Nan <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 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(-)

tools/perf/builtin-report.c +2 -0

index 18a8c52..545c51c 100644
@@ -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)

tools/perf/builtin-top.c +3 -0

index 1de381d..af849b1 100644
@@ -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);

tools/perf/util/callchain.c +2 -0

index 842be32..735ad48 100644
@@ -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;

tools/perf/util/callchain.h +1 -0

index 836d59a..aaf467c 100644
@@ -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;
 };

tools/perf/util/util.c +1 -1

index c1bf9ff..cd12c25 100644
@@ -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
 };

Frederic Weisbecker

2015-10-22 12:19 UTC #18 lore raw

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

Quoted 4 lines · 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.

                                 ^^^^
                                 callee?

Thanks!

Arnaldo Carvalho de Melo

maintainer Acked-by Frederic Weisbecker Acked-by Brendan Gregg on v1 3/3

2015-10-22 14:13 UTC #19 lore raw

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

Em Thu, Oct 22, 2015 at 03:28:50PM +0900, Namhyung Kim escreveu:

Review line 24

Quoted 9 lines

+++ b/tools/perf/util/util.c

(quoted diff) +1 -1

@@ -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:

Quoted 1 line

From fd2dcde26d09b34fccc3c164d9fc36e662c21fc4 Mon Sep 17 00:00:00 2001

From: Namhyung Kim <[email protected]>
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.

Acked-by Brendan Gregg <[email protected]>

Cc Adrian Hunter <[email protected]>

Cc Borislav Petkov <[email protected]>

Cc Chandler Carruth <[email protected]>

Cc David Ahern <[email protected]>

Cc Jiri Olsa <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Stephane Eranian <[email protected]>

Cc Wang Nan <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

[ Update 'perf report' man page ]

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 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(-)

tools/perf/Documentation/perf-report.txt +1 -1

index e4fdeeb51123..4ae1621f1def 100644
@@ -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

tools/perf/builtin-report.c +2 -0

index 3b23b25d1589..a554906a3e03 100644
@@ -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)

tools/perf/builtin-top.c +3 -0

index 6f641fd68296..be42e6eb6805 100644
@@ -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);

tools/perf/util/callchain.c +2 -0

index 773fe13ce627..a8ed80f77ff3 100644
@@ -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;

tools/perf/util/callchain.h +1 -0

index acee2b3cd801..0e96496567c5 100644
@@ -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;
 };

tools/perf/util/util.c +1 -1

index c1bf9ff210b0..cd12c25e4ea4 100644
@@ -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
 };

Signature · 1 line

2.1.0

Namhyung Kim

author maintainer

2015-10-22 14:23 UTC #20 lore raw

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

Hi Arnaldo,

Quoted 13 lines · Arnaldo Carvalho de Melo

Em Thu, Oct 22, 2015 at 03:28:50PM +0900, Namhyung Kim escreveu:
tools/perf/util/util.c line 24
Quoted 9 lines
+++ b/tools/perf/util/util.c
tools/perf/util/util.c +1 -1
@@ -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.

Quoted 3 lines

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

Quoted 120 lines

From fd2dcde26d09b34fccc3c164d9fc36e662c21fc4 Mon Sep 17 00:00:00 2001
From: Namhyung Kim <[email protected]>
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.

Acked-by Brendan Gregg <[email protected]>

Cc Adrian Hunter <[email protected]>

Cc Borislav Petkov <[email protected]>

Cc Chandler Carruth <[email protected]>

Cc David Ahern <[email protected]>

Cc Jiri Olsa <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Stephane Eranian <[email protected]>

Cc Wang Nan <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

[ Update 'perf report' man page ]

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 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(-)

tools/perf/Documentation/perf-report.txt +1 -1

index e4fdeeb51123..4ae1621f1def 100644
@@ -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

tools/perf/builtin-report.c +2 -0

index 3b23b25d1589..a554906a3e03 100644
@@ -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)

tools/perf/builtin-top.c +3 -0

index 6f641fd68296..be42e6eb6805 100644
@@ -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);

tools/perf/util/callchain.c +2 -0

index 773fe13ce627..a8ed80f77ff3 100644
@@ -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;

tools/perf/util/callchain.h +1 -0

index acee2b3cd801..0e96496567c5 100644
@@ -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;
 };

tools/perf/util/util.c +1 -2

index c1bf9ff210b0..cd12c25e4ea4 100644
@@ -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

Signature · 2 lines

Thanks,
Namhyung

Arnaldo Carvalho de Melo

maintainer

2015-10-22 14:37 UTC #21 lore raw

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

Em Thu, Oct 22, 2015 at 11:22:40PM +0900, Namhyung Kim escreveu:

Quoted 12 lines · Arnaldo Carvalho de Melo

Quoted 10 lines
Em Thu, Oct 22, 2015 at 03:28:50PM +0900, Namhyung Kim escreveu:
tools/perf/util/util.c line 21
Quoted 6 lines
+++ b/tools/perf/util/util.c
tools/perf/util/util.c +1 -1
@@ -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?

Quoted 1 line

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.

Quoted 5 lines

Quoted 2 lines
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.

Quoted 3 lines

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

Namhyung Kim

author maintainer

2015-10-22 15:51 UTC #22 lore raw

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

Quoted 29 lines · Arnaldo Carvalho de Melo

Em Thu, Oct 22, 2015 at 11:22:40PM +0900, Namhyung Kim escreveu:
Quoted 12 lines · Arnaldo Carvalho de Melo
Quoted 10 lines
Em Thu, Oct 22, 2015 at 03:28:50PM +0900, Namhyung Kim escreveu:
tools/perf/util/util.c line 21
Quoted 6 lines
+++ b/tools/perf/util/util.c
tools/perf/util/util.c +1 -1
@@ -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?
Quoted 1 line
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.
Quoted 5 lines
Quoted 2 lines
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".

Quoted 4 lines

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

Quoted 7 lines

Quoted 4 lines
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.. :)

Quoted 5 lines

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

Arnaldo Carvalho de Melo

maintainer

2015-10-22 16:22 UTC #23 lore raw

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

Em Fri, Oct 23, 2015 at 12:51:01AM +0900, Namhyung Kim escreveu:

Quoted 7 lines · Arnaldo Carvalho de Melo

Quoted 4 lines
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.

Quoted 8 lines

Quoted 6 lines
Quoted 3 lines
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.. :)

:-)

Quoted 6 lines

Quoted 4 lines
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

Ingo Molnar

2015-10-22 08:02 UTC #24 lore raw

tools/perf/util/callchain.h line 16

Quoted 7 lines · Namhyung Kim

tools/perf/util/callchain.h +7 -0

+#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:

tools/perf/util/callchain.h line 20

Quoted 5 lines

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

tools/perf/util/callchain.h +3 -0

+#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]

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

Namhyung Kim

author maintainer

2015-10-22 12:14 UTC #25 lore raw

Quoted 13 lines · Ingo Molnar

tools/perf/util/callchain.h line 16
Quoted 7 lines · Namhyung Kim
tools/perf/util/callchain.h +7 -0
+#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:

Quoted 7 lines

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

Quoted 5 lines

I also had another look at the help text:
Quoted 1 line
output_type,min_percent[,print_limit],call_order[,branch]

Quoted 7 lines

tools/perf/util/callchain.h line 20
Quoted 3 lines
tools/perf/util/callchain.h +3 -0
+#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.

Quoted 3 lines

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.

Quoted 8 lines

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.

Quoted 9 lines

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.

Quoted 4 lines

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.

Quoted 8 lines

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:

Quoted 7 lines

'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

Arnaldo Carvalho de Melo

maintainer

2015-10-22 14:17 UTC #26 lore raw

Em Thu, Oct 22, 2015 at 09:13:49PM +0900, Namhyung Kim escreveu:

Quoted 20 lines · Ingo Molnar

Quoted 19 lines · Namhyung Kim
tools/perf/util/callchain.h line 16
Quoted 8 lines
tools/perf/util/callchain.h +7 -0
+#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

Quoted 3 lines

Quoted 1 line
makes the construct stand out a lot better visually.
OK

Done.

Quoted 61 lines

Quoted 5 lines
I also had another look at the help text:
Quoted 1 line
output_type,min_percent[,print_limit],call_order[,branch]
Quoted 7 lines
tools/perf/util/callchain.h line 20
Quoted 3 lines
tools/perf/util/callchain.h +3 -0
+#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
Quoted 3 lines
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.
Quoted 8 lines
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.
Quoted 9 lines
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.
Quoted 1 line

Quoted 5 lines

Quoted 3 lines
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?

Quoted 20 lines

Quoted 8 lines
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:
Quoted 7 lines
'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

Ingo Molnar

2015-10-23 09:59 UTC #27 lore raw

Quoted 7 lines · Namhyung Kim

Quoted 3 lines
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

Arnaldo Carvalho de Melo

maintainer

2015-10-23 14:27 UTC #28 lore raw

Em Fri, Oct 23, 2015 at 11:59:32AM +0200, Ingo Molnar escreveu:

Quoted 8 lines

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:

Quoted 10 lines

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

Quoted 1 line

From 182ec8d7c105e331884def4f98f56b4f876a4f1d Mon Sep 17 00:00:00 2001

From: Arnaldo Carvalho de Melo <[email protected]>
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.

Cc Adrian Hunter <[email protected]>

Cc Borislav Petkov <[email protected]>

Cc David Ahern <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc Jiri Olsa <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Stephane Eranian <[email protected]>

Cc Wang Nan <[email protected]>

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/util/parse-options.c | 42 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

tools/perf/util/parse-options.c +42 -0

index 8aa7922397a9..47fb1405df7e 100644
@@ -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;
 }

Signature · 1 line

2.1.0

Arnaldo Carvalho de Melo

maintainer

2015-10-23 16:41 UTC #29 lore raw

Em Fri, Oct 23, 2015 at 11:27:15AM -0300, Arnaldo Carvalho de Melo escreveu:

Quoted 15 lines

Em Fri, Oct 23, 2015 at 11:59:32AM +0200, Ingo Molnar escreveu:
Quoted 8 lines
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:
Quoted 5 lines
-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

Quoted 8 lines

Quoted 5 lines
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 ;-)

Quoted 1 line

From 648b9931e6bccf1d1e1d14474d56b4b687e5fb71 Mon Sep 17 00:00:00 2001

From: Arnaldo Carvalho de Melo <[email protected]>
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

Cc Adrian Hunter <[email protected]>

Cc Borislav Petkov <[email protected]>

Cc Brendan Gregg <[email protected]>

: Chandler Carruth <[email protected]>

Cc David Ahern <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc Jiri Olsa <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Stephane Eranian <[email protected]>

Cc Wang Nan <[email protected]>

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/util/parse-options.c | 42 ++++++++++++++++++++++++++++++++---------
 1 file changed, 33 insertions(+), 9 deletions(-)

tools/perf/util/parse-options.c +33 -9

index fb26532d67c3..47ffa90d1ed4 100644
@@ -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);
 }

Signature · 1 line

2.1.0

Arnaldo Carvalho de Melo

maintainer Applied

2015-10-22 18:38 UTC #30 lore raw

Em Thu, Oct 22, 2015 at 03:28:48PM +0900, Namhyung Kim escreveu:

Quoted 1 line

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

Applied

tip-bot for Namhyung Kim

merge bot

2015-10-23 08:31 UTC #31 lore raw

perf tools: Move callchain help messages to callchain.h

Commit-ID:  21cf62847d29392e51c37460856d3c3c57769c5e
Gitweb:     http://git.kernel.org/tip/21cf62847d29392e51c37460856d3c3c57769c5e
Author:     Namhyung Kim <[email protected]>
AuthorDate: Thu, 22 Oct 2015 15:28:48 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
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.