[PATCHSET 0/5] perf tools: Add call-graph config options

[PATCHSET 0/5] perf tools: Add call-graph config options

[Namhyung Kim]

Hello,

This is patchset to add new callchain related config options so that
users don't need to pass their preference to the cmdline everytime.

Following config options will be added, and users can set appropriate
values to ~/.perfconfig file.  Note that the dump-size option is
meaningful only if record-mode = dwarf.

  $ cat ~/.perfconfig
  [call-graph]
    record-mode = dwarf
    dump-size = 4096
    print-type = graph
    order = callee
    threshold = 0.5
    print-limit = 128
    sort-key = function

  $ perf record -vg sleep 1
  callchain: type DWARF
  callchain: stack dump size 4096
  mmap size 528384B
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 0.054 MB perf.data (~2378 samples) ]
  Looking at the vmlinux_path (7 entries long)
  Using /lib/modules/3.16.3-1-ARCH/build/vmlinux for symbols


You can also get this from 'perf/callchain-config-v1' branch on my tree

  git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git


Any comments are welcomed, thanks
Namhyung


Namhyung Kim (5):
  perf hists browser: Fix callchain print bug on TUI
  perf tools: Move callchain config from record_opts to callchain_param
  perf tools: Move some callchain parser functions to callchain.c
  perf tools: Introduce perf_callchain_config()
  perf tools: Convert {record,top}.call-graph option to
    call-graph.record-mode

 tools/perf/builtin-record.c    | 119 ++++++-----------------------------------
 tools/perf/builtin-top.c       |   4 +-
 tools/perf/perf.h              |   3 --
 tools/perf/ui/browsers/hists.c |   3 --
 tools/perf/util/callchain.c    | 117 ++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/callchain.h    |   6 ++-
 tools/perf/util/config.c       |   3 ++
 tools/perf/util/evsel.c        |  11 ++--
 8 files changed, 146 insertions(+), 120 deletions(-)

-- 
2.1.0

[PATCH 1/5] perf hists browser: Fix callchain print bug on TUI

[Namhyung Kim]

Currently perf report -g graph option doesn't work as expected and
always work as same as -g fractal.  This was a bug during recent
callchain print code cleanup.

Before:
  $ perf report -g graph

    Children      Self  Command  Shared Object      Symbol
  ================================================================
  -   56.19%    35.41%  sleep    [kernel.kallsyms]  [k] page_fault
     - page_fault
        + 63.02% _dl_relocate_object
        + 36.98% clear_user

After:
    Children      Self  Command  Shared Object      Symbol
  ================================================================
  -   56.19%    35.41%  sleep    [kernel.kallsyms]  [k] page_fault
     - page_fault
        + 35.41% _dl_relocate_object
        + 20.78% clear_user

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/browsers/hists.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index d4cef68176da..8f60a970404f 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -804,9 +804,6 @@ static int hist_browser__show_entry(struct hist_browser *browser,
 			.is_current_entry = current_entry,
 		};
 
-		if (symbol_conf.cumulate_callchain)
-			total = entry->stat_acc->period;
-
 		printed += hist_browser__show_callchain(browser,
 					&entry->sorted_chain, 1, row, total,
 					hist_browser__show_callchain_entry, &arg,
-- 
2.1.0

[PATCH 2/5] perf tools: Move callchain config from record_opts to callchain_param

[Namhyung Kim]

So that all callchain config parameters can be read/written to a single place.
It's a preparation to consolidate handling of all callchain options.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-record.c | 45 ++++++++++++++++++++-------------------------
 tools/perf/builtin-top.c    |  4 +---
 tools/perf/perf.h           |  3 ---
 tools/perf/util/callchain.h |  5 ++++-
 tools/perf/util/evsel.c     | 11 +++++------
 5 files changed, 30 insertions(+), 38 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index a1b040394170..4de632cd677b 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -649,7 +649,7 @@ static int get_stack_size(char *str, unsigned long *_size)
 }
 #endif /* HAVE_DWARF_UNWIND_SUPPORT */
 
-int record_parse_callchain(const char *arg, struct record_opts *opts)
+int record_parse_callchain(const char *arg)
 {
 	char *tok, *name, *saveptr = NULL;
 	char *buf;
@@ -669,7 +669,7 @@ int record_parse_callchain(const char *arg, struct record_opts *opts)
 		/* Framepointer style */
 		if (!strncmp(name, "fp", sizeof("fp"))) {
 			if (!strtok_r(NULL, ",", &saveptr)) {
-				opts->call_graph = CALLCHAIN_FP;
+				callchain_param.record_mode = CALLCHAIN_FP;
 				ret = 0;
 			} else
 				pr_err("callchain: No more arguments "
@@ -682,15 +682,15 @@ int record_parse_callchain(const char *arg, struct record_opts *opts)
 			const unsigned long default_stack_dump_size = 8192;
 
 			ret = 0;
-			opts->call_graph = CALLCHAIN_DWARF;
-			opts->stack_dump_size = default_stack_dump_size;
+			callchain_param.record_mode = CALLCHAIN_DWARF;
+			callchain_param.dump_size = default_stack_dump_size;
 
 			tok = strtok_r(NULL, ",", &saveptr);
 			if (tok) {
 				unsigned long size = 0;
 
 				ret = get_stack_size(tok, &size);
-				opts->stack_dump_size = size;
+				callchain_param.dump_size = size;
 			}
 #endif /* HAVE_DWARF_UNWIND_SUPPORT */
 		} else {
@@ -705,61 +705,56 @@ int record_parse_callchain(const char *arg, struct record_opts *opts)
 	return ret;
 }
 
-static void callchain_debug(struct record_opts *opts)
+static void callchain_debug(void)
 {
 	static const char *str[CALLCHAIN_MAX] = { "NONE", "FP", "DWARF" };
 
-	pr_debug("callchain: type %s\n", str[opts->call_graph]);
+	pr_debug("callchain: type %s\n", str[callchain_param.record_mode]);
 
-	if (opts->call_graph == CALLCHAIN_DWARF)
+	if (callchain_param.record_mode == CALLCHAIN_DWARF)
 		pr_debug("callchain: stack dump size %d\n",
-			 opts->stack_dump_size);
+			 callchain_param.dump_size);
 }
 
-int record_parse_callchain_opt(const struct option *opt,
+int record_parse_callchain_opt(const struct option *opt __maybe_unused,
 			       const char *arg,
 			       int unset)
 {
-	struct record_opts *opts = opt->value;
 	int ret;
 
-	opts->call_graph_enabled = !unset;
+	callchain_param.enabled = !unset;
 
 	/* --no-call-graph */
 	if (unset) {
-		opts->call_graph = CALLCHAIN_NONE;
+		callchain_param.record_mode = CALLCHAIN_NONE;
 		pr_debug("callchain: disabled\n");
 		return 0;
 	}
 
-	ret = record_parse_callchain(arg, opts);
+	ret = record_parse_callchain(arg);
 	if (!ret)
-		callchain_debug(opts);
+		callchain_debug();
 
 	return ret;
 }
 
-int record_callchain_opt(const struct option *opt,
+int record_callchain_opt(const struct option *opt __maybe_unused,
 			 const char *arg __maybe_unused,
 			 int unset __maybe_unused)
 {
-	struct record_opts *opts = opt->value;
+	callchain_param.enabled = true;
 
-	opts->call_graph_enabled = !unset;
+	if (callchain_param.record_mode == CALLCHAIN_NONE)
+		callchain_param.record_mode = CALLCHAIN_FP;
 
-	if (opts->call_graph == CALLCHAIN_NONE)
-		opts->call_graph = CALLCHAIN_FP;
-
-	callchain_debug(opts);
+	callchain_debug();
 	return 0;
 }
 
 static int perf_record_config(const char *var, const char *value, void *cb)
 {
-	struct record *rec = cb;
-
 	if (!strcmp(var, "record.call-graph"))
-		return record_parse_callchain(value, &rec->opts);
+		return record_parse_callchain(value);
 
 	return perf_default_config(var, value, cb);
 }
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index e13864be2acb..200a44b370c3 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1020,10 +1020,8 @@ parse_callchain_opt(const struct option *opt, const char *arg, int unset)
 
 static int perf_top_config(const char *var, const char *value, void *cb)
 {
-	struct perf_top *top = cb;
-
 	if (!strcmp(var, "top.call-graph"))
-		return record_parse_callchain(value, &top->record_opts);
+		return record_parse_callchain(value);
 	if (!strcmp(var, "top.children")) {
 		symbol_conf.cumulate_callchain = perf_config_bool(var, value);
 		return 0;
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 510c65f72858..220d44e44c1b 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -41,8 +41,6 @@ void pthread__unblock_sigwinch(void);
 
 struct record_opts {
 	struct target target;
-	int	     call_graph;
-	bool         call_graph_enabled;
 	bool	     group;
 	bool	     inherit_stat;
 	bool	     no_buffering;
@@ -60,7 +58,6 @@ struct record_opts {
 	u64          branch_stack;
 	u64	     default_interval;
 	u64	     user_interval;
-	u16	     stack_dump_size;
 	bool	     sample_transaction;
 	unsigned     initial_delay;
 };
diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
index da43619d6173..819ae4f61e08 100644
--- a/tools/perf/util/callchain.h
+++ b/tools/perf/util/callchain.h
@@ -54,6 +54,9 @@ enum chain_key {
 };
 
 struct callchain_param {
+	bool			enabled;
+	enum perf_call_graph_mode record_mode;
+	u32			dump_size;
 	enum chain_mode 	mode;
 	u32			print_limit;
 	double			min_percent;
@@ -154,7 +157,7 @@ static inline void callchain_cursor_advance(struct callchain_cursor *cursor)
 struct option;
 struct hist_entry;
 
-int record_parse_callchain(const char *arg, struct record_opts *opts);
+int record_parse_callchain(const char *arg);
 int record_parse_callchain_opt(const struct option *opt, const char *arg, int unset);
 int record_callchain_opt(const struct option *opt, const char *arg, int unset);
 
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index b38de5819323..e0868a901c4a 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -503,20 +503,19 @@ int perf_evsel__group_desc(struct perf_evsel *evsel, char *buf, size_t size)
 }
 
 static void
-perf_evsel__config_callgraph(struct perf_evsel *evsel,
-			     struct record_opts *opts)
+perf_evsel__config_callgraph(struct perf_evsel *evsel)
 {
 	bool function = perf_evsel__is_function_event(evsel);
 	struct perf_event_attr *attr = &evsel->attr;
 
 	perf_evsel__set_sample_bit(evsel, CALLCHAIN);
 
-	if (opts->call_graph == CALLCHAIN_DWARF) {
+	if (callchain_param.record_mode == CALLCHAIN_DWARF) {
 		if (!function) {
 			perf_evsel__set_sample_bit(evsel, REGS_USER);
 			perf_evsel__set_sample_bit(evsel, STACK_USER);
 			attr->sample_regs_user = PERF_REGS_MASK;
-			attr->sample_stack_user = opts->stack_dump_size;
+			attr->sample_stack_user = callchain_param.dump_size;
 			attr->exclude_callchain_user = 1;
 		} else {
 			pr_info("Cannot use DWARF unwind for function trace event,"
@@ -625,8 +624,8 @@ void perf_evsel__config(struct perf_evsel *evsel, struct record_opts *opts)
 		attr->mmap_data = track;
 	}
 
-	if (opts->call_graph_enabled && !evsel->no_aux_samples)
-		perf_evsel__config_callgraph(evsel, opts);
+	if (callchain_param.enabled && !evsel->no_aux_samples)
+		perf_evsel__config_callgraph(evsel);
 
 	if (target__has_cpu(&opts->target))
 		perf_evsel__set_sample_bit(evsel, CPU);
-- 
2.1.0

[PATCH 3/5] perf tools: Move some callchain parser functions to callchain.c

[Namhyung Kim]

And rename record_callchain_parse() to parse_callchain_record_opt() in
accordance to parse_callchain_report_opt().

Signek-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-record.c | 88 ++-------------------------------------------
 tools/perf/builtin-top.c    |  2 +-
 tools/perf/util/callchain.c | 84 +++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/callchain.h |  2 +-
 4 files changed, 88 insertions(+), 88 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 4de632cd677b..12e1f2255f89 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -621,90 +621,6 @@ error:
 	return ret;
 }
 
-#ifdef HAVE_DWARF_UNWIND_SUPPORT
-static int get_stack_size(char *str, unsigned long *_size)
-{
-	char *endptr;
-	unsigned long size;
-	unsigned long max_size = round_down(USHRT_MAX, sizeof(u64));
-
-	size = strtoul(str, &endptr, 0);
-
-	do {
-		if (*endptr)
-			break;
-
-		size = round_up(size, sizeof(u64));
-		if (!size || size > max_size)
-			break;
-
-		*_size = size;
-		return 0;
-
-	} while (0);
-
-	pr_err("callchain: Incorrect stack dump size (max %ld): %s\n",
-	       max_size, str);
-	return -1;
-}
-#endif /* HAVE_DWARF_UNWIND_SUPPORT */
-
-int record_parse_callchain(const char *arg)
-{
-	char *tok, *name, *saveptr = NULL;
-	char *buf;
-	int ret = -1;
-
-	/* We need buffer that we know we can write to. */
-	buf = malloc(strlen(arg) + 1);
-	if (!buf)
-		return -ENOMEM;
-
-	strcpy(buf, arg);
-
-	tok = strtok_r((char *)buf, ",", &saveptr);
-	name = tok ? : (char *)buf;
-
-	do {
-		/* Framepointer style */
-		if (!strncmp(name, "fp", sizeof("fp"))) {
-			if (!strtok_r(NULL, ",", &saveptr)) {
-				callchain_param.record_mode = CALLCHAIN_FP;
-				ret = 0;
-			} else
-				pr_err("callchain: No more arguments "
-				       "needed for -g fp\n");
-			break;
-
-#ifdef HAVE_DWARF_UNWIND_SUPPORT
-		/* Dwarf style */
-		} else if (!strncmp(name, "dwarf", sizeof("dwarf"))) {
-			const unsigned long default_stack_dump_size = 8192;
-
-			ret = 0;
-			callchain_param.record_mode = CALLCHAIN_DWARF;
-			callchain_param.dump_size = default_stack_dump_size;
-
-			tok = strtok_r(NULL, ",", &saveptr);
-			if (tok) {
-				unsigned long size = 0;
-
-				ret = get_stack_size(tok, &size);
-				callchain_param.dump_size = size;
-			}
-#endif /* HAVE_DWARF_UNWIND_SUPPORT */
-		} else {
-			pr_err("callchain: Unknown --call-graph option "
-			       "value: %s\n", arg);
-			break;
-		}
-
-	} while (0);
-
-	free(buf);
-	return ret;
-}
-
 static void callchain_debug(void)
 {
 	static const char *str[CALLCHAIN_MAX] = { "NONE", "FP", "DWARF" };
@@ -731,7 +647,7 @@ int record_parse_callchain_opt(const struct option *opt __maybe_unused,
 		return 0;
 	}
 
-	ret = record_parse_callchain(arg);
+	ret = parse_callchain_record_opt(arg);
 	if (!ret)
 		callchain_debug();
 
@@ -754,7 +670,7 @@ int record_callchain_opt(const struct option *opt __maybe_unused,
 static int perf_record_config(const char *var, const char *value, void *cb)
 {
 	if (!strcmp(var, "record.call-graph"))
-		return record_parse_callchain(value);
+		return parse_callchain_record_opt(value);
 
 	return perf_default_config(var, value, cb);
 }
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 200a44b370c3..45a3a461286f 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1021,7 +1021,7 @@ parse_callchain_opt(const struct option *opt, const char *arg, int unset)
 static int perf_top_config(const char *var, const char *value, void *cb)
 {
 	if (!strcmp(var, "top.call-graph"))
-		return record_parse_callchain(value);
+		return parse_callchain_record_opt(value);
 	if (!strcmp(var, "top.children")) {
 		symbol_conf.cumulate_callchain = perf_config_bool(var, value);
 		return 0;
diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index 08f0fbf5527c..ba7297230143 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -25,6 +25,90 @@
 
 __thread struct callchain_cursor callchain_cursor;
 
+#ifdef HAVE_DWARF_UNWIND_SUPPORT
+static int get_stack_size(const char *str, unsigned long *_size)
+{
+	char *endptr;
+	unsigned long size;
+	unsigned long max_size = round_down(USHRT_MAX, sizeof(u64));
+
+	size = strtoul(str, &endptr, 0);
+
+	do {
+		if (*endptr)
+			break;
+
+		size = round_up(size, sizeof(u64));
+		if (!size || size > max_size)
+			break;
+
+		*_size = size;
+		return 0;
+
+	} while (0);
+
+	pr_err("callchain: Incorrect stack dump size (max %ld): %s\n",
+	       max_size, str);
+	return -1;
+}
+#endif /* HAVE_DWARF_UNWIND_SUPPORT */
+
+int parse_callchain_record_opt(const char *arg)
+{
+	char *tok, *name, *saveptr = NULL;
+	char *buf;
+	int ret = -1;
+
+	/* We need buffer that we know we can write to. */
+	buf = malloc(strlen(arg) + 1);
+	if (!buf)
+		return -ENOMEM;
+
+	strcpy(buf, arg);
+
+	tok = strtok_r((char *)buf, ",", &saveptr);
+	name = tok ? : (char *)buf;
+
+	do {
+		/* Framepointer style */
+		if (!strncmp(name, "fp", sizeof("fp"))) {
+			if (!strtok_r(NULL, ",", &saveptr)) {
+				callchain_param.record_mode = CALLCHAIN_FP;
+				ret = 0;
+			} else
+				pr_err("callchain: No more arguments "
+				       "needed for -g fp\n");
+			break;
+
+#ifdef HAVE_DWARF_UNWIND_SUPPORT
+		/* Dwarf style */
+		} else if (!strncmp(name, "dwarf", sizeof("dwarf"))) {
+			const unsigned long default_stack_dump_size = 8192;
+
+			ret = 0;
+			callchain_param.record_mode = CALLCHAIN_DWARF;
+			callchain_param.dump_size = default_stack_dump_size;
+
+			tok = strtok_r(NULL, ",", &saveptr);
+			if (tok) {
+				unsigned long size = 0;
+
+				ret = get_stack_size(tok, &size);
+				callchain_param.dump_size = size;
+			}
+#endif /* HAVE_DWARF_UNWIND_SUPPORT */
+		} else {
+			pr_err("callchain: Unknown --call-graph option "
+			       "value: %s\n", arg);
+			break;
+		}
+
+	} while (0);
+
+	free(buf);
+	return ret;
+}
+
 int
 parse_callchain_report_opt(const char *arg)
 {
diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
index 819ae4f61e08..8adfbf0bab5c 100644
--- a/tools/perf/util/callchain.h
+++ b/tools/perf/util/callchain.h
@@ -157,7 +157,6 @@ static inline void callchain_cursor_advance(struct callchain_cursor *cursor)
 struct option;
 struct hist_entry;
 
-int record_parse_callchain(const char *arg);
 int record_parse_callchain_opt(const struct option *opt, const char *arg, int unset);
 int record_callchain_opt(const struct option *opt, const char *arg, int unset);
 
@@ -169,6 +168,7 @@ int fill_callchain_info(struct addr_location *al, struct callchain_cursor_node *
 			bool hide_unresolved);
 
 extern const char record_callchain_help[];
+int parse_callchain_record_opt(const char *arg);
 int parse_callchain_report_opt(const char *arg);
 
 static inline void callchain_cursor_snapshot(struct callchain_cursor *dest,
-- 
2.1.0

[PATCH 4/5] perf tools: Introduce perf_callchain_config()

[Namhyung Kim]

This patch adds support for following config options to ~/.perfconfig file.

  [call-graph]
    record-mode = dwarf
    dump-size = 8192
    print-type = fractal
    order = callee
    threshold = 0.5
    print-limit = 128
    sort-key = function

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/callchain.c | 33 +++++++++++++++++++++++++++++++++
 tools/perf/util/callchain.h |  1 +
 tools/perf/util/config.c    |  3 +++
 3 files changed, 37 insertions(+)

diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index ba7297230143..ee489a1e3f4c 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -168,6 +168,39 @@ parse_callchain_report_opt(const char *arg)
 	return 0;
 }
 
+int perf_callchain_config(const char *var, const char *value)
+{
+	if (prefixcmp(var, "call-graph."))
+		return 0;
+	var += 11; /* strlen("call-graph.") == 11 */
+
+	if (!strcmp(var, "record-mode"))
+		return parse_callchain_record_opt(value);
+#ifdef HAVE_DWARF_UNWIND_SUPPORT
+	if (!strcmp(var, "dump-size")) {
+		unsigned long size = 0;
+		int ret;
+
+		ret = get_stack_size(value, &size);
+		callchain_param.dump_size = size;
+
+		return ret;
+	}
+#endif
+	if (!strcmp(var, "print-type"))
+		return parse_callchain_report_opt(value);
+	if (!strcmp(var, "order"))
+		return parse_callchain_report_opt(value);
+	if (!strcmp(var, "threshold"))
+		return parse_callchain_report_opt(value);
+	if (!strcmp(var, "print-limit"))
+		return parse_callchain_report_opt(value);
+	if (!strcmp(var, "sort-key"))
+		return parse_callchain_report_opt(value);
+
+	return 0;
+}
+
 static void
 rb_insert_callchain(struct rb_root *root, struct callchain_node *chain,
 		    enum chain_mode mode)
diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
index 8adfbf0bab5c..2a1f5a46543a 100644
--- a/tools/perf/util/callchain.h
+++ b/tools/perf/util/callchain.h
@@ -170,6 +170,7 @@ int fill_callchain_info(struct addr_location *al, struct callchain_cursor_node *
 extern const char record_callchain_help[];
 int parse_callchain_record_opt(const char *arg);
 int parse_callchain_report_opt(const char *arg);
+int perf_callchain_config(const char *var, const char *value);
 
 static inline void callchain_cursor_snapshot(struct callchain_cursor *dest,
 					     struct callchain_cursor *src)
diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index 9970b8b0190b..953512ed72ba 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -396,6 +396,9 @@ int perf_default_config(const char *var, const char *value,
 	if (!prefixcmp(var, "ui."))
 		return perf_ui_config(var, value);
 
+	if (!prefixcmp(var, "call-graph."))
+		return perf_callchain_config(var, value);
+
 	/* Add other config variables here. */
 	return 0;
 }
-- 
2.1.0

[PATCH 5/5] perf tools: Convert {record,top}.call-graph option to call-graph.record-mode

[Namhyung Kim]

So that it'll be passed to perf_callchain_config().

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/builtin-record.c | 2 +-
 tools/perf/builtin-top.c    | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 12e1f2255f89..86ed253fa1b8 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -670,7 +670,7 @@ int record_callchain_opt(const struct option *opt __maybe_unused,
 static int perf_record_config(const char *var, const char *value, void *cb)
 {
 	if (!strcmp(var, "record.call-graph"))
-		return parse_callchain_record_opt(value);
+		var = "call-graph.record-mode"; /* fall-through */
 
 	return perf_default_config(var, value, cb);
 }
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 45a3a461286f..d6cd4c3b13a0 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1021,7 +1021,7 @@ parse_callchain_opt(const struct option *opt, const char *arg, int unset)
 static int perf_top_config(const char *var, const char *value, void *cb)
 {
 	if (!strcmp(var, "top.call-graph"))
-		return parse_callchain_record_opt(value);
+		var = "call-graph.record-mode"; /* fall-through */
 	if (!strcmp(var, "top.children")) {
 		symbol_conf.cumulate_callchain = perf_config_bool(var, value);
 		return 0;
-- 
2.1.0

Re: [PATCHSET 0/5] perf tools: Add call-graph config options

[David Ahern]

On 9/20/14, 10:18 AM, Namhyung Kim wrote:
> Hello,
>
> This is patchset to add new callchain related config options so that
> users don't need to pass their preference to the cmdline everytime.
>
> Following config options will be added, and users can set appropriate
> values to ~/.perfconfig file.  Note that the dump-size option is
> meaningful only if record-mode = dwarf.
>
>    $ cat ~/.perfconfig
>    [call-graph]
>      record-mode = dwarf
>      dump-size = 4096
>      print-type = graph
>      order = callee
>      threshold = 0.5
>      print-limit = 128
>      sort-key = function
>
>    $ perf record -vg sleep 1
>    callchain: type DWARF
>    callchain: stack dump size 4096
>    mmap size 528384B
>    [ perf record: Woken up 1 times to write data ]
>    [ perf record: Captured and wrote 0.054 MB perf.data (~2378 samples) ]
>    Looking at the vmlinux_path (7 entries long)
>    Using /lib/modules/3.16.3-1-ARCH/build/vmlinux for symbols
>
>
> You can also get this from 'perf/callchain-config-v1' branch on my tree
>
>    git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
>

Looks ok to me. Reviewed-by: David Ahern <dsahern@gmail.com>

Re: [PATCH 4/5] perf tools: Introduce perf_callchain_config()

[Jiri Olsa]

On Sun, Sep 21, 2014 at 01:18:05AM +0900, Namhyung Kim wrote:

SNIP

> +int perf_callchain_config(const char *var, const char *value)
> +{
> +	if (prefixcmp(var, "call-graph."))
> +		return 0;
> +	var += 11; /* strlen("call-graph.") == 11 */
> +
> +	if (!strcmp(var, "record-mode"))
> +		return parse_callchain_record_opt(value);
> +#ifdef HAVE_DWARF_UNWIND_SUPPORT
> +	if (!strcmp(var, "dump-size")) {
> +		unsigned long size = 0;
> +		int ret;
> +
> +		ret = get_stack_size(value, &size);
> +		callchain_param.dump_size = size;
> +
> +		return ret;
> +	}
> +#endif
> +	if (!strcmp(var, "print-type"))
> +		return parse_callchain_report_opt(value);
> +	if (!strcmp(var, "order"))
> +		return parse_callchain_report_opt(value);
> +	if (!strcmp(var, "threshold"))
> +		return parse_callchain_report_opt(value);
> +	if (!strcmp(var, "print-limit"))
> +		return parse_callchain_report_opt(value);
> +	if (!strcmp(var, "sort-key"))
> +		return parse_callchain_report_opt(value);

so this allows to spcify option like:

[call-graph]
        sort-key = caller,function,1

which might be confusing for someone.. personaly I'm ok with that,
I'm just raising it, because it's not apparent and not sure how
strict we want to be in here ;-)

jirka

Re: [PATCH 4/5] perf tools: Introduce perf_callchain_config()

[Jiri Olsa]

On Sun, Sep 21, 2014 at 01:18:05AM +0900, Namhyung Kim wrote:
> This patch adds support for following config options to ~/.perfconfig file.
> 
>   [call-graph]
>     record-mode = dwarf
>     dump-size = 8192
>     print-type = fractal
>     order = callee
>     threshold = 0.5
>     print-limit = 128
>     sort-key = function
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/callchain.c | 33 +++++++++++++++++++++++++++++++++
>  tools/perf/util/callchain.h |  1 +
>  tools/perf/util/config.c    |  3 +++
>  3 files changed, 37 insertions(+)
> 
> diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
> index ba7297230143..ee489a1e3f4c 100644
> --- a/tools/perf/util/callchain.c
> +++ b/tools/perf/util/callchain.c
> @@ -168,6 +168,39 @@ parse_callchain_report_opt(const char *arg)
>  	return 0;
>  }
>  
> +int perf_callchain_config(const char *var, const char *value)
> +{
> +	if (prefixcmp(var, "call-graph."))
> +		return 0;
> +	var += 11; /* strlen("call-graph.") == 11 */

should we use sizeof("call-graph.") - 1 instead ?

jirka

Re: [PATCHSET 0/5] perf tools: Add call-graph config options

[Jiri Olsa]

On Sun, Sep 21, 2014 at 01:18:01AM +0900, Namhyung Kim wrote:
> Hello,
> 
> This is patchset to add new callchain related config options so that
> users don't need to pass their preference to the cmdline everytime.
> 
> Following config options will be added, and users can set appropriate
> values to ~/.perfconfig file.  Note that the dump-size option is
> meaningful only if record-mode = dwarf.
> 
>   $ cat ~/.perfconfig
>   [call-graph]
>     record-mode = dwarf
>     dump-size = 4096
>     print-type = graph
>     order = callee
>     threshold = 0.5
>     print-limit = 128
>     sort-key = function
> 
>   $ perf record -vg sleep 1
>   callchain: type DWARF
>   callchain: stack dump size 4096
>   mmap size 528384B
>   [ perf record: Woken up 1 times to write data ]
>   [ perf record: Captured and wrote 0.054 MB perf.data (~2378 samples) ]
>   Looking at the vmlinux_path (7 entries long)
>   Using /lib/modules/3.16.3-1-ARCH/build/vmlinux for symbols
> 
> 
> You can also get this from 'perf/callchain-config-v1' branch on my tree
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> 
> 
> Any comments are welcomed, thanks
> Namhyung
> 
> 
> Namhyung Kim (5):
>   perf hists browser: Fix callchain print bug on TUI
>   perf tools: Move callchain config from record_opts to callchain_param
>   perf tools: Move some callchain parser functions to callchain.c
>   perf tools: Introduce perf_callchain_config()
>   perf tools: Convert {record,top}.call-graph option to
>     call-graph.record-mode

apart from my other comments, for the patchset:

Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka

Re: [PATCH 4/5] perf tools: Introduce perf_callchain_config()

[Arnaldo Carvalho de Melo]

Em Mon, Sep 22, 2014 at 04:25:10PM +0200, Jiri Olsa escreveu:
> On Sun, Sep 21, 2014 at 01:18:05AM +0900, Namhyung Kim wrote:
> > +int perf_callchain_config(const char *var, const char *value)
> > +{
> > +	if (prefixcmp(var, "call-graph."))
> > +		return 0;
> > +	var += 11; /* strlen("call-graph.") == 11 */
 
> should we use sizeof("call-graph.") - 1 instead ?

Agreed, it should produce the same code and is more compact :-)

- Arnaldo

Re: [PATCH 4/5] perf tools: Introduce perf_callchain_config()

[Namhyung Kim]

2014-09-22 (월), 11:51 -0300, Arnaldo Carvalho de Melo:
> Em Mon, Sep 22, 2014 at 04:25:10PM +0200, Jiri Olsa escreveu:
> > On Sun, Sep 21, 2014 at 01:18:05AM +0900, Namhyung Kim wrote:
> > > +int perf_callchain_config(const char *var, const char *value)
> > > +{
> > > +	if (prefixcmp(var, "call-graph."))
> > > +		return 0;
> > > +	var += 11; /* strlen("call-graph.") == 11 */
>  
> > should we use sizeof("call-graph.") - 1 instead ?
> 
> Agreed, it should produce the same code and is more compact :-)

Will change.

Thanks,
Namhyung

Re: [PATCH 4/5] perf tools: Introduce perf_callchain_config()

[Namhyung Kim]

2014-09-22 (월), 16:24 +0200, Jiri Olsa:
> On Sun, Sep 21, 2014 at 01:18:05AM +0900, Namhyung Kim wrote:
> 
> SNIP
> 
> > +int perf_callchain_config(const char *var, const char *value)
> > +{
> > +	if (prefixcmp(var, "call-graph."))
> > +		return 0;
> > +	var += 11; /* strlen("call-graph.") == 11 */
> > +
> > +	if (!strcmp(var, "record-mode"))
> > +		return parse_callchain_record_opt(value);
> > +#ifdef HAVE_DWARF_UNWIND_SUPPORT
> > +	if (!strcmp(var, "dump-size")) {
> > +		unsigned long size = 0;
> > +		int ret;
> > +
> > +		ret = get_stack_size(value, &size);
> > +		callchain_param.dump_size = size;
> > +
> > +		return ret;
> > +	}
> > +#endif
> > +	if (!strcmp(var, "print-type"))
> > +		return parse_callchain_report_opt(value);
> > +	if (!strcmp(var, "order"))
> > +		return parse_callchain_report_opt(value);
> > +	if (!strcmp(var, "threshold"))
> > +		return parse_callchain_report_opt(value);
> > +	if (!strcmp(var, "print-limit"))
> > +		return parse_callchain_report_opt(value);
> > +	if (!strcmp(var, "sort-key"))
> > +		return parse_callchain_report_opt(value);
> 
> so this allows to spcify option like:
> 
> [call-graph]
>         sort-key = caller,function,1
> 
> which might be confusing for someone.. personaly I'm ok with that,
> I'm just raising it, because it's not apparent and not sure how
> strict we want to be in here ;-)

Right, I just reused parse_callchain_{record,report}_opt() functions to
simplify the code.

Hmm.. now I rethink that at least threshold and print-limit will be
treated differently as parse_callchain_report_opt() implies an order of
them.  Will change in the next version.

Thanks,
Namhyung