2009-10-17 00:07:23

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH -tip tracing/kprobes 0/9] tracing/kprobes, perf: perf probe and kprobe-tracer bugfixes

Hi Ingo and Frederic,

Here are the bugfix and update (mostly cleanup) patches for
previous patchset.

> I hope it's part of the last family of instruction set we
> are missing.

I added missing SSE opcodes and 3DNow! support too.
However, near future, x86 decoder may need AVX support.
(AFAIK, currently, there are no code using it.)

Thank you,

---

Masami Hiramatsu (9):
perf: Add perf-probe document
perf: Add DIE_IF() macro for error checking
perf: Use eprintf() for debug messages in perf-probe
perf: Use die() for error cases in perf-probe
perf: Check libdwarf APIs for perf probe
x86: Add AMD prefetch and 3DNow! opcodes to opcode map
x86: Add MMX/SSE opcode groups to opcode map
tracing/kprobes: Add failure messages for debugging
tracing/kprobes: Update kprobe-tracer selftest against new syntax


arch/x86/lib/x86-opcode-map.txt | 23 ++++-
kernel/trace/trace_kprobe.c | 39 ++++++--
tools/perf/Documentation/perf-probe.txt | 48 ++++++++++
tools/perf/Makefile | 5 +
tools/perf/builtin-probe.c | 70 ++++++---------
tools/perf/command-list.txt | 1
tools/perf/util/probe-finder.c | 149 ++++++++++++++-----------------
tools/perf/util/probe-finder.h | 17 ----
tools/perf/util/util.h | 9 ++
9 files changed, 206 insertions(+), 155 deletions(-)
create mode 100644 tools/perf/Documentation/perf-probe.txt

--
Masami Hiramatsu

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


2009-10-17 00:07:00

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH -tip tracing/kprobes 1/9] tracing/kprobes: Update kprobe-tracer selftest against new syntax

Update kprobe-tracer selftest since command syntax has been changed.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
-
---

kernel/trace/trace_kprobe.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 739f70e..cdacdab 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -1438,12 +1438,12 @@ static __init int kprobe_trace_self_tests_init(void)
pr_info("Testing kprobe tracing: ");

ret = command_trace_probe("p:testprobe kprobe_trace_selftest_target "
- "a1 a2 a3 a4 a5 a6");
+ "$arg1 $arg2 $arg3 $arg4 $stack $stack0");
if (WARN_ON_ONCE(ret))
pr_warning("error enabling function entry\n");

ret = command_trace_probe("r:testprobe2 kprobe_trace_selftest_target "
- "ra rv");
+ "$retval");
if (WARN_ON_ONCE(ret))
pr_warning("error enabling function return\n");



--
Masami Hiramatsu

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

e-mail: [email protected]

2009-10-17 00:07:29

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH -tip tracing/kprobes 2/9] tracing/kprobes: Add failure messages for debugging

Add vorbose failure messages to kprobe-tracer for debugging.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
-
---

kernel/trace/trace_kprobe.c | 35 +++++++++++++++++++++++++++--------
1 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index cdacdab..b8ef707 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -597,15 +597,19 @@ static int create_trace_probe(int argc, char **argv)
void *addr = NULL;
char buf[MAX_EVENT_NAME_LEN];

- if (argc < 2)
+ if (argc < 2) {
+ pr_info("Probe point is not specified.\n");
return -EINVAL;
+ }

if (argv[0][0] == 'p')
is_return = 0;
else if (argv[0][0] == 'r')
is_return = 1;
- else
+ else {
+ pr_info("Probe definition must be started with 'p' or 'r'.\n");
return -EINVAL;
+ }

if (argv[0][1] == ':') {
event = &argv[0][2];
@@ -625,21 +629,29 @@ static int create_trace_probe(int argc, char **argv)
}

if (isdigit(argv[1][0])) {
- if (is_return)
+ if (is_return) {
+ pr_info("Return probe point must be a symbol.\n");
return -EINVAL;
+ }
/* an address specified */
ret = strict_strtoul(&argv[0][2], 0, (unsigned long *)&addr);
- if (ret)
+ if (ret) {
+ pr_info("Failed to parse address.\n");
return ret;
+ }
} else {
/* a symbol specified */
symbol = argv[1];
/* TODO: support .init module functions */
ret = split_symbol_offset(symbol, &offset);
- if (ret)
+ if (ret) {
+ pr_info("Failed to parse symbol.\n");
return ret;
- if (offset && is_return)
+ }
+ if (offset && is_return) {
+ pr_info("Return probe must be used without offset.\n");
return -EINVAL;
+ }
}
argc -= 2; argv += 2;

@@ -658,8 +670,11 @@ static int create_trace_probe(int argc, char **argv)
}
tp = alloc_trace_probe(group, event, addr, symbol, offset, argc,
is_return);
- if (IS_ERR(tp))
+ if (IS_ERR(tp)) {
+ pr_info("Failed to allocate trace_probe.(%d)\n",
+ (int)PTR_ERR(tp));
return PTR_ERR(tp);
+ }

/* parse arguments */
ret = 0;
@@ -672,6 +687,8 @@ static int create_trace_probe(int argc, char **argv)
arg = argv[i];

if (conflict_field_name(argv[i], tp->args, i)) {
+ pr_info("Argument%d name '%s' conflicts with "
+ "another field.\n", i, argv[i]);
ret = -EINVAL;
goto error;
}
@@ -685,8 +702,10 @@ static int create_trace_probe(int argc, char **argv)
goto error;
}
ret = parse_probe_arg(arg, &tp->args[i].fetch, is_return);
- if (ret)
+ if (ret) {
+ pr_info("Parse error at argument%d. (%d)\n", i, ret);
goto error;
+ }
}
tp->nr_args = i;



--
Masami Hiramatsu

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

e-mail: [email protected]

2009-10-17 00:07:28

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH -tip tracing/kprobes 3/9] x86: Add MMX/SSE opcode groups to opcode map

Add missing MMX/SSE opcode groups to x86 opcode map.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
---

arch/x86/lib/x86-opcode-map.txt | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/x86/lib/x86-opcode-map.txt b/arch/x86/lib/x86-opcode-map.txt
index 78a0daf..e7285d8 100644
--- a/arch/x86/lib/x86-opcode-map.txt
+++ b/arch/x86/lib/x86-opcode-map.txt
@@ -777,12 +777,22 @@ GrpTable: Grp11
EndTable

GrpTable: Grp12
+2: psrlw Nq,Ib (11B) | psrlw Udq,Ib (66),(11B)
+4: psraw Nq,Ib (11B) | psraw Udq,Ib (66),(11B)
+6: psllw Nq,Ib (11B) | psllw Udq,Ib (66),(11B)
EndTable

GrpTable: Grp13
+2: psrld Nq,Ib (11B) | psrld Udq,Ib (66),(11B)
+4: psrad Nq,Ib (11B) | psrad Udq,Ib (66),(11B)
+6: pslld Nq,Ib (11B) | pslld Udq,Ib (66),(11B)
EndTable

GrpTable: Grp14
+2: psrlq Nq,Ib (11B) | psrlq Udq,Ib (66),(11B)
+3: psrldq Udq,Ib (66),(11B)
+6: psllq Nq,Ib (11B) | psllq Udq,Ib (66),(11B)
+7: pslldq Udq,Ib (66),(11B)
EndTable

GrpTable: Grp15


--
Masami Hiramatsu

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

e-mail: [email protected]

2009-10-17 00:07:58

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH -tip tracing/kprobes 4/9] x86: Add AMD prefetch and 3DNow! opcodes to opcode map

Add AMD prefetch and 3DNow! opcode including FEMMS. Since 3DNow!
uses the last immediate byte as an opcode extension byte, x86 insn
just treats the extenstion byte as an immediate byte instead of
a part of opcode (insn_get_opcode() decodes first "0x0f 0x0f"
bytes.)
Users who are interested in analyzing 3DNow! opcode still can
decode it by analyzing the immediate byte.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: H. Peter Anvin <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
---

arch/x86/lib/x86-opcode-map.txt | 13 ++++++++++---
1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/arch/x86/lib/x86-opcode-map.txt b/arch/x86/lib/x86-opcode-map.txt
index e7285d8..894497f 100644
--- a/arch/x86/lib/x86-opcode-map.txt
+++ b/arch/x86/lib/x86-opcode-map.txt
@@ -306,9 +306,10 @@ Referrer: 2-byte escape
0a:
0b: UD2 (1B)
0c:
-0d: NOP Ev
-0e:
-0f:
+0d: NOP Ev | GrpP
+0e: FEMMS
+# 3DNow! uses the last imm byte as opcode extension.
+0f: 3DNow! Pq,Qq,Ib
# 0x0f 0x10-0x1f
10: movups Vps,Wps | movss Vss,Wss (F3) | movupd Vpd,Wpd (66) | movsd Vsd,Wsd (F2)
11: movups Wps,Vps | movss Wss,Vss (F3) | movupd Wpd,Vpd (66) | movsd Wsd,Vsd (F2)
@@ -813,6 +814,12 @@ GrpTable: Grp16
3: prefetch T2
EndTable

+# AMD's Prefetch Group
+GrpTable: GrpP
+0: PREFETCH
+1: PREFETCHW
+EndTable
+
GrpTable: GrpPDLK
0: MONTMUL
1: XSHA1


--
Masami Hiramatsu

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

e-mail: [email protected]

2009-10-17 00:07:45

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH -tip tracing/kprobes 5/9] perf: Check libdwarf APIs for perf probe

Check libdwarf APIs for perf probe in tools/perf/Makefile. Since
dwarf_get_ranges() has been added from libdwarf 20081231 (and it's
the newest function used in probe-finder.c), this just checks
whether the function is defined.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
-
---

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

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 52b1f43..03c27b9 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -420,8 +420,8 @@ ifneq ($(shell sh -c "(echo '\#include <libelf.h>'; echo 'int main(void) { Elf *
msg := $(error No libelf.h/libelf found, please install libelf-dev/elfutils-libelf-devel);
endif

-ifneq ($(shell sh -c "(echo '\#include <libdwarf/dwarf.h>'; echo '\#include <libdwarf/libdwarf.h>'; echo 'int main(void) { Dwarf_Debug dbg; Dwarf_Error err; dwarf_init(0, DW_DLC_READ, 0, 0, &dbg, &err); return (long)dbg; }') | $(CC) -x c - $(ALL_CFLAGS) -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -ldwarf -lelf -o /dev/null $(ALL_LDFLAGS) > /dev/null 2>&1 && echo y"), y)
- msg := $(warning No libdwarf.h found, disables dwarf support. Please install libdwarf-dev/libdwarf-devel);
+ifneq ($(shell sh -c "(echo '\#include <libdwarf/dwarf.h>'; echo '\#include <libdwarf/libdwarf.h>'; echo 'int main(void) { Dwarf_Debug dbg; Dwarf_Error err; Dwarf_Ranges *rng; dwarf_init(0, DW_DLC_READ, 0, 0, &dbg, &err); dwarf_get_ranges(dbg, 0, &rng, 0, 0, &err); return (long)dbg; }') | $(CC) -x c - $(ALL_CFLAGS) -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -ldwarf -lelf -o /dev/null $(ALL_LDFLAGS) > /dev/null 2>&1 && echo y"), y)
+ msg := $(warning No libdwarf.h found or old libdwarf.h found, disables dwarf support. Please install libdwarf-dev/libdwarf-devel >= 20081231);
BASIC_CFLAGS += -DNO_LIBDWARF
else
EXTLIBS += -lelf -ldwarf


--
Masami Hiramatsu

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

e-mail: [email protected]

2009-10-17 00:07:59

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH -tip tracing/kprobes 6/9] perf: Use die() for error cases in perf-probe

Use die() for exiting perf-probe with errors. This replaces
perror_exit(), msg_exit() and fprintf()+exit() with die(), and
uses die() in semantic_error().
This also renames 'die' local variables to 'dw_die' for avoiding
name confliction.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
---

tools/perf/builtin-probe.c | 47 ++++++++++------------------------
tools/perf/util/probe-finder.c | 55 +++++++++++++++-------------------------
2 files changed, 34 insertions(+), 68 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 73c883b..a1467d1 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -49,6 +49,7 @@ const char *default_search_path[NR_SEARCH_PATH] = {

#define MAX_PATH_LEN 256
#define MAX_PROBES 128
+#define MAX_PROBE_ARGS 128

/* Session management structure */
static struct {
@@ -60,19 +61,7 @@ static struct {
char *events[MAX_PROBES];
} session;

-static void semantic_error(const char *msg)
-{
- fprintf(stderr, "Semantic error: %s\n", msg);
- exit(1);
-}
-
-static void perror_exit(const char *msg)
-{
- perror(msg);
- exit(1);
-}
-
-#define MAX_PROBE_ARGS 128
+#define semantic_error(msg ...) die("Semantic error :" msg)

static int parse_probepoint(const struct option *opt __used,
const char *str, int unset __used)
@@ -109,7 +98,7 @@ static int parse_probepoint(const struct option *opt __used,
/* Duplicate the argument */
argv[argc] = strndup(s, str - s);
if (argv[argc] == NULL)
- perror_exit("strndup");
+ die("strndup");
if (++argc == MAX_PROBE_ARGS)
semantic_error("Too many arguments");
debug("argv[%d]=%s\n", argc, argv[argc - 1]);
@@ -171,7 +160,7 @@ static int parse_probepoint(const struct option *opt __used,
if (pp->nr_args > 0) {
pp->args = (char **)malloc(sizeof(char *) * pp->nr_args);
if (!pp->args)
- perror_exit("malloc");
+ die("malloc");
memcpy(pp->args, &argv[2], sizeof(char *) * pp->nr_args);
}

@@ -260,7 +249,7 @@ static int write_new_event(int fd, const char *buf)
printf("Adding new event: %s\n", buf);
ret = write(fd, buf, strlen(buf));
if (ret <= 0)
- perror("Error: Failed to create event");
+ die("failed to create event.");

return ret;
}
@@ -273,7 +262,7 @@ static int synthesize_probepoint(struct probe_point *pp)
int i, len, ret;
pp->probes[0] = buf = (char *)calloc(MAX_CMDLEN, sizeof(char));
if (!buf)
- perror_exit("calloc");
+ die("calloc");
ret = snprintf(buf, MAX_CMDLEN, "%s+%d", pp->function, pp->offset);
if (ret <= 0 || ret >= MAX_CMDLEN)
goto error;
@@ -322,10 +311,8 @@ int cmd_probe(int argc, const char **argv, const char *prefix __used)
ret = synthesize_probepoint(&session.probes[j]);
if (ret == -E2BIG)
semantic_error("probe point is too long.");
- else if (ret < 0) {
- perror("snprintf");
- return -1;
- }
+ else if (ret < 0)
+ die("snprintf");
}

#ifndef NO_LIBDWARF
@@ -336,10 +323,8 @@ int cmd_probe(int argc, const char **argv, const char *prefix __used)
fd = open(session.vmlinux, O_RDONLY);
else
fd = open_default_vmlinux();
- if (fd < 0) {
- perror("vmlinux/module file open");
- return -1;
- }
+ if (fd < 0)
+ die("vmlinux/module file open");

/* Searching probe points */
for (j = 0; j < session.nr_probe; j++) {
@@ -349,10 +334,8 @@ int cmd_probe(int argc, const char **argv, const char *prefix __used)

lseek(fd, SEEK_SET, 0);
ret = find_probepoint(fd, pp);
- if (ret <= 0) {
- fprintf(stderr, "Error: No probe point found.\n");
- return -1;
- }
+ if (ret <= 0)
+ die("No probe point found.\n");
debug("probe event %s found\n", session.events[j]);
}
close(fd);
@@ -363,10 +346,8 @@ setup_probes:
/* Settng up probe points */
snprintf(buf, MAX_CMDLEN, "%s/../kprobe_events", debugfs_path);
fd = open(buf, O_WRONLY, O_APPEND);
- if (fd < 0) {
- perror("kprobe_events open");
- return -1;
- }
+ if (fd < 0)
+ die("kprobe_events open");
for (j = 0; j < session.nr_probe; j++) {
pp = &session.probes[j];
if (pp->found == 1) {
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index ec6f53f..338fdb9 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -31,6 +31,8 @@
#include <string.h>
#include <stdarg.h>
#include <ctype.h>
+
+#include "util.h"
#include "probe-finder.h"


@@ -43,20 +45,6 @@ struct die_link {
static Dwarf_Debug __dw_debug;
static Dwarf_Error __dw_error;

-static void msg_exit(int ret, const char *fmt, ...)
-{
- va_list ap;
-
- va_start(ap, fmt);
- fprintf(stderr, "Error: ");
- vfprintf(stderr, fmt, ap);
- va_end(ap);
-
- fprintf(stderr, "\n");
- exit(ret);
-}
-
-
/*
* Generic dwarf analysis helpers
*/
@@ -151,11 +139,11 @@ static Dwarf_Unsigned die_get_fileno(Dwarf_Die cu_die, const char *fname)
}

/* Compare diename and tname */
-static int die_compare_name(Dwarf_Die die, const char *tname)
+static int die_compare_name(Dwarf_Die dw_die, const char *tname)
{
char *name;
int ret;
- ret = dwarf_diename(die, &name, &__dw_error);
+ ret = dwarf_diename(dw_die, &name, &__dw_error);
ERR_IF(ret == DW_DLV_ERROR);
if (ret == DW_DLV_OK) {
ret = strcmp(tname, name);
@@ -187,25 +175,25 @@ static int die_within_subprogram(Dwarf_Die sp_die, Dwarf_Addr addr,
}

/* Check the die is inlined function */
-static Dwarf_Bool die_inlined_subprogram(Dwarf_Die die)
+static Dwarf_Bool die_inlined_subprogram(Dwarf_Die dw_die)
{
/* TODO: check strictly */
Dwarf_Bool inl;
int ret;

- ret = dwarf_hasattr(die, DW_AT_inline, &inl, &__dw_error);
+ ret = dwarf_hasattr(dw_die, DW_AT_inline, &inl, &__dw_error);
ERR_IF(ret == DW_DLV_ERROR);
return inl;
}

/* Get the offset of abstruct_origin */
-static Dwarf_Off die_get_abstract_origin(Dwarf_Die die)
+static Dwarf_Off die_get_abstract_origin(Dwarf_Die dw_die)
{
Dwarf_Attribute attr;
Dwarf_Off cu_offs;
int ret;

- ret = dwarf_attr(die, DW_AT_abstract_origin, &attr, &__dw_error);
+ ret = dwarf_attr(dw_die, DW_AT_abstract_origin, &attr, &__dw_error);
ERR_IF(ret != DW_DLV_OK);
ret = dwarf_formref(attr, &cu_offs, &__dw_error);
ERR_IF(ret != DW_DLV_OK);
@@ -214,7 +202,7 @@ static Dwarf_Off die_get_abstract_origin(Dwarf_Die die)
}

/* Get entry pc(or low pc, 1st entry of ranges) of the die */
-static Dwarf_Addr die_get_entrypc(Dwarf_Die die)
+static Dwarf_Addr die_get_entrypc(Dwarf_Die dw_die)
{
Dwarf_Attribute attr;
Dwarf_Addr addr;
@@ -224,7 +212,7 @@ static Dwarf_Addr die_get_entrypc(Dwarf_Die die)
int ret;

/* Try to get entry pc */
- ret = dwarf_attr(die, DW_AT_entry_pc, &attr, &__dw_error);
+ ret = dwarf_attr(dw_die, DW_AT_entry_pc, &attr, &__dw_error);
ERR_IF(ret == DW_DLV_ERROR);
if (ret == DW_DLV_OK) {
ret = dwarf_formaddr(attr, &addr, &__dw_error);
@@ -234,13 +222,13 @@ static Dwarf_Addr die_get_entrypc(Dwarf_Die die)
}

/* Try to get low pc */
- ret = dwarf_lowpc(die, &addr, &__dw_error);
+ ret = dwarf_lowpc(dw_die, &addr, &__dw_error);
ERR_IF(ret == DW_DLV_ERROR);
if (ret == DW_DLV_OK)
return addr;

/* Try to get ranges */
- ret = dwarf_attr(die, DW_AT_ranges, &attr, &__dw_error);
+ ret = dwarf_attr(dw_die, DW_AT_ranges, &attr, &__dw_error);
ERR_IF(ret != DW_DLV_OK);
ret = dwarf_formref(attr, &offs, &__dw_error);
ERR_IF(ret != DW_DLV_OK);
@@ -382,11 +370,11 @@ static void show_location(Dwarf_Loc *loc, struct probe_finder *pf)
} else if (op == DW_OP_regx) {
regn = loc->lr_number;
} else
- msg_exit(-EINVAL, "Dwarf_OP %d is not supported.\n", op);
+ die("Dwarf_OP %d is not supported.\n", op);

regs = get_arch_regstr(regn);
if (!regs)
- msg_exit(-EINVAL, "%lld exceeds max register number.\n", regn);
+ die("%lld exceeds max register number.\n", regn);

if (deref)
ret = snprintf(pf->buf, pf->len,
@@ -417,8 +405,8 @@ static void show_variable(Dwarf_Die vr_die, struct probe_finder *pf)
dwarf_dealloc(__dw_debug, attr, DW_DLA_ATTR);
return ;
error:
- msg_exit(-1, "Failed to find the location of %s at this address.\n"
- " Perhaps, it was optimized out.\n", pf->var);
+ die("Failed to find the location of %s at this address.\n"
+ " Perhaps, it has been optimized out.\n", pf->var);
}

static int variable_callback(struct die_link *dlink, void *data)
@@ -456,8 +444,7 @@ static void find_variable(Dwarf_Die sp_die, struct probe_finder *pf)
/* Search child die for local variables and parameters. */
ret = search_die_from_children(sp_die, variable_callback, pf);
if (!ret)
- msg_exit(-1, "Failed to find '%s' in this function.\n",
- pf->var);
+ die("Failed to find '%s' in this function.\n", pf->var);
}

/* Get a frame base on the address */
@@ -568,8 +555,7 @@ static void find_by_line(Dwarf_Die cu_die, struct probe_finder *pf)
/* Search a real subprogram including this line, */
ret = search_die_from_children(cu_die, probeaddr_callback, pf);
if (ret == 0)
- msg_exit(-1,
- "Probe point is not found in subprograms.\n");
+ die("Probe point is not found in subprograms.\n");
/* Continuing, because target line might be inlined. */
}
dwarf_srclines_dealloc(__dw_debug, lines, cnt);
@@ -621,7 +607,7 @@ static int probefunc_callback(struct die_link *dlink, void *data)
!die_inlined_subprogram(lk->die))
goto found;
}
- msg_exit(-1, "Failed to find real subprogram.\n");
+ die("Failed to find real subprogram.\n");
found:
/* Get offset from subprogram */
ret = die_within_subprogram(lk->die, pf->addr, &offs);
@@ -649,8 +635,7 @@ int find_probepoint(int fd, struct probe_point *pp)

ret = dwarf_init(fd, DW_DLC_READ, 0, 0, &__dw_debug, &__dw_error);
if (ret != DW_DLV_OK)
- msg_exit(-1, "Failed to call dwarf_init(). "
- "Maybe, not a dwarf file?\n");
+ die("Failed to call dwarf_init(). Maybe, not a dwarf file.\n");

pp->found = 0;
while (++cu_number) {


--
Masami Hiramatsu

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

e-mail: [email protected]

2009-10-17 00:08:36

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH -tip tracing/kprobes 7/9] perf: Use eprintf() for debug messages in perf-probe

Replace debug() macro with eprintf() and add -v option for showing
those messages in perf-probe.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
---

tools/perf/builtin-probe.c | 23 ++++++++++++++---------
tools/perf/util/probe-finder.c | 12 +++++++-----
tools/perf/util/probe-finder.h | 7 -------
3 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index a1467d1..b5ad86a 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -35,6 +35,8 @@
#include "perf.h"
#include "builtin.h"
#include "util/util.h"
+#include "util/event.h"
+#include "util/debug.h"
#include "util/parse-options.h"
#include "util/parse-events.h" /* For debugfs_path */
#include "util/probe-finder.h"
@@ -76,7 +78,7 @@ static int parse_probepoint(const struct option *opt __used,
if (!str) /* The end of probe points */
return 0;

- debug("Probe-define(%d): %s\n", session.nr_probe, str);
+ eprintf("probe-definition(%d): %s\n", session.nr_probe, str);
if (++session.nr_probe == MAX_PROBES)
semantic_error("Too many probes");

@@ -101,7 +103,7 @@ static int parse_probepoint(const struct option *opt __used,
die("strndup");
if (++argc == MAX_PROBE_ARGS)
semantic_error("Too many arguments");
- debug("argv[%d]=%s\n", argc, argv[argc - 1]);
+ eprintf("argv[%d]=%s\n", argc, argv[argc - 1]);
}
} while (*str != '\0');
if (argc < 2)
@@ -131,7 +133,7 @@ static int parse_probepoint(const struct option *opt __used,
pp->line = atoi(ptr);
if (!pp->file || !pp->line)
semantic_error("Failed to parse line.");
- debug("file:%s line:%d\n", pp->file, pp->line);
+ eprintf("file:%s line:%d\n", pp->file, pp->line);
} else {
/* Function name */
ptr = strchr(arg, '+');
@@ -148,7 +150,7 @@ static int parse_probepoint(const struct option *opt __used,
pp->file = strdup(ptr);
}
pp->function = strdup(arg);
- debug("symbol:%s file:%s offset:%d\n",
+ eprintf("symbol:%s file:%s offset:%d\n",
pp->function, pp->file, pp->offset);
}
free(argv[1]);
@@ -173,7 +175,7 @@ static int parse_probepoint(const struct option *opt __used,
session.need_dwarf = 1;
}

- debug("%d arguments\n", pp->nr_args);
+ eprintf("%d arguments\n", pp->nr_args);
return 0;
}

@@ -186,7 +188,7 @@ static int open_default_vmlinux(void)

ret = uname(&uts);
if (ret) {
- debug("uname() failed.\n");
+ eprintf("uname() failed.\n");
return -errno;
}
session.release = uts.release;
@@ -194,11 +196,12 @@ static int open_default_vmlinux(void)
ret = snprintf(fname, MAX_PATH_LEN,
default_search_path[i], session.release);
if (ret >= MAX_PATH_LEN || ret < 0) {
- debug("Filename(%d,%s) is too long.\n", i, uts.release);
+ eprintf("Filename(%d,%s) is too long.\n", i,
+ uts.release);
errno = E2BIG;
return -E2BIG;
}
- debug("try to open %s\n", fname);
+ eprintf("try to open %s\n", fname);
fd = open(fname, O_RDONLY);
if (fd >= 0)
break;
@@ -213,6 +216,8 @@ static const char * const probe_usage[] = {
};

static const struct option options[] = {
+ OPT_BOOLEAN('v', "verbose", &verbose,
+ "be more verbose (show parsed arguments, etc)"),
#ifndef NO_LIBDWARF
OPT_STRING('k', "vmlinux", &session.vmlinux, "file",
"vmlinux/module pathname"),
@@ -336,7 +341,7 @@ int cmd_probe(int argc, const char **argv, const char *prefix __used)
ret = find_probepoint(fd, pp);
if (ret <= 0)
die("No probe point found.\n");
- debug("probe event %s found\n", session.events[j]);
+ eprintf("probe event %s found\n", session.events[j]);
}
close(fd);

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 338fdb9..db24c91 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -32,6 +32,8 @@
#include <stdarg.h>
#include <ctype.h>

+#include "event.h"
+#include "debug.h"
#include "util.h"
#include "probe-finder.h"

@@ -134,7 +136,7 @@ static Dwarf_Unsigned die_get_fileno(Dwarf_Die cu_die, const char *fname)
dwarf_dealloc(__dw_debug, srcs, DW_DLA_LIST);
}
if (found)
- debug("found fno: %d\n", (int)found);
+ eprintf("found fno: %d\n", (int)found);
return found;
}

@@ -440,7 +442,7 @@ static void find_variable(Dwarf_Die sp_die, struct probe_finder *pf)
return ;
}

- debug("Searching '%s' variable in context.\n", pf->var);
+ eprintf("Searching '%s' variable in context.\n", pf->var);
/* Search child die for local variables and parameters. */
ret = search_die_from_children(sp_die, variable_callback, pf);
if (!ret)
@@ -550,7 +552,7 @@ static void find_by_line(Dwarf_Die cu_die, struct probe_finder *pf)

ret = dwarf_lineaddr(lines[i], &addr, &__dw_error);
ERR_IF(ret != DW_DLV_OK);
- debug("Probe point found: 0x%llx\n", addr);
+ eprintf("Probe point found: 0x%llx\n", addr);
pf->addr = addr;
/* Search a real subprogram including this line, */
ret = search_die_from_children(cu_die, probeaddr_callback, pf);
@@ -581,7 +583,7 @@ static int probefunc_callback(struct die_link *dlink, void *data)
&pf->inl_offs,
&__dw_error);
ERR_IF(ret != DW_DLV_OK);
- debug("inline definition offset %lld\n",
+ eprintf("inline definition offset %lld\n",
pf->inl_offs);
return 0;
}
@@ -597,7 +599,7 @@ static int probefunc_callback(struct die_link *dlink, void *data)
/* Get probe address */
pf->addr = die_get_entrypc(dlink->die);
pf->addr += pp->offset;
- debug("found inline addr: 0x%llx\n", pf->addr);
+ eprintf("found inline addr: 0x%llx\n", pf->addr);
/* Inlined function. Get a real subprogram */
for (lk = dlink->parent; lk != NULL; lk = lk->parent) {
tag = 0;
diff --git a/tools/perf/util/probe-finder.h b/tools/perf/util/probe-finder.h
index 306810c..6a7cb0c 100644
--- a/tools/perf/util/probe-finder.h
+++ b/tools/perf/util/probe-finder.h
@@ -4,13 +4,6 @@
#define _stringify(n) #n
#define stringify(n) _stringify(n)

-#ifdef DEBUG
-#define debug(fmt ...) \
- fprintf(stderr, "DBG(" __FILE__ ":" stringify(__LINE__) "): " fmt)
-#else
-#define debug(fmt ...) do {} while (0)
-#endif
-
#define ERR_IF(cnd) \
do { if (cnd) { \
fprintf(stderr, "Error (" __FILE__ ":" stringify(__LINE__) \


--
Masami Hiramatsu

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

e-mail: [email protected]

2009-10-17 00:08:06

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH -tip tracing/kprobes 8/9] perf: Add DIE_IF() macro for error checking

Add DIE_IF() macro and replace ERR_IF() with it, and use
linux/stringify.h.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
---

tools/perf/Makefile | 1
tools/perf/util/probe-finder.c | 82 ++++++++++++++++++++--------------------
tools/perf/util/probe-finder.h | 10 -----
tools/perf/util/util.h | 9 ++++
4 files changed, 51 insertions(+), 51 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 03c27b9..1abbf9a 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -321,6 +321,7 @@ LIB_FILE=libperf.a
LIB_H += ../../include/linux/perf_event.h
LIB_H += ../../include/linux/rbtree.h
LIB_H += ../../include/linux/list.h
+LIB_H += ../../include/linux/stringify.h
LIB_H += util/include/linux/list.h
LIB_H += perf.h
LIB_H += util/types.h
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index db24c91..be997ab 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -146,7 +146,7 @@ static int die_compare_name(Dwarf_Die dw_die, const char *tname)
char *name;
int ret;
ret = dwarf_diename(dw_die, &name, &__dw_error);
- ERR_IF(ret == DW_DLV_ERROR);
+ DIE_IF(ret == DW_DLV_ERROR);
if (ret == DW_DLV_OK) {
ret = strcmp(tname, name);
dwarf_dealloc(__dw_debug, name, DW_DLA_STRING);
@@ -164,11 +164,11 @@ static int die_within_subprogram(Dwarf_Die sp_die, Dwarf_Addr addr,

/* TODO: check ranges */
ret = dwarf_lowpc(sp_die, &lopc, &__dw_error);
- ERR_IF(ret == DW_DLV_ERROR);
+ DIE_IF(ret == DW_DLV_ERROR);
if (ret == DW_DLV_NO_ENTRY)
return 0;
ret = dwarf_highpc(sp_die, &hipc, &__dw_error);
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);
if (lopc <= addr && addr < hipc) {
*offs = addr - lopc;
return 1;
@@ -184,7 +184,7 @@ static Dwarf_Bool die_inlined_subprogram(Dwarf_Die dw_die)
int ret;

ret = dwarf_hasattr(dw_die, DW_AT_inline, &inl, &__dw_error);
- ERR_IF(ret == DW_DLV_ERROR);
+ DIE_IF(ret == DW_DLV_ERROR);
return inl;
}

@@ -196,9 +196,9 @@ static Dwarf_Off die_get_abstract_origin(Dwarf_Die dw_die)
int ret;

ret = dwarf_attr(dw_die, DW_AT_abstract_origin, &attr, &__dw_error);
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);
ret = dwarf_formref(attr, &cu_offs, &__dw_error);
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);
dwarf_dealloc(__dw_debug, attr, DW_DLA_ATTR);
return cu_offs;
}
@@ -215,28 +215,28 @@ static Dwarf_Addr die_get_entrypc(Dwarf_Die dw_die)

/* Try to get entry pc */
ret = dwarf_attr(dw_die, DW_AT_entry_pc, &attr, &__dw_error);
- ERR_IF(ret == DW_DLV_ERROR);
+ DIE_IF(ret == DW_DLV_ERROR);
if (ret == DW_DLV_OK) {
ret = dwarf_formaddr(attr, &addr, &__dw_error);
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);
dwarf_dealloc(__dw_debug, attr, DW_DLA_ATTR);
return addr;
}

/* Try to get low pc */
ret = dwarf_lowpc(dw_die, &addr, &__dw_error);
- ERR_IF(ret == DW_DLV_ERROR);
+ DIE_IF(ret == DW_DLV_ERROR);
if (ret == DW_DLV_OK)
return addr;

/* Try to get ranges */
ret = dwarf_attr(dw_die, DW_AT_ranges, &attr, &__dw_error);
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);
ret = dwarf_formref(attr, &offs, &__dw_error);
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);
ret = dwarf_get_ranges(__dw_debug, offs, &ranges, &cnt, NULL,
&__dw_error);
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);
addr = ranges[0].dwr_addr1;
dwarf_ranges_dealloc(__dw_debug, ranges, cnt);
return addr;
@@ -261,7 +261,7 @@ static int __search_die_tree(struct die_link *cur_link,
while (!(ret = die_cb(cur_link, data))) {
/* Check child die */
ret = dwarf_child(cur_link->die, &new_die, &__dw_error);
- ERR_IF(ret == DW_DLV_ERROR);
+ DIE_IF(ret == DW_DLV_ERROR);
if (ret == DW_DLV_OK) {
new_link.parent = cur_link;
new_link.die = new_die;
@@ -273,7 +273,7 @@ static int __search_die_tree(struct die_link *cur_link,
/* Move to next sibling */
ret = dwarf_siblingof(__dw_debug, cur_link->die, &new_die,
&__dw_error);
- ERR_IF(ret == DW_DLV_ERROR);
+ DIE_IF(ret == DW_DLV_ERROR);
dwarf_dealloc(__dw_debug, cur_link->die, DW_DLA_DIE);
cur_link->die = new_die;
if (ret == DW_DLV_NO_ENTRY)
@@ -293,7 +293,7 @@ static int search_die_from_children(Dwarf_Die parent_die,

new_link.parent = NULL;
ret = dwarf_child(parent_die, &new_link.die, &__dw_error);
- ERR_IF(ret == DW_DLV_ERROR);
+ DIE_IF(ret == DW_DLV_ERROR);
if (ret == DW_DLV_OK)
return __search_die_tree(&new_link, die_cb, data);
else
@@ -309,7 +309,7 @@ static int attr_get_locdesc(Dwarf_Attribute attr, Dwarf_Locdesc *desc,
int ret, i;

ret = dwarf_loclist_n(attr, &llbuf, &lcnt, &__dw_error);
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);
ret = DW_DLV_NO_ENTRY;
for (i = 0; i < lcnt; ++i) {
if (llbuf[i]->ld_lopc <= addr &&
@@ -317,7 +317,7 @@ static int attr_get_locdesc(Dwarf_Attribute attr, Dwarf_Locdesc *desc,
memcpy(desc, llbuf[i], sizeof(Dwarf_Locdesc));
desc->ld_s =
malloc(sizeof(Dwarf_Loc) * llbuf[i]->ld_cents);
- ERR_IF(desc->ld_s == NULL);
+ DIE_IF(desc->ld_s == NULL);
memcpy(desc->ld_s, llbuf[i]->ld_s,
sizeof(Dwarf_Loc) * llbuf[i]->ld_cents);
ret = DW_DLV_OK;
@@ -383,8 +383,8 @@ static void show_location(Dwarf_Loc *loc, struct probe_finder *pf)
" %s=%+lld(%s)", pf->var, offs, regs);
else
ret = snprintf(pf->buf, pf->len, " %s=%s", pf->var, regs);
- ERR_IF(ret < 0);
- ERR_IF(ret >= pf->len);
+ DIE_IF(ret < 0);
+ DIE_IF(ret >= pf->len);
}

/* Show a variables in kprobe event format */
@@ -401,7 +401,7 @@ static void show_variable(Dwarf_Die vr_die, struct probe_finder *pf)
if (ret != DW_DLV_OK)
goto error;
/* TODO? */
- ERR_IF(ld.ld_cents != 1);
+ DIE_IF(ld.ld_cents != 1);
show_location(&ld.ld_s[0], pf);
free(ld.ld_s);
dwarf_dealloc(__dw_debug, attr, DW_DLA_ATTR);
@@ -418,7 +418,7 @@ static int variable_callback(struct die_link *dlink, void *data)
int ret;

ret = dwarf_tag(dlink->die, &tag, &__dw_error);
- ERR_IF(ret == DW_DLV_ERROR);
+ DIE_IF(ret == DW_DLV_ERROR);
if ((tag == DW_TAG_formal_parameter ||
tag == DW_TAG_variable) &&
(die_compare_name(dlink->die, pf->var) == 0)) {
@@ -437,8 +437,8 @@ static void find_variable(Dwarf_Die sp_die, struct probe_finder *pf)
if (!is_c_varname(pf->var)) {
/* Output raw parameters */
ret = snprintf(pf->buf, pf->len, " %s", pf->var);
- ERR_IF(ret < 0);
- ERR_IF(ret >= pf->len);
+ DIE_IF(ret < 0);
+ DIE_IF(ret >= pf->len);
return ;
}

@@ -456,9 +456,9 @@ static void get_current_frame_base(Dwarf_Die sp_die, struct probe_finder *pf)
int ret;

ret = dwarf_attr(sp_die, DW_AT_frame_base, &attr, &__dw_error);
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);
ret = attr_get_locdesc(attr, &pf->fbloc, (pf->addr - pf->cu_base));
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);
dwarf_dealloc(__dw_debug, attr, DW_DLA_ATTR);
}

@@ -479,7 +479,7 @@ static void show_probepoint(Dwarf_Die sp_die, Dwarf_Signed offs,

/* Output name of probe point */
ret = dwarf_diename(sp_die, &name, &__dw_error);
- ERR_IF(ret == DW_DLV_ERROR);
+ DIE_IF(ret == DW_DLV_ERROR);
if (ret == DW_DLV_OK) {
ret = snprintf(tmp, MAX_PROBE_BUFFER, "%s+%u", name,
(unsigned int)offs);
@@ -488,8 +488,8 @@ static void show_probepoint(Dwarf_Die sp_die, Dwarf_Signed offs,
/* This function has no name. */
ret = snprintf(tmp, MAX_PROBE_BUFFER, "0x%llx", pf->addr);
}
- ERR_IF(ret < 0);
- ERR_IF(ret >= MAX_PROBE_BUFFER);
+ DIE_IF(ret < 0);
+ DIE_IF(ret >= MAX_PROBE_BUFFER);
len = ret;

/* Find each argument */
@@ -515,7 +515,7 @@ static int probeaddr_callback(struct die_link *dlink, void *data)
int ret;

ret = dwarf_tag(dlink->die, &tag, &__dw_error);
- ERR_IF(ret == DW_DLV_ERROR);
+ DIE_IF(ret == DW_DLV_ERROR);
/* Check the address is in this subprogram */
if (tag == DW_TAG_subprogram &&
die_within_subprogram(dlink->die, pf->addr, &offs)) {
@@ -537,21 +537,21 @@ static void find_by_line(Dwarf_Die cu_die, struct probe_finder *pf)
int ret;

ret = dwarf_srclines(cu_die, &lines, &cnt, &__dw_error);
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);

for (i = 0; i < cnt; i++) {
ret = dwarf_line_srcfileno(lines[i], &fno, &__dw_error);
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);
if (fno != pf->fno)
continue;

ret = dwarf_lineno(lines[i], &lineno, &__dw_error);
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);
if (lineno != (Dwarf_Unsigned)pp->line)
continue;

ret = dwarf_lineaddr(lines[i], &addr, &__dw_error);
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);
eprintf("Probe point found: 0x%llx\n", addr);
pf->addr = addr;
/* Search a real subprogram including this line, */
@@ -574,7 +574,7 @@ static int probefunc_callback(struct die_link *dlink, void *data)
int ret;

ret = dwarf_tag(dlink->die, &tag, &__dw_error);
- ERR_IF(ret == DW_DLV_ERROR);
+ DIE_IF(ret == DW_DLV_ERROR);
if (tag == DW_TAG_subprogram) {
if (die_compare_name(dlink->die, pp->function) == 0) {
if (die_inlined_subprogram(dlink->die)) {
@@ -582,7 +582,7 @@ static int probefunc_callback(struct die_link *dlink, void *data)
ret = dwarf_die_CU_offset(dlink->die,
&pf->inl_offs,
&__dw_error);
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);
eprintf("inline definition offset %lld\n",
pf->inl_offs);
return 0;
@@ -604,7 +604,7 @@ static int probefunc_callback(struct die_link *dlink, void *data)
for (lk = dlink->parent; lk != NULL; lk = lk->parent) {
tag = 0;
dwarf_tag(lk->die, &tag, &__dw_error);
- ERR_IF(ret == DW_DLV_ERROR);
+ DIE_IF(ret == DW_DLV_ERROR);
if (tag == DW_TAG_subprogram &&
!die_inlined_subprogram(lk->die))
goto found;
@@ -613,7 +613,7 @@ static int probefunc_callback(struct die_link *dlink, void *data)
found:
/* Get offset from subprogram */
ret = die_within_subprogram(lk->die, pf->addr, &offs);
- ERR_IF(!ret);
+ DIE_IF(!ret);
show_probepoint(lk->die, offs, pf);
/* Continue to search */
}
@@ -644,13 +644,13 @@ int find_probepoint(int fd, struct probe_point *pp)
/* Search CU (Compilation Unit) */
ret = dwarf_next_cu_header(__dw_debug, NULL, NULL, NULL,
&addr_size, &next_cuh, &__dw_error);
- ERR_IF(ret == DW_DLV_ERROR);
+ DIE_IF(ret == DW_DLV_ERROR);
if (ret == DW_DLV_NO_ENTRY)
break;

/* Get the DIE(Debugging Information Entry) of this CU */
ret = dwarf_siblingof(__dw_debug, 0, &cu_die, &__dw_error);
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);

/* Check if target file is included. */
if (pp->file)
@@ -659,7 +659,7 @@ int find_probepoint(int fd, struct probe_point *pp)
if (!pp->file || pf.fno) {
/* Save CU base address (for frame_base) */
ret = dwarf_lowpc(cu_die, &pf.cu_base, &__dw_error);
- ERR_IF(ret == DW_DLV_ERROR);
+ DIE_IF(ret == DW_DLV_ERROR);
if (ret == DW_DLV_NO_ENTRY)
pf.cu_base = 0;
if (pp->line)
@@ -670,7 +670,7 @@ int find_probepoint(int fd, struct probe_point *pp)
dwarf_dealloc(__dw_debug, cu_die, DW_DLA_DIE);
}
ret = dwarf_finish(__dw_debug, &__dw_error);
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);

return pp->found;
}
diff --git a/tools/perf/util/probe-finder.h b/tools/perf/util/probe-finder.h
index 6a7cb0c..d17fafc 100644
--- a/tools/perf/util/probe-finder.h
+++ b/tools/perf/util/probe-finder.h
@@ -1,16 +1,6 @@
#ifndef _PROBE_FINDER_H
#define _PROBE_FINDER_H

-#define _stringify(n) #n
-#define stringify(n) _stringify(n)
-
-#define ERR_IF(cnd) \
- do { if (cnd) { \
- fprintf(stderr, "Error (" __FILE__ ":" stringify(__LINE__) \
- "): " stringify(cnd) "\n"); \
- exit(1); \
- } } while (0)
-
#define MAX_PATH_LEN 256
#define MAX_PROBE_BUFFER 1024
#define MAX_PROBES 128
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 9de2329..0daa341 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -134,6 +134,15 @@ extern void die(const char *err, ...) NORETURN __attribute__((format (printf, 1,
extern int error(const char *err, ...) __attribute__((format (printf, 1, 2)));
extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2)));

+#include "../../../include/linux/stringify.h"
+
+#define DIE_IF(cnd) \
+ do { if (cnd) \
+ die(" at (" __FILE__ ":" __stringify(__LINE__) "): " \
+ __stringify(cnd) "\n"); \
+ } while (0)
+
+
extern void set_die_routine(void (*routine)(const char *err, va_list params) NORETURN);

extern int prefixcmp(const char *str, const char *prefix);


--
Masami Hiramatsu

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

e-mail: [email protected]

2009-10-17 00:08:14

by Masami Hiramatsu

[permalink] [raw]
Subject: [PATCH -tip tracing/kprobes 9/9] perf: Add perf-probe document

Add perf-probe subcommand document and add it to command-list.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Jim Keniston <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
---

tools/perf/Documentation/perf-probe.txt | 48 +++++++++++++++++++++++++++++++
tools/perf/command-list.txt | 1 +
2 files changed, 49 insertions(+), 0 deletions(-)
create mode 100644 tools/perf/Documentation/perf-probe.txt

diff --git a/tools/perf/Documentation/perf-probe.txt b/tools/perf/Documentation/perf-probe.txt
new file mode 100644
index 0000000..6b6c6ae
--- /dev/null
+++ b/tools/perf/Documentation/perf-probe.txt
@@ -0,0 +1,48 @@
+perf-probe(1)
+=============
+
+NAME
+----
+perf-probe - Define new dynamic tracepoints
+
+SYNOPSIS
+--------
+[verse]
+'perf probe' [-k <file>] -P 'PROBE' [-P 'PROBE' ...]
+
+
+DESCRIPTION
+-----------
+This command defines dynamic tracepoint events, by symbol and registers
+without debuginfo, or by C expressions (C line numbers, C function names,
+and C local variables) with debuginfo.
+
+
+OPTIONS
+-------
+-k::
+--vmlinux::
+ Specify vmlinux path which has debuginfo (Dwarf binary).
+
+-v::
+--verbose::
+ Be more verbose (show parsed arguments, etc).
+
+-P::
+--probe::
+ Define a probe point (see PROBE SYNTAX for detail)
+
+PROBE SYNTAX
+------------
+Probe points are defined by following syntax.
+
+ "TYPE:[GRP/]NAME FUNC[+OFFS][@SRC]|@SRC:LINE [ARG ...]"
+
+'TYPE' specifies the type of probe point, you can use either "p" (kprobe) or "r" (kretprobe) for 'TYPE'. 'GRP' specifies the group name of this probe, and 'NAME' specifies the event name. If 'GRP' is omitted, "kprobes" is used for its group name.
+'FUNC' and 'OFFS' specifies function and offset (in byte) where probe will be put. In addition, 'SRC' specifies a source file which has that function (this is mainly for inline functions).
+You can specify a probe point by the source line number by using '@SRC:LINE' syntax, where 'SRC' is the source file path and 'LINE' is the line number.
+'ARG' specifies the arguments of this probe point. You can use the name of local variable, or kprobe-tracer argument format (e.g. $retval, %ax, etc).
+
+SEE ALSO
+--------
+linkperf:perf-trace[1], linkperf:perf-record[1]
diff --git a/tools/perf/command-list.txt b/tools/perf/command-list.txt
index 00326e2..6475db4 100644
--- a/tools/perf/command-list.txt
+++ b/tools/perf/command-list.txt
@@ -11,3 +11,4 @@ perf-stat mainporcelain common
perf-timechart mainporcelain common
perf-top mainporcelain common
perf-trace mainporcelain common
+perf-probe mainporcelain common


--
Masami Hiramatsu

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

e-mail: [email protected]

2009-10-17 08:02:58

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH -tip tracing/kprobes 0/9] tracing/kprobes, perf: perf probe and kprobe-tracer bugfixes


* Masami Hiramatsu <[email protected]> wrote:

> Hi Ingo and Frederic,
>
> Here are the bugfix and update (mostly cleanup) patches for
> previous patchset.
>
> > I hope it's part of the last family of instruction set we
> > are missing.
>
> I added missing SSE opcodes and 3DNow! support too.
> However, near future, x86 decoder may need AVX support.
> (AFAIK, currently, there are no code using it.)
>
> Thank you,
>
> ---
>
> Masami Hiramatsu (9):
> perf: Add perf-probe document
> perf: Add DIE_IF() macro for error checking
> perf: Use eprintf() for debug messages in perf-probe
> perf: Use die() for error cases in perf-probe
> perf: Check libdwarf APIs for perf probe
> x86: Add AMD prefetch and 3DNow! opcodes to opcode map
> x86: Add MMX/SSE opcode groups to opcode map
> tracing/kprobes: Add failure messages for debugging
> tracing/kprobes: Update kprobe-tracer selftest against new syntax
>
>
> arch/x86/lib/x86-opcode-map.txt | 23 ++++-
> kernel/trace/trace_kprobe.c | 39 ++++++--
> tools/perf/Documentation/perf-probe.txt | 48 ++++++++++
> tools/perf/Makefile | 5 +
> tools/perf/builtin-probe.c | 70 ++++++---------
> tools/perf/command-list.txt | 1
> tools/perf/util/probe-finder.c | 149 ++++++++++++++-----------------
> tools/perf/util/probe-finder.h | 17 ----
> tools/perf/util/util.h | 9 ++
> 9 files changed, 206 insertions(+), 155 deletions(-)
> create mode 100644 tools/perf/Documentation/perf-probe.txt

Looks really nice, thanks Masami!

Note, i've created a new topic tree for this work: tip:perf/probes, and
have put all the commits there - since i expect most of the
enabling/completion work for this feature to happen on the perf events
side.

I also merged this tree upto v2.6.32-rc5 and resolved a conflict with
recent tracing fixes. Please base future patches on this new tree.

Thanks,

Ingo

2009-10-17 10:05:39

by Masami Hiramatsu

[permalink] [raw]
Subject: [tip:perf/probes] tracing/kprobes: Update kprobe-tracer selftest against new syntax

Commit-ID: f397af06e4c9bf5a0bc92facb8cb29905e338ab0
Gitweb: http://git.kernel.org/tip/f397af06e4c9bf5a0bc92facb8cb29905e338ab0
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Fri, 16 Oct 2009 20:07:20 -0400
Committer: Ingo Molnar <[email protected]>
CommitDate: Sat, 17 Oct 2009 09:53:57 +0200

tracing/kprobes: Update kprobe-tracer selftest against new syntax

Update kprobe-tracer selftest since command syntax has been
changed.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/trace/trace_kprobe.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 739f70e..cdacdab 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -1438,12 +1438,12 @@ static __init int kprobe_trace_self_tests_init(void)
pr_info("Testing kprobe tracing: ");

ret = command_trace_probe("p:testprobe kprobe_trace_selftest_target "
- "a1 a2 a3 a4 a5 a6");
+ "$arg1 $arg2 $arg3 $arg4 $stack $stack0");
if (WARN_ON_ONCE(ret))
pr_warning("error enabling function entry\n");

ret = command_trace_probe("r:testprobe2 kprobe_trace_selftest_target "
- "ra rv");
+ "$retval");
if (WARN_ON_ONCE(ret))
pr_warning("error enabling function return\n");

2009-10-17 10:05:56

by Masami Hiramatsu

[permalink] [raw]
Subject: [tip:perf/probes] tracing/kprobes: Add failure messages for debugging

Commit-ID: e63cc2397ecc0f2b604f22fb9cdbb05911c1e5d4
Gitweb: http://git.kernel.org/tip/e63cc2397ecc0f2b604f22fb9cdbb05911c1e5d4
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Fri, 16 Oct 2009 20:07:28 -0400
Committer: Ingo Molnar <[email protected]>
CommitDate: Sat, 17 Oct 2009 09:53:58 +0200

tracing/kprobes: Add failure messages for debugging

Add verbose failure messages to kprobe-tracer for debugging.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/trace/trace_kprobe.c | 35 +++++++++++++++++++++++++++--------
1 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index cdacdab..b8ef707 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -597,15 +597,19 @@ static int create_trace_probe(int argc, char **argv)
void *addr = NULL;
char buf[MAX_EVENT_NAME_LEN];

- if (argc < 2)
+ if (argc < 2) {
+ pr_info("Probe point is not specified.\n");
return -EINVAL;
+ }

if (argv[0][0] == 'p')
is_return = 0;
else if (argv[0][0] == 'r')
is_return = 1;
- else
+ else {
+ pr_info("Probe definition must be started with 'p' or 'r'.\n");
return -EINVAL;
+ }

if (argv[0][1] == ':') {
event = &argv[0][2];
@@ -625,21 +629,29 @@ static int create_trace_probe(int argc, char **argv)
}

if (isdigit(argv[1][0])) {
- if (is_return)
+ if (is_return) {
+ pr_info("Return probe point must be a symbol.\n");
return -EINVAL;
+ }
/* an address specified */
ret = strict_strtoul(&argv[0][2], 0, (unsigned long *)&addr);
- if (ret)
+ if (ret) {
+ pr_info("Failed to parse address.\n");
return ret;
+ }
} else {
/* a symbol specified */
symbol = argv[1];
/* TODO: support .init module functions */
ret = split_symbol_offset(symbol, &offset);
- if (ret)
+ if (ret) {
+ pr_info("Failed to parse symbol.\n");
return ret;
- if (offset && is_return)
+ }
+ if (offset && is_return) {
+ pr_info("Return probe must be used without offset.\n");
return -EINVAL;
+ }
}
argc -= 2; argv += 2;

@@ -658,8 +670,11 @@ static int create_trace_probe(int argc, char **argv)
}
tp = alloc_trace_probe(group, event, addr, symbol, offset, argc,
is_return);
- if (IS_ERR(tp))
+ if (IS_ERR(tp)) {
+ pr_info("Failed to allocate trace_probe.(%d)\n",
+ (int)PTR_ERR(tp));
return PTR_ERR(tp);
+ }

/* parse arguments */
ret = 0;
@@ -672,6 +687,8 @@ static int create_trace_probe(int argc, char **argv)
arg = argv[i];

if (conflict_field_name(argv[i], tp->args, i)) {
+ pr_info("Argument%d name '%s' conflicts with "
+ "another field.\n", i, argv[i]);
ret = -EINVAL;
goto error;
}
@@ -685,8 +702,10 @@ static int create_trace_probe(int argc, char **argv)
goto error;
}
ret = parse_probe_arg(arg, &tp->args[i].fetch, is_return);
- if (ret)
+ if (ret) {
+ pr_info("Parse error at argument%d. (%d)\n", i, ret);
goto error;
+ }
}
tp->nr_args = i;

2009-10-17 10:06:21

by Masami Hiramatsu

[permalink] [raw]
Subject: [tip:perf/probes] x86: Add MMX/SSE opcode groups to opcode map

Commit-ID: 8c95bc3e206cff7a55edd2fc5f0e2b305d57903f
Gitweb: http://git.kernel.org/tip/8c95bc3e206cff7a55edd2fc5f0e2b305d57903f
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Fri, 16 Oct 2009 20:07:36 -0400
Committer: Ingo Molnar <[email protected]>
CommitDate: Sat, 17 Oct 2009 09:53:58 +0200

x86: Add MMX/SSE opcode groups to opcode map

Add missing MMX/SSE opcode groups to x86 opcode map.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/lib/x86-opcode-map.txt | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/x86/lib/x86-opcode-map.txt b/arch/x86/lib/x86-opcode-map.txt
index 78a0daf..e7285d8 100644
--- a/arch/x86/lib/x86-opcode-map.txt
+++ b/arch/x86/lib/x86-opcode-map.txt
@@ -777,12 +777,22 @@ GrpTable: Grp11
EndTable

GrpTable: Grp12
+2: psrlw Nq,Ib (11B) | psrlw Udq,Ib (66),(11B)
+4: psraw Nq,Ib (11B) | psraw Udq,Ib (66),(11B)
+6: psllw Nq,Ib (11B) | psllw Udq,Ib (66),(11B)
EndTable

GrpTable: Grp13
+2: psrld Nq,Ib (11B) | psrld Udq,Ib (66),(11B)
+4: psrad Nq,Ib (11B) | psrad Udq,Ib (66),(11B)
+6: pslld Nq,Ib (11B) | pslld Udq,Ib (66),(11B)
EndTable

GrpTable: Grp14
+2: psrlq Nq,Ib (11B) | psrlq Udq,Ib (66),(11B)
+3: psrldq Udq,Ib (66),(11B)
+6: psllq Nq,Ib (11B) | psllq Udq,Ib (66),(11B)
+7: pslldq Udq,Ib (66),(11B)
EndTable

GrpTable: Grp15

2009-10-17 10:06:11

by Masami Hiramatsu

[permalink] [raw]
Subject: [tip:perf/probes] x86: Add AMD prefetch and 3DNow! opcodes to opcode map

Commit-ID: d1baf5a5a6088e2991b7dbbd370ff200bd6615ce
Gitweb: http://git.kernel.org/tip/d1baf5a5a6088e2991b7dbbd370ff200bd6615ce
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Fri, 16 Oct 2009 20:07:44 -0400
Committer: Ingo Molnar <[email protected]>
CommitDate: Sat, 17 Oct 2009 09:53:59 +0200

x86: Add AMD prefetch and 3DNow! opcodes to opcode map

Add AMD prefetch and 3DNow! opcode including FEMMS. Since 3DNow!
uses the last immediate byte as an opcode extension byte, x86
insn just treats the extenstion byte as an immediate byte
instead of a part of opcode (insn_get_opcode() decodes first
"0x0f 0x0f" bytes.)

Users who are interested in analyzing 3DNow! opcode still can
decode it by analyzing the immediate byte.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/lib/x86-opcode-map.txt | 13 ++++++++++---
1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/arch/x86/lib/x86-opcode-map.txt b/arch/x86/lib/x86-opcode-map.txt
index e7285d8..894497f 100644
--- a/arch/x86/lib/x86-opcode-map.txt
+++ b/arch/x86/lib/x86-opcode-map.txt
@@ -306,9 +306,10 @@ Referrer: 2-byte escape
0a:
0b: UD2 (1B)
0c:
-0d: NOP Ev
-0e:
-0f:
+0d: NOP Ev | GrpP
+0e: FEMMS
+# 3DNow! uses the last imm byte as opcode extension.
+0f: 3DNow! Pq,Qq,Ib
# 0x0f 0x10-0x1f
10: movups Vps,Wps | movss Vss,Wss (F3) | movupd Vpd,Wpd (66) | movsd Vsd,Wsd (F2)
11: movups Wps,Vps | movss Wss,Vss (F3) | movupd Wpd,Vpd (66) | movsd Wsd,Vsd (F2)
@@ -813,6 +814,12 @@ GrpTable: Grp16
3: prefetch T2
EndTable

+# AMD's Prefetch Group
+GrpTable: GrpP
+0: PREFETCH
+1: PREFETCHW
+EndTable
+
GrpTable: GrpPDLK
0: MONTMUL
1: XSHA1

2009-10-17 10:06:44

by Masami Hiramatsu

[permalink] [raw]
Subject: [tip:perf/probes] perf: Check libdwarf APIs for perf probe

Commit-ID: 4c20194c2de151bca14224ae384b47abf7636a95
Gitweb: http://git.kernel.org/tip/4c20194c2de151bca14224ae384b47abf7636a95
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Fri, 16 Oct 2009 20:07:52 -0400
Committer: Ingo Molnar <[email protected]>
CommitDate: Sat, 17 Oct 2009 09:53:59 +0200

perf: Check libdwarf APIs for perf probe

Check libdwarf APIs for perf probe in tools/perf/Makefile. Since
dwarf_get_ranges() has been added from libdwarf 20081231 (and
it's the newest function used in probe-finder.c), this just
checks whether the function is defined.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
tools/perf/Makefile | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 52b1f43..03c27b9 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -420,8 +420,8 @@ ifneq ($(shell sh -c "(echo '\#include <libelf.h>'; echo 'int main(void) { Elf *
msg := $(error No libelf.h/libelf found, please install libelf-dev/elfutils-libelf-devel);
endif

-ifneq ($(shell sh -c "(echo '\#include <libdwarf/dwarf.h>'; echo '\#include <libdwarf/libdwarf.h>'; echo 'int main(void) { Dwarf_Debug dbg; Dwarf_Error err; dwarf_init(0, DW_DLC_READ, 0, 0, &dbg, &err); return (long)dbg; }') | $(CC) -x c - $(ALL_CFLAGS) -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -ldwarf -lelf -o /dev/null $(ALL_LDFLAGS) > /dev/null 2>&1 && echo y"), y)
- msg := $(warning No libdwarf.h found, disables dwarf support. Please install libdwarf-dev/libdwarf-devel);
+ifneq ($(shell sh -c "(echo '\#include <libdwarf/dwarf.h>'; echo '\#include <libdwarf/libdwarf.h>'; echo 'int main(void) { Dwarf_Debug dbg; Dwarf_Error err; Dwarf_Ranges *rng; dwarf_init(0, DW_DLC_READ, 0, 0, &dbg, &err); dwarf_get_ranges(dbg, 0, &rng, 0, 0, &err); return (long)dbg; }') | $(CC) -x c - $(ALL_CFLAGS) -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -ldwarf -lelf -o /dev/null $(ALL_LDFLAGS) > /dev/null 2>&1 && echo y"), y)
+ msg := $(warning No libdwarf.h found or old libdwarf.h found, disables dwarf support. Please install libdwarf-dev/libdwarf-devel >= 20081231);
BASIC_CFLAGS += -DNO_LIBDWARF
else
EXTLIBS += -lelf -ldwarf

2009-10-17 10:06:38

by Masami Hiramatsu

[permalink] [raw]
Subject: [tip:perf/probes] perf: Use die() for error cases in perf-probe

Commit-ID: 074fc0e4b3f5d24306c2995f2f3b0bd4759e8aeb
Gitweb: http://git.kernel.org/tip/074fc0e4b3f5d24306c2995f2f3b0bd4759e8aeb
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Fri, 16 Oct 2009 20:08:01 -0400
Committer: Ingo Molnar <[email protected]>
CommitDate: Sat, 17 Oct 2009 09:54:00 +0200

perf: Use die() for error cases in perf-probe

Use die() for exiting perf-probe with errors. This replaces
perror_exit(), msg_exit() and fprintf()+exit() with die(), and
uses die() in semantic_error().

This also renames 'die' local variables to 'dw_die' for avoiding
name confliction.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
tools/perf/builtin-probe.c | 47 ++++++++++------------------------
tools/perf/util/probe-finder.c | 55 ++++++++++++++-------------------------
2 files changed, 34 insertions(+), 68 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 73c883b..a1467d1 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -49,6 +49,7 @@ const char *default_search_path[NR_SEARCH_PATH] = {

#define MAX_PATH_LEN 256
#define MAX_PROBES 128
+#define MAX_PROBE_ARGS 128

/* Session management structure */
static struct {
@@ -60,19 +61,7 @@ static struct {
char *events[MAX_PROBES];
} session;

-static void semantic_error(const char *msg)
-{
- fprintf(stderr, "Semantic error: %s\n", msg);
- exit(1);
-}
-
-static void perror_exit(const char *msg)
-{
- perror(msg);
- exit(1);
-}
-
-#define MAX_PROBE_ARGS 128
+#define semantic_error(msg ...) die("Semantic error :" msg)

static int parse_probepoint(const struct option *opt __used,
const char *str, int unset __used)
@@ -109,7 +98,7 @@ static int parse_probepoint(const struct option *opt __used,
/* Duplicate the argument */
argv[argc] = strndup(s, str - s);
if (argv[argc] == NULL)
- perror_exit("strndup");
+ die("strndup");
if (++argc == MAX_PROBE_ARGS)
semantic_error("Too many arguments");
debug("argv[%d]=%s\n", argc, argv[argc - 1]);
@@ -171,7 +160,7 @@ static int parse_probepoint(const struct option *opt __used,
if (pp->nr_args > 0) {
pp->args = (char **)malloc(sizeof(char *) * pp->nr_args);
if (!pp->args)
- perror_exit("malloc");
+ die("malloc");
memcpy(pp->args, &argv[2], sizeof(char *) * pp->nr_args);
}

@@ -260,7 +249,7 @@ static int write_new_event(int fd, const char *buf)
printf("Adding new event: %s\n", buf);
ret = write(fd, buf, strlen(buf));
if (ret <= 0)
- perror("Error: Failed to create event");
+ die("failed to create event.");

return ret;
}
@@ -273,7 +262,7 @@ static int synthesize_probepoint(struct probe_point *pp)
int i, len, ret;
pp->probes[0] = buf = (char *)calloc(MAX_CMDLEN, sizeof(char));
if (!buf)
- perror_exit("calloc");
+ die("calloc");
ret = snprintf(buf, MAX_CMDLEN, "%s+%d", pp->function, pp->offset);
if (ret <= 0 || ret >= MAX_CMDLEN)
goto error;
@@ -322,10 +311,8 @@ int cmd_probe(int argc, const char **argv, const char *prefix __used)
ret = synthesize_probepoint(&session.probes[j]);
if (ret == -E2BIG)
semantic_error("probe point is too long.");
- else if (ret < 0) {
- perror("snprintf");
- return -1;
- }
+ else if (ret < 0)
+ die("snprintf");
}

#ifndef NO_LIBDWARF
@@ -336,10 +323,8 @@ int cmd_probe(int argc, const char **argv, const char *prefix __used)
fd = open(session.vmlinux, O_RDONLY);
else
fd = open_default_vmlinux();
- if (fd < 0) {
- perror("vmlinux/module file open");
- return -1;
- }
+ if (fd < 0)
+ die("vmlinux/module file open");

/* Searching probe points */
for (j = 0; j < session.nr_probe; j++) {
@@ -349,10 +334,8 @@ int cmd_probe(int argc, const char **argv, const char *prefix __used)

lseek(fd, SEEK_SET, 0);
ret = find_probepoint(fd, pp);
- if (ret <= 0) {
- fprintf(stderr, "Error: No probe point found.\n");
- return -1;
- }
+ if (ret <= 0)
+ die("No probe point found.\n");
debug("probe event %s found\n", session.events[j]);
}
close(fd);
@@ -363,10 +346,8 @@ setup_probes:
/* Settng up probe points */
snprintf(buf, MAX_CMDLEN, "%s/../kprobe_events", debugfs_path);
fd = open(buf, O_WRONLY, O_APPEND);
- if (fd < 0) {
- perror("kprobe_events open");
- return -1;
- }
+ if (fd < 0)
+ die("kprobe_events open");
for (j = 0; j < session.nr_probe; j++) {
pp = &session.probes[j];
if (pp->found == 1) {
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index ec6f53f..338fdb9 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -31,6 +31,8 @@
#include <string.h>
#include <stdarg.h>
#include <ctype.h>
+
+#include "util.h"
#include "probe-finder.h"


@@ -43,20 +45,6 @@ struct die_link {
static Dwarf_Debug __dw_debug;
static Dwarf_Error __dw_error;

-static void msg_exit(int ret, const char *fmt, ...)
-{
- va_list ap;
-
- va_start(ap, fmt);
- fprintf(stderr, "Error: ");
- vfprintf(stderr, fmt, ap);
- va_end(ap);
-
- fprintf(stderr, "\n");
- exit(ret);
-}
-
-
/*
* Generic dwarf analysis helpers
*/
@@ -151,11 +139,11 @@ static Dwarf_Unsigned die_get_fileno(Dwarf_Die cu_die, const char *fname)
}

/* Compare diename and tname */
-static int die_compare_name(Dwarf_Die die, const char *tname)
+static int die_compare_name(Dwarf_Die dw_die, const char *tname)
{
char *name;
int ret;
- ret = dwarf_diename(die, &name, &__dw_error);
+ ret = dwarf_diename(dw_die, &name, &__dw_error);
ERR_IF(ret == DW_DLV_ERROR);
if (ret == DW_DLV_OK) {
ret = strcmp(tname, name);
@@ -187,25 +175,25 @@ static int die_within_subprogram(Dwarf_Die sp_die, Dwarf_Addr addr,
}

/* Check the die is inlined function */
-static Dwarf_Bool die_inlined_subprogram(Dwarf_Die die)
+static Dwarf_Bool die_inlined_subprogram(Dwarf_Die dw_die)
{
/* TODO: check strictly */
Dwarf_Bool inl;
int ret;

- ret = dwarf_hasattr(die, DW_AT_inline, &inl, &__dw_error);
+ ret = dwarf_hasattr(dw_die, DW_AT_inline, &inl, &__dw_error);
ERR_IF(ret == DW_DLV_ERROR);
return inl;
}

/* Get the offset of abstruct_origin */
-static Dwarf_Off die_get_abstract_origin(Dwarf_Die die)
+static Dwarf_Off die_get_abstract_origin(Dwarf_Die dw_die)
{
Dwarf_Attribute attr;
Dwarf_Off cu_offs;
int ret;

- ret = dwarf_attr(die, DW_AT_abstract_origin, &attr, &__dw_error);
+ ret = dwarf_attr(dw_die, DW_AT_abstract_origin, &attr, &__dw_error);
ERR_IF(ret != DW_DLV_OK);
ret = dwarf_formref(attr, &cu_offs, &__dw_error);
ERR_IF(ret != DW_DLV_OK);
@@ -214,7 +202,7 @@ static Dwarf_Off die_get_abstract_origin(Dwarf_Die die)
}

/* Get entry pc(or low pc, 1st entry of ranges) of the die */
-static Dwarf_Addr die_get_entrypc(Dwarf_Die die)
+static Dwarf_Addr die_get_entrypc(Dwarf_Die dw_die)
{
Dwarf_Attribute attr;
Dwarf_Addr addr;
@@ -224,7 +212,7 @@ static Dwarf_Addr die_get_entrypc(Dwarf_Die die)
int ret;

/* Try to get entry pc */
- ret = dwarf_attr(die, DW_AT_entry_pc, &attr, &__dw_error);
+ ret = dwarf_attr(dw_die, DW_AT_entry_pc, &attr, &__dw_error);
ERR_IF(ret == DW_DLV_ERROR);
if (ret == DW_DLV_OK) {
ret = dwarf_formaddr(attr, &addr, &__dw_error);
@@ -234,13 +222,13 @@ static Dwarf_Addr die_get_entrypc(Dwarf_Die die)
}

/* Try to get low pc */
- ret = dwarf_lowpc(die, &addr, &__dw_error);
+ ret = dwarf_lowpc(dw_die, &addr, &__dw_error);
ERR_IF(ret == DW_DLV_ERROR);
if (ret == DW_DLV_OK)
return addr;

/* Try to get ranges */
- ret = dwarf_attr(die, DW_AT_ranges, &attr, &__dw_error);
+ ret = dwarf_attr(dw_die, DW_AT_ranges, &attr, &__dw_error);
ERR_IF(ret != DW_DLV_OK);
ret = dwarf_formref(attr, &offs, &__dw_error);
ERR_IF(ret != DW_DLV_OK);
@@ -382,11 +370,11 @@ static void show_location(Dwarf_Loc *loc, struct probe_finder *pf)
} else if (op == DW_OP_regx) {
regn = loc->lr_number;
} else
- msg_exit(-EINVAL, "Dwarf_OP %d is not supported.\n", op);
+ die("Dwarf_OP %d is not supported.\n", op);

regs = get_arch_regstr(regn);
if (!regs)
- msg_exit(-EINVAL, "%lld exceeds max register number.\n", regn);
+ die("%lld exceeds max register number.\n", regn);

if (deref)
ret = snprintf(pf->buf, pf->len,
@@ -417,8 +405,8 @@ static void show_variable(Dwarf_Die vr_die, struct probe_finder *pf)
dwarf_dealloc(__dw_debug, attr, DW_DLA_ATTR);
return ;
error:
- msg_exit(-1, "Failed to find the location of %s at this address.\n"
- " Perhaps, it was optimized out.\n", pf->var);
+ die("Failed to find the location of %s at this address.\n"
+ " Perhaps, it has been optimized out.\n", pf->var);
}

static int variable_callback(struct die_link *dlink, void *data)
@@ -456,8 +444,7 @@ static void find_variable(Dwarf_Die sp_die, struct probe_finder *pf)
/* Search child die for local variables and parameters. */
ret = search_die_from_children(sp_die, variable_callback, pf);
if (!ret)
- msg_exit(-1, "Failed to find '%s' in this function.\n",
- pf->var);
+ die("Failed to find '%s' in this function.\n", pf->var);
}

/* Get a frame base on the address */
@@ -568,8 +555,7 @@ static void find_by_line(Dwarf_Die cu_die, struct probe_finder *pf)
/* Search a real subprogram including this line, */
ret = search_die_from_children(cu_die, probeaddr_callback, pf);
if (ret == 0)
- msg_exit(-1,
- "Probe point is not found in subprograms.\n");
+ die("Probe point is not found in subprograms.\n");
/* Continuing, because target line might be inlined. */
}
dwarf_srclines_dealloc(__dw_debug, lines, cnt);
@@ -621,7 +607,7 @@ static int probefunc_callback(struct die_link *dlink, void *data)
!die_inlined_subprogram(lk->die))
goto found;
}
- msg_exit(-1, "Failed to find real subprogram.\n");
+ die("Failed to find real subprogram.\n");
found:
/* Get offset from subprogram */
ret = die_within_subprogram(lk->die, pf->addr, &offs);
@@ -649,8 +635,7 @@ int find_probepoint(int fd, struct probe_point *pp)

ret = dwarf_init(fd, DW_DLC_READ, 0, 0, &__dw_debug, &__dw_error);
if (ret != DW_DLV_OK)
- msg_exit(-1, "Failed to call dwarf_init(). "
- "Maybe, not a dwarf file?\n");
+ die("Failed to call dwarf_init(). Maybe, not a dwarf file.\n");

pp->found = 0;
while (++cu_number) {

2009-10-17 10:06:56

by Masami Hiramatsu

[permalink] [raw]
Subject: [tip:perf/probes] perf: Use eprintf() for debug messages in perf-probe

Commit-ID: 89c69c0eee7515cdc217f4278de43547284b3458
Gitweb: http://git.kernel.org/tip/89c69c0eee7515cdc217f4278de43547284b3458
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Fri, 16 Oct 2009 20:08:10 -0400
Committer: Ingo Molnar <[email protected]>
CommitDate: Sat, 17 Oct 2009 09:54:00 +0200

perf: Use eprintf() for debug messages in perf-probe

Replace debug() macro with eprintf() and add -v option for
showing those messages in perf-probe.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
tools/perf/builtin-probe.c | 23 ++++++++++++++---------
tools/perf/util/probe-finder.c | 12 +++++++-----
tools/perf/util/probe-finder.h | 7 -------
3 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index a1467d1..b5ad86a 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -35,6 +35,8 @@
#include "perf.h"
#include "builtin.h"
#include "util/util.h"
+#include "util/event.h"
+#include "util/debug.h"
#include "util/parse-options.h"
#include "util/parse-events.h" /* For debugfs_path */
#include "util/probe-finder.h"
@@ -76,7 +78,7 @@ static int parse_probepoint(const struct option *opt __used,
if (!str) /* The end of probe points */
return 0;

- debug("Probe-define(%d): %s\n", session.nr_probe, str);
+ eprintf("probe-definition(%d): %s\n", session.nr_probe, str);
if (++session.nr_probe == MAX_PROBES)
semantic_error("Too many probes");

@@ -101,7 +103,7 @@ static int parse_probepoint(const struct option *opt __used,
die("strndup");
if (++argc == MAX_PROBE_ARGS)
semantic_error("Too many arguments");
- debug("argv[%d]=%s\n", argc, argv[argc - 1]);
+ eprintf("argv[%d]=%s\n", argc, argv[argc - 1]);
}
} while (*str != '\0');
if (argc < 2)
@@ -131,7 +133,7 @@ static int parse_probepoint(const struct option *opt __used,
pp->line = atoi(ptr);
if (!pp->file || !pp->line)
semantic_error("Failed to parse line.");
- debug("file:%s line:%d\n", pp->file, pp->line);
+ eprintf("file:%s line:%d\n", pp->file, pp->line);
} else {
/* Function name */
ptr = strchr(arg, '+');
@@ -148,7 +150,7 @@ static int parse_probepoint(const struct option *opt __used,
pp->file = strdup(ptr);
}
pp->function = strdup(arg);
- debug("symbol:%s file:%s offset:%d\n",
+ eprintf("symbol:%s file:%s offset:%d\n",
pp->function, pp->file, pp->offset);
}
free(argv[1]);
@@ -173,7 +175,7 @@ static int parse_probepoint(const struct option *opt __used,
session.need_dwarf = 1;
}

- debug("%d arguments\n", pp->nr_args);
+ eprintf("%d arguments\n", pp->nr_args);
return 0;
}

@@ -186,7 +188,7 @@ static int open_default_vmlinux(void)

ret = uname(&uts);
if (ret) {
- debug("uname() failed.\n");
+ eprintf("uname() failed.\n");
return -errno;
}
session.release = uts.release;
@@ -194,11 +196,12 @@ static int open_default_vmlinux(void)
ret = snprintf(fname, MAX_PATH_LEN,
default_search_path[i], session.release);
if (ret >= MAX_PATH_LEN || ret < 0) {
- debug("Filename(%d,%s) is too long.\n", i, uts.release);
+ eprintf("Filename(%d,%s) is too long.\n", i,
+ uts.release);
errno = E2BIG;
return -E2BIG;
}
- debug("try to open %s\n", fname);
+ eprintf("try to open %s\n", fname);
fd = open(fname, O_RDONLY);
if (fd >= 0)
break;
@@ -213,6 +216,8 @@ static const char * const probe_usage[] = {
};

static const struct option options[] = {
+ OPT_BOOLEAN('v', "verbose", &verbose,
+ "be more verbose (show parsed arguments, etc)"),
#ifndef NO_LIBDWARF
OPT_STRING('k', "vmlinux", &session.vmlinux, "file",
"vmlinux/module pathname"),
@@ -336,7 +341,7 @@ int cmd_probe(int argc, const char **argv, const char *prefix __used)
ret = find_probepoint(fd, pp);
if (ret <= 0)
die("No probe point found.\n");
- debug("probe event %s found\n", session.events[j]);
+ eprintf("probe event %s found\n", session.events[j]);
}
close(fd);

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 338fdb9..db24c91 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -32,6 +32,8 @@
#include <stdarg.h>
#include <ctype.h>

+#include "event.h"
+#include "debug.h"
#include "util.h"
#include "probe-finder.h"

@@ -134,7 +136,7 @@ static Dwarf_Unsigned die_get_fileno(Dwarf_Die cu_die, const char *fname)
dwarf_dealloc(__dw_debug, srcs, DW_DLA_LIST);
}
if (found)
- debug("found fno: %d\n", (int)found);
+ eprintf("found fno: %d\n", (int)found);
return found;
}

@@ -440,7 +442,7 @@ static void find_variable(Dwarf_Die sp_die, struct probe_finder *pf)
return ;
}

- debug("Searching '%s' variable in context.\n", pf->var);
+ eprintf("Searching '%s' variable in context.\n", pf->var);
/* Search child die for local variables and parameters. */
ret = search_die_from_children(sp_die, variable_callback, pf);
if (!ret)
@@ -550,7 +552,7 @@ static void find_by_line(Dwarf_Die cu_die, struct probe_finder *pf)

ret = dwarf_lineaddr(lines[i], &addr, &__dw_error);
ERR_IF(ret != DW_DLV_OK);
- debug("Probe point found: 0x%llx\n", addr);
+ eprintf("Probe point found: 0x%llx\n", addr);
pf->addr = addr;
/* Search a real subprogram including this line, */
ret = search_die_from_children(cu_die, probeaddr_callback, pf);
@@ -581,7 +583,7 @@ static int probefunc_callback(struct die_link *dlink, void *data)
&pf->inl_offs,
&__dw_error);
ERR_IF(ret != DW_DLV_OK);
- debug("inline definition offset %lld\n",
+ eprintf("inline definition offset %lld\n",
pf->inl_offs);
return 0;
}
@@ -597,7 +599,7 @@ static int probefunc_callback(struct die_link *dlink, void *data)
/* Get probe address */
pf->addr = die_get_entrypc(dlink->die);
pf->addr += pp->offset;
- debug("found inline addr: 0x%llx\n", pf->addr);
+ eprintf("found inline addr: 0x%llx\n", pf->addr);
/* Inlined function. Get a real subprogram */
for (lk = dlink->parent; lk != NULL; lk = lk->parent) {
tag = 0;
diff --git a/tools/perf/util/probe-finder.h b/tools/perf/util/probe-finder.h
index 306810c..6a7cb0c 100644
--- a/tools/perf/util/probe-finder.h
+++ b/tools/perf/util/probe-finder.h
@@ -4,13 +4,6 @@
#define _stringify(n) #n
#define stringify(n) _stringify(n)

-#ifdef DEBUG
-#define debug(fmt ...) \
- fprintf(stderr, "DBG(" __FILE__ ":" stringify(__LINE__) "): " fmt)
-#else
-#define debug(fmt ...) do {} while (0)
-#endif
-
#define ERR_IF(cnd) \
do { if (cnd) { \
fprintf(stderr, "Error (" __FILE__ ":" stringify(__LINE__) \

2009-10-17 10:07:11

by Masami Hiramatsu

[permalink] [raw]
Subject: [tip:perf/probes] perf: Add DIE_IF() macro for error checking

Commit-ID: 9769833b8e4425dc93fc837bf124c6cb02a51abb
Gitweb: http://git.kernel.org/tip/9769833b8e4425dc93fc837bf124c6cb02a51abb
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Fri, 16 Oct 2009 20:08:18 -0400
Committer: Ingo Molnar <[email protected]>
CommitDate: Sat, 17 Oct 2009 09:54:01 +0200

perf: Add DIE_IF() macro for error checking

Add DIE_IF() macro and replace ERR_IF() with it, and use
linux/stringify.h.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
tools/perf/Makefile | 1 +
tools/perf/util/probe-finder.c | 82 ++++++++++++++++++++--------------------
tools/perf/util/probe-finder.h | 10 -----
tools/perf/util/util.h | 9 ++++
4 files changed, 51 insertions(+), 51 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 03c27b9..1abbf9a 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -321,6 +321,7 @@ LIB_FILE=libperf.a
LIB_H += ../../include/linux/perf_event.h
LIB_H += ../../include/linux/rbtree.h
LIB_H += ../../include/linux/list.h
+LIB_H += ../../include/linux/stringify.h
LIB_H += util/include/linux/list.h
LIB_H += perf.h
LIB_H += util/types.h
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index db24c91..be997ab 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -146,7 +146,7 @@ static int die_compare_name(Dwarf_Die dw_die, const char *tname)
char *name;
int ret;
ret = dwarf_diename(dw_die, &name, &__dw_error);
- ERR_IF(ret == DW_DLV_ERROR);
+ DIE_IF(ret == DW_DLV_ERROR);
if (ret == DW_DLV_OK) {
ret = strcmp(tname, name);
dwarf_dealloc(__dw_debug, name, DW_DLA_STRING);
@@ -164,11 +164,11 @@ static int die_within_subprogram(Dwarf_Die sp_die, Dwarf_Addr addr,

/* TODO: check ranges */
ret = dwarf_lowpc(sp_die, &lopc, &__dw_error);
- ERR_IF(ret == DW_DLV_ERROR);
+ DIE_IF(ret == DW_DLV_ERROR);
if (ret == DW_DLV_NO_ENTRY)
return 0;
ret = dwarf_highpc(sp_die, &hipc, &__dw_error);
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);
if (lopc <= addr && addr < hipc) {
*offs = addr - lopc;
return 1;
@@ -184,7 +184,7 @@ static Dwarf_Bool die_inlined_subprogram(Dwarf_Die dw_die)
int ret;

ret = dwarf_hasattr(dw_die, DW_AT_inline, &inl, &__dw_error);
- ERR_IF(ret == DW_DLV_ERROR);
+ DIE_IF(ret == DW_DLV_ERROR);
return inl;
}

@@ -196,9 +196,9 @@ static Dwarf_Off die_get_abstract_origin(Dwarf_Die dw_die)
int ret;

ret = dwarf_attr(dw_die, DW_AT_abstract_origin, &attr, &__dw_error);
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);
ret = dwarf_formref(attr, &cu_offs, &__dw_error);
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);
dwarf_dealloc(__dw_debug, attr, DW_DLA_ATTR);
return cu_offs;
}
@@ -215,28 +215,28 @@ static Dwarf_Addr die_get_entrypc(Dwarf_Die dw_die)

/* Try to get entry pc */
ret = dwarf_attr(dw_die, DW_AT_entry_pc, &attr, &__dw_error);
- ERR_IF(ret == DW_DLV_ERROR);
+ DIE_IF(ret == DW_DLV_ERROR);
if (ret == DW_DLV_OK) {
ret = dwarf_formaddr(attr, &addr, &__dw_error);
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);
dwarf_dealloc(__dw_debug, attr, DW_DLA_ATTR);
return addr;
}

/* Try to get low pc */
ret = dwarf_lowpc(dw_die, &addr, &__dw_error);
- ERR_IF(ret == DW_DLV_ERROR);
+ DIE_IF(ret == DW_DLV_ERROR);
if (ret == DW_DLV_OK)
return addr;

/* Try to get ranges */
ret = dwarf_attr(dw_die, DW_AT_ranges, &attr, &__dw_error);
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);
ret = dwarf_formref(attr, &offs, &__dw_error);
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);
ret = dwarf_get_ranges(__dw_debug, offs, &ranges, &cnt, NULL,
&__dw_error);
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);
addr = ranges[0].dwr_addr1;
dwarf_ranges_dealloc(__dw_debug, ranges, cnt);
return addr;
@@ -261,7 +261,7 @@ static int __search_die_tree(struct die_link *cur_link,
while (!(ret = die_cb(cur_link, data))) {
/* Check child die */
ret = dwarf_child(cur_link->die, &new_die, &__dw_error);
- ERR_IF(ret == DW_DLV_ERROR);
+ DIE_IF(ret == DW_DLV_ERROR);
if (ret == DW_DLV_OK) {
new_link.parent = cur_link;
new_link.die = new_die;
@@ -273,7 +273,7 @@ static int __search_die_tree(struct die_link *cur_link,
/* Move to next sibling */
ret = dwarf_siblingof(__dw_debug, cur_link->die, &new_die,
&__dw_error);
- ERR_IF(ret == DW_DLV_ERROR);
+ DIE_IF(ret == DW_DLV_ERROR);
dwarf_dealloc(__dw_debug, cur_link->die, DW_DLA_DIE);
cur_link->die = new_die;
if (ret == DW_DLV_NO_ENTRY)
@@ -293,7 +293,7 @@ static int search_die_from_children(Dwarf_Die parent_die,

new_link.parent = NULL;
ret = dwarf_child(parent_die, &new_link.die, &__dw_error);
- ERR_IF(ret == DW_DLV_ERROR);
+ DIE_IF(ret == DW_DLV_ERROR);
if (ret == DW_DLV_OK)
return __search_die_tree(&new_link, die_cb, data);
else
@@ -309,7 +309,7 @@ static int attr_get_locdesc(Dwarf_Attribute attr, Dwarf_Locdesc *desc,
int ret, i;

ret = dwarf_loclist_n(attr, &llbuf, &lcnt, &__dw_error);
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);
ret = DW_DLV_NO_ENTRY;
for (i = 0; i < lcnt; ++i) {
if (llbuf[i]->ld_lopc <= addr &&
@@ -317,7 +317,7 @@ static int attr_get_locdesc(Dwarf_Attribute attr, Dwarf_Locdesc *desc,
memcpy(desc, llbuf[i], sizeof(Dwarf_Locdesc));
desc->ld_s =
malloc(sizeof(Dwarf_Loc) * llbuf[i]->ld_cents);
- ERR_IF(desc->ld_s == NULL);
+ DIE_IF(desc->ld_s == NULL);
memcpy(desc->ld_s, llbuf[i]->ld_s,
sizeof(Dwarf_Loc) * llbuf[i]->ld_cents);
ret = DW_DLV_OK;
@@ -383,8 +383,8 @@ static void show_location(Dwarf_Loc *loc, struct probe_finder *pf)
" %s=%+lld(%s)", pf->var, offs, regs);
else
ret = snprintf(pf->buf, pf->len, " %s=%s", pf->var, regs);
- ERR_IF(ret < 0);
- ERR_IF(ret >= pf->len);
+ DIE_IF(ret < 0);
+ DIE_IF(ret >= pf->len);
}

/* Show a variables in kprobe event format */
@@ -401,7 +401,7 @@ static void show_variable(Dwarf_Die vr_die, struct probe_finder *pf)
if (ret != DW_DLV_OK)
goto error;
/* TODO? */
- ERR_IF(ld.ld_cents != 1);
+ DIE_IF(ld.ld_cents != 1);
show_location(&ld.ld_s[0], pf);
free(ld.ld_s);
dwarf_dealloc(__dw_debug, attr, DW_DLA_ATTR);
@@ -418,7 +418,7 @@ static int variable_callback(struct die_link *dlink, void *data)
int ret;

ret = dwarf_tag(dlink->die, &tag, &__dw_error);
- ERR_IF(ret == DW_DLV_ERROR);
+ DIE_IF(ret == DW_DLV_ERROR);
if ((tag == DW_TAG_formal_parameter ||
tag == DW_TAG_variable) &&
(die_compare_name(dlink->die, pf->var) == 0)) {
@@ -437,8 +437,8 @@ static void find_variable(Dwarf_Die sp_die, struct probe_finder *pf)
if (!is_c_varname(pf->var)) {
/* Output raw parameters */
ret = snprintf(pf->buf, pf->len, " %s", pf->var);
- ERR_IF(ret < 0);
- ERR_IF(ret >= pf->len);
+ DIE_IF(ret < 0);
+ DIE_IF(ret >= pf->len);
return ;
}

@@ -456,9 +456,9 @@ static void get_current_frame_base(Dwarf_Die sp_die, struct probe_finder *pf)
int ret;

ret = dwarf_attr(sp_die, DW_AT_frame_base, &attr, &__dw_error);
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);
ret = attr_get_locdesc(attr, &pf->fbloc, (pf->addr - pf->cu_base));
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);
dwarf_dealloc(__dw_debug, attr, DW_DLA_ATTR);
}

@@ -479,7 +479,7 @@ static void show_probepoint(Dwarf_Die sp_die, Dwarf_Signed offs,

/* Output name of probe point */
ret = dwarf_diename(sp_die, &name, &__dw_error);
- ERR_IF(ret == DW_DLV_ERROR);
+ DIE_IF(ret == DW_DLV_ERROR);
if (ret == DW_DLV_OK) {
ret = snprintf(tmp, MAX_PROBE_BUFFER, "%s+%u", name,
(unsigned int)offs);
@@ -488,8 +488,8 @@ static void show_probepoint(Dwarf_Die sp_die, Dwarf_Signed offs,
/* This function has no name. */
ret = snprintf(tmp, MAX_PROBE_BUFFER, "0x%llx", pf->addr);
}
- ERR_IF(ret < 0);
- ERR_IF(ret >= MAX_PROBE_BUFFER);
+ DIE_IF(ret < 0);
+ DIE_IF(ret >= MAX_PROBE_BUFFER);
len = ret;

/* Find each argument */
@@ -515,7 +515,7 @@ static int probeaddr_callback(struct die_link *dlink, void *data)
int ret;

ret = dwarf_tag(dlink->die, &tag, &__dw_error);
- ERR_IF(ret == DW_DLV_ERROR);
+ DIE_IF(ret == DW_DLV_ERROR);
/* Check the address is in this subprogram */
if (tag == DW_TAG_subprogram &&
die_within_subprogram(dlink->die, pf->addr, &offs)) {
@@ -537,21 +537,21 @@ static void find_by_line(Dwarf_Die cu_die, struct probe_finder *pf)
int ret;

ret = dwarf_srclines(cu_die, &lines, &cnt, &__dw_error);
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);

for (i = 0; i < cnt; i++) {
ret = dwarf_line_srcfileno(lines[i], &fno, &__dw_error);
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);
if (fno != pf->fno)
continue;

ret = dwarf_lineno(lines[i], &lineno, &__dw_error);
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);
if (lineno != (Dwarf_Unsigned)pp->line)
continue;

ret = dwarf_lineaddr(lines[i], &addr, &__dw_error);
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);
eprintf("Probe point found: 0x%llx\n", addr);
pf->addr = addr;
/* Search a real subprogram including this line, */
@@ -574,7 +574,7 @@ static int probefunc_callback(struct die_link *dlink, void *data)
int ret;

ret = dwarf_tag(dlink->die, &tag, &__dw_error);
- ERR_IF(ret == DW_DLV_ERROR);
+ DIE_IF(ret == DW_DLV_ERROR);
if (tag == DW_TAG_subprogram) {
if (die_compare_name(dlink->die, pp->function) == 0) {
if (die_inlined_subprogram(dlink->die)) {
@@ -582,7 +582,7 @@ static int probefunc_callback(struct die_link *dlink, void *data)
ret = dwarf_die_CU_offset(dlink->die,
&pf->inl_offs,
&__dw_error);
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);
eprintf("inline definition offset %lld\n",
pf->inl_offs);
return 0;
@@ -604,7 +604,7 @@ static int probefunc_callback(struct die_link *dlink, void *data)
for (lk = dlink->parent; lk != NULL; lk = lk->parent) {
tag = 0;
dwarf_tag(lk->die, &tag, &__dw_error);
- ERR_IF(ret == DW_DLV_ERROR);
+ DIE_IF(ret == DW_DLV_ERROR);
if (tag == DW_TAG_subprogram &&
!die_inlined_subprogram(lk->die))
goto found;
@@ -613,7 +613,7 @@ static int probefunc_callback(struct die_link *dlink, void *data)
found:
/* Get offset from subprogram */
ret = die_within_subprogram(lk->die, pf->addr, &offs);
- ERR_IF(!ret);
+ DIE_IF(!ret);
show_probepoint(lk->die, offs, pf);
/* Continue to search */
}
@@ -644,13 +644,13 @@ int find_probepoint(int fd, struct probe_point *pp)
/* Search CU (Compilation Unit) */
ret = dwarf_next_cu_header(__dw_debug, NULL, NULL, NULL,
&addr_size, &next_cuh, &__dw_error);
- ERR_IF(ret == DW_DLV_ERROR);
+ DIE_IF(ret == DW_DLV_ERROR);
if (ret == DW_DLV_NO_ENTRY)
break;

/* Get the DIE(Debugging Information Entry) of this CU */
ret = dwarf_siblingof(__dw_debug, 0, &cu_die, &__dw_error);
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);

/* Check if target file is included. */
if (pp->file)
@@ -659,7 +659,7 @@ int find_probepoint(int fd, struct probe_point *pp)
if (!pp->file || pf.fno) {
/* Save CU base address (for frame_base) */
ret = dwarf_lowpc(cu_die, &pf.cu_base, &__dw_error);
- ERR_IF(ret == DW_DLV_ERROR);
+ DIE_IF(ret == DW_DLV_ERROR);
if (ret == DW_DLV_NO_ENTRY)
pf.cu_base = 0;
if (pp->line)
@@ -670,7 +670,7 @@ int find_probepoint(int fd, struct probe_point *pp)
dwarf_dealloc(__dw_debug, cu_die, DW_DLA_DIE);
}
ret = dwarf_finish(__dw_debug, &__dw_error);
- ERR_IF(ret != DW_DLV_OK);
+ DIE_IF(ret != DW_DLV_OK);

return pp->found;
}
diff --git a/tools/perf/util/probe-finder.h b/tools/perf/util/probe-finder.h
index 6a7cb0c..d17fafc 100644
--- a/tools/perf/util/probe-finder.h
+++ b/tools/perf/util/probe-finder.h
@@ -1,16 +1,6 @@
#ifndef _PROBE_FINDER_H
#define _PROBE_FINDER_H

-#define _stringify(n) #n
-#define stringify(n) _stringify(n)
-
-#define ERR_IF(cnd) \
- do { if (cnd) { \
- fprintf(stderr, "Error (" __FILE__ ":" stringify(__LINE__) \
- "): " stringify(cnd) "\n"); \
- exit(1); \
- } } while (0)
-
#define MAX_PATH_LEN 256
#define MAX_PROBE_BUFFER 1024
#define MAX_PROBES 128
diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h
index 9de2329..0daa341 100644
--- a/tools/perf/util/util.h
+++ b/tools/perf/util/util.h
@@ -134,6 +134,15 @@ extern void die(const char *err, ...) NORETURN __attribute__((format (printf, 1,
extern int error(const char *err, ...) __attribute__((format (printf, 1, 2)));
extern void warning(const char *err, ...) __attribute__((format (printf, 1, 2)));

+#include "../../../include/linux/stringify.h"
+
+#define DIE_IF(cnd) \
+ do { if (cnd) \
+ die(" at (" __FILE__ ":" __stringify(__LINE__) "): " \
+ __stringify(cnd) "\n"); \
+ } while (0)
+
+
extern void set_die_routine(void (*routine)(const char *err, va_list params) NORETURN);

extern int prefixcmp(const char *str, const char *prefix);

2009-10-17 10:09:23

by Masami Hiramatsu

[permalink] [raw]
Subject: [tip:perf/probes] perf: Add perf-probe document

Commit-ID: 595c36490deb49381dc51231a3d5e6b66786ed27
Gitweb: http://git.kernel.org/tip/595c36490deb49381dc51231a3d5e6b66786ed27
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Fri, 16 Oct 2009 20:08:27 -0400
Committer: Ingo Molnar <[email protected]>
CommitDate: Sat, 17 Oct 2009 09:54:01 +0200

perf: Add perf-probe document

Add perf-probe subcommand document and add it to command-list.

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
tools/perf/Documentation/perf-probe.txt | 48 +++++++++++++++++++++++++++++++
tools/perf/command-list.txt | 1 +
2 files changed, 49 insertions(+), 0 deletions(-)

diff --git a/tools/perf/Documentation/perf-probe.txt b/tools/perf/Documentation/perf-probe.txt
new file mode 100644
index 0000000..6b6c6ae
--- /dev/null
+++ b/tools/perf/Documentation/perf-probe.txt
@@ -0,0 +1,48 @@
+perf-probe(1)
+=============
+
+NAME
+----
+perf-probe - Define new dynamic tracepoints
+
+SYNOPSIS
+--------
+[verse]
+'perf probe' [-k <file>] -P 'PROBE' [-P 'PROBE' ...]
+
+
+DESCRIPTION
+-----------
+This command defines dynamic tracepoint events, by symbol and registers
+without debuginfo, or by C expressions (C line numbers, C function names,
+and C local variables) with debuginfo.
+
+
+OPTIONS
+-------
+-k::
+--vmlinux::
+ Specify vmlinux path which has debuginfo (Dwarf binary).
+
+-v::
+--verbose::
+ Be more verbose (show parsed arguments, etc).
+
+-P::
+--probe::
+ Define a probe point (see PROBE SYNTAX for detail)
+
+PROBE SYNTAX
+------------
+Probe points are defined by following syntax.
+
+ "TYPE:[GRP/]NAME FUNC[+OFFS][@SRC]|@SRC:LINE [ARG ...]"
+
+'TYPE' specifies the type of probe point, you can use either "p" (kprobe) or "r" (kretprobe) for 'TYPE'. 'GRP' specifies the group name of this probe, and 'NAME' specifies the event name. If 'GRP' is omitted, "kprobes" is used for its group name.
+'FUNC' and 'OFFS' specifies function and offset (in byte) where probe will be put. In addition, 'SRC' specifies a source file which has that function (this is mainly for inline functions).
+You can specify a probe point by the source line number by using '@SRC:LINE' syntax, where 'SRC' is the source file path and 'LINE' is the line number.
+'ARG' specifies the arguments of this probe point. You can use the name of local variable, or kprobe-tracer argument format (e.g. $retval, %ax, etc).
+
+SEE ALSO
+--------
+linkperf:perf-trace[1], linkperf:perf-record[1]
diff --git a/tools/perf/command-list.txt b/tools/perf/command-list.txt
index 00326e2..6475db4 100644
--- a/tools/perf/command-list.txt
+++ b/tools/perf/command-list.txt
@@ -11,3 +11,4 @@ perf-stat mainporcelain common
perf-timechart mainporcelain common
perf-top mainporcelain common
perf-trace mainporcelain common
+perf-probe mainporcelain common

2009-10-17 10:35:35

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH -tip tracing/kprobes 0/9] tracing/kprobes, perf: perf probe and kprobe-tracer bugfixes


I took a good look at the current bits, and there are a few more things
that need to be fixed before we can consider 'perf probe' for upstream.

Firstly, this decoder bug is still not fixed:

CHK include/linux/compile.h
TEST posttest
Error: ffffffff81068fe0: 66 0f 73 fd 04 pslldq $0x4,%xmm5
Error: objdump says 5 bytes, but insn_get_length() says 4 (attr:8000)
make[1]: *** [posttest] Error 2

64-bit allyesconfig will trigger this for example, but the attached
smaller config too. This needs to be fixed before we can apply any
new commits.

Secondly, the probe syntax is quite non-obvious currently. All the 'p'
and -P gymnastics is just a barrier to the first-time user getting his
first probe inserted successfully.

The user need not worry about whether it's a 'kprobe' or a 'kretprobe'.
The user should _specify_ what it wants to probe, and _that_ will lead
to 'perf probe' picking the most suitable facility to achieve that kind
of probing.

It might be a kprobe, a kretprobe, or an mcount driven function probe,
an existing tracepoint if it happens to be present in that place already
- or some other future mechanism. The driving force must be a robust
specification of 'what', not the mechanics of 'how'.

Considering that, the current 'perf probe' syntax does not achieve that
goal yet.

Here are a few syntax suggestions

The simpest probe syntax should be to add a probe to a single function
name:

perf probe +schedule

_nothing else_.

To remove it, the user should just do something like:

perf probe -schedule

(to be symmetric 'perf probe +schedule' should work as well)

perf probe will make up a synthetic probe name for that - probe-1 for
example. It will also pick the suitable probe mechanism (kprobes).

All the other extensions and possibilities - arguments, variables,
source code lines, etc. should be natural and intuitive extensions of
this basic, minimal syntax.

To insert a simple probe no -P should be needed, 'p', no ':' - no probe
name even.

Furthermore, there should be a way to list existing probes (and only
probes), probably via 'perf list --probes' or 'perf probe --list'.

Plus, 'perf probe --help' should list a few simple examples, beyond the
syntax.

Ok?

Ingo


Attachments:
(No filename) (2.21 kB)
config (64.20 kB)
Download all attachments

2009-10-17 10:37:38

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH -tip tracing/kprobes 0/9] tracing/kprobes, perf: perf probe and kprobe-tracer bugfixes


* Ingo Molnar <[email protected]> wrote:

> The simpest probe syntax should be to add a probe to a single function
> name:
>
> perf probe +schedule
>
> _nothing else_.

here i meant the shortcut without the '+':

perf probe schedule

> To remove it, the user should just do something like:
>
> perf probe -schedule
>
> (to be symmetric 'perf probe +schedule' should work as well)

Ingo

2009-10-18 06:00:53

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH -tip tracing/kprobes 0/9] tracing/kprobes, perf: perf probe and kprobe-tracer bugfixes

Ingo Molnar wrote:
>
> I took a good look at the current bits, and there are a few more things
> that need to be fixed before we can consider 'perf probe' for upstream.
>
> Firstly, this decoder bug is still not fixed:
>
> CHK include/linux/compile.h
> TEST posttest
> Error: ffffffff81068fe0: 66 0f 73 fd 04 pslldq $0x4,%xmm5
> Error: objdump says 5 bytes, but insn_get_length() says 4 (attr:8000)
> make[1]: *** [posttest] Error 2
>
> 64-bit allyesconfig will trigger this for example, but the attached
> smaller config too. This needs to be fixed before we can apply any
> new commits.

Absolutely, yes. Thank you for reporting. I'm checking it again.

> Secondly, the probe syntax is quite non-obvious currently. All the 'p'
> and -P gymnastics is just a barrier to the first-time user getting his
> first probe inserted successfully.

Hmm...

> The user need not worry about whether it's a 'kprobe' or a 'kretprobe'.
> The user should _specify_ what it wants to probe, and _that_ will lead
> to 'perf probe' picking the most suitable facility to achieve that kind
> of probing.
>
> It might be a kprobe, a kretprobe, or an mcount driven function probe,
> an existing tracepoint if it happens to be present in that place already
> - or some other future mechanism. The driving force must be a robust
> specification of 'what', not the mechanics of 'how'.

Agreed.

> Considering that, the current 'perf probe' syntax does not achieve that
> goal yet.
>
> Here are a few syntax suggestions
>
> The simpest probe syntax should be to add a probe to a single function
> name:
>
> perf probe +schedule
>
> _nothing else_.
>
> To remove it, the user should just do something like:
>
> perf probe -schedule
>
> (to be symmetric 'perf probe +schedule' should work as well)

I think '-<symbol>' syntax doesn't work good with other command-line
options and multiple definitions.
(However, it will be good for input-from-file syntax. :-))

So, what would you think about using -D (def) and -U (undef) ?

> perf probe will make up a synthetic probe name for that - probe-1 for
> example. It will also pick the suitable probe mechanism (kprobes).

How about [perfprobe:symbol_offs] ?

> All the other extensions and possibilities - arguments, variables,
> source code lines, etc. should be natural and intuitive extensions of
> this basic, minimal syntax.

Don't you like current space(' ') separated arguments? :-)
I mean, what is 'natural' syntax in your opinion?

>
> To insert a simple probe no -P should be needed, 'p', no ':' - no probe
> name even.

Yeah, return-probe and event-name should be optional.

> Furthermore, there should be a way to list existing probes (and only
> probes), probably via 'perf list --probes' or 'perf probe --list'.

OK. I think perf probe --list will list up all probes including
user-defined ftrace-event via debugfs (not perf-probe), since
perf can use and delete it.

> Plus, 'perf probe --help' should list a few simple examples, beyond the
> syntax.
>
> Ok?

Sure.

Thank you!


--
Masami Hiramatsu

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

e-mail: [email protected]

2009-10-19 07:52:39

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH -tip tracing/kprobes 0/9] tracing/kprobes, perf: perf probe and kprobe-tracer bugfixes


* Masami Hiramatsu <[email protected]> wrote:

> Ingo Molnar wrote:
> >
> > I took a good look at the current bits, and there are a few more things
> > that need to be fixed before we can consider 'perf probe' for upstream.
> >
> > Firstly, this decoder bug is still not fixed:
> >
> > CHK include/linux/compile.h
> > TEST posttest
> > Error: ffffffff81068fe0: 66 0f 73 fd 04 pslldq $0x4,%xmm5
> > Error: objdump says 5 bytes, but insn_get_length() says 4 (attr:8000)
> > make[1]: *** [posttest] Error 2
> >
> > 64-bit allyesconfig will trigger this for example, but the attached
> > smaller config too. This needs to be fixed before we can apply any
> > new commits.
>
> Absolutely, yes. Thank you for reporting. I'm checking it again.

Thanks!

> > Secondly, the probe syntax is quite non-obvious currently. All the
> > 'p' and -P gymnastics is just a barrier to the first-time user
> > getting his first probe inserted successfully.
>
> Hmm...
>
> > The user need not worry about whether it's a 'kprobe' or a
> > 'kretprobe'. The user should _specify_ what it wants to probe, and
> > _that_ will lead to 'perf probe' picking the most suitable facility
> > to achieve that kind of probing.
> >
> > It might be a kprobe, a kretprobe, or an mcount driven function
> > probe, an existing tracepoint if it happens to be present in that
> > place already - or some other future mechanism. The driving force
> > must be a robust specification of 'what', not the mechanics of
> > 'how'.
>
> Agreed.
>
> > Considering that, the current 'perf probe' syntax does not achieve
> > that goal yet.
> >
> > Here are a few syntax suggestions
> >
> > The simpest probe syntax should be to add a probe to a single
> > function name:
> >
> > perf probe +schedule
> >
> > _nothing else_.
> >
> > To remove it, the user should just do something like:
> >
> > perf probe -schedule
> >
> > (to be symmetric 'perf probe +schedule' should work as well)
>
> I think '-<symbol>' syntax doesn't work good with other command-line
> options and multiple definitions. (However, it will be good for
> input-from-file syntax. :-))

dash can be used too - perf has the options library from Git and there's
a wide spectrum of option parsing available, via
tools/perf/util/parse-options.h.

But yes, it complicates things a bit.

> So, what would you think about using -D (def) and -U (undef) ?

The simpest case should be no extra character at all:

perf probe schedule

There's a few well-known command line idioms to add/remove stuff, but -D
/ -U is not one of them i'm afraid =B-)

The following ones might work too:

perf probe +schedule
perf probe -schedule

perf probe add schedule
perf probe del schedule

perf probe --add schedule
perf probe --del schedule

[ Plain 'add/del' has a minor complication as we could have a similar
symbol defined. ]

+ / - is certainly the simplest.

--add/--del works like routes do, so that's intuitive as well. As long
as we have the simple default to add a new probe at a function, without
any extra options we can do this too initially.

> > perf probe will make up a synthetic probe name for that - probe-1
> > for example. It will also pick the suitable probe mechanism
> > (kprobes).
>
> How about [perfprobe:symbol_offs] ?

Yeah, that's a nice idea - naming it after the symbol keeps the probe
namespace still very readable.

> > All the other extensions and possibilities - arguments, variables,
> > source code lines, etc. should be natural and intuitive extensions
> > of this basic, minimal syntax.
>
> Don't you like current space(' ') separated arguments? :-) I mean,
> what is 'natural' syntax in your opinion?

Yeah, space separated arguments are nice too. The question is how to
specify a more precise coordinate for the bit we want to probe - and how
to specify the information we want to extract. Something like:

perf schedule+15

would be a rather intuitive shortcut for '15 lines into the schedule()
function' - and it might even be a largely cross-kernel-version
compatible way of specifying probe points.

Or this:

perf schedule:'switch_count = &prev->nivcsw'

would insert the probe to the source code that matches that statement
pattern. Rarely will people want to insert a probe to an absolutely line
number - that's a usage mode for higher level tools. (so we definitely
want to support it - but it should not use up valuable spots in our
options space.) Same goes for symbol offsets, etc. - humans will rarely
use them.

We also want to have functionality that helps people find probe spots
within a function:

perf probe --list-lines schedule

Would list the line numbers and source code of the schedule() function.
(similar to how GDB 'list' works) That way someone can have an ad-hoc
session of deciding what place to probe, and the line numbers make for
an easy ID of the statement to probe.

Anyway, these details make or break the actual utility of this tool, so
lets iterate this some more and we'll see the limitations and the needs
in practice. As usual, tool design rarely survives first contact with an
actual user - so we have to show some adaptibility ;-)

Ingo

2009-10-19 11:01:01

by Frederic Weisbecker

[permalink] [raw]
Subject: Re: [PATCH -tip tracing/kprobes 0/9] tracing/kprobes, perf: perf probe and kprobe-tracer bugfixes

On Mon, Oct 19, 2009 at 09:51:03AM +0200, Ingo Molnar wrote:
> > So, what would you think about using -D (def) and -U (undef) ?
>
> The simpest case should be no extra character at all:
>
> perf probe schedule


Yeah, I really prefer that too.



> > > All the other extensions and possibilities - arguments, variables,
> > > source code lines, etc. should be natural and intuitive extensions
> > > of this basic, minimal syntax.
> >
> > Don't you like current space(' ') separated arguments? :-) I mean,
> > what is 'natural' syntax in your opinion?
>
> Yeah, space separated arguments are nice too. The question is how to
> specify a more precise coordinate for the bit we want to probe - and how
> to specify the information we want to extract. Something like:
>
> perf schedule+15


I personally don't imagine common easy usecases that imply relative line
offsets but rather absolute lines.

I guess the most immediate usecase is a direct function probe:

perf probe schedule

Just to know if a function is matched.

If you want more precision, it also means you have you code editor opened
and want to set a precise point. Since you also have the absolute
line directly displayed by your editor, you don't want to calculate the relative
line but rather the absolute one.

Hmm?

Hence I rather imagine the following:

perf probe schedule.c:line

(Unfortunately, schedule:line is shorter but less intuitive
but that could be a shortcut).



> Or this:
>
> perf schedule:'switch_count = &prev->nivcsw'
>
> would insert the probe to the source code that matches that statement
> pattern. Rarely will people want to insert a probe to an absolutely line
> number - that's a usage mode for higher level tools. (so we definitely
> want to support it - but it should not use up valuable spots in our
> options space.) Same goes for symbol offsets, etc. - humans will rarely
> use them.



I don't understand your point. If your editor is opened and you have
the source code in front of you, why would you cut'n'paste a line instead
of actually write the line number?



>
> We also want to have functionality that helps people find probe spots
> within a function:
>
> perf probe --list-lines schedule
>
> Would list the line numbers and source code of the schedule() function.
> (similar to how GDB 'list' works) That way someone can have an ad-hoc
> session of deciding what place to probe, and the line numbers make for
> an easy ID of the statement to probe.


Agreed!

Thanks.

2009-10-19 11:22:48

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH -tip tracing/kprobes 0/9] tracing/kprobes, perf: perf probe and kprobe-tracer bugfixes


* Frederic Weisbecker <[email protected]> wrote:

> On Mon, Oct 19, 2009 at 09:51:03AM +0200, Ingo Molnar wrote:
> > > So, what would you think about using -D (def) and -U (undef) ?
> >
> > The simpest case should be no extra character at all:
> >
> > perf probe schedule
>
> Yeah, I really prefer that too.
>
>
>
> > > > All the other extensions and possibilities - arguments,
> > > > variables, source code lines, etc. should be natural and
> > > > intuitive extensions of this basic, minimal syntax.
> > >
> > > Don't you like current space(' ') separated arguments? :-) I mean,
> > > what is 'natural' syntax in your opinion?
> >
> > Yeah, space separated arguments are nice too. The question is how to
> > specify a more precise coordinate for the bit we want to probe - and
> > how to specify the information we want to extract. Something like:
> >
> > perf schedule+15
>
> I personally don't imagine common easy usecases that imply relative
> line offsets but rather absolute lines.

Absolute line numbers could be expressed too, via:

perf schedule@5396

There's no immediate need to express 'sched.c'. (you can do it, or you
can do it for extra clarity or for the case of local scope functions of
which multiple instances exist)

> I guess the most immediate usecase is a direct function probe:
>
> perf probe schedule
>
> Just to know if a function is matched.

Correct.

> If you want more precision, it also means you have you code editor
> opened and want to set a precise point. Since you also have the
> absolute line directly displayed by your editor, you don't want to
> calculate the relative line but rather the absolute one.

Not necessarily - see below:

> Hmm?
>
> Hence I rather imagine the following:
>
> perf probe schedule.c:line
>
> (Unfortunately, schedule:line is shorter but less intuitive but that
> could be a shortcut).
>
> > Or this:
> >
> > perf schedule:'switch_count = &prev->nivcsw'
> >
> > would insert the probe to the source code that matches that statement
> > pattern. Rarely will people want to insert a probe to an absolutely line
> > number - that's a usage mode for higher level tools. (so we definitely
> > want to support it - but it should not use up valuable spots in our
> > options space.) Same goes for symbol offsets, etc. - humans will rarely
> > use them.
>
> I don't understand your point. If your editor is opened and you have
> the source code in front of you, why would you cut'n'paste a line
> instead of actually write the line number?

The 'perf probe --list schedule' sub-tool i outlined would display
relative line numbers for the function - starting at 0:

000 /*
001 * schedule() is the main scheduler function.
002 */
003 asmlinkage void __sched schedule(void)
004 {
005 struct task_struct *prev, *next;
006 unsigned long *switch_count;
007 struct rq *rq;
008 int cpu;
009
010 need_resched:
011 preempt_disable();
012 cpu = smp_processor_id();
013 rq = cpu_rq(cpu);
014 rcu_sched_qs(cpu);
015 prev = rq->curr;
016 switch_count = &prev->nivcsw;
017
018 release_kernel_lock(prev);

That way the following two are equivalent:

perf probe schedule
perf probe schedule+0

The advantage of relative line numbers is that they are much more
version invariant than absolute line numbers. Relative line numbers into
schedule() will only change if the function itself changes.

This means that expressions like 'schedule+16' will have a lot longer
life-time than absolute line number driven probes. You can quote it in
an email and chances are that it will still be valid even a few kernel
releases down the road.

> > We also want to have functionality that helps people find probe
> > spots within a function:
> >
> > perf probe --list-lines schedule
> >
> > Would list the line numbers and source code of the schedule()
> > function. (similar to how GDB 'list' works) That way someone can
> > have an ad-hoc session of deciding what place to probe, and the line
> > numbers make for an easy ID of the statement to probe.
>
> Agreed!

Furthermore - to answer another question you raised above - the
following syntax:

perf probe schedule:'switch_count = &prev->nivcsw'

is basically a 'fuzzy string match' based probe to within a function.

For example you might want to probe the point within schedule that calls
switch_mm() - this could be done via:

perf probe schedule@switch_mm

Or the point where 'next' gets assigned? Sure, you dont need to even
open the editor, if you know the rough outline of the function you can
probe it via:

perf probe schedule@'next ='

Note that i was able to specify both probes without having opened an
editor - just based on the general knowledge of the scheduler.

The point is to prefer intuitive, non-mechanic, fundamentally human
expressions of information above mechanic ones (absolute line numbers,
addresses, ways of probing, etc.) - and to have a rich variety of them.

String based pattern matching and intuitive syntax that reuses existing
paradigms of arithmetics and pattern-matching is good - limited syntax
and extra, arbitrary syntactic hoops to jump through is bad.

If we provide all that, people will start using this stuff - and i'd
only like to merge this upstream once it's clear that people like me
will (be able to) use this facility for ad-hoc probe insertion.

In other words: this facility has to 'live within' our source code and
has to be able to interact with it on a very broad basis - for it to be
maximally useful for everyday development.

Ingo

2009-10-19 16:19:14

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH -tip tracing/kprobes 0/9] tracing/kprobes, perf: perf probe and kprobe-tracer bugfixes

Em Mon, Oct 19, 2009 at 01:00:57PM +0200, Frederic Weisbecker escreveu:
> On Mon, Oct 19, 2009 at 09:51:03AM +0200, Ingo Molnar wrote:
> > > So, what would you think about using -D (def) and -U (undef) ?
> > The simpest case should be no extra character at all:
> > perf probe schedule
> Yeah, I really prefer that too.

/me too

> If you want more precision, it also means you have you code editor opened
> and want to set a precise point. Since you also have the absolute
> line directly displayed by your editor, you don't want to calculate the relative
> line but rather the absolute one.

Perhaps this should come with vim/emacs key bindings so that all this
gets hidden by just opening the source code and pressing
control+SOMEKEY?

- Arnaldo

2009-10-19 18:55:15

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH -tip tracing/kprobes 0/9] tracing/kprobes, perf: perf probe and kprobe-tracer bugfixes

Ingo Molnar wrote:
>>> Here are a few syntax suggestions
>>>
>>> The simpest probe syntax should be to add a probe to a single
>>> function name:
>>>
>>> perf probe +schedule
>>>
>>> _nothing else_.
>>>
>>> To remove it, the user should just do something like:
>>>
>>> perf probe -schedule
>>>
>>> (to be symmetric 'perf probe +schedule' should work as well)
>>
>> I think '-<symbol>' syntax doesn't work good with other command-line
>> options and multiple definitions. (However, it will be good for
>> input-from-file syntax. :-))
>
> dash can be used too - perf has the options library from Git and there's
> a wide spectrum of option parsing available, via
> tools/perf/util/parse-options.h.
>
> But yes, it complicates things a bit.

Yeah, what I'm concerning about is that user will confuse when deleting
probe points which starts with other option, like 'k'.
(-kmalloc can mean -k malloc too)

>> So, what would you think about using -D (def) and -U (undef) ?
>
> The simpest case should be no extra character at all:
>
> perf probe schedule
>
> There's a few well-known command line idioms to add/remove stuff, but -D
> / -U is not one of them i'm afraid =B-)
>
> The following ones might work too:
>
> perf probe +schedule
> perf probe -schedule
>
> perf probe add schedule
> perf probe del schedule
>
> perf probe --add schedule
> perf probe --del schedule
>
> [ Plain 'add/del' has a minor complication as we could have a similar
> symbol defined. ]
>
> + / - is certainly the simplest.
>
> --add/--del works like routes do, so that's intuitive as well. As long
> as we have the simple default to add a new probe at a function, without
> any extra options we can do this too initially.

How about the following syntax?
<adding>
perf probe schedule
perf probe --add schedule

<deleting>
perf probe --del schedule
perf probe --del all /* delete all probepoints */

So, this doesn't symmetric, but provides simple way to add a probe.

>>> All the other extensions and possibilities - arguments, variables,
>>> source code lines, etc. should be natural and intuitive extensions
>>> of this basic, minimal syntax.
>>
>> Don't you like current space(' ') separated arguments? :-) I mean,
>> what is 'natural' syntax in your opinion?
>
> Yeah, space separated arguments are nice too. The question is how to
> specify a more precise coordinate for the bit we want to probe - and how
> to specify the information we want to extract. Something like:
>
> perf schedule+15
>
> would be a rather intuitive shortcut for '15 lines into the schedule()
> function' - and it might even be a largely cross-kernel-version
> compatible way of specifying probe points.

I agreed with the cross-kernel-version issue. I'd rather like

perf probe symbol:relative-line

and

perf probe file:absolute-line

since it will be familiar for GDB users.

And I'd like to preserve

perf probe symbol+offs-byte

for assembly users who might want to trace assembly code with
objdump.

> Or this:
>
> perf schedule:'switch_count = &prev->nivcsw'
>
> would insert the probe to the source code that matches that statement
> pattern. Rarely will people want to insert a probe to an absolutely line
> number - that's a usage mode for higher level tools. (so we definitely
> want to support it - but it should not use up valuable spots in our
> options space.) Same goes for symbol offsets, etc. - humans will rarely
> use them.

Hmm, maybe, it's possible. I should investigate dwarf more...


Thank you!

--
Masami Hiramatsu

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

e-mail: [email protected]

2009-10-19 19:32:53

by Frederic Weisbecker

[permalink] [raw]
Subject: Re: [PATCH -tip tracing/kprobes 0/9] tracing/kprobes, perf: perf probe and kprobe-tracer bugfixes

On Mon, Oct 19, 2009 at 01:21:25PM +0200, Ingo Molnar wrote:
>
> * Frederic Weisbecker <[email protected]> wrote:
> > I personally don't imagine common easy usecases that imply relative
> > line offsets but rather absolute lines.
>
> Absolute line numbers could be expressed too, via:
>
> perf schedule@5396
>
> There's no immediate need to express 'sched.c'. (you can do it, or you
> can do it for extra clarity or for the case of local scope functions of
> which multiple instances exist)



Ok. As long as there is an easy way to do that, like in the above
example, then I think it's fine.

(More argument below)


> > I don't understand your point. If your editor is opened and you have
> > the source code in front of you, why would you cut'n'paste a line
> > instead of actually write the line number?
>
> The 'perf probe --list schedule' sub-tool i outlined would display
> relative line numbers for the function - starting at 0:
>
> 000 /*
> 001 * schedule() is the main scheduler function.
> 002 */
> 003 asmlinkage void __sched schedule(void)
> 004 {
> 005 struct task_struct *prev, *next;
> 006 unsigned long *switch_count;
> 007 struct rq *rq;
> 008 int cpu;
> 009
> 010 need_resched:
> 011 preempt_disable();
> 012 cpu = smp_processor_id();
> 013 rq = cpu_rq(cpu);
> 014 rcu_sched_qs(cpu);
> 015 prev = rq->curr;
> 016 switch_count = &prev->nivcsw;
> 017
> 018 release_kernel_lock(prev);
>
> That way the following two are equivalent:
>
> perf probe schedule
> perf probe schedule+0
>
> The advantage of relative line numbers is that they are much more
> version invariant than absolute line numbers. Relative line numbers into
> schedule() will only change if the function itself changes.
>
> This means that expressions like 'schedule+16' will have a lot longer
> life-time than absolute line number driven probes. You can quote it in
> an email and chances are that it will still be valid even a few kernel
> releases down the road.


I haven't thought that relative lines would make it less short-lived.
So yeah that's a good point.

I think absolute and relative line modes are not colliding/contending
at all but actually fit two different needs.

- absolute is nice when you are lonely doing kernel debugging.
(can be expanded at will once you imagine user probes)
You are stuck in your code editor, trying to figure out the
origin of your problem and then you think it would be nice
to set a probe in branch 1 and in branch 2 inside func_foo().
Then you already have absolute lines and relying in
perf probe --list func_foo() to resolve an absolute line into
a relative one is a very undesired middle step.

- relative is nice in some other cases. When you already have
the function target in mind, you even don't need to check your
editor, just a quick check to this command and get the relative
line. But also when you want to transmit a probe reference
in a mailing list because of its better lifetime.


Both are important IMO.


> Furthermore - to answer another question you raised above - the
> following syntax:
>
> perf probe schedule:'switch_count = &prev->nivcsw'
>
> is basically a 'fuzzy string match' based probe to within a function.
>
> For example you might want to probe the point within schedule that calls
> switch_mm() - this could be done via:
>
> perf probe schedule@switch_mm
>
> Or the point where 'next' gets assigned? Sure, you dont need to even
> open the editor, if you know the rough outline of the function you can
> probe it via:
>
> perf probe schedule@'next ='



Yeah, that's a nice advanced usage.


>
> Note that i was able to specify both probes without having opened an
> editor - just based on the general knowledge of the scheduler.


Yeah, that a good workflow for someone who knows well the targeted
code.

That can't cover every needs though, but I guess you agree that
abs_line/rel_line/smart_resolution methods need to coexist anyway,
all of them fit different level of needs.


> The point is to prefer intuitive, non-mechanic, fundamentally human
> expressions of information above mechanic ones (absolute line numbers,
> addresses, ways of probing, etc.) - and to have a rich variety of them.


Agreed!


> String based pattern matching and intuitive syntax that reuses existing
> paradigms of arithmetics and pattern-matching is good - limited syntax
> and extra, arbitrary syntactic hoops to jump through is bad.
>
> If we provide all that, people will start using this stuff - and i'd
> only like to merge this upstream once it's clear that people like me
> will (be able to) use this facility for ad-hoc probe insertion.
>
> In other words: this facility has to 'live within' our source code and
> has to be able to interact with it on a very broad basis - for it to be
> maximally useful for everyday development.
>
> Ingo


Ok.

Thanks.

2009-10-19 19:50:25

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH -tip tracing/kprobes 0/9] tracing/kprobes, perf: perf probe and kprobe-tracer bugfixes

Ingo Molnar wrote:
> The 'perf probe --list schedule' sub-tool i outlined would display
> relative line numbers for the function - starting at 0:
>
> 000 /*
> 001 * schedule() is the main scheduler function.
> 002 */
> 003 asmlinkage void __sched schedule(void)
> 004 {
> 005 struct task_struct *prev, *next;
> 006 unsigned long *switch_count;
> 007 struct rq *rq;
> 008 int cpu;
> 009
> 010 need_resched:
> 011 preempt_disable();
> 012 cpu = smp_processor_id();
> 013 rq = cpu_rq(cpu);
> 014 rcu_sched_qs(cpu);
> 015 prev = rq->curr;
> 016 switch_count = &prev->nivcsw;
> 017
> 018 release_kernel_lock(prev);

Ah, I've gotten what you said, and many lines will be optimized out
it would be better to show line numbers which can be probed, as below.

000 asmlinkage void __sched schedule(void)
{
struct task_struct *prev, *next;
unsigned long *switch_count;
struct rq *rq;
int cpu;

007 need_resched:
008 preempt_disable();
009 cpu = smp_processor_id();
010 rq = cpu_rq(cpu);
011 rcu_sched_qs(cpu);
012 prev = rq->curr;
013 switch_count = &prev->nivcsw;

015 release_kernel_lock(prev);


> That way the following two are equivalent:
>
> perf probe schedule
> perf probe schedule+0
>
> The advantage of relative line numbers is that they are much more
> version invariant than absolute line numbers. Relative line numbers into
> schedule() will only change if the function itself changes.
>
> This means that expressions like 'schedule+16' will have a lot longer
> life-time than absolute line number driven probes. You can quote it in
> an email and chances are that it will still be valid even a few kernel
> releases down the road.

Hmm, I imagines 'schedule() + 16 byte offset' from 'schedule+16', because
many in-kernel(and oops) messages means that. :-)
So I'd like to use 'schedule:16'.

>>> We also want to have functionality that helps people find probe
>>> spots within a function:
>>>
>>> perf probe --list-lines schedule
>>>
>>> Would list the line numbers and source code of the schedule()
>>> function. (similar to how GDB 'list' works) That way someone can
>>> have an ad-hoc session of deciding what place to probe, and the line
>>> numbers make for an easy ID of the statement to probe.
>>
>> Agreed!
>
> Furthermore - to answer another question you raised above - the
> following syntax:
>
> perf probe schedule:'switch_count = &prev->nivcsw'
>
> is basically a 'fuzzy string match' based probe to within a function.
>
> For example you might want to probe the point within schedule that calls
> switch_mm() - this could be done via:
>
> perf probe schedule@switch_mm
>
> Or the point where 'next' gets assigned? Sure, you dont need to even
> open the editor, if you know the rough outline of the function you can
> probe it via:
>
> perf probe schedule@'next ='
>
> Note that i was able to specify both probes without having opened an
> editor - just based on the general knowledge of the scheduler.

It may be useful for return probe too :-)

perf probe schedule@return

> The point is to prefer intuitive, non-mechanic, fundamentally human
> expressions of information above mechanic ones (absolute line numbers,
> addresses, ways of probing, etc.) - and to have a rich variety of them.
>
> String based pattern matching and intuitive syntax that reuses existing
> paradigms of arithmetics and pattern-matching is good - limited syntax
> and extra, arbitrary syntactic hoops to jump through is bad.
>
> If we provide all that, people will start using this stuff - and i'd
> only like to merge this upstream once it's clear that people like me
> will (be able to) use this facility for ad-hoc probe insertion.
>
> In other words: this facility has to 'live within' our source code and
> has to be able to interact with it on a very broad basis - for it to be
> maximally useful for everyday development.

Hmm, so you mean perf-probe should work with source-code?
Without source code (but with debuginfo), maybe we can't use
string matching, is that OK?

Thank you,

--
Masami Hiramatsu

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

e-mail: [email protected]

2009-10-19 22:53:09

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH -tip tracing/kprobes 0/9] tracing/kprobes, perf: perf probe and kprobe-tracer bugfixes

Masami Hiramatsu wrote:
> Ingo Molnar wrote:
>> For example you might want to probe the point within schedule that calls
>> switch_mm() - this could be done via:
>>
>> perf probe schedule@switch_mm
>>
>> Or the point where 'next' gets assigned? Sure, you dont need to even
>> open the editor, if you know the rough outline of the function you can
>> probe it via:
>>
>> perf probe schedule@'next ='
>>
>> Note that i was able to specify both probes without having opened an
>> editor - just based on the general knowledge of the scheduler.
>
> It may be useful for return probe too :-)
>
> perf probe schedule@return

Hmm, IMHO,

>> perf probe schedule@switch_mm

might be confused as 'probe schedule() called from switch_mm()'.

BTW, there might be several local/inline functions which have
same name.
I think we'd better provide a syntax for solving this issue.
And current syntax uses @ for this purpose as below.

perf probe localfunc@file

Maybe, we still can use % for special matching,

perf probe schedule%switch_mm

These can be combined with each other, as below.

perf probe schedule@kernel/sched.c%switch_mm

Or, supporting lazy string pattern matching
(reusing glob matching in ftrace?)

perf probe schedule:'switch_mm(*);'

Just my thought.

Thank you,

--
Masami Hiramatsu

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

e-mail: [email protected]

2009-10-19 23:10:04

by Ashwin Chaugule

[permalink] [raw]
Subject: Re: [PATCH -tip tracing/kprobes 0/9] tracing/kprobes, perf: perf probe and kprobe-tracer bugfixes

On Fri, Oct 16, 2009 at 8:07 PM, Masami Hiramatsu <[email protected]> wrote:

> ?arch/x86/lib/x86-opcode-map.txt ? ? ? ? | ? 23 ++++-
> ?kernel/trace/trace_kprobe.c ? ? ? ? ? ? | ? 39 ++++++--
> ?tools/perf/Documentation/perf-probe.txt | ? 48 ++++++++++
> ?tools/perf/Makefile ? ? ? ? ? ? ? ? ? ? | ? ?5 +
> ?tools/perf/builtin-probe.c ? ? ? ? ? ? ?| ? 70 ++++++---------
> ?tools/perf/command-list.txt ? ? ? ? ? ? | ? ?1
> ?tools/perf/util/probe-finder.c ? ? ? ? ?| ?149 ++++++++++++++-----------------
> ?tools/perf/util/probe-finder.h ? ? ? ? ?| ? 17 ----
> ?tools/perf/util/util.h ? ? ? ? ? ? ? ? ?| ? ?9 ++
> ?9 files changed, 206 insertions(+), 155 deletions(-)
> ?create mode 100644 tools/perf/Documentation/perf-probe.txt
>

Masami,
I really like your idea with kprobes ! I havent yet run
this stuff on my machine, but I've been planning to utilize kprobes
with perfevents too. Please let me know if the following functionality
is already possible with your patches.

The basic idea I had was to be able to profile any function in the
kernel using kprobes and use the perfevents framework to monitor
things like incorrect branch predictions, cache misses etc. for the
scope of that function. That way, we can fine tune kernel functions
that are in the hot path of kernel control flow, using gcc tricks,
inline assembly, or even architecture specific tricks.

Alternately, I think even dynamic trace could provide similar insight
with perfevents ? If none of this is already done, I plan to work on
this in my spare time and would like to team up with anyone else
interested.

Cheers,
Ashwin

2009-10-20 00:28:38

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH -tip tracing/kprobes 0/9] tracing/kprobes, perf: perf probe and kprobe-tracer bugfixes

Hi Ashwin,

Ashwin Chaugule wrote:
> Masami,
> I really like your idea with kprobes ! I havent yet run
> this stuff on my machine, but I've been planning to utilize kprobes
> with perfevents too. Please let me know if the following functionality
> is already possible with your patches.

Thanks! I'm happy to here that you are interested in.

> The basic idea I had was to be able to profile any function in the
> kernel using kprobes and use the perfevents framework to monitor
> things like incorrect branch predictions, cache misses etc. for the
> scope of that function. That way, we can fine tune kernel functions
> that are in the hot path of kernel control flow, using gcc tricks,
> inline assembly, or even architecture specific tricks.

Actually, perf-probe can add tracepoint-like events. It will allow you
to probe function inside by both C-source-line level and address level.
perf-probe already supports inline function too.

Currently, even its syntax is unstable, but you can add an event inside
some function which is at cfile.c line 100, as below;

perf probe -P 'p:probe1 cfile.c:100'

and you can trace it by using perf record as same as other tracepoints

perf record -f -e kprobes:myprobe:record -F 1 -a ls


> Alternately, I think even dynamic trace could provide similar insight
> with perfevents ? If none of this is already done, I plan to work on
> this in my spare time and would like to team up with anyone else
> interested.

Yes, it's done as I said above.
But it still has long TODO list, including support type of arguments,
arrays, fields of structures and so on (of course, defining useful
syntax too). So any comments and contributes are welcome :-)

Thank you,

--
Masami Hiramatsu

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

e-mail: [email protected]

2009-10-20 01:59:39

by Ashwin Chaugule

[permalink] [raw]
Subject: Re: [PATCH -tip tracing/kprobes 0/9] tracing/kprobes, perf: perf probe and kprobe-tracer bugfixes

On Mon, Oct 19, 2009 at 8:30 PM, Masami Hiramatsu <[email protected]> wrote:
>
> Actually, perf-probe can add tracepoint-like events. It will allow you
> to probe function inside by both C-source-line level and address level.
> perf-probe already supports inline function too.
>
> Currently, even its syntax is unstable, but you can add an event inside
> some function which is at cfile.c line 100, as below;
>
> ?perf probe -P 'p:probe1 cfile.c:100'
>
> and you can trace it by using perf record as same as other tracepoints
>
> ?perf record -f -e kprobes:myprobe:record -F 1 -a ls
>
>
>> Alternately, I think even dynamic trace could provide similar insight
>> with perfevents ? If none of this is already done, I plan to work on
>> this in my spare time and would like to team up with anyone else
>> interested.
>
> Yes, it's done as I said above.
> But it still has long TODO list, including support type of arguments,
> arrays, fields of structures and so on (of course, defining useful
> syntax too). So any comments and contributes are welcome :-)
>
Excellent ! Time to give it a spin :)

Cheers,
Ashwin

2009-10-20 06:44:01

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH -tip tracing/kprobes 0/9] tracing/kprobes, perf: perf probe and kprobe-tracer bugfixes


* Frederic Weisbecker <[email protected]> wrote:

[...]
>
> I think absolute and relative line modes are not colliding/contending
> at all but actually fit two different needs.

Definitely so.

> - absolute is nice when you are lonely doing kernel debugging.
> (can be expanded at will once you imagine user probes)
> You are stuck in your code editor, trying to figure out the
> origin of your problem and then you think it would be nice
> to set a probe in branch 1 and in branch 2 inside func_foo().
> Then you already have absolute lines and relying in
> perf probe --list func_foo() to resolve an absolute line into
> a relative one is a very undesired middle step.

Of course - absolute numbers definitely rule for everything that works
on a whole-file basis. (I'd argue that if you do that from an editor
then you want a short macro that just sets a probe there - much like a
breakpoint. Such an editor macro would want to use absolute numbers.))

> - relative is nice in some other cases. When you already have
> the function target in mind, you even don't need to check your
> editor, just a quick check to this command and get the relative
> line. But also when you want to transmit a probe reference
> in a mailing list because of its better lifetime.

also useful for command line workflows: 'perf probe --list' output - i
think we users to generate func_symbol+rel_position kind of probes.

Plus a relative position is more intuitive as well. If you see
'schedule+10' versus 'schedule+102', you'll know it immediate that the
first one is early in the function while the second one is near the end.

If you see 'schedule@2465' versus 'schedule@2555' that kind of 'where in
the function is the probe, roughly' subjective impression is lost.

Ingo

2009-10-20 06:51:24

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH -tip tracing/kprobes 0/9] tracing/kprobes, perf: perf probe and kprobe-tracer bugfixes


* Masami Hiramatsu <[email protected]> wrote:

> Ah, I've gotten what you said, and many lines will be optimized out it
> would be better to show line numbers which can be probed, as below.
>
> 000 asmlinkage void __sched schedule(void)
> {
> struct task_struct *prev, *next;
> unsigned long *switch_count;
> struct rq *rq;
> int cpu;
>
> 007 need_resched:
> 008 preempt_disable();
> 009 cpu = smp_processor_id();
> 010 rq = cpu_rq(cpu);
> 011 rcu_sched_qs(cpu);
> 012 prev = rq->curr;
> 013 switch_count = &prev->nivcsw;
>
> 015 release_kernel_lock(prev);

Agreed, that's a very good idea!

( You could also make use of color_fprintf() to color-code some of the
numbers - for example lines with existing probes might be marked red. )

> > That way the following two are equivalent:
> >
> > perf probe schedule
> > perf probe schedule+0
> >
> > The advantage of relative line numbers is that they are much more
> > version invariant than absolute line numbers. Relative line numbers into
> > schedule() will only change if the function itself changes.
> >
> > This means that expressions like 'schedule+16' will have a lot longer
> > life-time than absolute line number driven probes. You can quote it in
> > an email and chances are that it will still be valid even a few kernel
> > releases down the road.
>
> Hmm, I imagines 'schedule() + 16 byte offset' from 'schedule+16',
> because many in-kernel(and oops) messages means that. :-) So I'd like
> to use 'schedule:16'.

kernel symbol addresses are in the form of schedule+0x16/0x510.

I like the plus sign because it also allows the negative positions:
relative position from the _end_ of the function. So for example
'schedule-1' would probe the (last) return line of the function.
[admittedly this is not as useful as the + operation though.]

But schedule:16 [and schedule:+16] would be fine too i suspect.

> >>> We also want to have functionality that helps people find probe
> >>> spots within a function:
> >>>
> >>> perf probe --list-lines schedule
> >>>
> >>> Would list the line numbers and source code of the schedule()
> >>> function. (similar to how GDB 'list' works) That way someone can
> >>> have an ad-hoc session of deciding what place to probe, and the line
> >>> numbers make for an easy ID of the statement to probe.
> >>
> >> Agreed!
> >
> > Furthermore - to answer another question you raised above - the
> > following syntax:
> >
> > perf probe schedule:'switch_count = &prev->nivcsw'
> >
> > is basically a 'fuzzy string match' based probe to within a function.
> >
> > For example you might want to probe the point within schedule that calls
> > switch_mm() - this could be done via:
> >
> > perf probe schedule@switch_mm
> >
> > Or the point where 'next' gets assigned? Sure, you dont need to even
> > open the editor, if you know the rough outline of the function you can
> > probe it via:
> >
> > perf probe schedule@'next ='
> >
> > Note that i was able to specify both probes without having opened an
> > editor - just based on the general knowledge of the scheduler.
>
> It may be useful for return probe too :-)
>
> perf probe schedule@return
>
> > The point is to prefer intuitive, non-mechanic, fundamentally human
> > expressions of information above mechanic ones (absolute line numbers,
> > addresses, ways of probing, etc.) - and to have a rich variety of them.
> >
> > String based pattern matching and intuitive syntax that reuses existing
> > paradigms of arithmetics and pattern-matching is good - limited syntax
> > and extra, arbitrary syntactic hoops to jump through is bad.
> >
> > If we provide all that, people will start using this stuff - and i'd
> > only like to merge this upstream once it's clear that people like me
> > will (be able to) use this facility for ad-hoc probe insertion.
> >
> > In other words: this facility has to 'live within' our source code and
> > has to be able to interact with it on a very broad basis - for it to be
> > maximally useful for everyday development.
>
> Hmm, so you mean perf-probe should work with source-code? Without
> source code (but with debuginfo), maybe we can't use string matching,
> is that OK?

Well most forms of debuginfo embedd the full source code in the
debuginfo, right? If it's not there (or we dont know where it is) then
we cannot use it, obviously.

But we obviously want the whole 'perf probe' workflow to primarily
operate on source code - we are humans.

Ingo

2009-10-20 06:52:24

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH -tip tracing/kprobes 0/9] tracing/kprobes, perf: perf probe and kprobe-tracer bugfixes


* Masami Hiramatsu <[email protected]> wrote:

> Masami Hiramatsu wrote:
> > Ingo Molnar wrote:
> >> For example you might want to probe the point within schedule that calls
> >> switch_mm() - this could be done via:
> >>
> >> perf probe schedule@switch_mm
> >>
> >> Or the point where 'next' gets assigned? Sure, you dont need to even
> >> open the editor, if you know the rough outline of the function you can
> >> probe it via:
> >>
> >> perf probe schedule@'next ='
> >>
> >> Note that i was able to specify both probes without having opened an
> >> editor - just based on the general knowledge of the scheduler.
> >
> > It may be useful for return probe too :-)
> >
> > perf probe schedule@return
>
> Hmm, IMHO,
>
> >> perf probe schedule@switch_mm
>
> might be confused as 'probe schedule() called from switch_mm()'.
>
> BTW, there might be several local/inline functions which have
> same name.
> I think we'd better provide a syntax for solving this issue.
> And current syntax uses @ for this purpose as below.
>
> perf probe localfunc@file
>
> Maybe, we still can use % for special matching,
>
> perf probe schedule%switch_mm
>
> These can be combined with each other, as below.
>
> perf probe schedule@kernel/sched.c%switch_mm
>
> Or, supporting lazy string pattern matching
> (reusing glob matching in ftrace?)
>
> perf probe schedule:'switch_mm(*);'
>
> Just my thought.

I'm not attached to any particular form of syntax here (other than it
should be simple and obvious) - we can try and see how it works out.

Ingo

2009-10-20 06:55:05

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH -tip tracing/kprobes 0/9] tracing/kprobes, perf: perf probe and kprobe-tracer bugfixes


* Masami Hiramatsu <[email protected]> wrote:

> How about the following syntax?
> <adding>
> perf probe schedule
> perf probe --add schedule
>
> <deleting>
> perf probe --del schedule
> perf probe --del all /* delete all probepoints */
>
> So, this doesn't symmetric, but provides simple way to add a probe.

agreed. For deletion, eventually this:

perf probe --del "*"

should also work - as should other regexp (or glob) matches on a range
of existing probes.

plus there should be a perf probe call to output all current probes as
perf probe commands - so that it can be saved (and restored).

Ingo

2009-10-20 06:56:31

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH -tip tracing/kprobes 0/9] tracing/kprobes, perf: perf probe and kprobe-tracer bugfixes


* Ashwin Chaugule <[email protected]> wrote:

> > Yes, it's done as I said above. But it still has long TODO list,
> > including support type of arguments, arrays, fields of structures
> > and so on (of course, defining useful syntax too). So any comments
> > and contributes are welcome :-)
>
> Excellent ! Time to give it a spin :)

btw., you can pick it up from:

http://people.redhat.com/mingo/tip.git/README

git checkout tip/perf/probes

Ingo

2009-10-20 14:25:54

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH -tip tracing/kprobes 0/9] tracing/kprobes, perf: perf probe and kprobe-tracer bugfixes

Ingo Molnar wrote:
>
> * Masami Hiramatsu <[email protected]> wrote:
>
>> How about the following syntax?
>> <adding>
>> perf probe schedule
>> perf probe --add schedule
>>
>> <deleting>
>> perf probe --del schedule
>> perf probe --del all /* delete all probepoints */
>>
>> So, this doesn't symmetric, but provides simple way to add a probe.
>
> agreed. For deletion, eventually this:
>
> perf probe --del "*"
>
> should also work - as should other regexp (or glob) matches on a range
> of existing probes.

Ah, that's a nice idea :-), it allows user to delete a group of
probes, like --del "schedule*".

> plus there should be a perf probe call to output all current probes as
> perf probe commands - so that it can be saved (and restored).

Sure. 'perf probe --save file' & 'perf probe --restore file' will be
fine for me.

Thank you,
--
Masami Hiramatsu

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

e-mail: [email protected]

2009-10-20 14:38:32

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH -tip tracing/kprobes 0/9] tracing/kprobes, perf: perf probe and kprobe-tracer bugfixes

Ingo Molnar wrote:
>>> The point is to prefer intuitive, non-mechanic, fundamentally human
>>> expressions of information above mechanic ones (absolute line numbers,
>>> addresses, ways of probing, etc.) - and to have a rich variety of them.
>>>
>>> String based pattern matching and intuitive syntax that reuses existing
>>> paradigms of arithmetics and pattern-matching is good - limited syntax
>>> and extra, arbitrary syntactic hoops to jump through is bad.
>>>
>>> If we provide all that, people will start using this stuff - and i'd
>>> only like to merge this upstream once it's clear that people like me
>>> will (be able to) use this facility for ad-hoc probe insertion.
>>>
>>> In other words: this facility has to 'live within' our source code and
>>> has to be able to interact with it on a very broad basis - for it to be
>>> maximally useful for everyday development.
>>
>> Hmm, so you mean perf-probe should work with source-code? Without
>> source code (but with debuginfo), maybe we can't use string matching,
>> is that OK?
>
> Well most forms of debuginfo embedd the full source code in the
> debuginfo, right? If it's not there (or we dont know where it is) then
> we cannot use it, obviously.

Um, actually debuginfo doesn't have the full source code, but has
the source file path. So, only if there are source files,
we can use string-based matching. Even if there are no source files,
that means users can't change their kernel:-). So we don't care
about kernel-version dependency.

> But we obviously want the whole 'perf probe' workflow to primarily
> operate on source code - we are humans.

Sure :-)

Thank you,

--
Masami Hiramatsu

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

e-mail: [email protected]

2009-10-20 17:45:14

by Frederic Weisbecker

[permalink] [raw]
Subject: Re: [PATCH -tip tracing/kprobes 0/9] tracing/kprobes, perf: perf probe and kprobe-tracer bugfixes

On Mon, Oct 19, 2009 at 02:18:24PM -0200, Arnaldo Carvalho de Melo wrote:
> Em Mon, Oct 19, 2009 at 01:00:57PM +0200, Frederic Weisbecker escreveu:
> > On Mon, Oct 19, 2009 at 09:51:03AM +0200, Ingo Molnar wrote:
> > > > So, what would you think about using -D (def) and -U (undef) ?
> > > The simpest case should be no extra character at all:
> > > perf probe schedule
> > Yeah, I really prefer that too.
>
> /me too
>
> > If you want more precision, it also means you have you code editor opened
> > and want to set a precise point. Since you also have the absolute
> > line directly displayed by your editor, you don't want to calculate the relative
> > line but rather the absolute one.
>
> Perhaps this should come with vim/emacs key bindings so that all this
> gets hidden by just opening the source code and pressing
> control+SOMEKEY?
>
> - Arnaldo


Yep, would be a nice idea.

2009-10-20 17:51:22

by Frederic Weisbecker

[permalink] [raw]
Subject: Re: [PATCH -tip tracing/kprobes 0/9] tracing/kprobes, perf: perf probe and kprobe-tracer bugfixes

On Tue, Oct 20, 2009 at 08:43:34AM +0200, Ingo Molnar wrote:
> also useful for command line workflows: 'perf probe --list' output - i
> think we users to generate func_symbol+rel_position kind of probes.
>
> Plus a relative position is more intuitive as well. If you see
> 'schedule+10' versus 'schedule+102', you'll know it immediate that the
> first one is early in the function while the second one is near the end.
>
> If you see 'schedule@2465' versus 'schedule@2555' that kind of 'where in
> the function is the probe, roughly' subjective impression is lost.
>
> Ingo


Yeah, from a probe id reading POV, that's much better.

2009-10-21 00:04:38

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH -tip tracing/kprobes 0/9] tracing/kprobes, perf: perf probe and kprobe-tracer bugfixes

Ingo Molnar wrote:
> * Masami Hiramatsu <[email protected]> wrote:
>> Masami Hiramatsu wrote:
>>> Ingo Molnar wrote:
>>>> For example you might want to probe the point within schedule that calls
>>>> switch_mm() - this could be done via:
>>>>
>>>> perf probe schedule@switch_mm
>>>>
>>>> Or the point where 'next' gets assigned? Sure, you dont need to even
>>>> open the editor, if you know the rough outline of the function you can
>>>> probe it via:
>>>>
>>>> perf probe schedule@'next ='
>>>>
>>>> Note that i was able to specify both probes without having opened an
>>>> editor - just based on the general knowledge of the scheduler.
>>>
>>> It may be useful for return probe too :-)
>>>
>>> perf probe schedule@return
>>
>> Hmm, IMHO,
>>
>>>> perf probe schedule@switch_mm
>>
>> might be confused as 'probe schedule() called from switch_mm()'.
>>
>> BTW, there might be several local/inline functions which have
>> same name.
>> I think we'd better provide a syntax for solving this issue.
>> And current syntax uses @ for this purpose as below.
>>
>> perf probe localfunc@file
>>
>> Maybe, we still can use % for special matching,
>>
>> perf probe schedule%switch_mm
>>
>> These can be combined with each other, as below.
>>
>> perf probe schedule@kernel/sched.c%switch_mm
>>
>> Or, supporting lazy string pattern matching
>> (reusing glob matching in ftrace?)
>>
>> perf probe schedule:'switch_mm(*);'
>>
>> Just my thought.
>
> I'm not attached to any particular form of syntax here (other than it
> should be simple and obvious) - we can try and see how it works out.

OK, so I'll try to implement it and see how it works out:-)

--
Masami Hiramatsu

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

e-mail: [email protected]