2009-12-01 00:19:41

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH -tip 0/9] perf-probe updates

Hi,

Here are bugfixes and updates for perf-probe and kprobe-tracer.
I've fixed some minor bugs and added --list option and simple
probe naming.

TODO:
- Support build-id checking.
- Support --del option to remove probes.
- Support --line option to show which lines user can probe.
- Support lazy string matching.

Thank you,

---

Masami Hiramatsu (9):
perf probe: Simplify event naming
perf probe: Add --list option for listing current probe events
perf probe: Add argv_split() from lib/argv_split.c
perf probe: Move probe event utility functions to probe-event.c
perf probe: Fix probe array index for multiple probe point
perf probe: Fix argv array size in probe parser
perf probe: Fix to add probe-finder.h without libdwarf
perf probe: Fix to change a debugging message from pr_info to pr_debug
trace_kprobes: Fix a memory leak bug and check kstrdup return value


kernel/trace/trace_kprobe.c | 26 ++
tools/perf/Makefile | 4
tools/perf/builtin-probe.c | 236 ++------------------
tools/perf/util/probe-event.c | 484 +++++++++++++++++++++++++++++++++++++++++
tools/perf/util/probe-event.h | 18 ++
tools/perf/util/string.c | 101 +++++++++
tools/perf/util/string.h | 2
7 files changed, 647 insertions(+), 224 deletions(-)
create mode 100644 tools/perf/util/probe-event.c
create mode 100644 tools/perf/util/probe-event.h

--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division
e-mail: [email protected]


2009-12-01 00:19:58

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH -tip 1/9] trace_kprobes: Fix a memory leak bug and check kstrdup return value

Fix a memory leak case in create_trace_probe(). When an argument is
too long (> MAX_ARGSTR_LEN), it just jumps to error path. In that case
tp->args[i].name is not released.
This also fixes a bug to check kstrdup()'s return value.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: K.Prasad <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
---

kernel/trace/trace_kprobe.c | 26 +++++++++++++++++++-------
1 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 72d0c65..aff5f80 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -483,7 +483,8 @@ static int parse_probe_vars(char *arg, struct fetch_func *ff, int is_return)
return ret;
}

-static int parse_probe_arg(char *arg, struct fetch_func *ff, int is_return)
+/* Recursive argument parser */
+static int __parse_probe_arg(char *arg, struct fetch_func *ff, int is_return)
{
int ret = 0;
unsigned long param;
@@ -543,7 +544,7 @@ static int parse_probe_arg(char *arg, struct fetch_func *ff, int is_return)
if (!id)
return -ENOMEM;
id->offset = offset;
- ret = parse_probe_arg(arg, &id->orig, is_return);
+ ret = __parse_probe_arg(arg, &id->orig, is_return);
if (ret)
kfree(id);
else {
@@ -560,6 +561,16 @@ static int parse_probe_arg(char *arg, struct fetch_func *ff, int is_return)
return ret;
}

+/* String length checking wrapper */
+static int parse_probe_arg(char *arg, struct fetch_func *ff, int is_return)
+{
+ if (strlen(arg) > MAX_ARGSTR_LEN) {
+ pr_info("Argument is too long.: %s\n", arg);
+ return -ENOSPC;
+ }
+ return __parse_probe_arg(arg, ff, is_return);
+}
+
/* Return 1 if name is reserved or already used by another argument */
static int conflict_field_name(const char *name,
struct probe_arg *args, int narg)
@@ -698,13 +709,14 @@ static int create_trace_probe(int argc, char **argv)
}

tp->args[i].name = kstrdup(argv[i], GFP_KERNEL);
-
- /* Parse fetch argument */
- if (strlen(arg) > MAX_ARGSTR_LEN) {
- pr_info("Argument%d(%s) is too long.\n", i, arg);
- ret = -ENOSPC;
+ if (!tp->args[i].name) {
+ pr_info("Failed to allocate argument%d name '%s'.\n",
+ i, argv[i]);
+ ret = -ENOMEM;
goto error;
}
+
+ /* Parse fetch argument */
ret = parse_probe_arg(arg, &tp->args[i].fetch, is_return);
if (ret) {
pr_info("Parse error at argument%d. (%d)\n", i, ret);


--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: [email protected]

2009-12-01 00:23:55

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH -tip 2/9] perf probe: Fix to change a debugging message from pr_info to pr_debug

Change annoying debug-info using notice from pr_info() to pr_debug(),
since the message always printed when user adds a probe point which
requires debug-info.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: K.Prasad <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
---

tools/perf/builtin-probe.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index a2f6daf..4e418af 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -351,7 +351,7 @@ int cmd_probe(int argc, const char **argv, const char *prefix __used)
#ifdef NO_LIBDWARF
semantic_error("Debuginfo-analysis is not supported");
#else /* !NO_LIBDWARF */
- pr_info("Some probes require debuginfo.\n");
+ pr_debug("Some probes require debuginfo.\n");

if (session.vmlinux)
fd = open(session.vmlinux, O_RDONLY);


--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: [email protected]

2009-12-01 00:23:36

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH -tip 3/9] perf probe: Fix to add probe-finder.h without libdwarf

Add probe-finder.h as LIB_H without libdwarf, because that
header is included even if no libdwarf.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: K.Prasad <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
---

tools/perf/Makefile | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index f1537a9..76e4b04 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -369,6 +369,7 @@ LIB_H += util/sort.h
LIB_H += util/hist.h
LIB_H += util/thread.h
LIB_H += util/data_map.h
+LIB_H += util/probe-finder.h

LIB_OBJS += util/abspath.o
LIB_OBJS += util/alias.o
@@ -485,7 +486,6 @@ ifneq ($(shell sh -c "(echo '\#include <libdwarf/dwarf.h>'; echo '\#include <lib
BASIC_CFLAGS += -DNO_LIBDWARF
else
EXTLIBS += -lelf -ldwarf
- LIB_H += util/probe-finder.h
LIB_OBJS += util/probe-finder.o
endif



--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: [email protected]

2009-12-01 00:23:47

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH -tip 4/9] perf probe: Fix argv array size in probe parser

Since the syntax had been changed, probe definition needs parameters
less than MAX_PROBE_ARGS + 1 (probe-point + arguments).

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: K.Prasad <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
---

tools/perf/builtin-probe.c | 11 +++++++----
1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 4e418af..510fdd4 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -151,7 +151,7 @@ static void parse_probe_point(char *arg, struct probe_point *pp)
/* Parse an event definition. Note that any error must die. */
static void parse_probe_event(const char *str)
{
- char *argv[MAX_PROBE_ARGS + 2]; /* Event + probe + args */
+ char *argv[MAX_PROBE_ARGS + 1]; /* probe + args */
int argc, i;
struct probe_point *pp = &session.probes[session.nr_probe];

@@ -169,6 +169,9 @@ static void parse_probe_event(const char *str)
/* Add an argument */
if (*str != '\0') {
const char *s = str;
+ /* Check the limit number of arguments */
+ if (argc == MAX_PROBE_ARGS + 1)
+ semantic_error("Too many arguments");

/* Skip the argument */
while (!isspace(*str) && *str != '\0')
@@ -178,9 +181,9 @@ static void parse_probe_event(const char *str)
argv[argc] = strndup(s, str - s);
if (argv[argc] == NULL)
die("strndup");
- if (++argc == MAX_PROBE_ARGS)
- semantic_error("Too many arguments");
- pr_debug("argv[%d]=%s\n", argc, argv[argc - 1]);
+ pr_debug("argv[%d]=%s\n", argc, argv[argc]);
+ argc++;
+
}
} while (*str != '\0');
if (!argc)


--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: [email protected]

2009-12-01 00:20:27

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH -tip 5/9] perf probe: Fix probe array index for multiple probe point

Fix the index of formatted probe array for multiple probe
point, which should be probes[i] instead of probes[0].

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: K.Prasad <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
---

tools/perf/builtin-probe.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 510fdd4..5f47e62 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -428,7 +428,7 @@ end_dwarf:
pp->retprobe ? 'r' : 'p',
PERFPROBE_GROUP,
pp->function, pp->offset, i,
- pp->probes[0]);
+ pp->probes[i]);
write_new_event(fd, buf);
}
}


--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: [email protected]

2009-12-01 00:24:12

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH -tip 6/9] perf probe: Move probe event utility functions to probe-event.c

Split probe event (kprobe-events and perf probe events) utility
functions from builtin-probe.c to probe-event.c.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: K.Prasad <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
---

tools/perf/Makefile | 2
tools/perf/builtin-probe.c | 227 +---------------------------------
tools/perf/util/probe-event.c | 273 +++++++++++++++++++++++++++++++++++++++++
tools/perf/util/probe-event.h | 10 ++
4 files changed, 294 insertions(+), 218 deletions(-)
create mode 100644 tools/perf/util/probe-event.c
create mode 100644 tools/perf/util/probe-event.h

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 76e4b04..f8537cf 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -370,6 +370,7 @@ LIB_H += util/hist.h
LIB_H += util/thread.h
LIB_H += util/data_map.h
LIB_H += util/probe-finder.h
+LIB_H += util/probe-event.h

LIB_OBJS += util/abspath.o
LIB_OBJS += util/alias.o
@@ -412,6 +413,7 @@ LIB_OBJS += util/svghelper.o
LIB_OBJS += util/sort.o
LIB_OBJS += util/hist.o
LIB_OBJS += util/data_map.o
+LIB_OBJS += util/probe-event.o

BUILTIN_OBJS += builtin-annotate.o

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 5f47e62..bf20df2 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -40,6 +40,7 @@
#include "util/parse-options.h"
#include "util/parse-events.h" /* For debugfs_path */
#include "util/probe-finder.h"
+#include "util/probe-event.h"

/* Default vmlinux search paths */
#define NR_SEARCH_PATH 3
@@ -51,8 +52,6 @@ const char *default_search_path[NR_SEARCH_PATH] = {

#define MAX_PATH_LEN 256
#define MAX_PROBES 128
-#define MAX_PROBE_ARGS 128
-#define PERFPROBE_GROUP "probe"

/* Session management structure */
static struct {
@@ -63,155 +62,17 @@ static struct {
struct probe_point probes[MAX_PROBES];
} session;

-#define semantic_error(msg ...) die("Semantic error :" msg)
-
-/* Parse probe point. Return 1 if return probe */
-static void parse_probe_point(char *arg, struct probe_point *pp)
-{
- char *ptr, *tmp;
- char c, nc = 0;
- /*
- * <Syntax>
- * perf probe SRC:LN
- * perf probe FUNC[+OFFS|%return][@SRC]
- */
-
- ptr = strpbrk(arg, ":+@%");
- if (ptr) {
- nc = *ptr;
- *ptr++ = '\0';
- }
-
- /* Check arg is function or file and copy it */
- if (strchr(arg, '.')) /* File */
- pp->file = strdup(arg);
- else /* Function */
- pp->function = strdup(arg);
- DIE_IF(pp->file == NULL && pp->function == NULL);
-
- /* Parse other options */
- while (ptr) {
- arg = ptr;
- c = nc;
- ptr = strpbrk(arg, ":+@%");
- if (ptr) {
- nc = *ptr;
- *ptr++ = '\0';
- }
- switch (c) {
- case ':': /* Line number */
- pp->line = strtoul(arg, &tmp, 0);
- if (*tmp != '\0')
- semantic_error("There is non-digit charactor"
- " in line number.");
- break;
- case '+': /* Byte offset from a symbol */
- pp->offset = strtoul(arg, &tmp, 0);
- if (*tmp != '\0')
- semantic_error("There is non-digit charactor"
- " in offset.");
- break;
- case '@': /* File name */
- if (pp->file)
- semantic_error("SRC@SRC is not allowed.");
- pp->file = strdup(arg);
- DIE_IF(pp->file == NULL);
- if (ptr)
- semantic_error("@SRC must be the last "
- "option.");
- break;
- case '%': /* Probe places */
- if (strcmp(arg, "return") == 0) {
- pp->retprobe = 1;
- } else /* Others not supported yet */
- semantic_error("%%%s is not supported.", arg);
- break;
- default:
- DIE_IF("Program has a bug.");
- break;
- }
- }
-
- /* Exclusion check */
- if (pp->line && pp->offset)
- semantic_error("Offset can't be used with line number.");
- if (!pp->line && pp->file && !pp->function)
- semantic_error("File always requires line number.");
- if (pp->offset && !pp->function)
- semantic_error("Offset requires an entry function.");
- if (pp->retprobe && !pp->function)
- semantic_error("Return probe requires an entry function.");
- if ((pp->offset || pp->line) && pp->retprobe)
- semantic_error("Offset/Line can't be used with return probe.");
-
- pr_debug("symbol:%s file:%s line:%d offset:%d, return:%d\n",
- pp->function, pp->file, pp->line, pp->offset, pp->retprobe);
-}
-
/* Parse an event definition. Note that any error must die. */
static void parse_probe_event(const char *str)
{
- char *argv[MAX_PROBE_ARGS + 1]; /* probe + args */
- int argc, i;
struct probe_point *pp = &session.probes[session.nr_probe];

pr_debug("probe-definition(%d): %s\n", session.nr_probe, str);
if (++session.nr_probe == MAX_PROBES)
- semantic_error("Too many probes");
-
- /* Separate arguments, similar to argv_split */
- argc = 0;
- do {
- /* Skip separators */
- while (isspace(*str))
- str++;
-
- /* Add an argument */
- if (*str != '\0') {
- const char *s = str;
- /* Check the limit number of arguments */
- if (argc == MAX_PROBE_ARGS + 1)
- semantic_error("Too many arguments");
-
- /* Skip the argument */
- while (!isspace(*str) && *str != '\0')
- str++;
-
- /* Duplicate the argument */
- argv[argc] = strndup(s, str - s);
- if (argv[argc] == NULL)
- die("strndup");
- pr_debug("argv[%d]=%s\n", argc, argv[argc]);
- argc++;
-
- }
- } while (*str != '\0');
- if (!argc)
- semantic_error("An empty argument.");
-
- /* Parse probe point */
- parse_probe_point(argv[0], pp);
- free(argv[0]);
- if (pp->file || pp->line)
- session.need_dwarf = 1;
-
- /* Copy arguments */
- pp->nr_args = argc - 1;
- if (pp->nr_args > 0) {
- pp->args = (char **)malloc(sizeof(char *) * pp->nr_args);
- if (!pp->args)
- die("malloc");
- memcpy(pp->args, &argv[1], sizeof(char *) * pp->nr_args);
- }
+ die("Too many probes (> %d) are specified.", MAX_PROBES);

- /* Ensure return probe has no C argument */
- for (i = 0; i < pp->nr_args; i++)
- if (is_c_varname(pp->args[i])) {
- if (pp->retprobe)
- semantic_error("You can't specify local"
- " variable for kretprobe");
- session.need_dwarf = 1;
- }
+ /* Parse perf-probe event into probe_point */
+ session.need_dwarf = parse_perf_probe_event(str, pp);

pr_debug("%d arguments\n", pp->nr_args);
}
@@ -288,59 +149,15 @@ static const struct option options[] = {
"\t\tALN:\tAbsolute line number in file.\n"
"\t\tARG:\tProbe argument (local variable name or\n"
#endif
- "\t\t\tkprobe-tracer argument format is supported.)\n",
+ "\t\t\tkprobe-tracer argument format.)\n",
opt_add_probe_event),
OPT_END()
};

-static int write_new_event(int fd, const char *buf)
-{
- int ret;
-
- ret = write(fd, buf, strlen(buf));
- if (ret <= 0)
- die("Failed to create event.");
- else
- printf("Added new event: %s\n", buf);
-
- return ret;
-}
-
-#define MAX_CMDLEN 256
-
-static int synthesize_probe_event(struct probe_point *pp)
-{
- char *buf;
- int i, len, ret;
- pp->probes[0] = buf = zalloc(MAX_CMDLEN);
- if (!buf)
- die("Failed to allocate memory by zalloc.");
- ret = snprintf(buf, MAX_CMDLEN, "%s+%d", pp->function, pp->offset);
- if (ret <= 0 || ret >= MAX_CMDLEN)
- goto error;
- len = ret;
-
- for (i = 0; i < pp->nr_args; i++) {
- ret = snprintf(&buf[len], MAX_CMDLEN - len, " %s",
- pp->args[i]);
- if (ret <= 0 || ret >= MAX_CMDLEN - len)
- goto error;
- len += ret;
- }
- pp->found = 1;
- return pp->found;
-error:
- free(pp->probes[0]);
- if (ret > 0)
- ret = -E2BIG;
- return ret;
-}
-
int cmd_probe(int argc, const char **argv, const char *prefix __used)
{
int i, j, fd, ret;
struct probe_point *pp;
- char buf[MAX_CMDLEN];

argc = parse_options(argc, argv, options, probe_usage,
PARSE_OPT_STOP_AT_NON_OPTION);
@@ -352,7 +169,7 @@ int cmd_probe(int argc, const char **argv, const char *prefix __used)

if (session.need_dwarf)
#ifdef NO_LIBDWARF
- semantic_error("Debuginfo-analysis is not supported");
+ die("Debuginfo-analysis is not supported");
#else /* !NO_LIBDWARF */
pr_debug("Some probes require debuginfo.\n");

@@ -398,41 +215,15 @@ end_dwarf:
if (pp->found) /* This probe is already found. */
continue;

- ret = synthesize_probe_event(pp);
+ ret = synthesize_trace_kprobe_event(pp);
if (ret == -E2BIG)
- semantic_error("probe point is too long.");
+ die("probe point definition becomes too long.");
else if (ret < 0)
die("Failed to synthesize a probe point.");
}

/* Settng up probe points */
- snprintf(buf, MAX_CMDLEN, "%s/../kprobe_events", debugfs_path);
- fd = open(buf, O_WRONLY, O_APPEND);
- if (fd < 0) {
- if (errno == ENOENT)
- die("kprobe_events file does not exist - please rebuild with CONFIG_KPROBE_TRACER.");
- else
- die("Could not open kprobe_events file: %s",
- strerror(errno));
- }
- for (j = 0; j < session.nr_probe; j++) {
- pp = &session.probes[j];
- if (pp->found == 1) {
- snprintf(buf, MAX_CMDLEN, "%c:%s/%s_%x %s\n",
- pp->retprobe ? 'r' : 'p', PERFPROBE_GROUP,
- pp->function, pp->offset, pp->probes[0]);
- write_new_event(fd, buf);
- } else
- for (i = 0; i < pp->found; i++) {
- snprintf(buf, MAX_CMDLEN, "%c:%s/%s_%x_%d %s\n",
- pp->retprobe ? 'r' : 'p',
- PERFPROBE_GROUP,
- pp->function, pp->offset, i,
- pp->probes[i]);
- write_new_event(fd, buf);
- }
- }
- close(fd);
+ add_trace_kprobe_events(session.probes, session.nr_probe);
return 0;
}

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
new file mode 100644
index 0000000..7335a3b
--- /dev/null
+++ b/tools/perf/util/probe-event.c
@@ -0,0 +1,273 @@
+/*
+ * probe-event.c : perf-probe definition to kprobe_events format converter
+ *
+ * Written by Masami Hiramatsu <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ */
+
+#define _GNU_SOURCE
+#include <sys/utsname.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <string.h>
+
+#undef _GNU_SOURCE
+#include "event.h"
+#include "debug.h"
+#include "parse-events.h" /* For debugfs_path */
+#include "probe-event.h"
+
+#define MAX_CMDLEN 256
+#define MAX_PROBE_ARGS 128
+#define PERFPROBE_GROUP "probe"
+
+#define semantic_error(msg ...) die("Semantic error :" msg)
+
+/* Parse probepoint definition. */
+static void parse_perf_probe_probepoint(char *arg, struct probe_point *pp)
+{
+ char *ptr, *tmp;
+ char c, nc = 0;
+ /*
+ * <Syntax>
+ * perf probe SRC:LN
+ * perf probe FUNC[+OFFS|%return][@SRC]
+ */
+
+ ptr = strpbrk(arg, ":+@%");
+ if (ptr) {
+ nc = *ptr;
+ *ptr++ = '\0';
+ }
+
+ /* Check arg is function or file and copy it */
+ if (strchr(arg, '.')) /* File */
+ pp->file = strdup(arg);
+ else /* Function */
+ pp->function = strdup(arg);
+ DIE_IF(pp->file == NULL && pp->function == NULL);
+
+ /* Parse other options */
+ while (ptr) {
+ arg = ptr;
+ c = nc;
+ ptr = strpbrk(arg, ":+@%");
+ if (ptr) {
+ nc = *ptr;
+ *ptr++ = '\0';
+ }
+ switch (c) {
+ case ':': /* Line number */
+ pp->line = strtoul(arg, &tmp, 0);
+ if (*tmp != '\0')
+ semantic_error("There is non-digit charactor"
+ " in line number.");
+ break;
+ case '+': /* Byte offset from a symbol */
+ pp->offset = strtoul(arg, &tmp, 0);
+ if (*tmp != '\0')
+ semantic_error("There is non-digit charactor"
+ " in offset.");
+ break;
+ case '@': /* File name */
+ if (pp->file)
+ semantic_error("SRC@SRC is not allowed.");
+ pp->file = strdup(arg);
+ DIE_IF(pp->file == NULL);
+ if (ptr)
+ semantic_error("@SRC must be the last "
+ "option.");
+ break;
+ case '%': /* Probe places */
+ if (strcmp(arg, "return") == 0) {
+ pp->retprobe = 1;
+ } else /* Others not supported yet */
+ semantic_error("%%%s is not supported.", arg);
+ break;
+ default:
+ DIE_IF("Program has a bug.");
+ break;
+ }
+ }
+
+ /* Exclusion check */
+ if (pp->line && pp->offset)
+ semantic_error("Offset can't be used with line number.");
+
+ if (!pp->line && pp->file && !pp->function)
+ semantic_error("File always requires line number.");
+
+ if (pp->offset && !pp->function)
+ semantic_error("Offset requires an entry function.");
+
+ if (pp->retprobe && !pp->function)
+ semantic_error("Return probe requires an entry function.");
+
+ if ((pp->offset || pp->line) && pp->retprobe)
+ semantic_error("Offset/Line can't be used with return probe.");
+
+ pr_debug("symbol:%s file:%s line:%d offset:%d, return:%d\n",
+ pp->function, pp->file, pp->line, pp->offset, pp->retprobe);
+}
+
+/* Parse perf-probe event definition */
+int parse_perf_probe_event(const char *str, struct probe_point *pp)
+{
+ char *argv[MAX_PROBE_ARGS + 1]; /* probe + args */
+ int argc, i, need_dwarf = 0;
+
+ /* Separate arguments, similar to argv_split */
+ argc = 0;
+ do {
+ /* Skip separators */
+ while (isspace(*str))
+ str++;
+
+ /* Add an argument */
+ if (*str != '\0') {
+ const char *s = str;
+ /* Check the limit number of arguments */
+ if (argc == MAX_PROBE_ARGS + 1)
+ semantic_error("Too many arguments");
+
+ /* Skip the argument */
+ while (!isspace(*str) && *str != '\0')
+ str++;
+
+ /* Duplicate the argument */
+ argv[argc] = strndup(s, str - s);
+ if (argv[argc] == NULL)
+ die("strndup");
+ pr_debug("argv[%d]=%s\n", argc, argv[argc]);
+ argc++;
+ }
+ } while (*str != '\0');
+ if (!argc)
+ semantic_error("An empty argument.");
+
+ /* Parse probe point */
+ parse_perf_probe_probepoint(argv[0], pp);
+ free(argv[0]);
+ if (pp->file || pp->line)
+ need_dwarf = 1;
+
+ /* Copy arguments */
+ pp->nr_args = argc - 1;
+ if (pp->nr_args > 0) {
+ pp->args = (char **)malloc(sizeof(char *) * pp->nr_args);
+ if (!pp->args)
+ die("malloc");
+ memcpy(pp->args, &argv[1], sizeof(char *) * pp->nr_args);
+ }
+
+ /* Ensure return probe has no C argument */
+ for (i = 0; i < pp->nr_args; i++)
+ if (is_c_varname(pp->args[i])) {
+ if (pp->retprobe)
+ semantic_error("You can't specify local"
+ " variable for kretprobe");
+ need_dwarf = 1;
+ }
+
+ return need_dwarf;
+}
+
+int synthesize_trace_kprobe_event(struct probe_point *pp)
+{
+ char *buf;
+ int i, len, ret;
+
+ pp->probes[0] = buf = zalloc(MAX_CMDLEN);
+ if (!buf)
+ die("Failed to allocate memory by zalloc.");
+ ret = snprintf(buf, MAX_CMDLEN, "%s+%d", pp->function, pp->offset);
+ if (ret <= 0 || ret >= MAX_CMDLEN)
+ goto error;
+ len = ret;
+
+ for (i = 0; i < pp->nr_args; i++) {
+ ret = snprintf(&buf[len], MAX_CMDLEN - len, " %s",
+ pp->args[i]);
+ if (ret <= 0 || ret >= MAX_CMDLEN - len)
+ goto error;
+ len += ret;
+ }
+ pp->found = 1;
+
+ return pp->found;
+error:
+ free(pp->probes[0]);
+ if (ret > 0)
+ ret = -E2BIG;
+
+ return ret;
+}
+
+static int write_trace_kprobe_event(int fd, const char *buf)
+{
+ int ret;
+
+ ret = write(fd, buf, strlen(buf));
+ if (ret <= 0)
+ die("Failed to create event.");
+ else
+ printf("Added new event: %s\n", buf);
+
+ return ret;
+}
+
+void add_trace_kprobe_events(struct probe_point *probes, int nr_probes)
+{
+ int i, j, fd;
+ struct probe_point *pp;
+ char buf[MAX_CMDLEN];
+
+ snprintf(buf, MAX_CMDLEN, "%s/../kprobe_events", debugfs_path);
+ fd = open(buf, O_WRONLY, O_APPEND);
+ if (fd < 0) {
+ if (errno == ENOENT)
+ die("kprobe_events file does not exist -"
+ " please rebuild with CONFIG_KPROBE_TRACER.");
+ else
+ die("Could not open kprobe_events file: %s",
+ strerror(errno));
+ }
+
+ for (j = 0; j < nr_probes; j++) {
+ pp = probes + j;
+ if (pp->found == 1) {
+ snprintf(buf, MAX_CMDLEN, "%c:%s/%s_%x %s\n",
+ pp->retprobe ? 'r' : 'p', PERFPROBE_GROUP,
+ pp->function, pp->offset, pp->probes[0]);
+ write_trace_kprobe_event(fd, buf);
+ } else
+ for (i = 0; i < pp->found; i++) {
+ snprintf(buf, MAX_CMDLEN, "%c:%s/%s_%x_%d %s\n",
+ pp->retprobe ? 'r' : 'p',
+ PERFPROBE_GROUP,
+ pp->function, pp->offset, i,
+ pp->probes[i]);
+ write_trace_kprobe_event(fd, buf);
+ }
+ }
+ close(fd);
+}
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
new file mode 100644
index 0000000..0089c45
--- /dev/null
+++ b/tools/perf/util/probe-event.h
@@ -0,0 +1,10 @@
+#ifndef _PROBE_EVENT_H
+#define _PROBE_EVENT_H
+
+#include "probe-finder.h"
+
+extern int parse_perf_probe_event(const char *str, struct probe_point *pp);
+extern int synthesize_trace_kprobe_event(struct probe_point *pp);
+extern void add_trace_kprobe_events(struct probe_point *probes, int nr_probes);
+
+#endif /*_PROBE_EVENT_H */


--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: [email protected]

2009-12-01 00:20:49

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH -tip 7/9] perf probe: Add argv_split() from lib/argv_split.c

Add argv_split() ported from lib/argv_split.c to string.c and use it
in util/probe-event.c.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: K.Prasad <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
---

tools/perf/util/probe-event.c | 55 ++++++----------------
tools/perf/util/string.c | 101 +++++++++++++++++++++++++++++++++++++++++
tools/perf/util/string.h | 2 +
3 files changed, 118 insertions(+), 40 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 7335a3b..e3a683a 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -32,6 +32,7 @@

#undef _GNU_SOURCE
#include "event.h"
+#include "string.h"
#include "debug.h"
#include "parse-events.h" /* For debugfs_path */
#include "probe-event.h"
@@ -132,62 +133,36 @@ static void parse_perf_probe_probepoint(char *arg, struct probe_point *pp)
/* Parse perf-probe event definition */
int parse_perf_probe_event(const char *str, struct probe_point *pp)
{
- char *argv[MAX_PROBE_ARGS + 1]; /* probe + args */
+ char **argv;
int argc, i, need_dwarf = 0;

- /* Separate arguments, similar to argv_split */
- argc = 0;
- do {
- /* Skip separators */
- while (isspace(*str))
- str++;
-
- /* Add an argument */
- if (*str != '\0') {
- const char *s = str;
- /* Check the limit number of arguments */
- if (argc == MAX_PROBE_ARGS + 1)
- semantic_error("Too many arguments");
-
- /* Skip the argument */
- while (!isspace(*str) && *str != '\0')
- str++;
-
- /* Duplicate the argument */
- argv[argc] = strndup(s, str - s);
- if (argv[argc] == NULL)
- die("strndup");
- pr_debug("argv[%d]=%s\n", argc, argv[argc]);
- argc++;
- }
- } while (*str != '\0');
- if (!argc)
- semantic_error("An empty argument.");
+ argv = argv_split(str, &argc);
+ if (!argv)
+ die("argv_split failed.");
+ if (argc > MAX_PROBE_ARGS + 1)
+ semantic_error("Too many arguments");

/* Parse probe point */
parse_perf_probe_probepoint(argv[0], pp);
- free(argv[0]);
if (pp->file || pp->line)
need_dwarf = 1;

- /* Copy arguments */
+ /* Copy arguments and ensure return probe has no C argument */
pp->nr_args = argc - 1;
- if (pp->nr_args > 0) {
- pp->args = (char **)malloc(sizeof(char *) * pp->nr_args);
- if (!pp->args)
- die("malloc");
- memcpy(pp->args, &argv[1], sizeof(char *) * pp->nr_args);
- }
-
- /* Ensure return probe has no C argument */
- for (i = 0; i < pp->nr_args; i++)
+ pp->args = zalloc(sizeof(char *) * pp->nr_args);
+ for (i = 0; i < pp->nr_args; i++) {
+ pp->args[i] = strdup(argv[i + 1]);
+ if (!pp->args[i])
+ die("Failed to copy argument.");
if (is_c_varname(pp->args[i])) {
if (pp->retprobe)
semantic_error("You can't specify local"
" variable for kretprobe");
need_dwarf = 1;
}
+ }

+ argv_free(argv);
return need_dwarf;
}

diff --git a/tools/perf/util/string.c b/tools/perf/util/string.c
index 2270435..0977cf4 100644
--- a/tools/perf/util/string.c
+++ b/tools/perf/util/string.c
@@ -127,3 +127,104 @@ out_err:
out:
return length;
}
+
+/*
+ * Helper function for splitting a string into an argv-like array.
+ * originaly copied from lib/argv_split.c
+ */
+static const char *skip_sep(const char *cp)
+{
+ while (*cp && isspace(*cp))
+ cp++;
+
+ return cp;
+}
+
+static const char *skip_arg(const char *cp)
+{
+ while (*cp && !isspace(*cp))
+ cp++;
+
+ return cp;
+}
+
+static int count_argc(const char *str)
+{
+ int count = 0;
+
+ while (*str) {
+ str = skip_sep(str);
+ if (*str) {
+ count++;
+ str = skip_arg(str);
+ }
+ }
+
+ return count;
+}
+
+/**
+ * argv_free - free an argv
+ * @argv - the argument vector to be freed
+ *
+ * Frees an argv and the strings it points to.
+ */
+void argv_free(char **argv)
+{
+ char **p;
+ for (p = argv; *p; p++)
+ free(*p);
+
+ free(argv);
+}
+
+/**
+ * argv_split - split a string at whitespace, returning an argv
+ * @str: the string to be split
+ * @argcp: returned argument count
+ *
+ * Returns an array of pointers to strings which are split out from
+ * @str. This is performed by strictly splitting on white-space; no
+ * quote processing is performed. Multiple whitespace characters are
+ * considered to be a single argument separator. The returned array
+ * is always NULL-terminated. Returns NULL on memory allocation
+ * failure.
+ */
+char **argv_split(const char *str, int *argcp)
+{
+ int argc = count_argc(str);
+ char **argv = zalloc(sizeof(*argv) * (argc+1));
+ char **argvp;
+
+ if (argv == NULL)
+ goto out;
+
+ if (argcp)
+ *argcp = argc;
+
+ argvp = argv;
+
+ while (*str) {
+ str = skip_sep(str);
+
+ if (*str) {
+ const char *p = str;
+ char *t;
+
+ str = skip_arg(str);
+
+ t = strndup(p, str-p);
+ if (t == NULL)
+ goto fail;
+ *argvp++ = t;
+ }
+ }
+ *argvp = NULL;
+
+out:
+ return argv;
+
+fail:
+ argv_free(argv);
+ return NULL;
+}
diff --git a/tools/perf/util/string.h b/tools/perf/util/string.h
index e50b07f..bfecec2 100644
--- a/tools/perf/util/string.h
+++ b/tools/perf/util/string.h
@@ -6,6 +6,8 @@
int hex2u64(const char *ptr, u64 *val);
char *strxfrchar(char *s, char from, char to);
s64 perf_atoll(const char *str);
+char **argv_split(const char *str, int *argcp);
+void argv_free(char **argv);

#define _STR(x) #x
#define STR(x) _STR(x)


--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: [email protected]

2009-12-01 00:24:19

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH -tip 8/9] perf probe: Add --list option for listing current probe events

Add --list option for listing currently defined probe events
in the kernel. This shows events in below format;

[group:event] <perf-probe probe-definition>

e.g.
[probe:schedule_0] schedule+30 cpu

Note that source file/line information is not supported yet.
So even if you added a probe by line, it will be shown in
<symbol+offset>.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: K.Prasad <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
---

tools/perf/builtin-probe.c | 12 ++
tools/perf/util/probe-event.c | 231 ++++++++++++++++++++++++++++++++++++++---
tools/perf/util/probe-event.h | 5 +
3 files changed, 230 insertions(+), 18 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index bf20df2..b5d15cf 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -62,6 +62,8 @@ static struct {
struct probe_point probes[MAX_PROBES];
} session;

+static bool listing;
+
/* Parse an event definition. Note that any error must die. */
static void parse_probe_event(const char *str)
{
@@ -119,6 +121,7 @@ static int open_default_vmlinux(void)
static const char * const probe_usage[] = {
"perf probe [<options>] 'PROBEDEF' ['PROBEDEF' ...]",
"perf probe [<options>] --add 'PROBEDEF' [--add 'PROBEDEF' ...]",
+ "perf probe --list",
NULL
};

@@ -129,6 +132,7 @@ static const struct option options[] = {
OPT_STRING('k', "vmlinux", &session.vmlinux, "file",
"vmlinux/module pathname"),
#endif
+ OPT_BOOLEAN('l', "list", &listing, "list up current probes"),
OPT_CALLBACK('a', "add", NULL,
#ifdef NO_LIBDWARF
"FUNC[+OFFS|%return] [ARG ...]",
@@ -164,9 +168,15 @@ int cmd_probe(int argc, const char **argv, const char *prefix __used)
for (i = 0; i < argc; i++)
parse_probe_event(argv[i]);

- if (session.nr_probe == 0)
+ if ((session.nr_probe == 0 && !listing) ||
+ (session.nr_probe != 0 && listing))
usage_with_options(probe_usage, options);

+ if (listing) {
+ show_perf_probe_events();
+ return 0;
+ }
+
if (session.need_dwarf)
#ifdef NO_LIBDWARF
die("Debuginfo-analysis is not supported");
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index e3a683a..7f4f288 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -29,10 +29,13 @@
#include <unistd.h>
#include <stdlib.h>
#include <string.h>
+#include <stdarg.h>
+#include <limits.h>

#undef _GNU_SOURCE
#include "event.h"
#include "string.h"
+#include "strlist.h"
#include "debug.h"
#include "parse-events.h" /* For debugfs_path */
#include "probe-event.h"
@@ -43,6 +46,19 @@

#define semantic_error(msg ...) die("Semantic error :" msg)

+/* If there is no space to write, returns -E2BIG. */
+static int e_snprintf(char *str, size_t size, const char *format, ...)
+{
+ int ret;
+ va_list ap;
+ va_start(ap, format);
+ ret = vsnprintf(str, size, format, ap);
+ va_end(ap);
+ if (ret >= (int)size)
+ ret = -E2BIG;
+ return ret;
+}
+
/* Parse probepoint definition. */
static void parse_perf_probe_probepoint(char *arg, struct probe_point *pp)
{
@@ -166,6 +182,103 @@ int parse_perf_probe_event(const char *str, struct probe_point *pp)
return need_dwarf;
}

+/* Parse kprobe_events event into struct probe_point */
+void parse_trace_kprobe_event(const char *str, char **group, char **event,
+ struct probe_point *pp)
+{
+ char pr;
+ char *p;
+ int ret, i, argc;
+ char **argv;
+
+ pr_debug("Parsing kprobe_events: %s\n", str);
+ argv = argv_split(str, &argc);
+ if (!argv)
+ die("argv_split failed.");
+ if (argc < 2)
+ semantic_error("Too less arguments.");
+
+ /* Scan event and group name. */
+ ret = sscanf(argv[0], "%c:%m[^/ \t]/%m[^ \t]",
+ &pr, group, event);
+ if (ret != 3)
+ semantic_error("Failed to parse event name: %s", argv[0]);
+ pr_debug("Group:%s Event:%s probe:%c\n", *group, *event, pr);
+
+ if (!pp)
+ goto end;
+
+ pp->retprobe = (pr == 'r');
+
+ /* Scan function name and offset */
+ ret = sscanf(argv[1], "%m[^+]+%d", &pp->function, &pp->offset);
+ if (ret == 1)
+ pp->offset = 0;
+
+ /* kprobe_events doesn't have this information */
+ pp->line = 0;
+ pp->file = NULL;
+
+ pp->nr_args = argc - 2;
+ pp->args = zalloc(sizeof(char *) * pp->nr_args);
+ for (i = 0; i < pp->nr_args; i++) {
+ p = strchr(argv[i + 2], '=');
+ if (p) /* We don't need which register is assigned. */
+ *p = '\0';
+ pp->args[i] = strdup(argv[i + 2]);
+ if (!pp->args[i])
+ die("Failed to copy argument.");
+ }
+
+end:
+ argv_free(argv);
+}
+
+int synthesize_perf_probe_event(struct probe_point *pp)
+{
+ char *buf;
+ char offs[64] = "", line[64] = "";
+ int i, len, ret;
+
+ pp->probes[0] = buf = zalloc(MAX_CMDLEN);
+ if (!buf)
+ die("Failed to allocate memory by zalloc.");
+ if (pp->offset) {
+ ret = e_snprintf(offs, 64, "+%d", pp->offset);
+ if (ret <= 0)
+ goto error;
+ }
+ if (pp->line) {
+ ret = e_snprintf(line, 64, ":%d", pp->line);
+ if (ret <= 0)
+ goto error;
+ }
+
+ if (pp->function)
+ ret = e_snprintf(buf, MAX_CMDLEN, "%s%s%s%s", pp->function,
+ offs, pp->retprobe ? "%return" : "", line);
+ else
+ ret = e_snprintf(buf, MAX_CMDLEN, "%s%s%s%s", pp->file, line);
+ if (ret <= 0)
+ goto error;
+ len = ret;
+
+ for (i = 0; i < pp->nr_args; i++) {
+ ret = e_snprintf(&buf[len], MAX_CMDLEN - len, " %s",
+ pp->args[i]);
+ if (ret <= 0)
+ goto error;
+ len += ret;
+ }
+ pp->found = 1;
+
+ return pp->found;
+error:
+ free(pp->probes[0]);
+
+ return ret;
+}
+
int synthesize_trace_kprobe_event(struct probe_point *pp)
{
char *buf;
@@ -174,15 +287,15 @@ int synthesize_trace_kprobe_event(struct probe_point *pp)
pp->probes[0] = buf = zalloc(MAX_CMDLEN);
if (!buf)
die("Failed to allocate memory by zalloc.");
- ret = snprintf(buf, MAX_CMDLEN, "%s+%d", pp->function, pp->offset);
- if (ret <= 0 || ret >= MAX_CMDLEN)
+ ret = e_snprintf(buf, MAX_CMDLEN, "%s+%d", pp->function, pp->offset);
+ if (ret <= 0)
goto error;
len = ret;

for (i = 0; i < pp->nr_args; i++) {
- ret = snprintf(&buf[len], MAX_CMDLEN - len, " %s",
- pp->args[i]);
- if (ret <= 0 || ret >= MAX_CMDLEN - len)
+ ret = e_snprintf(&buf[len], MAX_CMDLEN - len, " %s",
+ pp->args[i]);
+ if (ret <= 0)
goto error;
len += ret;
}
@@ -191,12 +304,105 @@ int synthesize_trace_kprobe_event(struct probe_point *pp)
return pp->found;
error:
free(pp->probes[0]);
- if (ret > 0)
- ret = -E2BIG;

return ret;
}

+static int open_kprobe_events(int flags, int mode)
+{
+ char buf[PATH_MAX];
+ int ret;
+
+ ret = e_snprintf(buf, PATH_MAX, "%s/../kprobe_events", debugfs_path);
+ if (ret < 0)
+ die("Failed to make kprobe_events path.");
+
+ ret = open(buf, flags, mode);
+ if (ret < 0) {
+ if (errno == ENOENT)
+ die("kprobe_events file does not exist -"
+ " please rebuild with CONFIG_KPROBE_TRACER.");
+ else
+ die("Could not open kprobe_events file: %s",
+ strerror(errno));
+ }
+ return ret;
+}
+
+/* Get raw string list of current kprobe_events */
+static struct strlist *get_trace_kprobe_event_rawlist(int fd)
+{
+ int ret, idx;
+ FILE *fp;
+ char buf[MAX_CMDLEN];
+ char *p;
+ struct strlist *sl;
+
+ sl = strlist__new(true, NULL);
+
+ fp = fdopen(dup(fd), "r");
+ while (!feof(fp)) {
+ p = fgets(buf, MAX_CMDLEN, fp);
+ if (!p)
+ break;
+
+ idx = strlen(p) - 1;
+ if (p[idx] == '\n')
+ p[idx] = '\0';
+ ret = strlist__add(sl, buf);
+ if (ret < 0)
+ die("strlist__add failed: %s", strerror(-ret));
+ }
+ fclose(fp);
+
+ return sl;
+}
+
+/* Free and zero clear probe_point */
+static void clear_probe_point(struct probe_point *pp)
+{
+ int i;
+
+ if (pp->function)
+ free(pp->function);
+ if (pp->file)
+ free(pp->file);
+ for (i = 0; i < pp->nr_args; i++)
+ free(pp->args[i]);
+ if (pp->args)
+ free(pp->args);
+ for (i = 0; i < pp->found; i++)
+ free(pp->probes[i]);
+ memset(pp, 0, sizeof(pp));
+}
+
+/* List up current perf-probe events */
+void show_perf_probe_events(void)
+{
+ unsigned int i;
+ int fd;
+ char *group, *event;
+ struct probe_point pp;
+ struct strlist *rawlist;
+ struct str_node *ent;
+
+ fd = open_kprobe_events(O_RDONLY, 0);
+ rawlist = get_trace_kprobe_event_rawlist(fd);
+ close(fd);
+
+ for (i = 0; i < strlist__nr_entries(rawlist); i++) {
+ ent = strlist__entry(rawlist, i);
+ parse_trace_kprobe_event(ent->s, &group, &event, &pp);
+ synthesize_perf_probe_event(&pp);
+ printf("[%s:%s]\t%s\n", group, event, pp.probes[0]);
+ free(group);
+ free(event);
+ clear_probe_point(&pp);
+ }
+
+ strlist__delete(rawlist);
+}
+
static int write_trace_kprobe_event(int fd, const char *buf)
{
int ret;
@@ -216,16 +422,7 @@ void add_trace_kprobe_events(struct probe_point *probes, int nr_probes)
struct probe_point *pp;
char buf[MAX_CMDLEN];

- snprintf(buf, MAX_CMDLEN, "%s/../kprobe_events", debugfs_path);
- fd = open(buf, O_WRONLY, O_APPEND);
- if (fd < 0) {
- if (errno == ENOENT)
- die("kprobe_events file does not exist -"
- " please rebuild with CONFIG_KPROBE_TRACER.");
- else
- die("Could not open kprobe_events file: %s",
- strerror(errno));
- }
+ fd = open_kprobe_events(O_WRONLY, O_APPEND);

for (j = 0; j < nr_probes; j++) {
pp = probes + j;
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index 0089c45..88db7d1 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -2,9 +2,14 @@
#define _PROBE_EVENT_H

#include "probe-finder.h"
+#include "strlist.h"

extern int parse_perf_probe_event(const char *str, struct probe_point *pp);
+extern int synthesize_perf_probe_event(struct probe_point *pp);
+extern void parse_trace_kprobe_event(const char *str, char **group,
+ char **event, struct probe_point *pp);
extern int synthesize_trace_kprobe_event(struct probe_point *pp);
extern void add_trace_kprobe_events(struct probe_point *probes, int nr_probes);
+extern void show_perf_probe_events(void);

#endif /*_PROBE_EVENT_H */


--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: [email protected]

2009-12-01 00:21:01

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH -tip 9/9] perf probe: Simplify event naming

Simplify event naming as <symbol>_<seqnum>. Each event name is
globally unique (group name is not checked). So, if there is
schedule_0, next probe event on schedule() will be schedule_1.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: K.Prasad <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
---

tools/perf/util/probe-event.c | 67 ++++++++++++++++++++++++++++++++---------
tools/perf/util/probe-event.h | 3 ++
2 files changed, 56 insertions(+), 14 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 7f4f288..e42f3ac 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -403,6 +403,29 @@ void show_perf_probe_events(void)
strlist__delete(rawlist);
}

+/* Get current perf-probe event names */
+static struct strlist *get_perf_event_names(int fd)
+{
+ unsigned int i;
+ char *group, *event;
+ struct strlist *sl, *rawlist;
+ struct str_node *ent;
+
+ rawlist = get_trace_kprobe_event_rawlist(fd);
+
+ sl = strlist__new(false, NULL);
+ for (i = 0; i < strlist__nr_entries(rawlist); i++) {
+ ent = strlist__entry(rawlist, i);
+ parse_trace_kprobe_event(ent->s, &group, &event, NULL);
+ strlist__add(sl, event);
+ free(group);
+ }
+
+ strlist__delete(rawlist);
+
+ return sl;
+}
+
static int write_trace_kprobe_event(int fd, const char *buf)
{
int ret;
@@ -416,30 +439,46 @@ static int write_trace_kprobe_event(int fd, const char *buf)
return ret;
}

+static void get_new_event_name(char *buf, size_t len, const char *base,
+ struct strlist *namelist)
+{
+ int i, ret;
+ for (i = 0; i < MAX_EVENT_INDEX; i++) {
+ ret = e_snprintf(buf, len, "%s_%d", base, i);
+ if (ret < 0)
+ die("snprintf() failed: %s", strerror(-ret));
+ if (!strlist__has_entry(namelist, buf))
+ break;
+ }
+ if (i == MAX_EVENT_INDEX)
+ die("Too many events are on the same function.");
+}
+
void add_trace_kprobe_events(struct probe_point *probes, int nr_probes)
{
int i, j, fd;
struct probe_point *pp;
char buf[MAX_CMDLEN];
+ char event[64];
+ struct strlist *namelist;

- fd = open_kprobe_events(O_WRONLY, O_APPEND);
+ fd = open_kprobe_events(O_RDWR, O_APPEND);
+ /* Get current event names */
+ namelist = get_perf_event_names(fd);

for (j = 0; j < nr_probes; j++) {
pp = probes + j;
- if (pp->found == 1) {
- snprintf(buf, MAX_CMDLEN, "%c:%s/%s_%x %s\n",
- pp->retprobe ? 'r' : 'p', PERFPROBE_GROUP,
- pp->function, pp->offset, pp->probes[0]);
+ for (i = 0; i < pp->found; i++) {
+ /* Get an unused new event name */
+ get_new_event_name(event, 64, pp->function, namelist);
+ snprintf(buf, MAX_CMDLEN, "%c:%s/%s %s\n",
+ pp->retprobe ? 'r' : 'p',
+ PERFPROBE_GROUP, event,
+ pp->probes[i]);
write_trace_kprobe_event(fd, buf);
- } else
- for (i = 0; i < pp->found; i++) {
- snprintf(buf, MAX_CMDLEN, "%c:%s/%s_%x_%d %s\n",
- pp->retprobe ? 'r' : 'p',
- PERFPROBE_GROUP,
- pp->function, pp->offset, i,
- pp->probes[i]);
- write_trace_kprobe_event(fd, buf);
- }
+ /* Add added event name to namelist */
+ strlist__add(namelist, event);
+ }
}
close(fd);
}
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index 88db7d1..0c6fe56 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -12,4 +12,7 @@ extern int synthesize_trace_kprobe_event(struct probe_point *pp);
extern void add_trace_kprobe_events(struct probe_point *probes, int nr_probes);
extern void show_perf_probe_events(void);

+/* Maximum index number of event-name postfix */
+#define MAX_EVENT_INDEX 1024
+
#endif /*_PROBE_EVENT_H */


--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: [email protected]

2009-12-01 07:30:03

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH -tip 0/9] perf-probe updates


* Masami Hiramatsu <[email protected]> wrote:

> Hi,
>
> Here are bugfixes and updates for perf-probe and kprobe-tracer. I've
> fixed some minor bugs and added --list option and simple probe naming.

Applied, thanks Masami!

> TODO:
> - Support build-id checking.
> - Support --del option to remove probes.
> - Support --line option to show which lines user can probe.
> - Support lazy string matching.

ok, cool!

One other small detail i noticed wrt. probe naming. Right now if we
insert a single probe into a function it gets named schedule_0:

# perf probe schedule
Could not open vmlinux/module file. Try to use symbols.
Added new event: p:probe/schedule_0 schedule+0

the next one gets named schedule_1, schedule_2, etc.

It would be nice to special-case the first one and name it 'schedule'.
Most of the time people insert a single probe into a function, so the _0
postfix is extra and in most cases unnecessary typing for them.

Another small detail is that i dont think we should emit this line:

Could not open vmlinux/module file. Try to use symbols.

when we can create a probe successfully - it's just unnecessary noise,
the user does not care how we pulled it off, as long as we were able to
get a reliable symbol address and the insertion worked fine.

A third detail is this line:

Added new event: p:probe/schedule_0 schedule+0

If that is pasted to perf stat directly it wont work because the syntax
is probe:schedule_0. So i'd suggest to print something like:

Added new event: probe/schedule_0 (on schedule+0)

Perhaps even print another line:

You can now use it on all perf tools, such as:

perf probe -e probe/schedule_0 -a sleep 1
perf record -e probe/schedule_0 -a sleep 1

... to show people how to make use of it.

Thanks,

Ingo

2009-12-01 07:35:02

by Masami Hiramatsu

[permalink] [raw]
Subject: [tip:perf/core] trace_kprobes: Fix a memory leak bug and check kstrdup() return value

Commit-ID: ba8665d7dd95eb6093ee06f8f624b6acb1e73206
Gitweb: http://git.kernel.org/tip/ba8665d7dd95eb6093ee06f8f624b6acb1e73206
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Mon, 30 Nov 2009 19:19:20 -0500
Committer: Ingo Molnar <[email protected]>
CommitDate: Tue, 1 Dec 2009 08:19:59 +0100

trace_kprobes: Fix a memory leak bug and check kstrdup() return value

Fix a memory leak case in create_trace_probe(). When an argument
is too long (> MAX_ARGSTR_LEN), it just jumps to error path. In
that case tp->args[i].name is not released.
This also fixes a bug to check kstrdup()'s return value.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: systemtap <[email protected]>
Cc: DLE <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: K.Prasad <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
LKML-Reference: <20091201001919.10235.56455.stgit@harusame>
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/trace/trace_kprobe.c | 26 +++++++++++++++++++-------
1 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 72d0c65..aff5f80 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -483,7 +483,8 @@ static int parse_probe_vars(char *arg, struct fetch_func *ff, int is_return)
return ret;
}

-static int parse_probe_arg(char *arg, struct fetch_func *ff, int is_return)
+/* Recursive argument parser */
+static int __parse_probe_arg(char *arg, struct fetch_func *ff, int is_return)
{
int ret = 0;
unsigned long param;
@@ -543,7 +544,7 @@ static int parse_probe_arg(char *arg, struct fetch_func *ff, int is_return)
if (!id)
return -ENOMEM;
id->offset = offset;
- ret = parse_probe_arg(arg, &id->orig, is_return);
+ ret = __parse_probe_arg(arg, &id->orig, is_return);
if (ret)
kfree(id);
else {
@@ -560,6 +561,16 @@ static int parse_probe_arg(char *arg, struct fetch_func *ff, int is_return)
return ret;
}

+/* String length checking wrapper */
+static int parse_probe_arg(char *arg, struct fetch_func *ff, int is_return)
+{
+ if (strlen(arg) > MAX_ARGSTR_LEN) {
+ pr_info("Argument is too long.: %s\n", arg);
+ return -ENOSPC;
+ }
+ return __parse_probe_arg(arg, ff, is_return);
+}
+
/* Return 1 if name is reserved or already used by another argument */
static int conflict_field_name(const char *name,
struct probe_arg *args, int narg)
@@ -698,13 +709,14 @@ static int create_trace_probe(int argc, char **argv)
}

tp->args[i].name = kstrdup(argv[i], GFP_KERNEL);
-
- /* Parse fetch argument */
- if (strlen(arg) > MAX_ARGSTR_LEN) {
- pr_info("Argument%d(%s) is too long.\n", i, arg);
- ret = -ENOSPC;
+ if (!tp->args[i].name) {
+ pr_info("Failed to allocate argument%d name '%s'.\n",
+ i, argv[i]);
+ ret = -ENOMEM;
goto error;
}
+
+ /* Parse fetch argument */
ret = parse_probe_arg(arg, &tp->args[i].fetch, is_return);
if (ret) {
pr_info("Parse error at argument%d. (%d)\n", i, ret);

2009-12-01 07:33:42

by Masami Hiramatsu

[permalink] [raw]
Subject: [tip:perf/core] perf probe: Change a debugging message from pr_info to pr_debug

Commit-ID: f41b1e43c41e99c39a2222578a7806032c045c79
Gitweb: http://git.kernel.org/tip/f41b1e43c41e99c39a2222578a7806032c045c79
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Mon, 30 Nov 2009 19:19:27 -0500
Committer: Ingo Molnar <[email protected]>
CommitDate: Tue, 1 Dec 2009 08:20:00 +0100

perf probe: Change a debugging message from pr_info to pr_debug

Change annoying debug-info using notice from pr_info() to
pr_debug(), since the message always printed when user adds a
probe point which requires debug-info.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: systemtap <[email protected]>
Cc: DLE <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: K.Prasad <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
LKML-Reference: <20091201001927.10235.63645.stgit@harusame>
Signed-off-by: Ingo Molnar <[email protected]>
---
tools/perf/builtin-probe.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index a2f6daf..4e418af 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -351,7 +351,7 @@ int cmd_probe(int argc, const char **argv, const char *prefix __used)
#ifdef NO_LIBDWARF
semantic_error("Debuginfo-analysis is not supported");
#else /* !NO_LIBDWARF */
- pr_info("Some probes require debuginfo.\n");
+ pr_debug("Some probes require debuginfo.\n");

if (session.vmlinux)
fd = open(session.vmlinux, O_RDONLY);

2009-12-01 07:33:54

by Masami Hiramatsu

[permalink] [raw]
Subject: [tip:perf/core] perf probe: Add probe-finder.h without libdwarf

Commit-ID: 57d250df7deb3e1742fbf3cc3230119731109552
Gitweb: http://git.kernel.org/tip/57d250df7deb3e1742fbf3cc3230119731109552
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Mon, 30 Nov 2009 19:19:34 -0500
Committer: Ingo Molnar <[email protected]>
CommitDate: Tue, 1 Dec 2009 08:20:00 +0100

perf probe: Add probe-finder.h without libdwarf

Add probe-finder.h as LIB_H without libdwarf, because that
header is included even if no libdwarf.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: systemtap <[email protected]>
Cc: DLE <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: K.Prasad <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
LKML-Reference: <20091201001934.10235.44656.stgit@harusame>
Signed-off-by: Ingo Molnar <[email protected]>
---
tools/perf/Makefile | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index f1537a9..76e4b04 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -369,6 +369,7 @@ LIB_H += util/sort.h
LIB_H += util/hist.h
LIB_H += util/thread.h
LIB_H += util/data_map.h
+LIB_H += util/probe-finder.h

LIB_OBJS += util/abspath.o
LIB_OBJS += util/alias.o
@@ -485,7 +486,6 @@ ifneq ($(shell sh -c "(echo '\#include <libdwarf/dwarf.h>'; echo '\#include <lib
BASIC_CFLAGS += -DNO_LIBDWARF
else
EXTLIBS += -lelf -ldwarf
- LIB_H += util/probe-finder.h
LIB_OBJS += util/probe-finder.o
endif

2009-12-01 07:34:24

by Masami Hiramatsu

[permalink] [raw]
Subject: [tip:perf/core] perf probe: Fix argv array size in probe parser

Commit-ID: 74ca4c0ece52a2d19dae1bcbfc24fcfc5facfeb4
Gitweb: http://git.kernel.org/tip/74ca4c0ece52a2d19dae1bcbfc24fcfc5facfeb4
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Mon, 30 Nov 2009 19:19:43 -0500
Committer: Ingo Molnar <[email protected]>
CommitDate: Tue, 1 Dec 2009 08:20:01 +0100

perf probe: Fix argv array size in probe parser

Since the syntax has been changed, probe definition needs
parameters less than MAX_PROBE_ARGS + 1 (probe-point +
arguments).

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: systemtap <[email protected]>
Cc: DLE <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: K.Prasad <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
LKML-Reference: <20091201001943.10235.80367.stgit@harusame>
Signed-off-by: Ingo Molnar <[email protected]>
---
tools/perf/builtin-probe.c | 11 +++++++----
1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 4e418af..510fdd4 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -151,7 +151,7 @@ static void parse_probe_point(char *arg, struct probe_point *pp)
/* Parse an event definition. Note that any error must die. */
static void parse_probe_event(const char *str)
{
- char *argv[MAX_PROBE_ARGS + 2]; /* Event + probe + args */
+ char *argv[MAX_PROBE_ARGS + 1]; /* probe + args */
int argc, i;
struct probe_point *pp = &session.probes[session.nr_probe];

@@ -169,6 +169,9 @@ static void parse_probe_event(const char *str)
/* Add an argument */
if (*str != '\0') {
const char *s = str;
+ /* Check the limit number of arguments */
+ if (argc == MAX_PROBE_ARGS + 1)
+ semantic_error("Too many arguments");

/* Skip the argument */
while (!isspace(*str) && *str != '\0')
@@ -178,9 +181,9 @@ static void parse_probe_event(const char *str)
argv[argc] = strndup(s, str - s);
if (argv[argc] == NULL)
die("strndup");
- if (++argc == MAX_PROBE_ARGS)
- semantic_error("Too many arguments");
- pr_debug("argv[%d]=%s\n", argc, argv[argc - 1]);
+ pr_debug("argv[%d]=%s\n", argc, argv[argc]);
+ argc++;
+
}
} while (*str != '\0');
if (!argc)

2009-12-01 07:34:34

by Masami Hiramatsu

[permalink] [raw]
Subject: [tip:perf/core] perf probe: Fix probe array index for multiple probe points

Commit-ID: 934b1f5fd0c9a2ddde5a4487695c126243d9a42b
Gitweb: http://git.kernel.org/tip/934b1f5fd0c9a2ddde5a4487695c126243d9a42b
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Mon, 30 Nov 2009 19:19:51 -0500
Committer: Ingo Molnar <[email protected]>
CommitDate: Tue, 1 Dec 2009 08:20:01 +0100

perf probe: Fix probe array index for multiple probe points

Fix the index of formatted probe array for multiple probe
points, which should be probes[i] instead of probes[0].

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: systemtap <[email protected]>
Cc: DLE <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: K.Prasad <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
LKML-Reference: <20091201001950.10235.54781.stgit@harusame>
Signed-off-by: Ingo Molnar <[email protected]>
---
tools/perf/builtin-probe.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 510fdd4..5f47e62 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -428,7 +428,7 @@ end_dwarf:
pp->retprobe ? 'r' : 'p',
PERFPROBE_GROUP,
pp->function, pp->offset, i,
- pp->probes[0]);
+ pp->probes[i]);
write_new_event(fd, buf);
}
}

2009-12-01 07:33:49

by Masami Hiramatsu

[permalink] [raw]
Subject: [tip:perf/core] perf probe: Move probe event utility functions to probe-event.c

Commit-ID: 50656eec82684d03add0f4f4b4875a20bd8f9755
Gitweb: http://git.kernel.org/tip/50656eec82684d03add0f4f4b4875a20bd8f9755
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Mon, 30 Nov 2009 19:19:58 -0500
Committer: Ingo Molnar <[email protected]>
CommitDate: Tue, 1 Dec 2009 08:20:01 +0100

perf probe: Move probe event utility functions to probe-event.c

Split probe event (kprobe-events and perf probe events) utility
functions from builtin-probe.c to probe-event.c.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: systemtap <[email protected]>
Cc: DLE <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: K.Prasad <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
LKML-Reference: <20091201001958.10235.90243.stgit@harusame>
Signed-off-by: Ingo Molnar <[email protected]>
---
tools/perf/Makefile | 2 +
tools/perf/builtin-probe.c | 227 ++--------------------------------
tools/perf/util/probe-event.c | 273 +++++++++++++++++++++++++++++++++++++++++
tools/perf/util/probe-event.h | 10 ++
4 files changed, 294 insertions(+), 218 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 76e4b04..f8537cf 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -370,6 +370,7 @@ LIB_H += util/hist.h
LIB_H += util/thread.h
LIB_H += util/data_map.h
LIB_H += util/probe-finder.h
+LIB_H += util/probe-event.h

LIB_OBJS += util/abspath.o
LIB_OBJS += util/alias.o
@@ -412,6 +413,7 @@ LIB_OBJS += util/svghelper.o
LIB_OBJS += util/sort.o
LIB_OBJS += util/hist.o
LIB_OBJS += util/data_map.o
+LIB_OBJS += util/probe-event.o

BUILTIN_OBJS += builtin-annotate.o

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 5f47e62..bf20df2 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -40,6 +40,7 @@
#include "util/parse-options.h"
#include "util/parse-events.h" /* For debugfs_path */
#include "util/probe-finder.h"
+#include "util/probe-event.h"

/* Default vmlinux search paths */
#define NR_SEARCH_PATH 3
@@ -51,8 +52,6 @@ const char *default_search_path[NR_SEARCH_PATH] = {

#define MAX_PATH_LEN 256
#define MAX_PROBES 128
-#define MAX_PROBE_ARGS 128
-#define PERFPROBE_GROUP "probe"

/* Session management structure */
static struct {
@@ -63,155 +62,17 @@ static struct {
struct probe_point probes[MAX_PROBES];
} session;

-#define semantic_error(msg ...) die("Semantic error :" msg)
-
-/* Parse probe point. Return 1 if return probe */
-static void parse_probe_point(char *arg, struct probe_point *pp)
-{
- char *ptr, *tmp;
- char c, nc = 0;
- /*
- * <Syntax>
- * perf probe SRC:LN
- * perf probe FUNC[+OFFS|%return][@SRC]
- */
-
- ptr = strpbrk(arg, ":+@%");
- if (ptr) {
- nc = *ptr;
- *ptr++ = '\0';
- }
-
- /* Check arg is function or file and copy it */
- if (strchr(arg, '.')) /* File */
- pp->file = strdup(arg);
- else /* Function */
- pp->function = strdup(arg);
- DIE_IF(pp->file == NULL && pp->function == NULL);
-
- /* Parse other options */
- while (ptr) {
- arg = ptr;
- c = nc;
- ptr = strpbrk(arg, ":+@%");
- if (ptr) {
- nc = *ptr;
- *ptr++ = '\0';
- }
- switch (c) {
- case ':': /* Line number */
- pp->line = strtoul(arg, &tmp, 0);
- if (*tmp != '\0')
- semantic_error("There is non-digit charactor"
- " in line number.");
- break;
- case '+': /* Byte offset from a symbol */
- pp->offset = strtoul(arg, &tmp, 0);
- if (*tmp != '\0')
- semantic_error("There is non-digit charactor"
- " in offset.");
- break;
- case '@': /* File name */
- if (pp->file)
- semantic_error("SRC@SRC is not allowed.");
- pp->file = strdup(arg);
- DIE_IF(pp->file == NULL);
- if (ptr)
- semantic_error("@SRC must be the last "
- "option.");
- break;
- case '%': /* Probe places */
- if (strcmp(arg, "return") == 0) {
- pp->retprobe = 1;
- } else /* Others not supported yet */
- semantic_error("%%%s is not supported.", arg);
- break;
- default:
- DIE_IF("Program has a bug.");
- break;
- }
- }
-
- /* Exclusion check */
- if (pp->line && pp->offset)
- semantic_error("Offset can't be used with line number.");
- if (!pp->line && pp->file && !pp->function)
- semantic_error("File always requires line number.");
- if (pp->offset && !pp->function)
- semantic_error("Offset requires an entry function.");
- if (pp->retprobe && !pp->function)
- semantic_error("Return probe requires an entry function.");
- if ((pp->offset || pp->line) && pp->retprobe)
- semantic_error("Offset/Line can't be used with return probe.");
-
- pr_debug("symbol:%s file:%s line:%d offset:%d, return:%d\n",
- pp->function, pp->file, pp->line, pp->offset, pp->retprobe);
-}
-
/* Parse an event definition. Note that any error must die. */
static void parse_probe_event(const char *str)
{
- char *argv[MAX_PROBE_ARGS + 1]; /* probe + args */
- int argc, i;
struct probe_point *pp = &session.probes[session.nr_probe];

pr_debug("probe-definition(%d): %s\n", session.nr_probe, str);
if (++session.nr_probe == MAX_PROBES)
- semantic_error("Too many probes");
-
- /* Separate arguments, similar to argv_split */
- argc = 0;
- do {
- /* Skip separators */
- while (isspace(*str))
- str++;
-
- /* Add an argument */
- if (*str != '\0') {
- const char *s = str;
- /* Check the limit number of arguments */
- if (argc == MAX_PROBE_ARGS + 1)
- semantic_error("Too many arguments");
-
- /* Skip the argument */
- while (!isspace(*str) && *str != '\0')
- str++;
-
- /* Duplicate the argument */
- argv[argc] = strndup(s, str - s);
- if (argv[argc] == NULL)
- die("strndup");
- pr_debug("argv[%d]=%s\n", argc, argv[argc]);
- argc++;
-
- }
- } while (*str != '\0');
- if (!argc)
- semantic_error("An empty argument.");
-
- /* Parse probe point */
- parse_probe_point(argv[0], pp);
- free(argv[0]);
- if (pp->file || pp->line)
- session.need_dwarf = 1;
-
- /* Copy arguments */
- pp->nr_args = argc - 1;
- if (pp->nr_args > 0) {
- pp->args = (char **)malloc(sizeof(char *) * pp->nr_args);
- if (!pp->args)
- die("malloc");
- memcpy(pp->args, &argv[1], sizeof(char *) * pp->nr_args);
- }
+ die("Too many probes (> %d) are specified.", MAX_PROBES);

- /* Ensure return probe has no C argument */
- for (i = 0; i < pp->nr_args; i++)
- if (is_c_varname(pp->args[i])) {
- if (pp->retprobe)
- semantic_error("You can't specify local"
- " variable for kretprobe");
- session.need_dwarf = 1;
- }
+ /* Parse perf-probe event into probe_point */
+ session.need_dwarf = parse_perf_probe_event(str, pp);

pr_debug("%d arguments\n", pp->nr_args);
}
@@ -288,59 +149,15 @@ static const struct option options[] = {
"\t\tALN:\tAbsolute line number in file.\n"
"\t\tARG:\tProbe argument (local variable name or\n"
#endif
- "\t\t\tkprobe-tracer argument format is supported.)\n",
+ "\t\t\tkprobe-tracer argument format.)\n",
opt_add_probe_event),
OPT_END()
};

-static int write_new_event(int fd, const char *buf)
-{
- int ret;
-
- ret = write(fd, buf, strlen(buf));
- if (ret <= 0)
- die("Failed to create event.");
- else
- printf("Added new event: %s\n", buf);
-
- return ret;
-}
-
-#define MAX_CMDLEN 256
-
-static int synthesize_probe_event(struct probe_point *pp)
-{
- char *buf;
- int i, len, ret;
- pp->probes[0] = buf = zalloc(MAX_CMDLEN);
- if (!buf)
- die("Failed to allocate memory by zalloc.");
- ret = snprintf(buf, MAX_CMDLEN, "%s+%d", pp->function, pp->offset);
- if (ret <= 0 || ret >= MAX_CMDLEN)
- goto error;
- len = ret;
-
- for (i = 0; i < pp->nr_args; i++) {
- ret = snprintf(&buf[len], MAX_CMDLEN - len, " %s",
- pp->args[i]);
- if (ret <= 0 || ret >= MAX_CMDLEN - len)
- goto error;
- len += ret;
- }
- pp->found = 1;
- return pp->found;
-error:
- free(pp->probes[0]);
- if (ret > 0)
- ret = -E2BIG;
- return ret;
-}
-
int cmd_probe(int argc, const char **argv, const char *prefix __used)
{
int i, j, fd, ret;
struct probe_point *pp;
- char buf[MAX_CMDLEN];

argc = parse_options(argc, argv, options, probe_usage,
PARSE_OPT_STOP_AT_NON_OPTION);
@@ -352,7 +169,7 @@ int cmd_probe(int argc, const char **argv, const char *prefix __used)

if (session.need_dwarf)
#ifdef NO_LIBDWARF
- semantic_error("Debuginfo-analysis is not supported");
+ die("Debuginfo-analysis is not supported");
#else /* !NO_LIBDWARF */
pr_debug("Some probes require debuginfo.\n");

@@ -398,41 +215,15 @@ end_dwarf:
if (pp->found) /* This probe is already found. */
continue;

- ret = synthesize_probe_event(pp);
+ ret = synthesize_trace_kprobe_event(pp);
if (ret == -E2BIG)
- semantic_error("probe point is too long.");
+ die("probe point definition becomes too long.");
else if (ret < 0)
die("Failed to synthesize a probe point.");
}

/* Settng up probe points */
- snprintf(buf, MAX_CMDLEN, "%s/../kprobe_events", debugfs_path);
- fd = open(buf, O_WRONLY, O_APPEND);
- if (fd < 0) {
- if (errno == ENOENT)
- die("kprobe_events file does not exist - please rebuild with CONFIG_KPROBE_TRACER.");
- else
- die("Could not open kprobe_events file: %s",
- strerror(errno));
- }
- for (j = 0; j < session.nr_probe; j++) {
- pp = &session.probes[j];
- if (pp->found == 1) {
- snprintf(buf, MAX_CMDLEN, "%c:%s/%s_%x %s\n",
- pp->retprobe ? 'r' : 'p', PERFPROBE_GROUP,
- pp->function, pp->offset, pp->probes[0]);
- write_new_event(fd, buf);
- } else
- for (i = 0; i < pp->found; i++) {
- snprintf(buf, MAX_CMDLEN, "%c:%s/%s_%x_%d %s\n",
- pp->retprobe ? 'r' : 'p',
- PERFPROBE_GROUP,
- pp->function, pp->offset, i,
- pp->probes[i]);
- write_new_event(fd, buf);
- }
- }
- close(fd);
+ add_trace_kprobe_events(session.probes, session.nr_probe);
return 0;
}

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
new file mode 100644
index 0000000..7335a3b
--- /dev/null
+++ b/tools/perf/util/probe-event.c
@@ -0,0 +1,273 @@
+/*
+ * probe-event.c : perf-probe definition to kprobe_events format converter
+ *
+ * Written by Masami Hiramatsu <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ */
+
+#define _GNU_SOURCE
+#include <sys/utsname.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <string.h>
+
+#undef _GNU_SOURCE
+#include "event.h"
+#include "debug.h"
+#include "parse-events.h" /* For debugfs_path */
+#include "probe-event.h"
+
+#define MAX_CMDLEN 256
+#define MAX_PROBE_ARGS 128
+#define PERFPROBE_GROUP "probe"
+
+#define semantic_error(msg ...) die("Semantic error :" msg)
+
+/* Parse probepoint definition. */
+static void parse_perf_probe_probepoint(char *arg, struct probe_point *pp)
+{
+ char *ptr, *tmp;
+ char c, nc = 0;
+ /*
+ * <Syntax>
+ * perf probe SRC:LN
+ * perf probe FUNC[+OFFS|%return][@SRC]
+ */
+
+ ptr = strpbrk(arg, ":+@%");
+ if (ptr) {
+ nc = *ptr;
+ *ptr++ = '\0';
+ }
+
+ /* Check arg is function or file and copy it */
+ if (strchr(arg, '.')) /* File */
+ pp->file = strdup(arg);
+ else /* Function */
+ pp->function = strdup(arg);
+ DIE_IF(pp->file == NULL && pp->function == NULL);
+
+ /* Parse other options */
+ while (ptr) {
+ arg = ptr;
+ c = nc;
+ ptr = strpbrk(arg, ":+@%");
+ if (ptr) {
+ nc = *ptr;
+ *ptr++ = '\0';
+ }
+ switch (c) {
+ case ':': /* Line number */
+ pp->line = strtoul(arg, &tmp, 0);
+ if (*tmp != '\0')
+ semantic_error("There is non-digit charactor"
+ " in line number.");
+ break;
+ case '+': /* Byte offset from a symbol */
+ pp->offset = strtoul(arg, &tmp, 0);
+ if (*tmp != '\0')
+ semantic_error("There is non-digit charactor"
+ " in offset.");
+ break;
+ case '@': /* File name */
+ if (pp->file)
+ semantic_error("SRC@SRC is not allowed.");
+ pp->file = strdup(arg);
+ DIE_IF(pp->file == NULL);
+ if (ptr)
+ semantic_error("@SRC must be the last "
+ "option.");
+ break;
+ case '%': /* Probe places */
+ if (strcmp(arg, "return") == 0) {
+ pp->retprobe = 1;
+ } else /* Others not supported yet */
+ semantic_error("%%%s is not supported.", arg);
+ break;
+ default:
+ DIE_IF("Program has a bug.");
+ break;
+ }
+ }
+
+ /* Exclusion check */
+ if (pp->line && pp->offset)
+ semantic_error("Offset can't be used with line number.");
+
+ if (!pp->line && pp->file && !pp->function)
+ semantic_error("File always requires line number.");
+
+ if (pp->offset && !pp->function)
+ semantic_error("Offset requires an entry function.");
+
+ if (pp->retprobe && !pp->function)
+ semantic_error("Return probe requires an entry function.");
+
+ if ((pp->offset || pp->line) && pp->retprobe)
+ semantic_error("Offset/Line can't be used with return probe.");
+
+ pr_debug("symbol:%s file:%s line:%d offset:%d, return:%d\n",
+ pp->function, pp->file, pp->line, pp->offset, pp->retprobe);
+}
+
+/* Parse perf-probe event definition */
+int parse_perf_probe_event(const char *str, struct probe_point *pp)
+{
+ char *argv[MAX_PROBE_ARGS + 1]; /* probe + args */
+ int argc, i, need_dwarf = 0;
+
+ /* Separate arguments, similar to argv_split */
+ argc = 0;
+ do {
+ /* Skip separators */
+ while (isspace(*str))
+ str++;
+
+ /* Add an argument */
+ if (*str != '\0') {
+ const char *s = str;
+ /* Check the limit number of arguments */
+ if (argc == MAX_PROBE_ARGS + 1)
+ semantic_error("Too many arguments");
+
+ /* Skip the argument */
+ while (!isspace(*str) && *str != '\0')
+ str++;
+
+ /* Duplicate the argument */
+ argv[argc] = strndup(s, str - s);
+ if (argv[argc] == NULL)
+ die("strndup");
+ pr_debug("argv[%d]=%s\n", argc, argv[argc]);
+ argc++;
+ }
+ } while (*str != '\0');
+ if (!argc)
+ semantic_error("An empty argument.");
+
+ /* Parse probe point */
+ parse_perf_probe_probepoint(argv[0], pp);
+ free(argv[0]);
+ if (pp->file || pp->line)
+ need_dwarf = 1;
+
+ /* Copy arguments */
+ pp->nr_args = argc - 1;
+ if (pp->nr_args > 0) {
+ pp->args = (char **)malloc(sizeof(char *) * pp->nr_args);
+ if (!pp->args)
+ die("malloc");
+ memcpy(pp->args, &argv[1], sizeof(char *) * pp->nr_args);
+ }
+
+ /* Ensure return probe has no C argument */
+ for (i = 0; i < pp->nr_args; i++)
+ if (is_c_varname(pp->args[i])) {
+ if (pp->retprobe)
+ semantic_error("You can't specify local"
+ " variable for kretprobe");
+ need_dwarf = 1;
+ }
+
+ return need_dwarf;
+}
+
+int synthesize_trace_kprobe_event(struct probe_point *pp)
+{
+ char *buf;
+ int i, len, ret;
+
+ pp->probes[0] = buf = zalloc(MAX_CMDLEN);
+ if (!buf)
+ die("Failed to allocate memory by zalloc.");
+ ret = snprintf(buf, MAX_CMDLEN, "%s+%d", pp->function, pp->offset);
+ if (ret <= 0 || ret >= MAX_CMDLEN)
+ goto error;
+ len = ret;
+
+ for (i = 0; i < pp->nr_args; i++) {
+ ret = snprintf(&buf[len], MAX_CMDLEN - len, " %s",
+ pp->args[i]);
+ if (ret <= 0 || ret >= MAX_CMDLEN - len)
+ goto error;
+ len += ret;
+ }
+ pp->found = 1;
+
+ return pp->found;
+error:
+ free(pp->probes[0]);
+ if (ret > 0)
+ ret = -E2BIG;
+
+ return ret;
+}
+
+static int write_trace_kprobe_event(int fd, const char *buf)
+{
+ int ret;
+
+ ret = write(fd, buf, strlen(buf));
+ if (ret <= 0)
+ die("Failed to create event.");
+ else
+ printf("Added new event: %s\n", buf);
+
+ return ret;
+}
+
+void add_trace_kprobe_events(struct probe_point *probes, int nr_probes)
+{
+ int i, j, fd;
+ struct probe_point *pp;
+ char buf[MAX_CMDLEN];
+
+ snprintf(buf, MAX_CMDLEN, "%s/../kprobe_events", debugfs_path);
+ fd = open(buf, O_WRONLY, O_APPEND);
+ if (fd < 0) {
+ if (errno == ENOENT)
+ die("kprobe_events file does not exist -"
+ " please rebuild with CONFIG_KPROBE_TRACER.");
+ else
+ die("Could not open kprobe_events file: %s",
+ strerror(errno));
+ }
+
+ for (j = 0; j < nr_probes; j++) {
+ pp = probes + j;
+ if (pp->found == 1) {
+ snprintf(buf, MAX_CMDLEN, "%c:%s/%s_%x %s\n",
+ pp->retprobe ? 'r' : 'p', PERFPROBE_GROUP,
+ pp->function, pp->offset, pp->probes[0]);
+ write_trace_kprobe_event(fd, buf);
+ } else
+ for (i = 0; i < pp->found; i++) {
+ snprintf(buf, MAX_CMDLEN, "%c:%s/%s_%x_%d %s\n",
+ pp->retprobe ? 'r' : 'p',
+ PERFPROBE_GROUP,
+ pp->function, pp->offset, i,
+ pp->probes[i]);
+ write_trace_kprobe_event(fd, buf);
+ }
+ }
+ close(fd);
+}
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
new file mode 100644
index 0000000..0089c45
--- /dev/null
+++ b/tools/perf/util/probe-event.h
@@ -0,0 +1,10 @@
+#ifndef _PROBE_EVENT_H
+#define _PROBE_EVENT_H
+
+#include "probe-finder.h"
+
+extern int parse_perf_probe_event(const char *str, struct probe_point *pp);
+extern int synthesize_trace_kprobe_event(struct probe_point *pp);
+extern void add_trace_kprobe_events(struct probe_point *probes, int nr_probes);
+
+#endif /*_PROBE_EVENT_H */

2009-12-01 07:35:11

by Masami Hiramatsu

[permalink] [raw]
Subject: [tip:perf/core] perf probe: Add argv_split() from lib/argv_split.c

Commit-ID: e1c01d61a98703fcc80d15b8068ec36d5a215f7e
Gitweb: http://git.kernel.org/tip/e1c01d61a98703fcc80d15b8068ec36d5a215f7e
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Mon, 30 Nov 2009 19:20:05 -0500
Committer: Ingo Molnar <[email protected]>
CommitDate: Tue, 1 Dec 2009 08:20:02 +0100

perf probe: Add argv_split() from lib/argv_split.c

Add argv_split() ported from lib/argv_split.c to string.c and
use it in util/probe-event.c.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: systemtap <[email protected]>
Cc: DLE <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: K.Prasad <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
LKML-Reference: <20091201002005.10235.55602.stgit@harusame>
Signed-off-by: Ingo Molnar <[email protected]>
---
tools/perf/util/probe-event.c | 55 ++++++----------------
tools/perf/util/string.c | 101 +++++++++++++++++++++++++++++++++++++++++
tools/perf/util/string.h | 2 +
3 files changed, 118 insertions(+), 40 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 7335a3b..e3a683a 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -32,6 +32,7 @@

#undef _GNU_SOURCE
#include "event.h"
+#include "string.h"
#include "debug.h"
#include "parse-events.h" /* For debugfs_path */
#include "probe-event.h"
@@ -132,62 +133,36 @@ static void parse_perf_probe_probepoint(char *arg, struct probe_point *pp)
/* Parse perf-probe event definition */
int parse_perf_probe_event(const char *str, struct probe_point *pp)
{
- char *argv[MAX_PROBE_ARGS + 1]; /* probe + args */
+ char **argv;
int argc, i, need_dwarf = 0;

- /* Separate arguments, similar to argv_split */
- argc = 0;
- do {
- /* Skip separators */
- while (isspace(*str))
- str++;
-
- /* Add an argument */
- if (*str != '\0') {
- const char *s = str;
- /* Check the limit number of arguments */
- if (argc == MAX_PROBE_ARGS + 1)
- semantic_error("Too many arguments");
-
- /* Skip the argument */
- while (!isspace(*str) && *str != '\0')
- str++;
-
- /* Duplicate the argument */
- argv[argc] = strndup(s, str - s);
- if (argv[argc] == NULL)
- die("strndup");
- pr_debug("argv[%d]=%s\n", argc, argv[argc]);
- argc++;
- }
- } while (*str != '\0');
- if (!argc)
- semantic_error("An empty argument.");
+ argv = argv_split(str, &argc);
+ if (!argv)
+ die("argv_split failed.");
+ if (argc > MAX_PROBE_ARGS + 1)
+ semantic_error("Too many arguments");

/* Parse probe point */
parse_perf_probe_probepoint(argv[0], pp);
- free(argv[0]);
if (pp->file || pp->line)
need_dwarf = 1;

- /* Copy arguments */
+ /* Copy arguments and ensure return probe has no C argument */
pp->nr_args = argc - 1;
- if (pp->nr_args > 0) {
- pp->args = (char **)malloc(sizeof(char *) * pp->nr_args);
- if (!pp->args)
- die("malloc");
- memcpy(pp->args, &argv[1], sizeof(char *) * pp->nr_args);
- }
-
- /* Ensure return probe has no C argument */
- for (i = 0; i < pp->nr_args; i++)
+ pp->args = zalloc(sizeof(char *) * pp->nr_args);
+ for (i = 0; i < pp->nr_args; i++) {
+ pp->args[i] = strdup(argv[i + 1]);
+ if (!pp->args[i])
+ die("Failed to copy argument.");
if (is_c_varname(pp->args[i])) {
if (pp->retprobe)
semantic_error("You can't specify local"
" variable for kretprobe");
need_dwarf = 1;
}
+ }

+ argv_free(argv);
return need_dwarf;
}

diff --git a/tools/perf/util/string.c b/tools/perf/util/string.c
index 2270435..0977cf4 100644
--- a/tools/perf/util/string.c
+++ b/tools/perf/util/string.c
@@ -127,3 +127,104 @@ out_err:
out:
return length;
}
+
+/*
+ * Helper function for splitting a string into an argv-like array.
+ * originaly copied from lib/argv_split.c
+ */
+static const char *skip_sep(const char *cp)
+{
+ while (*cp && isspace(*cp))
+ cp++;
+
+ return cp;
+}
+
+static const char *skip_arg(const char *cp)
+{
+ while (*cp && !isspace(*cp))
+ cp++;
+
+ return cp;
+}
+
+static int count_argc(const char *str)
+{
+ int count = 0;
+
+ while (*str) {
+ str = skip_sep(str);
+ if (*str) {
+ count++;
+ str = skip_arg(str);
+ }
+ }
+
+ return count;
+}
+
+/**
+ * argv_free - free an argv
+ * @argv - the argument vector to be freed
+ *
+ * Frees an argv and the strings it points to.
+ */
+void argv_free(char **argv)
+{
+ char **p;
+ for (p = argv; *p; p++)
+ free(*p);
+
+ free(argv);
+}
+
+/**
+ * argv_split - split a string at whitespace, returning an argv
+ * @str: the string to be split
+ * @argcp: returned argument count
+ *
+ * Returns an array of pointers to strings which are split out from
+ * @str. This is performed by strictly splitting on white-space; no
+ * quote processing is performed. Multiple whitespace characters are
+ * considered to be a single argument separator. The returned array
+ * is always NULL-terminated. Returns NULL on memory allocation
+ * failure.
+ */
+char **argv_split(const char *str, int *argcp)
+{
+ int argc = count_argc(str);
+ char **argv = zalloc(sizeof(*argv) * (argc+1));
+ char **argvp;
+
+ if (argv == NULL)
+ goto out;
+
+ if (argcp)
+ *argcp = argc;
+
+ argvp = argv;
+
+ while (*str) {
+ str = skip_sep(str);
+
+ if (*str) {
+ const char *p = str;
+ char *t;
+
+ str = skip_arg(str);
+
+ t = strndup(p, str-p);
+ if (t == NULL)
+ goto fail;
+ *argvp++ = t;
+ }
+ }
+ *argvp = NULL;
+
+out:
+ return argv;
+
+fail:
+ argv_free(argv);
+ return NULL;
+}
diff --git a/tools/perf/util/string.h b/tools/perf/util/string.h
index e50b07f..bfecec2 100644
--- a/tools/perf/util/string.h
+++ b/tools/perf/util/string.h
@@ -6,6 +6,8 @@
int hex2u64(const char *ptr, u64 *val);
char *strxfrchar(char *s, char from, char to);
s64 perf_atoll(const char *str);
+char **argv_split(const char *str, int *argcp);
+void argv_free(char **argv);

#define _STR(x) #x
#define STR(x) _STR(x)

2009-12-01 07:34:09

by Masami Hiramatsu

[permalink] [raw]
Subject: [tip:perf/core] perf probe: Add --list option for listing current probe events

Commit-ID: 4de189fe6e5ad8241f6f8709d2e2ba4c3aeae33a
Gitweb: http://git.kernel.org/tip/4de189fe6e5ad8241f6f8709d2e2ba4c3aeae33a
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Mon, 30 Nov 2009 19:20:17 -0500
Committer: Ingo Molnar <[email protected]>
CommitDate: Tue, 1 Dec 2009 08:20:02 +0100

perf probe: Add --list option for listing current probe events

Add --list option for listing currently defined probe events
in the kernel. This shows events in below format;

[group:event] <perf-probe probe-definition>

for example:

[probe:schedule_0] schedule+30 cpu

Note that source file/line information is not supported yet.
So even if you added a probe by line, it will be shown in
<symbol+offset>.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: systemtap <[email protected]>
Cc: DLE <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: K.Prasad <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
LKML-Reference: <20091201002017.10235.76575.stgit@harusame>
Signed-off-by: Ingo Molnar <[email protected]>
---
tools/perf/builtin-probe.c | 12 ++-
tools/perf/util/probe-event.c | 231 ++++++++++++++++++++++++++++++++++++++---
tools/perf/util/probe-event.h | 5 +
3 files changed, 230 insertions(+), 18 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index bf20df2..b5d15cf 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -62,6 +62,8 @@ static struct {
struct probe_point probes[MAX_PROBES];
} session;

+static bool listing;
+
/* Parse an event definition. Note that any error must die. */
static void parse_probe_event(const char *str)
{
@@ -119,6 +121,7 @@ static int open_default_vmlinux(void)
static const char * const probe_usage[] = {
"perf probe [<options>] 'PROBEDEF' ['PROBEDEF' ...]",
"perf probe [<options>] --add 'PROBEDEF' [--add 'PROBEDEF' ...]",
+ "perf probe --list",
NULL
};

@@ -129,6 +132,7 @@ static const struct option options[] = {
OPT_STRING('k', "vmlinux", &session.vmlinux, "file",
"vmlinux/module pathname"),
#endif
+ OPT_BOOLEAN('l', "list", &listing, "list up current probes"),
OPT_CALLBACK('a', "add", NULL,
#ifdef NO_LIBDWARF
"FUNC[+OFFS|%return] [ARG ...]",
@@ -164,9 +168,15 @@ int cmd_probe(int argc, const char **argv, const char *prefix __used)
for (i = 0; i < argc; i++)
parse_probe_event(argv[i]);

- if (session.nr_probe == 0)
+ if ((session.nr_probe == 0 && !listing) ||
+ (session.nr_probe != 0 && listing))
usage_with_options(probe_usage, options);

+ if (listing) {
+ show_perf_probe_events();
+ return 0;
+ }
+
if (session.need_dwarf)
#ifdef NO_LIBDWARF
die("Debuginfo-analysis is not supported");
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index e3a683a..7f4f288 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -29,10 +29,13 @@
#include <unistd.h>
#include <stdlib.h>
#include <string.h>
+#include <stdarg.h>
+#include <limits.h>

#undef _GNU_SOURCE
#include "event.h"
#include "string.h"
+#include "strlist.h"
#include "debug.h"
#include "parse-events.h" /* For debugfs_path */
#include "probe-event.h"
@@ -43,6 +46,19 @@

#define semantic_error(msg ...) die("Semantic error :" msg)

+/* If there is no space to write, returns -E2BIG. */
+static int e_snprintf(char *str, size_t size, const char *format, ...)
+{
+ int ret;
+ va_list ap;
+ va_start(ap, format);
+ ret = vsnprintf(str, size, format, ap);
+ va_end(ap);
+ if (ret >= (int)size)
+ ret = -E2BIG;
+ return ret;
+}
+
/* Parse probepoint definition. */
static void parse_perf_probe_probepoint(char *arg, struct probe_point *pp)
{
@@ -166,6 +182,103 @@ int parse_perf_probe_event(const char *str, struct probe_point *pp)
return need_dwarf;
}

+/* Parse kprobe_events event into struct probe_point */
+void parse_trace_kprobe_event(const char *str, char **group, char **event,
+ struct probe_point *pp)
+{
+ char pr;
+ char *p;
+ int ret, i, argc;
+ char **argv;
+
+ pr_debug("Parsing kprobe_events: %s\n", str);
+ argv = argv_split(str, &argc);
+ if (!argv)
+ die("argv_split failed.");
+ if (argc < 2)
+ semantic_error("Too less arguments.");
+
+ /* Scan event and group name. */
+ ret = sscanf(argv[0], "%c:%m[^/ \t]/%m[^ \t]",
+ &pr, group, event);
+ if (ret != 3)
+ semantic_error("Failed to parse event name: %s", argv[0]);
+ pr_debug("Group:%s Event:%s probe:%c\n", *group, *event, pr);
+
+ if (!pp)
+ goto end;
+
+ pp->retprobe = (pr == 'r');
+
+ /* Scan function name and offset */
+ ret = sscanf(argv[1], "%m[^+]+%d", &pp->function, &pp->offset);
+ if (ret == 1)
+ pp->offset = 0;
+
+ /* kprobe_events doesn't have this information */
+ pp->line = 0;
+ pp->file = NULL;
+
+ pp->nr_args = argc - 2;
+ pp->args = zalloc(sizeof(char *) * pp->nr_args);
+ for (i = 0; i < pp->nr_args; i++) {
+ p = strchr(argv[i + 2], '=');
+ if (p) /* We don't need which register is assigned. */
+ *p = '\0';
+ pp->args[i] = strdup(argv[i + 2]);
+ if (!pp->args[i])
+ die("Failed to copy argument.");
+ }
+
+end:
+ argv_free(argv);
+}
+
+int synthesize_perf_probe_event(struct probe_point *pp)
+{
+ char *buf;
+ char offs[64] = "", line[64] = "";
+ int i, len, ret;
+
+ pp->probes[0] = buf = zalloc(MAX_CMDLEN);
+ if (!buf)
+ die("Failed to allocate memory by zalloc.");
+ if (pp->offset) {
+ ret = e_snprintf(offs, 64, "+%d", pp->offset);
+ if (ret <= 0)
+ goto error;
+ }
+ if (pp->line) {
+ ret = e_snprintf(line, 64, ":%d", pp->line);
+ if (ret <= 0)
+ goto error;
+ }
+
+ if (pp->function)
+ ret = e_snprintf(buf, MAX_CMDLEN, "%s%s%s%s", pp->function,
+ offs, pp->retprobe ? "%return" : "", line);
+ else
+ ret = e_snprintf(buf, MAX_CMDLEN, "%s%s%s%s", pp->file, line);
+ if (ret <= 0)
+ goto error;
+ len = ret;
+
+ for (i = 0; i < pp->nr_args; i++) {
+ ret = e_snprintf(&buf[len], MAX_CMDLEN - len, " %s",
+ pp->args[i]);
+ if (ret <= 0)
+ goto error;
+ len += ret;
+ }
+ pp->found = 1;
+
+ return pp->found;
+error:
+ free(pp->probes[0]);
+
+ return ret;
+}
+
int synthesize_trace_kprobe_event(struct probe_point *pp)
{
char *buf;
@@ -174,15 +287,15 @@ int synthesize_trace_kprobe_event(struct probe_point *pp)
pp->probes[0] = buf = zalloc(MAX_CMDLEN);
if (!buf)
die("Failed to allocate memory by zalloc.");
- ret = snprintf(buf, MAX_CMDLEN, "%s+%d", pp->function, pp->offset);
- if (ret <= 0 || ret >= MAX_CMDLEN)
+ ret = e_snprintf(buf, MAX_CMDLEN, "%s+%d", pp->function, pp->offset);
+ if (ret <= 0)
goto error;
len = ret;

for (i = 0; i < pp->nr_args; i++) {
- ret = snprintf(&buf[len], MAX_CMDLEN - len, " %s",
- pp->args[i]);
- if (ret <= 0 || ret >= MAX_CMDLEN - len)
+ ret = e_snprintf(&buf[len], MAX_CMDLEN - len, " %s",
+ pp->args[i]);
+ if (ret <= 0)
goto error;
len += ret;
}
@@ -191,12 +304,105 @@ int synthesize_trace_kprobe_event(struct probe_point *pp)
return pp->found;
error:
free(pp->probes[0]);
- if (ret > 0)
- ret = -E2BIG;

return ret;
}

+static int open_kprobe_events(int flags, int mode)
+{
+ char buf[PATH_MAX];
+ int ret;
+
+ ret = e_snprintf(buf, PATH_MAX, "%s/../kprobe_events", debugfs_path);
+ if (ret < 0)
+ die("Failed to make kprobe_events path.");
+
+ ret = open(buf, flags, mode);
+ if (ret < 0) {
+ if (errno == ENOENT)
+ die("kprobe_events file does not exist -"
+ " please rebuild with CONFIG_KPROBE_TRACER.");
+ else
+ die("Could not open kprobe_events file: %s",
+ strerror(errno));
+ }
+ return ret;
+}
+
+/* Get raw string list of current kprobe_events */
+static struct strlist *get_trace_kprobe_event_rawlist(int fd)
+{
+ int ret, idx;
+ FILE *fp;
+ char buf[MAX_CMDLEN];
+ char *p;
+ struct strlist *sl;
+
+ sl = strlist__new(true, NULL);
+
+ fp = fdopen(dup(fd), "r");
+ while (!feof(fp)) {
+ p = fgets(buf, MAX_CMDLEN, fp);
+ if (!p)
+ break;
+
+ idx = strlen(p) - 1;
+ if (p[idx] == '\n')
+ p[idx] = '\0';
+ ret = strlist__add(sl, buf);
+ if (ret < 0)
+ die("strlist__add failed: %s", strerror(-ret));
+ }
+ fclose(fp);
+
+ return sl;
+}
+
+/* Free and zero clear probe_point */
+static void clear_probe_point(struct probe_point *pp)
+{
+ int i;
+
+ if (pp->function)
+ free(pp->function);
+ if (pp->file)
+ free(pp->file);
+ for (i = 0; i < pp->nr_args; i++)
+ free(pp->args[i]);
+ if (pp->args)
+ free(pp->args);
+ for (i = 0; i < pp->found; i++)
+ free(pp->probes[i]);
+ memset(pp, 0, sizeof(pp));
+}
+
+/* List up current perf-probe events */
+void show_perf_probe_events(void)
+{
+ unsigned int i;
+ int fd;
+ char *group, *event;
+ struct probe_point pp;
+ struct strlist *rawlist;
+ struct str_node *ent;
+
+ fd = open_kprobe_events(O_RDONLY, 0);
+ rawlist = get_trace_kprobe_event_rawlist(fd);
+ close(fd);
+
+ for (i = 0; i < strlist__nr_entries(rawlist); i++) {
+ ent = strlist__entry(rawlist, i);
+ parse_trace_kprobe_event(ent->s, &group, &event, &pp);
+ synthesize_perf_probe_event(&pp);
+ printf("[%s:%s]\t%s\n", group, event, pp.probes[0]);
+ free(group);
+ free(event);
+ clear_probe_point(&pp);
+ }
+
+ strlist__delete(rawlist);
+}
+
static int write_trace_kprobe_event(int fd, const char *buf)
{
int ret;
@@ -216,16 +422,7 @@ void add_trace_kprobe_events(struct probe_point *probes, int nr_probes)
struct probe_point *pp;
char buf[MAX_CMDLEN];

- snprintf(buf, MAX_CMDLEN, "%s/../kprobe_events", debugfs_path);
- fd = open(buf, O_WRONLY, O_APPEND);
- if (fd < 0) {
- if (errno == ENOENT)
- die("kprobe_events file does not exist -"
- " please rebuild with CONFIG_KPROBE_TRACER.");
- else
- die("Could not open kprobe_events file: %s",
- strerror(errno));
- }
+ fd = open_kprobe_events(O_WRONLY, O_APPEND);

for (j = 0; j < nr_probes; j++) {
pp = probes + j;
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index 0089c45..88db7d1 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -2,9 +2,14 @@
#define _PROBE_EVENT_H

#include "probe-finder.h"
+#include "strlist.h"

extern int parse_perf_probe_event(const char *str, struct probe_point *pp);
+extern int synthesize_perf_probe_event(struct probe_point *pp);
+extern void parse_trace_kprobe_event(const char *str, char **group,
+ char **event, struct probe_point *pp);
extern int synthesize_trace_kprobe_event(struct probe_point *pp);
extern void add_trace_kprobe_events(struct probe_point *probes, int nr_probes);
+extern void show_perf_probe_events(void);

#endif /*_PROBE_EVENT_H */

2009-12-01 07:33:58

by Masami Hiramatsu

[permalink] [raw]
Subject: [tip:perf/core] perf probe: Simplify event naming

Commit-ID: b498ce1f2753b9724b2fc05d2057f7d1490cfa93
Gitweb: http://git.kernel.org/tip/b498ce1f2753b9724b2fc05d2057f7d1490cfa93
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Mon, 30 Nov 2009 19:20:25 -0500
Committer: Ingo Molnar <[email protected]>
CommitDate: Tue, 1 Dec 2009 08:20:03 +0100

perf probe: Simplify event naming

Simplify event naming as <symbol>_<seqnum>. Each event name is
globally unique (group name is not checked). So, if there is
schedule_0, next probe event on schedule() will be schedule_1.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: systemtap <[email protected]>
Cc: DLE <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Jason Baron <[email protected]>
Cc: K.Prasad <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
LKML-Reference: <20091201002024.10235.2353.stgit@harusame>
Signed-off-by: Ingo Molnar <[email protected]>
---
tools/perf/util/probe-event.c | 67 ++++++++++++++++++++++++++++++++--------
tools/perf/util/probe-event.h | 3 ++
2 files changed, 56 insertions(+), 14 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 7f4f288..e42f3ac 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -403,6 +403,29 @@ void show_perf_probe_events(void)
strlist__delete(rawlist);
}

+/* Get current perf-probe event names */
+static struct strlist *get_perf_event_names(int fd)
+{
+ unsigned int i;
+ char *group, *event;
+ struct strlist *sl, *rawlist;
+ struct str_node *ent;
+
+ rawlist = get_trace_kprobe_event_rawlist(fd);
+
+ sl = strlist__new(false, NULL);
+ for (i = 0; i < strlist__nr_entries(rawlist); i++) {
+ ent = strlist__entry(rawlist, i);
+ parse_trace_kprobe_event(ent->s, &group, &event, NULL);
+ strlist__add(sl, event);
+ free(group);
+ }
+
+ strlist__delete(rawlist);
+
+ return sl;
+}
+
static int write_trace_kprobe_event(int fd, const char *buf)
{
int ret;
@@ -416,30 +439,46 @@ static int write_trace_kprobe_event(int fd, const char *buf)
return ret;
}

+static void get_new_event_name(char *buf, size_t len, const char *base,
+ struct strlist *namelist)
+{
+ int i, ret;
+ for (i = 0; i < MAX_EVENT_INDEX; i++) {
+ ret = e_snprintf(buf, len, "%s_%d", base, i);
+ if (ret < 0)
+ die("snprintf() failed: %s", strerror(-ret));
+ if (!strlist__has_entry(namelist, buf))
+ break;
+ }
+ if (i == MAX_EVENT_INDEX)
+ die("Too many events are on the same function.");
+}
+
void add_trace_kprobe_events(struct probe_point *probes, int nr_probes)
{
int i, j, fd;
struct probe_point *pp;
char buf[MAX_CMDLEN];
+ char event[64];
+ struct strlist *namelist;

- fd = open_kprobe_events(O_WRONLY, O_APPEND);
+ fd = open_kprobe_events(O_RDWR, O_APPEND);
+ /* Get current event names */
+ namelist = get_perf_event_names(fd);

for (j = 0; j < nr_probes; j++) {
pp = probes + j;
- if (pp->found == 1) {
- snprintf(buf, MAX_CMDLEN, "%c:%s/%s_%x %s\n",
- pp->retprobe ? 'r' : 'p', PERFPROBE_GROUP,
- pp->function, pp->offset, pp->probes[0]);
+ for (i = 0; i < pp->found; i++) {
+ /* Get an unused new event name */
+ get_new_event_name(event, 64, pp->function, namelist);
+ snprintf(buf, MAX_CMDLEN, "%c:%s/%s %s\n",
+ pp->retprobe ? 'r' : 'p',
+ PERFPROBE_GROUP, event,
+ pp->probes[i]);
write_trace_kprobe_event(fd, buf);
- } else
- for (i = 0; i < pp->found; i++) {
- snprintf(buf, MAX_CMDLEN, "%c:%s/%s_%x_%d %s\n",
- pp->retprobe ? 'r' : 'p',
- PERFPROBE_GROUP,
- pp->function, pp->offset, i,
- pp->probes[i]);
- write_trace_kprobe_event(fd, buf);
- }
+ /* Add added event name to namelist */
+ strlist__add(namelist, event);
+ }
}
close(fd);
}
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index 88db7d1..0c6fe56 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -12,4 +12,7 @@ extern int synthesize_trace_kprobe_event(struct probe_point *pp);
extern void add_trace_kprobe_events(struct probe_point *probes, int nr_probes);
extern void show_perf_probe_events(void);

+/* Maximum index number of event-name postfix */
+#define MAX_EVENT_INDEX 1024
+
#endif /*_PROBE_EVENT_H */

2009-12-02 04:11:03

by tip-bot for Liming Wang

[permalink] [raw]
Subject: Re: [tip:perf/core] perf probe: Add --list option for listing current probe events

Hi Masami,

tip-bot for Masami Hiramatsu wrote:
> Commit-ID: 4de189fe6e5ad8241f6f8709d2e2ba4c3aeae33a
> Gitweb: http://git.kernel.org/tip/4de189fe6e5ad8241f6f8709d2e2ba4c3aeae33a
> Author: Masami Hiramatsu <[email protected]>
> AuthorDate: Mon, 30 Nov 2009 19:20:17 -0500
> Committer: Ingo Molnar <[email protected]>
> CommitDate: Tue, 1 Dec 2009 08:20:02 +0100
>
> perf probe: Add --list option for listing current probe events
>
> Add --list option for listing currently defined probe events
> in the kernel. This shows events in below format;
>
> [group:event] <perf-probe probe-definition>
>
> for example:
>
> [probe:schedule_0] schedule+30 cpu
>
> Note that source file/line information is not supported yet.
> So even if you added a probe by line, it will be shown in
> <symbol+offset>.
...
> +
> + /* Scan event and group name. */
> + ret = sscanf(argv[0], "%c:%m[^/ \t]/%m[^ \t]",
> + &pr, group, event);
"%m" are typos?
> + if (ret != 3)
> + semantic_error("Failed to parse event name: %s", argv[0]);
> + pr_debug("Group:%s Event:%s probe:%c\n", *group, *event, pr);
> +
> + if (!pp)
> + goto end;
> +
> + pp->retprobe = (pr == 'r');
> +
> + /* Scan function name and offset */
> + ret = sscanf(argv[1], "%m[^+]+%d", &pp->function, &pp->offset);
the same error? fix attached below.

--

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index e42f3ac..8d46521 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -199,8 +199,8 @@ void parse_trace_kprobe_event(const char *str, char **group,
char **event,
semantic_error("Too less arguments.");

/* Scan event and group name. */
- ret = sscanf(argv[0], "%c:%m[^/ \t]/%m[^ \t]",
- &pr, group, event);
+ ret = sscanf(argv[0], "%c:%s[^/ \t]/%s[^ \t]",
+ &pr, *group, *event);
if (ret != 3)
semantic_error("Failed to parse event name: %s", argv[0]);
pr_debug("Group:%s Event:%s probe:%c\n", *group, *event, pr);
@@ -211,7 +211,7 @@ void parse_trace_kprobe_event(const char *str, char **group,
char **event,
pp->retprobe = (pr == 'r');

/* Scan function name and offset */
- ret = sscanf(argv[1], "%m[^+]+%d", &pp->function, &pp->offset);
+ ret = sscanf(argv[1], "%s[^+]+%d", pp->function, &pp->offset);
if (ret == 1)
pp->offset = 0;



2009-12-02 04:54:05

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [tip:perf/core] perf probe: Add --list option for listing current probe events

Wang Liming wrote:
> Hi Masami,
>
> tip-bot for Masami Hiramatsu wrote:
>> Commit-ID: 4de189fe6e5ad8241f6f8709d2e2ba4c3aeae33a
>> Gitweb:
>> http://git.kernel.org/tip/4de189fe6e5ad8241f6f8709d2e2ba4c3aeae33a
>> Author: Masami Hiramatsu <[email protected]>
>> AuthorDate: Mon, 30 Nov 2009 19:20:17 -0500
>> Committer: Ingo Molnar <[email protected]>
>> CommitDate: Tue, 1 Dec 2009 08:20:02 +0100
>>
>> perf probe: Add --list option for listing current probe events
>>
>> Add --list option for listing currently defined probe events
>> in the kernel. This shows events in below format;
>>
>> [group:event] <perf-probe probe-definition>
>>
>> for example:
>>
>> [probe:schedule_0] schedule+30 cpu
>>
>> Note that source file/line information is not supported yet.
>> So even if you added a probe by line, it will be shown in
>> <symbol+offset>.
> ...
>> +
>> + /* Scan event and group name. */
>> + ret = sscanf(argv[0], "%c:%m[^/ \t]/%m[^ \t]",
>> + &pr, group, event);
> "%m" are typos?

No, %m is glibc extension of sscanf modifier, which allocate
memory for scanned string in sscanf. You can see "man sscanf"
if your glibc is not so old (hopefully..). Please refer below;

http://www.kernel.org/doc/man-pages/online/pages/man3/scanf.3.html#NOTES

Anyway, thank you for reviewing!

>> + if (ret != 3)
>> + semantic_error("Failed to parse event name: %s", argv[0]);
>> + pr_debug("Group:%s Event:%s probe:%c\n", *group, *event, pr);
>> +
>> + if (!pp)
>> + goto end;
>> +
>> + pp->retprobe = (pr == 'r');
>> +
>> + /* Scan function name and offset */
>> + ret = sscanf(argv[1], "%m[^+]+%d", &pp->function, &pp->offset);
> the same error? fix attached below.
>
> --
>
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index e42f3ac..8d46521 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -199,8 +199,8 @@ void parse_trace_kprobe_event(const char *str, char
> **group, char **event,
> semantic_error("Too less arguments.");
>
> /* Scan event and group name. */
> - ret = sscanf(argv[0], "%c:%m[^/ \t]/%m[^ \t]",
> - &pr, group, event);
> + ret = sscanf(argv[0], "%c:%s[^/ \t]/%s[^ \t]",
> + &pr, *group, *event);
> if (ret != 3)
> semantic_error("Failed to parse event name: %s", argv[0]);
> pr_debug("Group:%s Event:%s probe:%c\n", *group, *event, pr);
> @@ -211,7 +211,7 @@ void parse_trace_kprobe_event(const char *str, char
> **group, char **event,
> pp->retprobe = (pr == 'r');
>
> /* Scan function name and offset */
> - ret = sscanf(argv[1], "%m[^+]+%d", &pp->function, &pp->offset);
> + ret = sscanf(argv[1], "%s[^+]+%d", pp->function, &pp->offset);
> if (ret == 1)
> pp->offset = 0;
>
>
>
>

--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: [email protected]

2009-12-02 05:35:28

by tip-bot for Liming Wang

[permalink] [raw]
Subject: Re: [tip:perf/core] perf probe: Add --list option for listing current probe events

Masami Hiramatsu wrote:
> Wang Liming wrote:
>> Hi Masami,
>>
>> tip-bot for Masami Hiramatsu wrote:
>>> Commit-ID: 4de189fe6e5ad8241f6f8709d2e2ba4c3aeae33a
>>> Gitweb:
>>> http://git.kernel.org/tip/4de189fe6e5ad8241f6f8709d2e2ba4c3aeae33a
>>> Author: Masami Hiramatsu <[email protected]>
>>> AuthorDate: Mon, 30 Nov 2009 19:20:17 -0500
>>> Committer: Ingo Molnar <[email protected]>
>>> CommitDate: Tue, 1 Dec 2009 08:20:02 +0100
>>>
>>> perf probe: Add --list option for listing current probe events
>>>
>>> Add --list option for listing currently defined probe events
>>> in the kernel. This shows events in below format;
>>>
>>> [group:event] <perf-probe probe-definition>
>>>
>>> for example:
>>>
>>> [probe:schedule_0] schedule+30 cpu
>>>
>>> Note that source file/line information is not supported yet.
>>> So even if you added a probe by line, it will be shown in
>>> <symbol+offset>.
>> ...
>>> +
>>> + /* Scan event and group name. */
>>> + ret = sscanf(argv[0], "%c:%m[^/ \t]/%m[^ \t]",
>>> + &pr, group, event);
>> "%m" are typos?
>
> No, %m is glibc extension of sscanf modifier, which allocate
> memory for scanned string in sscanf. You can see "man sscanf"
> if your glibc is not so old (hopefully..). Please refer below;
>
> http://www.kernel.org/doc/man-pages/online/pages/man3/scanf.3.html#NOTES
Yes, my glibc is old so that "perf" can't be built. I'm using ubuntu 8.04.

Thanks a lot.
>
> Anyway, thank you for reviewing!
>
>>> + if (ret != 3)
>>> + semantic_error("Failed to parse event name: %s", argv[0]);
>>> + pr_debug("Group:%s Event:%s probe:%c\n", *group, *event, pr);
>>> +
>>> + if (!pp)
>>> + goto end;
>>> +
>>> + pp->retprobe = (pr == 'r');
>>> +
>>> + /* Scan function name and offset */
>>> + ret = sscanf(argv[1], "%m[^+]+%d", &pp->function, &pp->offset);
>> the same error? fix attached below.
>>
>> --
>>
>> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
>> index e42f3ac..8d46521 100644
>> --- a/tools/perf/util/probe-event.c
>> +++ b/tools/perf/util/probe-event.c
>> @@ -199,8 +199,8 @@ void parse_trace_kprobe_event(const char *str, char
>> **group, char **event,
>> semantic_error("Too less arguments.");
>>
>> /* Scan event and group name. */
>> - ret = sscanf(argv[0], "%c:%m[^/ \t]/%m[^ \t]",
>> - &pr, group, event);
>> + ret = sscanf(argv[0], "%c:%s[^/ \t]/%s[^ \t]",
>> + &pr, *group, *event);
>> if (ret != 3)
>> semantic_error("Failed to parse event name: %s", argv[0]);
>> pr_debug("Group:%s Event:%s probe:%c\n", *group, *event, pr);
>> @@ -211,7 +211,7 @@ void parse_trace_kprobe_event(const char *str, char
>> **group, char **event,
>> pp->retprobe = (pr == 'r');
>>
>> /* Scan function name and offset */
>> - ret = sscanf(argv[1], "%m[^+]+%d", &pp->function, &pp->offset);
>> + ret = sscanf(argv[1], "%s[^+]+%d", pp->function, &pp->offset);
>> if (ret == 1)
>> pp->offset = 0;
>>
>>
>>
>>
>

2009-12-02 05:42:11

by tip-bot for Liming Wang

[permalink] [raw]
Subject: Re: [tip:perf/core] perf probe: Add argv_split() from lib/argv_split.c

Hi Masami,

tip-bot for Masami Hiramatsu wrote:
...
> diff --git a/tools/perf/util/string.c b/tools/perf/util/string.c
> index 2270435..0977cf4 100644
> --- a/tools/perf/util/string.c
> +++ b/tools/perf/util/string.c
> @@ -127,3 +127,104 @@ out_err:
> out:
> return length;
> }
> +
> +/*
> + * Helper function for splitting a string into an argv-like array.
> + * originaly copied from lib/argv_split.c
> + */
> +static const char *skip_sep(const char *cp)
> +{
> + while (*cp && isspace(*cp))
> + cp++;
> +
> + return cp;
> +}
> +
> +static const char *skip_arg(const char *cp)
> +{
> + while (*cp && !isspace(*cp))
> + cp++;
> +
> + return cp;
> +}
> +
> +static int count_argc(const char *str)
> +{
> + int count = 0;
> +
> + while (*str) {
> + str = skip_sep(str);
> + if (*str) {
> + count++;
> + str = skip_arg(str);
> + }
> + }
> +
> + return count;
> +}
> +
> +/**
> + * argv_free - free an argv
> + * @argv - the argument vector to be freed
> + *
> + * Frees an argv and the strings it points to.
> + */
> +void argv_free(char **argv)
> +{
> + char **p;
> + for (p = argv; *p; p++)
> + free(*p);
> +
> + free(argv);
> +}
> +
> +/**
> + * argv_split - split a string at whitespace, returning an argv
> + * @str: the string to be split
> + * @argcp: returned argument count
> + *
> + * Returns an array of pointers to strings which are split out from
> + * @str. This is performed by strictly splitting on white-space; no
> + * quote processing is performed. Multiple whitespace characters are
> + * considered to be a single argument separator. The returned array
> + * is always NULL-terminated. Returns NULL on memory allocation
> + * failure.
> + */
> +char **argv_split(const char *str, int *argcp)
> +{
> + int argc = count_argc(str);
> + char **argv = zalloc(sizeof(*argv) * (argc+1));
> + char **argvp;
> +
> + if (argv == NULL)
> + goto out;
> +
> + if (argcp)
> + *argcp = argc;
> +
> + argvp = argv;
> +
> + while (*str) {
> + str = skip_sep(str);
> +
> + if (*str) {
> + const char *p = str;
> + char *t;
> +
> + str = skip_arg(str);
> +
> + t = strndup(p, str-p);
When I compiled "perf", I encountered following error:

CC util/string.o
cc1: warnings being treated as errors
util/string.c: In function 'argv_split':
util/string.c:216: warning: implicit declaration of function 'strndup'
util/string.c:216: warning: incompatible implicit declaration of built-in
function 'strndup'
make: *** [util/string.o] Error 1

Do we need to define _GNU_SOURCE in the head? Or maybe I used rather old glibc
version.

diff --git a/tools/perf/util/string.c b/tools/perf/util/string.c
index 0977cf4..ea3eb39 100644
--- a/tools/perf/util/string.c
+++ b/tools/perf/util/string.c
@@ -1,5 +1,8 @@
+#define _GNU_SOURCE
#include <string.h>
#include <stdlib.h>
+
+#undef _GNU_SOURCE
#include "string.h"
#include "util.h"

2009-12-02 05:51:17

by Frederic Weisbecker

[permalink] [raw]
Subject: Re: [tip:perf/core] perf probe: Add argv_split() from lib/argv_split.c

On Wed, Dec 02, 2009 at 01:44:56PM +0800, Wang Liming wrote:
> When I compiled "perf", I encountered following error:
>
> CC util/string.o
> cc1: warnings being treated as errors
> util/string.c: In function 'argv_split':
> util/string.c:216: warning: implicit declaration of function 'strndup'
> util/string.c:216: warning: incompatible implicit declaration of built-in
> function 'strndup'
> make: *** [util/string.o] Error 1
>
> Do we need to define _GNU_SOURCE in the head? Or maybe I used rather old
> glibc version.
>
> diff --git a/tools/perf/util/string.c b/tools/perf/util/string.c
> index 0977cf4..ea3eb39 100644
> --- a/tools/perf/util/string.c
> +++ b/tools/perf/util/string.c
> @@ -1,5 +1,8 @@
> +#define _GNU_SOURCE
> #include <string.h>
> #include <stdlib.h>
> +
> +#undef _GNU_SOURCE
> #include "string.h"
> #include "util.h"



Right, strndup is a GNU extension.
Could you please resend this patch with your signed-off-by
and a proper changelog?

Thanks!

Acked-by: Frederic Weisbecker <[email protected]>

2009-12-02 06:06:52

by Frederic Weisbecker

[permalink] [raw]
Subject: Re: [tip:perf/core] perf probe: Add --list option for listing current probe events

On Wed, Dec 02, 2009 at 01:35:39PM +0800, Wang Liming wrote:
> Yes, my glibc is old so that "perf" can't be built. I'm using ubuntu 8.04.
>
> Thanks a lot.



What is your glibc version?
%m appears to be supported since 2.7

But if possible we would like perf to be buildable on most
boxes. May be should we use %a instead there.

odd gcc versions will whine because of the confusion with the %a for
floating point things but that can be worked around with (float *) casts (sigh).
Well, since we also have strict aliasing checks, we'll actually need:

scanf("%a", (float *)(void *)str);

We do that in util/trace-event-parse.c

2009-12-02 06:20:59

by tip-bot for Liming Wang

[permalink] [raw]
Subject: Re: [tip:perf/core] perf probe: Add --list option for listing current probe events

Frederic Weisbecker wrote:
> On Wed, Dec 02, 2009 at 01:35:39PM +0800, Wang Liming wrote:
>> Yes, my glibc is old so that "perf" can't be built. I'm using ubuntu 8.04.
>>
>> Thanks a lot.
>
>
>
> What is your glibc version?
> %m appears to be supported since 2.7
I'm using ubuntu 8.04, and glibc version seems to be 2.7.

$dpkg -l |grep libc|less
ii libc6 2.7-10ubuntu4
GNU C Library: Shared libraries
ii libc6-dev 2.7-10ubuntu4
GNU C Library: Development Libraries and Header

$gcc -v
Using built-in specs.
Target: i486-linux-gnu
Configured with: ../src/configure -v
--enable-languages=c,c++,fortran,objc,obj-c++,treelang --prefix=/usr
--enable-shared --with-system-zlib --libexecdir=/usr/lib
--without-included-gettext --enable-threads=posix --enable-nls
--with-gxx-include-dir=/usr/include/c++/4.2 --program-suffix=-4.2
--enable-clocale=gnu --enable-libstdcxx-debug --enable-objc-gc --enable-mpfr
--enable-targets=all --enable-checking=release --build=i486-linux-gnu
--host=i486-linux-gnu --target=i486-linux-gnu
Thread model: posix
gcc version 4.2.4 (Ubuntu 4.2.4-1ubuntu4)

>
> But if possible we would like perf to be buildable on most
> boxes. May be should we use %a instead there.
I think so. We couldn't assume that all users use the latest glibc.

Liming Wang
>
> odd gcc versions will whine because of the confusion with the %a for
> floating point things but that can be worked around with (float *) casts (sigh).
> Well, since we also have strict aliasing checks, we'll actually need:
>
> scanf("%a", (float *)(void *)str);
>
> We do that in util/trace-event-parse.c
>
>

2009-12-02 08:32:33

by Ingo Molnar

[permalink] [raw]
Subject: Re: [tip:perf/core] perf probe: Add --list option for listing current probe events


* Frederic Weisbecker <[email protected]> wrote:

> On Wed, Dec 02, 2009 at 01:35:39PM +0800, Wang Liming wrote:
> > Yes, my glibc is old so that "perf" can't be built. I'm using ubuntu 8.04.
> >
> > Thanks a lot.
>
>
>
> What is your glibc version?
> %m appears to be supported since 2.7
>
> But if possible we would like perf to be buildable on most
> boxes. May be should we use %a instead there.

Definitely so.

Wang, mind sending a patch for that?

Thanks,

Ingo

2009-12-02 09:08:24

by tip-bot for Liming Wang

[permalink] [raw]
Subject: [PATCH] perf tools: replace %m with %a in sscanf

Not all glibc support %m and it results in a compile error if
%m not supported. Replace it with %a and (float *) casts.

Signed-off-by: Liming Wang <[email protected]>
Acked-by: Frederic Weisbecker <[email protected]>
---
tools/perf/util/probe-event.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index e42f3ac..cd7fbda 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -199,8 +199,8 @@ void parse_trace_kprobe_event(const char *str, char **group, char **event,
semantic_error("Too less arguments.");

/* Scan event and group name. */
- ret = sscanf(argv[0], "%c:%m[^/ \t]/%m[^ \t]",
- &pr, group, event);
+ ret = sscanf(argv[0], "%c:%a[^/ \t]/%a[^ \t]",
+ &pr, (float *)(void *)group, (float *)(void *)event);
if (ret != 3)
semantic_error("Failed to parse event name: %s", argv[0]);
pr_debug("Group:%s Event:%s probe:%c\n", *group, *event, pr);
@@ -211,7 +211,7 @@ void parse_trace_kprobe_event(const char *str, char **group, char **event,
pp->retprobe = (pr == 'r');

/* Scan function name and offset */
- ret = sscanf(argv[1], "%m[^+]+%d", &pp->function, &pp->offset);
+ ret = sscanf(argv[1], "%a[^+]+%d", (float *)(void *)&pp->function, &pp->offset);
if (ret == 1)
pp->offset = 0;

--
1.6.0.3

2009-12-02 10:45:02

by tip-bot for Liming Wang

[permalink] [raw]
Subject: [tip:perf/core] perf tools: Replace %m with %a in sscanf

Commit-ID: 93aaa45a6ad3f983180223601fc663cc551ad499
Gitweb: http://git.kernel.org/tip/93aaa45a6ad3f983180223601fc663cc551ad499
Author: Liming Wang <[email protected]>
AuthorDate: Wed, 2 Dec 2009 16:42:54 +0800
Committer: Ingo Molnar <[email protected]>
CommitDate: Wed, 2 Dec 2009 10:12:16 +0100

perf tools: Replace %m with %a in sscanf

Not all glibc support %m and it results in a compile error if
%m not supported. Replace it with %a and (float *) casts.

Signed-off-by: Liming Wang <[email protected]>
Acked-by: Frederic Weisbecker <[email protected]>
Cc: [email protected]
Cc: [email protected]
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
tools/perf/util/probe-event.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index e42f3ac..cd7fbda 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -199,8 +199,8 @@ void parse_trace_kprobe_event(const char *str, char **group, char **event,
semantic_error("Too less arguments.");

/* Scan event and group name. */
- ret = sscanf(argv[0], "%c:%m[^/ \t]/%m[^ \t]",
- &pr, group, event);
+ ret = sscanf(argv[0], "%c:%a[^/ \t]/%a[^ \t]",
+ &pr, (float *)(void *)group, (float *)(void *)event);
if (ret != 3)
semantic_error("Failed to parse event name: %s", argv[0]);
pr_debug("Group:%s Event:%s probe:%c\n", *group, *event, pr);
@@ -211,7 +211,7 @@ void parse_trace_kprobe_event(const char *str, char **group, char **event,
pp->retprobe = (pr == 'r');

/* Scan function name and offset */
- ret = sscanf(argv[1], "%m[^+]+%d", &pp->function, &pp->offset);
+ ret = sscanf(argv[1], "%a[^+]+%d", (float *)(void *)&pp->function, &pp->offset);
if (ret == 1)
pp->offset = 0;

2009-12-02 16:45:53

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH] perf tools: replace %m with %a in sscanf

Liming Wang wrote:
> Not all glibc support %m and it results in a compile error if
> %m not supported. Replace it with %a and (float *) casts.
>
> Signed-off-by: Liming Wang<[email protected]>
> Acked-by: Frederic Weisbecker<[email protected]>
> ---
> tools/perf/util/probe-event.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index e42f3ac..cd7fbda 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -199,8 +199,8 @@ void parse_trace_kprobe_event(const char *str, char **group, char **event,
> semantic_error("Too less arguments.");
>
> /* Scan event and group name. */
> - ret = sscanf(argv[0], "%c:%m[^/ \t]/%m[^ \t]",
> - &pr, group, event);
> + ret = sscanf(argv[0], "%c:%a[^/ \t]/%a[^ \t]",
> + &pr, (float *)(void *)group, (float *)(void *)event);

Hmm, the code itself is OK for me, but I think we need a comment
why we cast (void *) to (float *)...

Thank you,

> if (ret != 3)
> semantic_error("Failed to parse event name: %s", argv[0]);
> pr_debug("Group:%s Event:%s probe:%c\n", *group, *event, pr);
> @@ -211,7 +211,7 @@ void parse_trace_kprobe_event(const char *str, char **group, char **event,
> pp->retprobe = (pr == 'r');
>
> /* Scan function name and offset */
> - ret = sscanf(argv[1], "%m[^+]+%d",&pp->function,&pp->offset);
> + ret = sscanf(argv[1], "%a[^+]+%d", (float *)(void *)&pp->function,&pp->offset);
> if (ret == 1)
> pp->offset = 0;
>

--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: [email protected]

2009-12-02 21:53:34

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH -tip 0/9] perf-probe updates

Ingo Molnar wrote:
>
> * Masami Hiramatsu<[email protected]> wrote:
>
>> Hi,
>>
>> Here are bugfixes and updates for perf-probe and kprobe-tracer. I've
>> fixed some minor bugs and added --list option and simple probe naming.
>
> Applied, thanks Masami!
>
>> TODO:
>> - Support build-id checking.
>> - Support --del option to remove probes.
>> - Support --line option to show which lines user can probe.
>> - Support lazy string matching.
>
> ok, cool!
>
> One other small detail i noticed wrt. probe naming. Right now if we
> insert a single probe into a function it gets named schedule_0:
>
> # perf probe schedule
> Could not open vmlinux/module file. Try to use symbols.
> Added new event: p:probe/schedule_0 schedule+0
>
> the next one gets named schedule_1, schedule_2, etc.
>
> It would be nice to special-case the first one and name it 'schedule'.
> Most of the time people insert a single probe into a function, so the _0
> postfix is extra and in most cases unnecessary typing for them.

Sure, that's reasonable.

>
> Another small detail is that i dont think we should emit this line:
>
> Could not open vmlinux/module file. Try to use symbols.
>
> when we can create a probe successfully - it's just unnecessary noise,
> the user does not care how we pulled it off, as long as we were able to
> get a reliable symbol address and the insertion worked fine.

OK, I'll make it a debug message.

> A third detail is this line:
>
> Added new event: p:probe/schedule_0 schedule+0
>
> If that is pasted to perf stat directly it wont work because the syntax
> is probe:schedule_0. So i'd suggest to print something like:
>
> Added new event: probe/schedule_0 (on schedule+0)

Sure, perf always use ':' for event group separator, so

Added new event: probe:schedule (on schedule+0 [with ...(args)])

>
> Perhaps even print another line:
>
> You can now use it on all perf tools, such as:
>
> perf probe -e probe/schedule_0 -a sleep 1
> perf record -e probe/schedule_0 -a sleep 1

OK.

Thank you for good advice!

>
> ... to show people how to make use of it.
>
> Thanks,
>
> Ingo

--
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: [email protected]