perf callchain: Refactor callchain option parsing

record_opts__parse_callchain is shared by builtin-record and
builtin-trace, it is declared in callchain.h. Move the declaration to
callchain.c for consistency with the header. In other cases make the
option callback a small static stub that then calls into callchain.c.

Make the no argument '-g' callchain option just a short-cut for
'--call-graph fp' so that there is consistency in how the arguments
are handled. This requires the const char* string to be strdup-ed in
__parse_callchain_report_opt. For consistency also make
parse_callchain_record use strdup and remove some unnecessary
casts. Also, be more explicit about the '-g' behavior if there is a
.perfconfig file setting.

Signed-off-by: Ian Rogers <irogers@google.com>
Tested-by: Thomas Richter <tmricht@linux.ibm.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 40917a0be2389567810a4f5ba901529e5aded930..59b8125d1b13a79ed22b9ddde79ff65ab1de0182 100644 (file)
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -2975,65 +2975,30 @@ out_delete_session:
        return status;
 }
 
-static void callchain_debug(struct callchain_param *callchain)
-{
-       static const char *str[CALLCHAIN_MAX] = { "NONE", "FP", "DWARF", "LBR" };
-
-       pr_debug("callchain: type %s\n", str[callchain->record_mode]);
-
-       if (callchain->record_mode == CALLCHAIN_DWARF)
-               pr_debug("callchain: stack dump size %d\n",
-                        callchain->dump_size);
-}
-
-int record_opts__parse_callchain(struct record_opts *record,
-                                struct callchain_param *callchain,
-                                const char *arg, bool unset)
-{
-       int ret;
-       callchain->enabled = !unset;
-
-       /* --no-call-graph */
-       if (unset) {
-               callchain->record_mode = CALLCHAIN_NONE;
-               pr_debug("callchain: disabled\n");
-               return 0;
-       }
-
-       ret = parse_callchain_record_opt(arg, callchain);
-       if (!ret) {
-               /* Enable data address sampling for DWARF unwind. */
-               if (callchain->record_mode == CALLCHAIN_DWARF &&
-                   !record->record_data_mmap_set)
-                       record->record_data_mmap = true;
-               callchain_debug(callchain);
-       }
-
-       return ret;
-}
-
-int record_parse_callchain_opt(const struct option *opt,
+static int record_parse_callchain_opt(const struct option *opt,
                               const char *arg,
                               int unset)
 {
        return record_opts__parse_callchain(opt->value, &callchain_param, arg, unset);
 }
 
-int record_callchain_opt(const struct option *opt,
-                        const char *arg __maybe_unused,
-                        int unset __maybe_unused)
+static int record_callchain_opt(const struct option *opt,
+                               const char *arg __maybe_unused,
+                               int unset)
 {
-       struct callchain_param *callchain = opt->value;
-
-       callchain->enabled = true;
-
-       if (callchain->record_mode == CALLCHAIN_NONE)
-               callchain->record_mode = CALLCHAIN_FP;
+       /*
+        * The -g option only sets the callchain if not already configured by
+        * .perfconfig. It does, however, enable it.
+        */
+       if (callchain_param.record_mode != CALLCHAIN_NONE) {
+               callchain_param.enabled = true;
+               return 0;
+       }
 
-       callchain_debug(callchain);
-       return 0;
+       return record_opts__parse_callchain(opt->value, &callchain_param, "fp", unset);
 }
 
+
 static int perf_record_config(const char *var, const char *value, void *cb)
 {
        struct record *rec = cb;
@@ -3525,7 +3490,7 @@ static struct option __record_options[] = {
        OPT_CALLBACK(0, "mmap-flush", &record.opts, "number",
                     "Minimal number of bytes that is extracted from mmap data pages (default: 1)",
                     record__mmap_flush_parse),
-       OPT_CALLBACK_NOOPT('g', NULL, &callchain_param,
+       OPT_CALLBACK_NOOPT('g', NULL, &record.opts,
                           NULL, "enables call-graph recording" ,
                           &record_callchain_opt),
        OPT_CALLBACK(0, "call-graph", &record.opts,

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 710604c4f6f6cbe82714c84f461072acb0901b7e..b6726f4dffb3a2b75bf50699c007e015b020afb9 100644 (file)
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1386,13 +1386,6 @@ out_join_thread:
        return ret;
 }
 
-static int
-callchain_opt(const struct option *opt, const char *arg, int unset)
-{
-       symbol_conf.use_callchain = true;
-       return record_callchain_opt(opt, arg, unset);
-}
-
 static int
 parse_callchain_opt(const struct option *opt, const char *arg, int unset)
 {
@@ -1413,6 +1406,24 @@ parse_callchain_opt(const struct option *opt, const char *arg, int unset)
        return parse_callchain_top_opt(arg);
 }
 
+static int
+callchain_opt(const struct option *opt, const char *arg __maybe_unused, int unset)
+{
+       struct callchain_param *callchain = opt->value;
+
+       /*
+        * The -g option only sets the callchain if not already configured by
+        * .perfconfig. It does, however, enable it.
+        */
+       if (callchain->record_mode != CALLCHAIN_NONE) {
+               callchain->enabled = true;
+               return 0;
+       }
+
+       return parse_callchain_opt(opt, "fp", unset);
+}
+
+
 static int perf_top_config(const char *var, const char *value, void *cb __maybe_unused)
 {
        if (!strcmp(var, "top.call-graph")) {

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 1c38f3d16a31ed3a784c42532b4c3808c5490534..f487fbaa0ad60028633685527303aa3474b078d6 100644 (file)
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -5300,6 +5300,13 @@ static int trace__parse_summary_mode(const struct option *opt, const char *str,
        return 0;
 }
 
+static int trace_parse_callchain_opt(const struct option *opt,
+                                    const char *arg,
+                                    int unset)
+{
+       return record_opts__parse_callchain(opt->value, &callchain_param, arg, unset);
+}
+
 static int trace__config(const char *var, const char *value, void *arg)
 {
        struct trace *trace = arg;
@@ -5447,7 +5454,7 @@ int cmd_trace(int argc, const char **argv)
        OPT_BOOLEAN('f', "force", &trace.force, "don't complain, do it"),
        OPT_CALLBACK(0, "call-graph", &trace.opts,
                     "record_mode[,record_size]", record_callchain_help,
-                    &record_parse_callchain_opt),
+                    &trace_parse_callchain_opt),
        OPT_BOOLEAN(0, "libtraceevent_print", &trace.libtraceevent_print,
                    "Use libtraceevent to print the tracepoint arguments."),
        OPT_BOOLEAN(0, "kernel-syscall-graph", &trace.kernel_syscallchains,

diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index 8ff0898799ee0cc00e47383f055705f0852947db..f879b84f8ff998cab2e24fc3cf36632c8d8fc3d5 100644 (file)
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -30,6 +30,7 @@
 #include "map.h"
 #include "callchain.h"
 #include "branch.h"
+#include "record.h"
 #include "symbol.h"
 #include "thread.h"
 #include "util.h"
@@ -170,7 +171,7 @@ static int get_stack_size(const char *str, unsigned long *_size)
 static int
 __parse_callchain_report_opt(const char *arg, bool allow_record_opt)
 {
-       char *tok;
+       char *tok, *arg_copy;
        char *endptr, *saveptr = NULL;
        bool minpcnt_set = false;
        bool record_opt_set = false;
@@ -182,12 +183,17 @@ __parse_callchain_report_opt(const char *arg, bool allow_record_opt)
        if (!arg)
                return 0;
 
-       while ((tok = strtok_r((char *)arg, ",", &saveptr)) != NULL) {
+       arg_copy = strdup(arg);
+       if (!arg_copy)
+               return -ENOMEM;
+
+       tok = strtok_r(arg_copy, ",", &saveptr);
+       while (tok) {
                if (!strncmp(tok, "none", strlen(tok))) {
                        callchain_param.mode = CHAIN_NONE;
                        callchain_param.enabled = false;
                        symbol_conf.use_callchain = false;
-                       return 0;
+                       goto out;
                }
 
                if (!parse_callchain_mode(tok) ||
@@ -214,30 +220,35 @@ try_numbers:
                        unsigned long size = 0;
 
                        if (get_stack_size(tok, &size) < 0)
-                               return -1;
+                               goto err_out;
                        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);
                        if (tok == endptr)
-                               return -1;
+                               goto err_out;
                        minpcnt_set = true;
                } else {
                        /* try print limit at last */
                        callchain_param.print_limit = strtoul(tok, &endptr, 0);
                        if (tok == endptr)
-                               return -1;
+                               goto err_out;
                }
 next:
-               arg = NULL;
+               tok = strtok_r(NULL, ",", &saveptr);
        }
 
        if (callchain_register_param(&callchain_param) < 0) {
                pr_err("Can't register callchain params\n");
-               return -1;
+               goto err_out;
        }
+out:
+       free(arg_copy);
        return 0;
+err_out:
+       free(arg_copy);
+       return -1;
 }
 
 int parse_callchain_report_opt(const char *arg)
@@ -257,14 +268,12 @@ int parse_callchain_record(const char *arg, struct callchain_param *param)
        int ret = -1;
 
        /* We need buffer that we know we can write to. */
-       buf = malloc(strlen(arg) + 1);
+       buf = strdup(arg);
        if (!buf)
                return -ENOMEM;
 
-       strcpy(buf, arg);
-
-       tok = strtok_r((char *)buf, ",", &saveptr);
-       name = tok ? : (char *)buf;
+       tok = strtok_r(buf, ",", &saveptr);
+       name = tok ? : buf;
 
        do {
                /* Framepointer style */
@@ -328,6 +337,44 @@ int parse_callchain_record(const char *arg, struct callchain_param *param)
        return ret;
 }
 
+static void callchain_debug(const struct callchain_param *callchain)
+{
+       static const char *str[CALLCHAIN_MAX] = { "NONE", "FP", "DWARF", "LBR" };
+
+       pr_debug("callchain: type %s\n", str[callchain->record_mode]);
+
+       if (callchain->record_mode == CALLCHAIN_DWARF)
+               pr_debug("callchain: stack dump size %d\n",
+                        callchain->dump_size);
+}
+
+int record_opts__parse_callchain(struct record_opts *record,
+                                struct callchain_param *callchain,
+                                const char *arg, bool unset)
+{
+       int ret;
+
+       callchain->enabled = !unset;
+
+       /* --no-call-graph */
+       if (unset) {
+               callchain->record_mode = CALLCHAIN_NONE;
+               pr_debug("callchain: disabled\n");
+               return 0;
+       }
+
+       ret = parse_callchain_record_opt(arg, callchain);
+       if (!ret) {
+               /* Enable data address sampling for DWARF unwind. */
+               if (callchain->record_mode == CALLCHAIN_DWARF &&
+                   !record->record_data_mmap_set)
+                       record->record_data_mmap = true;
+               callchain_debug(callchain);
+       }
+
+       return ret;
+}
+
 int perf_callchain_config(const char *var, const char *value)
 {
        char *endptr;

diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
index df54ddb8c0cb107492119afa12c0349bc853a675..06d463ccc7a04fc4b0f9f06f1bc18adc10ae14b7 100644 (file)
--- a/tools/perf/util/callchain.h
+++ b/tools/perf/util/callchain.h
@@ -9,11 +9,13 @@
 
 struct addr_location;
 struct evsel;
+struct hist_entry;
+struct hists;
 struct ip_callchain;
 struct map;
 struct perf_sample;
+struct record_opts;
 struct thread;
-struct hists;
 
 #define HELP_PAD "\t\t\t\t"
 
@@ -237,14 +239,6 @@ struct callchain_cursor *get_tls_callchain_cursor(void);
 int callchain_cursor__copy(struct callchain_cursor *dst,
                           struct callchain_cursor *src);
 
-struct option;
-struct hist_entry;
-
-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);
-
-struct record_opts;
-
 int record_opts__parse_callchain(struct record_opts *record,
                                 struct callchain_param *callchain,
                                 const char *arg, bool unset);