> > +   /* User explicitly set per-event callgraph, clear the old setting and
> reset. */
> > +   if ((callgraph_buf != NULL) || (dump_size > 0)) {
> > +
> > +           /* parse callgraph parameters */
> > +           if (callgraph_buf != NULL) {
> > +                   if (!strcmp(callgraph_buf, "no")) {
> > +                           param.enabled = false;
> > +                           param.record_mode = CALLCHAIN_NONE;
> > +                   } else {
> > +                           param.enabled = true;
> > +                           if
> (parse_callchain_record_opt(callgraph_buf, &param)) {
> > +                                   pr_err("per-event callgraph setting
> for %s failed. "
> > +                                          "Apply callgraph global setting
> for it\n",
> > +                                          evsel->name);
> > +                                   return;
> 
> hum, calling parse_callchain_record_opt from evsel hurts the python code:
> 
> 17: Try 'import perf' in python, checking link problems      :
> --- start ---
> test child forked, pid 25751
> Traceback (most recent call last):
>   File "<stdin>", line 1, in <module>
> ImportError: python/perf.so: undefined symbol:
> parse_callchain_record_opt test child finished with -1
> ---- end ----
> Try 'import perf' in python, checking link problems: FAILED!
> 
> 
> not sure if we can call it from some place else (I guess not), then we'd need
> to either put the util/callchain.c under python objects,

We cannot only put the util/callchain.c under python objects, since
there are too many dependency for callchain.c. 

> or somehow refine
> needed parsing code..

Could we just move the related code to util.c as below?

---
diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index 931cca8..773fe13 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -25,96 +25,9 @@
 
 __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, struct callchain_param *param)
 {
-       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)) {
-                               param->record_mode = CALLCHAIN_FP;
-                               ret = 0;
-                       } else
-                               pr_err("callchain: No more arguments "
-                                      "needed for --call-graph 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;
-                       param->record_mode = CALLCHAIN_DWARF;
-                       param->dump_size = default_stack_dump_size;
-
-                       tok = strtok_r(NULL, ",", &saveptr);
-                       if (tok) {
-                               unsigned long size = 0;
-
-                               ret = get_stack_size(tok, &size);
-                               param->dump_size = size;
-                       }
-#endif /* HAVE_DWARF_UNWIND_SUPPORT */
-               } else if (!strncmp(name, "lbr", sizeof("lbr"))) {
-                       if (!strtok_r(NULL, ",", &saveptr)) {
-                               param->record_mode = CALLCHAIN_LBR;
-                               ret = 0;
-                       } else
-                               pr_err("callchain: No more arguments "
-                                       "needed for --call-graph lbr\n");
-                       break;
-               } else {
-                       pr_err("callchain: Unknown --call-graph option "
-                              "value: %s\n", arg);
-                       break;
-               }
-
-       } while (0);
-
-       free(buf);
-       return ret;
+       return parse_callchain_record(arg, param);
 }
 
 static int parse_callchain_mode(const char *value)
diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h
index 68a32c2..acee2b3 100644
--- a/tools/perf/util/callchain.h
+++ b/tools/perf/util/callchain.h
@@ -177,6 +177,7 @@ int fill_callchain_info(struct addr_location *al, struct 
callchain_cursor_node *
                        bool hide_unresolved);
 
 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 perf_callchain_config(const char *var, const char *value);
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 106cd20..675d237 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -651,7 +651,7 @@ static void apply_config_terms(struct perf_evsel *evsel,
                                param.record_mode = CALLCHAIN_NONE;
                        } else {
                                param.enabled = true;
-                               if (parse_callchain_record_opt(callgraph_buf, 
&param)) {
+                               if (parse_callchain_record(callgraph_buf, 
&param)) {
                                        pr_err("per-event callgraph setting for 
%s failed. "
                                               "Apply callgraph global setting 
for it\n",
                                               evsel->name);
diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
index edc2d63..f7adf12 100644
--- a/tools/perf/util/util.c
+++ b/tools/perf/util/util.c
@@ -566,6 +566,96 @@ unsigned long parse_tag_value(const char *str, struct 
parse_tag *tags)
        return (unsigned long) -1;
 }
 
+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;
+}
+
+int parse_callchain_record(const char *arg, struct callchain_param *param)
+{
+       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)) {
+                               param->record_mode = CALLCHAIN_FP;
+                               ret = 0;
+                       } else
+                               pr_err("callchain: No more arguments "
+                                      "needed for --call-graph 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;
+                       param->record_mode = CALLCHAIN_DWARF;
+                       param->dump_size = default_stack_dump_size;
+
+                       tok = strtok_r(NULL, ",", &saveptr);
+                       if (tok) {
+                               unsigned long size = 0;
+
+                               ret = get_stack_size(tok, &size);
+                               param->dump_size = size;
+                       }
+#endif /* HAVE_DWARF_UNWIND_SUPPORT */
+               } else if (!strncmp(name, "lbr", sizeof("lbr"))) {
+                       if (!strtok_r(NULL, ",", &saveptr)) {
+                               param->record_mode = CALLCHAIN_LBR;
+                               ret = 0;
+                       } else
+                               pr_err("callchain: No more arguments "
+                                       "needed for --call-graph lbr\n");
+                       break;
+               } else {
+                       pr_err("callchain: Unknown --call-graph option "
+                              "value: %s\n", arg);
+                       break;
+               }
+
+       } while (0);
+
+       free(buf);
+       return ret;
+}
+
 int filename__read_str(const char *filename, char **buf, size_t *sizep)
 {
        size_t size = 0, alloc_size = 0;
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 20d625a..8148703 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -351,4 +351,6 @@ static inline char *asprintf_expr_not_in_ints(const char 
*var, size_t nints, int
        return asprintf_expr_inout_ints(var, false, nints, ints);
 }
 
+int get_stack_size(const char *str, unsigned long *_size);
+
 #endif /* GIT_COMPAT_UTIL_H */
 --

Thanks,
Kan

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to