[PATCH tracing/kprobes v2 2/5] tracing/kprobes: Avoid field name confliction

[PATCH tracing/kprobes v2 2/5] tracing/kprobes: Avoid field name confliction

[Masami Hiramatsu]

Check whether the argument name is conflict with other field names.

Changes in v2:
 - Add common_lock_depth to reserved name list.

Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Jim Keniston <jkenisto@us.ibm.com>
Cc: Frank Ch. Eigler <fche@redhat.com>
---

 kernel/trace/trace_kprobe.c |   65 +++++++++++++++++++++++++++++++++++--------
 1 files changed, 53 insertions(+), 12 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index f63ead0..eb1fa0f 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -38,6 +38,25 @@
 #define MAX_EVENT_NAME_LEN 64
 #define KPROBE_EVENT_SYSTEM "kprobes"
 
+/* Reserved field names */
+#define FIELD_STRING_IP "ip"
+#define FIELD_STRING_NARGS "nargs"
+#define FIELD_STRING_RETIP "ret_ip"
+#define FIELD_STRING_FUNC "func"
+
+const char *reserved_field_names[] = {
+	"common_type",
+	"common_flags",
+	"common_preempt_count",
+	"common_pid",
+	"common_tgid",
+	"common_lock_depth",
+	FIELD_STRING_IP,
+	FIELD_STRING_NARGS,
+	FIELD_STRING_RETIP,
+	FIELD_STRING_FUNC,
+};
+
 /* currently, trace_kprobe only supports X86. */
 
 struct fetch_func {
@@ -551,6 +570,20 @@ static int parse_probe_arg(char *arg, struct fetch_func *ff, int is_return)
 	return ret;
 }
 
+/* Return 1 if name is reserved or already used by another argument */
+static int conflict_field_name(const char *name,
+			       struct probe_arg *args, int narg)
+{
+	int i;
+	for (i = 0; i < ARRAY_SIZE(reserved_field_names); i++)
+		if (!strcmp(reserved_field_names[i], name))
+			return 1;
+	for (i = 0; i < narg; i++)
+		if (!strcmp(args[i].name, name))
+			return 1;
+	return 0;
+}
+
 static int create_trace_probe(int argc, char **argv)
 {
 	/*
@@ -652,6 +685,12 @@ static int create_trace_probe(int argc, char **argv)
 			*arg++ = '\0';
 		else
 			arg = argv[i];
+
+		if (conflict_field_name(argv[i], tp->args, i)) {
+			ret = -EINVAL;
+			goto error;
+		}
+
 		tp->args[i].name = kstrdup(argv[i], GFP_KERNEL);
 
 		/* Parse fetch argument */
@@ -1054,8 +1093,8 @@ static int kprobe_event_define_fields(struct ftrace_event_call *event_call)
 	if (!ret)
 		return ret;
 
-	DEFINE_FIELD(unsigned long, ip, "ip", 0);
-	DEFINE_FIELD(int, nargs, "nargs", 1);
+	DEFINE_FIELD(unsigned long, ip, FIELD_STRING_IP, 0);
+	DEFINE_FIELD(int, nargs, FIELD_STRING_NARGS, 1);
 	/* Set argument names as fields */
 	for (i = 0; i < tp->nr_args; i++)
 		DEFINE_FIELD(unsigned long, args[i], tp->args[i].name, 0);
@@ -1072,9 +1111,9 @@ static int kretprobe_event_define_fields(struct ftrace_event_call *event_call)
 	if (!ret)
 		return ret;
 
-	DEFINE_FIELD(unsigned long, func, "func", 0);
-	DEFINE_FIELD(unsigned long, ret_ip, "ret_ip", 0);
-	DEFINE_FIELD(int, nargs, "nargs", 1);
+	DEFINE_FIELD(unsigned long, func, FIELD_STRING_FUNC, 0);
+	DEFINE_FIELD(unsigned long, ret_ip, FIELD_STRING_RETIP, 0);
+	DEFINE_FIELD(int, nargs, FIELD_STRING_NARGS, 1);
 	/* Set argument names as fields */
 	for (i = 0; i < tp->nr_args; i++)
 		DEFINE_FIELD(unsigned long, args[i], tp->args[i].name, 0);
@@ -1123,15 +1162,16 @@ static int kprobe_event_show_format(struct ftrace_event_call *call,
 	int ret, i;
 	struct trace_probe *tp = (struct trace_probe *)call->data;
 
-	SHOW_FIELD(unsigned long, ip, "ip");
-	SHOW_FIELD(int, nargs, "nargs");
+	SHOW_FIELD(unsigned long, ip, FIELD_STRING_IP);
+	SHOW_FIELD(int, nargs, FIELD_STRING_NARGS);
 
 	/* Show fields */
 	for (i = 0; i < tp->nr_args; i++)
 		SHOW_FIELD(unsigned long, args[i], tp->args[i].name);
 	trace_seq_puts(s, "\n");
 
-	return __probe_event_show_format(s, tp, "(%lx)", "REC->ip");
+	return __probe_event_show_format(s, tp, "(%lx)",
+					 "REC->" FIELD_STRING_IP);
 }
 
 static int kretprobe_event_show_format(struct ftrace_event_call *call,
@@ -1141,9 +1181,9 @@ static int kretprobe_event_show_format(struct ftrace_event_call *call,
 	int ret, i;
 	struct trace_probe *tp = (struct trace_probe *)call->data;
 
-	SHOW_FIELD(unsigned long, func, "func");
-	SHOW_FIELD(unsigned long, ret_ip, "ret_ip");
-	SHOW_FIELD(int, nargs, "nargs");
+	SHOW_FIELD(unsigned long, func, FIELD_STRING_FUNC);
+	SHOW_FIELD(unsigned long, ret_ip, FIELD_STRING_RETIP);
+	SHOW_FIELD(int, nargs, FIELD_STRING_NARGS);
 
 	/* Show fields */
 	for (i = 0; i < tp->nr_args; i++)
@@ -1151,7 +1191,8 @@ static int kretprobe_event_show_format(struct ftrace_event_call *call,
 	trace_seq_puts(s, "\n");
 
 	return __probe_event_show_format(s, tp, "(%lx <- %lx)",
-					  "REC->func, REC->ret_ip");
+					 "REC->" FIELD_STRING_FUNC
+					 ", REC->" FIELD_STRING_RETIP);
 }
 
 #ifdef CONFIG_EVENT_PROFILE


-- 
Masami Hiramatsu

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

e-mail: mhiramat@redhat.com

[PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Masami Hiramatsu]

Add $ prefix to the special variables(e.g. sa, rv) of kprobe-tracer.
This resolves consistency issue between kprobe_events and perf-kprobe.

Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Jim Keniston <jkenisto@us.ibm.com>
Cc: Frank Ch. Eigler <fche@redhat.com>
---

 Documentation/trace/kprobetrace.txt |   10 +++---
 kernel/trace/trace_kprobe.c         |   60 ++++++++++++++++++++++-------------
 2 files changed, 42 insertions(+), 28 deletions(-)

diff --git a/Documentation/trace/kprobetrace.txt b/Documentation/trace/kprobetrace.txt
index 9b8f7c6..40caef0 100644
--- a/Documentation/trace/kprobetrace.txt
+++ b/Documentation/trace/kprobetrace.txt
@@ -36,13 +36,13 @@ Synopsis of kprobe_events
 
  FETCHARGS	: Arguments. Each probe can have up to 128 args.
   %REG	: Fetch register REG
-  sN	: Fetch Nth entry of stack (N >= 0)
-  sa	: Fetch stack address.
   @ADDR	: Fetch memory at ADDR (ADDR should be in kernel)
   @SYM[+|-offs]	: Fetch memory at SYM +|- offs (SYM should be a data symbol)
-  aN	: Fetch function argument. (N >= 0)(*)
-  rv	: Fetch return value.(**)
-  ra	: Fetch return address.(**)
+  $sN	: Fetch Nth entry of stack (N >= 0)
+  $sa	: Fetch stack address.
+  $aN	: Fetch function argument. (N >= 0)(*)
+  $rv	: Fetch return value.(**)
+  $ra	: Fetch return address.(**)
   +|-offs(FETCHARG) : Fetch memory at FETCHARG +|- offs address.(***)
   NAME=FETCHARG: Set NAME as the argument name of FETCHARG.
 
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 97309d4..f63ead0 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -220,24 +220,24 @@ static int probe_arg_string(char *buf, size_t n, struct fetch_func *ff)
 	int ret = -EINVAL;
 
 	if (ff->func == fetch_argument)
-		ret = snprintf(buf, n, "a%lu", (unsigned long)ff->data);
+		ret = snprintf(buf, n, "$a%lu", (unsigned long)ff->data);
 	else if (ff->func == fetch_register) {
 		const char *name;
 		name = regs_query_register_name((unsigned int)((long)ff->data));
 		ret = snprintf(buf, n, "%%%s", name);
 	} else if (ff->func == fetch_stack)
-		ret = snprintf(buf, n, "s%lu", (unsigned long)ff->data);
+		ret = snprintf(buf, n, "$s%lu", (unsigned long)ff->data);
 	else if (ff->func == fetch_memory)
 		ret = snprintf(buf, n, "@0x%p", ff->data);
 	else if (ff->func == fetch_symbol) {
 		struct symbol_cache *sc = ff->data;
 		ret = snprintf(buf, n, "@%s%+ld", sc->symbol, sc->offset);
 	} else if (ff->func == fetch_retvalue)
-		ret = snprintf(buf, n, "rv");
+		ret = snprintf(buf, n, "$rv");
 	else if (ff->func == fetch_ip)
-		ret = snprintf(buf, n, "ra");
+		ret = snprintf(buf, n, "$ra");
 	else if (ff->func == fetch_stack_address)
-		ret = snprintf(buf, n, "sa");
+		ret = snprintf(buf, n, "$sa");
 	else if (ff->func == fetch_indirect) {
 		struct indirect_fetch_data *id = ff->data;
 		size_t l = 0;
@@ -429,12 +429,10 @@ static int split_symbol_offset(char *symbol, unsigned long *offset)
 #define PARAM_MAX_ARGS 16
 #define PARAM_MAX_STACK (THREAD_SIZE / sizeof(unsigned long))
 
-static int parse_probe_arg(char *arg, struct fetch_func *ff, int is_return)
+static int parse_probe_vars(char *arg, struct fetch_func *ff, int is_return)
 {
 	int ret = 0;
 	unsigned long param;
-	long offset;
-	char *tmp;
 
 	switch (arg[0]) {
 	case 'a':	/* argument */
@@ -456,14 +454,6 @@ static int parse_probe_arg(char *arg, struct fetch_func *ff, int is_return)
 		} else
 			ret = -EINVAL;
 		break;
-	case '%':	/* named register */
-		ret = regs_query_register_offset(arg + 1);
-		if (ret >= 0) {
-			ff->func = fetch_register;
-			ff->data = (void *)(unsigned long)ret;
-			ret = 0;
-		}
-		break;
 	case 's':	/* stack */
 		if (arg[1] == 'a') {
 			ff->func = fetch_stack_address;
@@ -478,6 +468,31 @@ static int parse_probe_arg(char *arg, struct fetch_func *ff, int is_return)
 			}
 		}
 		break;
+	default:
+		ret = -EINVAL;
+	}
+	return ret;
+}
+
+static int parse_probe_arg(char *arg, struct fetch_func *ff, int is_return)
+{
+	int ret = 0;
+	unsigned long param;
+	long offset;
+	char *tmp;
+
+	switch (arg[0]) {
+	case '$':
+		ret = parse_probe_vars(arg + 1, ff, is_return);
+		break;
+	case '%':	/* named register */
+		ret = regs_query_register_offset(arg + 1);
+		if (ret >= 0) {
+			ff->func = fetch_register;
+			ff->data = (void *)(unsigned long)ret;
+			ret = 0;
+		}
+		break;
 	case '@':	/* memory or symbol */
 		if (isdigit(arg[1])) {
 			ret = strict_strtoul(arg + 1, 0, &param);
@@ -489,8 +504,7 @@ static int parse_probe_arg(char *arg, struct fetch_func *ff, int is_return)
 			ret = split_symbol_offset(arg + 1, &offset);
 			if (ret)
 				break;
-			ff->data = alloc_symbol_cache(arg + 1,
-							      offset);
+			ff->data = alloc_symbol_cache(arg + 1, offset);
 			if (ff->data)
 				ff->func = fetch_symbol;
 			else
@@ -544,11 +558,11 @@ static int create_trace_probe(int argc, char **argv)
 	 *  - Add kprobe: p[:[GRP/]EVENT] KSYM[+OFFS]|KADDR [FETCHARGS]
 	 *  - Add kretprobe: r[:[GRP/]EVENT] KSYM[+0] [FETCHARGS]
 	 * Fetch args:
-	 *  aN	: fetch Nth of function argument. (N:0-)
-	 *  rv	: fetch return value
-	 *  ra	: fetch return address
-	 *  sa	: fetch stack address
-	 *  sN	: fetch Nth of stack (N:0-)
+	 *  $aN	: fetch Nth of function argument. (N:0-)
+	 *  $rv	: fetch return value
+	 *  $ra	: fetch return address
+	 *  $sa	: fetch stack address
+	 *  $sN	: fetch Nth of stack (N:0-)
 	 *  @ADDR	: fetch memory at ADDR (ADDR should be in kernel)
 	 *  @SYM[+|-offs] : fetch memory at SYM +|- offs (SYM is a data symbol)
 	 *  %REG	: fetch register REG


-- 
Masami Hiramatsu

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

e-mail: mhiramat@redhat.com

[PATCH tracing/kprobes v2 3/5] tracing/kprobes: Rename fixed field name

[Masami Hiramatsu]

Rename probe-common fixed field name to the name harder conflictable,
because current 'ip', 'func', and other probe field names are easy to
conflict with user-specified variable names.

Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Jim Keniston <jkenisto@us.ibm.com>
Cc: Frank Ch. Eigler <fche@redhat.com>
---

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

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index eb1fa0f..4838d22 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -39,10 +39,10 @@
 #define KPROBE_EVENT_SYSTEM "kprobes"
 
 /* Reserved field names */
-#define FIELD_STRING_IP "ip"
-#define FIELD_STRING_NARGS "nargs"
-#define FIELD_STRING_RETIP "ret_ip"
-#define FIELD_STRING_FUNC "func"
+#define FIELD_STRING_IP "__probe_ip"
+#define FIELD_STRING_NARGS "__probe_nargs"
+#define FIELD_STRING_RETIP "__probe_ret_ip"
+#define FIELD_STRING_FUNC "__probe_func"
 
 const char *reserved_field_names[] = {
 	"common_type",


-- 
Masami Hiramatsu

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

e-mail: mhiramat@redhat.com

[PATCH tracing/kprobes v2 0/5] tracing/kprobes, perf: perf probe support take 2

[Masami Hiramatsu]

Hi,

These patches introduce 'perf probe' command and update kprobe-tracer.
perf probe command allows you to add new probe points by C line number
and local variable names.

This version fixes some bugs, changes subcommand name from kprobe to
probe and use spaces for separator instead of ',' for visibility (this
also make it easy to support probe list from stdin).

Usage
-----
 perf probe [<options>] -P 'PROBEDEF' [-P 'PROBEDEF' ...]

    -k, --vmlinux <file>  vmlinux/module pathname
    -r, --release <rel>   kernel release
    -P, --probe <p|r:[GRP/]NAME FUNC[+OFFS][@SRC]|@SRC:LINE [ARG ...]>
                          probe point definition, where
		p:	kprobe probe
		r:	kretprobe probe
		GRP:	Group name (optional)
		NAME:	Event name
		FUNC:	Function name
		OFFS:	Offset from function entry (in byte)
		SRC:	Source code path
		LINE:	Line number
		ARG:	Probe argument (local variable name or
			kprobe-tracer argument format is supported.)

Examples
--------
1) Add a new kprobe probe on a line of C source code.
./perf probe -P 'p:myprobe @fs/read_write.c:285 file buf count'
Adding new event: p:myprobe vfs_read+57 file=%bx buf=%si count=%ax

2) Add a new kretprobe probe on a function return.
./perf probe -P 'r:myretprobe vfs_read $rv'
Adding new event: r:myretprobe vfs_read+0 $rv

3) Check it in the perf list.
./perf list
...
  rNNN                                       [raw hardware event descriptor]

  kprobes:myprobe                            [Tracepoint event]
  kprobes:myretprobe                         [Tracepoint event]
  skb:kfree_skb                              [Tracepoint event]
...

4) Record the event by perf
./perf record -f -e kprobes:myprobe:record  -F 1 -a ls
...
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.081 MB perf.data (~3540 samples) ]

5) Trace the event
./perf trace
            perf-11445 [000] 95862.048894383: myprobe: (c04bbed5) file=dae15e80 buf=b78b2000 count=400
            perf-11445 [000] 95862.049066533: myprobe: (c04bbed5) file=dae15d80 buf=b78b2000 count=400
            perf-11445 [000] 95862.049134394: myprobe: (c04bbed5) file=dae15d80 buf=b78b2000 count=400
            perf-11445 [000] 95862.049171495: myprobe: (c04bbed5) file=dae15a80 buf=b78b2000 count=400

NOTE
----
 perf still fails to parse format if arguments have special charactors
(e.g. $rv, +10($sp) etc.) So, tracing myretprobe will fail with this
version. This will be solved by naming arguments automatically if it
doesn't have C-language name.

TODO
----
 - Support sys_perf_counter_open (non-root)
 - Input from stdin/output to stdout
 - Non-auto static variable
 - Fields of data structures (var->field)
 - Type support
   - Bit fields
 - Array (var[N])
 - Dynamic array indexing (var[var2])
 - String/dynamic arrays (var:string, var[N..M])
 - Force Type casting ((type)var)
 - Non-inline search
 - libdw, libdwfl
 - etc.

Thank you,

---

Masami Hiramatsu (5):
      perf: kprobe command supports without libdwarf
      perf: Add perf probe subcommand for kprobe-event setup helper
      tracing/kprobes: Rename fixed field name
      tracing/kprobes: Avoid field name confliction
      tracing/kprobes: Rename special variables syntax


 Documentation/trace/kprobetrace.txt |   10 -
 kernel/trace/trace_kprobe.c         |  125 +++++-
 tools/perf/Makefile                 |   10 +
 tools/perf/builtin-probe.c          |  384 +++++++++++++++++++
 tools/perf/builtin.h                |    1 
 tools/perf/perf.c                   |    1 
 tools/perf/util/probe-finder.c      |  690 +++++++++++++++++++++++++++++++++++
 tools/perf/util/probe-finder.h      |   70 ++++
 8 files changed, 1251 insertions(+), 40 deletions(-)
 create mode 100644 tools/perf/builtin-probe.c
 create mode 100644 tools/perf/util/probe-finder.c
 create mode 100644 tools/perf/util/probe-finder.h

-- 
Masami Hiramatsu

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

e-mail: mhiramat@redhat.com

[PATCH tracing/kprobes v2 4/5] perf: Add perf probe subcommand for kprobe-event setup helper

[Masami Hiramatsu]

Add perf probe subcommand for kprobe-event setup helper to perf command.
This allows user to define kprobe events by C expressions (C line numbers,
C function names, and C local variables).

Usage
-----
 perf probe [<options>] -P 'PROBEDEF' [-P 'PROBEDEF' ...]

    -k, --vmlinux <file>  vmlinux/module pathname
    -r, --release <rel>   kernel release
    -P, --probe <p|r:[GRP/]NAME FUNC[+OFFS][@SRC]|@SRC:LINE [ARG ...]>
                          probe point definition, where
		p:	kprobe probe
		r:	kretprobe probe
		GRP:	Group name (optional)
		NAME:	Event name
		FUNC:	Function name
		OFFS:	Offset from function entry (in byte)
		SRC:	Source code path
		LINE:	Line number
		ARG:	Probe argument (local variable name or
			kprobe-tracer argument format is supported.)

Changes in v2:
 - Check synthesized string length.
 - Rename perf kprobe to perf probe.
 - Use spaces for separator and update usage comment.
 - Check error paths in parse_probepoint().
 - Check optimized-out variables.

Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Jim Keniston <jkenisto@us.ibm.com>
Cc: Frank Ch. Eigler <fche@redhat.com>
---

 tools/perf/Makefile            |   10 +
 tools/perf/builtin-probe.c     |  356 +++++++++++++++++++++
 tools/perf/builtin.h           |    1 
 tools/perf/perf.c              |    3 
 tools/perf/util/probe-finder.c |  690 ++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/probe-finder.h |   68 ++++
 6 files changed, 1128 insertions(+), 0 deletions(-)
 create mode 100644 tools/perf/builtin-probe.c
 create mode 100644 tools/perf/util/probe-finder.c
 create mode 100644 tools/perf/util/probe-finder.h

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index b5f1953..6dabcf1 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -419,6 +419,16 @@ 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 probe subcommand. Please install libdwarf-dev/libdwarf-devel);
+else
+	EXTLIBS += -lelf -ldwarf
+	LIB_H += util/probe-finder.h
+	LIB_OBJS += util/probe-finder.o
+	BUILTIN_OBJS += builtin-probe.o
+	BASIC_CFLAGS += -DSUPPORT_DWARF
+endif
+
 ifdef NO_DEMANGLE
 	BASIC_CFLAGS += -DNO_DEMANGLE
 else
diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
new file mode 100644
index 0000000..47c5fb8
--- /dev/null
+++ b/tools/perf/builtin-probe.c
@@ -0,0 +1,356 @@
+/*
+ * builtin-probe.c
+ *
+ * Builtin probe command: Set up probe events by C expression
+ *
+ * Written by Masami Hiramatsu <mhiramat@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ */
+
+#include <sys/utsname.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "perf.h"
+#include "builtin.h"
+#include "util/util.h"
+#include "util/parse-options.h"
+#include "util/parse-events.h"	/* For debugfs_path */
+#include "util/probe-finder.h"
+
+/* Default vmlinux search paths */
+#define NR_SEARCH_PATH 3
+const char *default_search_path[NR_SEARCH_PATH] = {
+"/lib/modules/%s/build/vmlinux",		/* Custom build kernel */
+"/usr/lib/debug/lib/modules/%s/vmlinux",	/* Red Hat debuginfo */
+"/boot/vmlinux-debug-%s",			/* Ubuntu */
+};
+
+#define MAX_PATH_LEN 256
+#define MAX_PROBES 128
+
+/* Session management structure */
+static struct {
+	char *vmlinux;
+	char *release;
+	int nr_probe;
+	struct probe_point probes[MAX_PROBES];
+	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
+
+static int parse_probepoint(const struct option *opt __used,
+			    const char *str, int unset __used)
+{
+	char *argv[MAX_PROBE_ARGS + 2];	/* Event + probe + args */
+	int argc, i;
+	char *arg, *ptr;
+	struct probe_point *pp = &session.probes[session.nr_probe];
+	char **event = &session.events[session.nr_probe];
+	int retp = 0;
+
+	if (!str)	/* The end of probe points */
+		return 0;
+
+	debug("Probe-define(%d): %s\n", session.nr_probe, str);
+	if (++session.nr_probe == MAX_PROBES)
+		semantic_error("Too many probes");
+
+	/* Separate arguments, similar to argv_split */
+	argc = 0;
+	do {
+		/* Skip separators */
+		while (isspace(*str))
+			str++;
+
+		/* Add an argument */
+		if (*str != '\0') {
+			const char *s = str;
+
+			/* Skip the argument */
+			while (!isspace(*str) && *str != '\0')
+				str++;
+
+			/* Duplicate the argument */
+			argv[argc] = strndup(s, str - s);
+			if (argv[argc] == NULL)
+				perror_exit("strndup");
+			if (++argc == MAX_PROBE_ARGS)
+				semantic_error("Too many arguments");
+			debug("argv[%d]=%s\n", argc, argv[argc - 1]);
+		}
+	} while (*str != '\0');
+	if (argc < 2)
+		semantic_error("Need event-name and probe-point at least.");
+
+	/* Parse the event name */
+	if (argv[0][0] == 'r')
+		retp = 1;
+	else if (argv[0][0] != 'p')
+		semantic_error("You must specify 'p'(kprobe) or"
+				" 'r'(kretprobe) first.");
+	/* TODO: check event name */
+	*event = argv[0];
+
+	/* Parse probe point */
+	arg = argv[1];
+	if (arg[0] == '@') {
+		/* Source Line */
+		arg++;
+		ptr = strchr(arg, ':');
+		if (!ptr || !isdigit(ptr[1]))
+			semantic_error("Line number is required.");
+		*ptr++ = '\0';
+		if (strlen(arg) == 0)
+			semantic_error("No file name.");
+		pp->file = strdup(arg);
+		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);
+	} else {
+		/* Function name */
+		ptr = strchr(arg, '+');
+		if (ptr) {
+			if (!isdigit(ptr[1]))
+				semantic_error("Offset is required.");
+			*ptr++ = '\0';
+			pp->offset = atoi(ptr);
+		} else
+			ptr = arg;
+		ptr = strchr(ptr, '@');
+		if (ptr) {
+			*ptr++ = '\0';
+			pp->file = strdup(ptr);
+		}
+		pp->function = strdup(arg);
+		debug("symbol:%s file:%s offset:%d\n",
+		      pp->function, pp->file, pp->offset);
+	}
+	free(argv[1]);
+
+	/* Copy arguments */
+	pp->nr_args = argc - 2;
+	if (pp->nr_args > 0) {
+		pp->args = (char **)malloc(sizeof(char *) * pp->nr_args);
+		memcpy(pp->args, &argv[2], sizeof(char *) * pp->nr_args);
+	}
+
+	/* Ensure return probe has no C argument */
+	if (retp)
+		for (i = 0; i < pp->nr_args; i++)
+			if (is_c_varname(pp->args[i]))
+				semantic_error("You can't specify local"
+						" variable for kretprobe");
+	debug("%d arguments\n", pp->nr_args);
+	return 0;
+}
+
+static int open_default_vmlinux(void)
+{
+	struct utsname uts;
+	char fname[MAX_PATH_LEN];
+	int fd, ret, i;
+
+	if (!session.release) {
+		ret = uname(&uts);
+		if (ret) {
+			debug("uname() failed.\n");
+			return -errno;
+		}
+		session.release = uts.release;
+	}
+	for (i = 0; i < NR_SEARCH_PATH; i++) {
+		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);
+			errno = E2BIG;
+			return -E2BIG;
+		}
+		debug("try to open %s\n", fname);
+		fd = open(fname, O_RDONLY);
+		if (fd >= 0)
+			break;
+	}
+	return fd;
+}
+
+static const char * const probe_usage[] = {
+	"perf probe [<options>] -P 'PROBEDEF' [-P 'PROBEDEF' ...]",
+	NULL
+};
+
+static const struct option options[] = {
+	OPT_STRING('k', "vmlinux", &session.vmlinux, "file",
+		"vmlinux/module pathname"),
+	OPT_STRING('r', "release", &session.release, "rel", "kernel release"),
+	OPT_CALLBACK('P', "probe", NULL,
+		"p|r:[GRP/]NAME FUNC[+OFFS][@SRC]|@SRC:LINE [ARG ...]",
+		"probe point definition, where\n"
+		"\t\tp:\tkprobe probe\n"
+		"\t\tr:\tkretprobe probe\n"
+		"\t\tGRP:\tGroup name (optional)\n"
+		"\t\tNAME:\tEvent name\n"
+		"\t\tFUNC:\tFunction name\n"
+		"\t\tOFFS:\tOffset from function entry (in byte)\n"
+		"\t\tSRC:\tSource code path\n"
+		"\t\tLINE:\tLine number\n"
+		"\t\tARG:\tProbe argument (local variable name or\n"
+		"\t\t\tkprobe-tracer argument format is supported.)\n",
+		parse_probepoint),
+	OPT_END()
+};
+
+static int write_new_event(int fd, const char *buf)
+{
+	int ret;
+
+	printf("Adding new event: %s\n", buf);
+	ret = write(fd, buf, strlen(buf));
+	if (ret <= 0)
+		perror("Error: Failed to create event");
+
+	return ret;
+}
+
+#define MAX_CMDLEN 256
+
+static int synthesize_probepoint(struct probe_point *pp)
+{
+	char *buf;
+	int i, len, ret;
+	pp->probes[0] = buf = (char *)calloc(MAX_CMDLEN, sizeof(char));
+	ret = snprintf(buf, MAX_CMDLEN, "%s+%d", pp->function, pp->offset);
+	if (ret <= 0 || ret >= MAX_CMDLEN)
+		goto error;
+	len = ret;
+
+	for (i = 0; i < pp->nr_args; i++) {
+		ret = snprintf(&buf[len], MAX_CMDLEN - len, " %s",
+			       pp->args[i]);
+		if (ret <= 0 || ret >= MAX_CMDLEN - len)
+			goto error;
+		len += ret;
+	}
+	pp->found = 1;
+	return pp->found;
+error:
+	free(pp->probes[0]);
+	if (ret > 0)
+		ret = -E2BIG;
+	return ret;
+}
+
+int cmd_probe(int argc, const char **argv, const char *prefix __used)
+{
+	int i, j, fd, ret, need_dwarf = 0;
+	struct probe_point *pp;
+	char buf[MAX_CMDLEN];
+
+	argc = parse_options(argc, argv, options, probe_usage,
+		PARSE_OPT_STOP_AT_NON_OPTION);
+	if (argc || session.nr_probe == 0)
+		usage_with_options(probe_usage, options);
+
+	/* Synthesize return probes */
+	for (j = 0; j < session.nr_probe; j++) {
+		if (session.events[j][0] != 'r') {
+			need_dwarf = 1;
+			continue;
+		}
+		ret = synthesize_probepoint(&session.probes[j]);
+		if (ret == -E2BIG)
+			semantic_error("probe point is too long.");
+		else if (ret < 0) {
+			perror("snprintf");
+			return -1;
+		}
+	}
+
+	if (!need_dwarf)
+		goto setup_probes;
+
+	if (session.vmlinux)
+		fd = open(session.vmlinux, O_RDONLY);
+	else
+		fd = open_default_vmlinux();
+	if (fd < 0) {
+		perror("vmlinux/module file open");
+		return -1;
+	}
+
+	/* Searching probe points */
+	for (j = 0; j < session.nr_probe; j++) {
+		pp = &session.probes[j];
+		if (pp->found)
+			continue;
+
+		lseek(fd, SEEK_SET, 0);
+		ret = find_probepoint(fd, pp);
+		if (ret <= 0) {
+			fprintf(stderr, "Error: No probe point found.\n");
+			return -1;
+		}
+		debug("probe event %s found\n", session.events[j]);
+	}
+	close(fd);
+
+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;
+	}
+	for (j = 0; j < session.nr_probe; j++) {
+		pp = &session.probes[j];
+		if (pp->found == 1) {
+			snprintf(buf, MAX_CMDLEN, "%s %s\n",
+				session.events[j], pp->probes[0]);
+			write_new_event(fd, buf);
+		} else
+			for (i = 0; i < pp->found; i++) {
+				snprintf(buf, MAX_CMDLEN, "%s%d %s\n",
+					session.events[j], i, pp->probes[i]);
+				write_new_event(fd, buf);
+			}
+	}
+	close(fd);
+	return 0;
+}
+
diff --git a/tools/perf/builtin.h b/tools/perf/builtin.h
index e11d8d2..ad5f0f4 100644
--- a/tools/perf/builtin.h
+++ b/tools/perf/builtin.h
@@ -25,5 +25,6 @@ extern int cmd_timechart(int argc, const char **argv, const char *prefix);
 extern int cmd_top(int argc, const char **argv, const char *prefix);
 extern int cmd_trace(int argc, const char **argv, const char *prefix);
 extern int cmd_version(int argc, const char **argv, const char *prefix);
+extern int cmd_probe(int argc, const char **argv, const char *prefix);
 
 #endif
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index 19fc7fe..f598120 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -295,6 +295,9 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "version", cmd_version, 0 },
 		{ "trace", cmd_trace, 0 },
 		{ "sched", cmd_sched, 0 },
+#ifdef SUPPORT_DWARF
+		{ "probe", cmd_probe, 0 },
+#endif
 	};
 	unsigned int i;
 	static const char ext[] = STRIP_EXTENSION;
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
new file mode 100644
index 0000000..25e22fe
--- /dev/null
+++ b/tools/perf/util/probe-finder.c
@@ -0,0 +1,690 @@
+/*
+ * probe-finder.c : C expression to kprobe event converter
+ *
+ * Written by Masami Hiramatsu <mhiramat@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ */
+
+#include <sys/utsname.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <stdio.h>
+#include <unistd.h>
+#include <getopt.h>
+#include <stdlib.h>
+#include <string.h>
+#include <stdarg.h>
+#include <ctype.h>
+#include "probe-finder.h"
+
+
+/* Dwarf_Die Linkage to parent Die */
+struct die_link {
+	struct die_link *parent;	/* Parent die */
+	Dwarf_Die die;			/* Current die */
+};
+
+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
+ */
+
+#define X86_32_MAX_REGS 8
+const char *x86_32_regs_table[X86_32_MAX_REGS] = {
+	"%ax",
+	"%cx",
+	"%dx",
+	"%bx",
+	"$sa",	/* Stack address */
+	"%bp",
+	"%si",
+	"%di",
+};
+
+#define X86_64_MAX_REGS 16
+const char *x86_64_regs_table[X86_64_MAX_REGS] = {
+	"%ax",
+	"%dx",
+	"%cx",
+	"%bx",
+	"%si",
+	"%di",
+	"%bp",
+	"%sp",
+	"%r8",
+	"%r9",
+	"%r10",
+	"%r11",
+	"%r12",
+	"%r13",
+	"%r14",
+	"%r15",
+};
+
+/* TODO: switching by dwarf address size */
+#ifdef __x86_64__
+#define ARCH_MAX_REGS X86_64_MAX_REGS
+#define arch_regs_table x86_64_regs_table
+#else
+#define ARCH_MAX_REGS X86_32_MAX_REGS
+#define arch_regs_table x86_32_regs_table
+#endif
+
+/* Return architecture dependent register string (for kprobe-tracer) */
+static const char *get_arch_regstr(unsigned int n)
+{
+	return (n <= ARCH_MAX_REGS) ? arch_regs_table[n] : NULL;
+}
+
+/*
+ * Compare the tail of two strings.
+ * Return 0 if whole of either string is same as another's tail part.
+ */
+static int strtailcmp(const char *s1, const char *s2)
+{
+	int i1 = strlen(s1);
+	int i2 = strlen(s2);
+	while (--i1 > 0 && --i2 > 0) {
+		if (s1[i1] != s2[i2])
+			return s1[i1] - s2[i2];
+	}
+	return 0;
+}
+
+/* Find the fileno of the target file. */
+static Dwarf_Unsigned die_get_fileno(Dwarf_Die cu_die, const char *fname)
+{
+	Dwarf_Signed cnt, i;
+	Dwarf_Unsigned found = 0;
+	char **srcs;
+	int ret;
+
+	if (!fname)
+		return 0;
+
+	ret = dwarf_srcfiles(cu_die, &srcs, &cnt, &__dw_error);
+	if (ret == DW_DLV_OK) {
+		for (i = 0; i < cnt && !found; i++) {
+			if (strtailcmp(srcs[i], fname) == 0)
+				found = i + 1;
+			dwarf_dealloc(__dw_debug, srcs[i], DW_DLA_STRING);
+		}
+		for (; i < cnt; i++)
+			dwarf_dealloc(__dw_debug, srcs[i], DW_DLA_STRING);
+		dwarf_dealloc(__dw_debug, srcs, DW_DLA_LIST);
+	}
+	if (found)
+		debug("found fno: %d\n", (int)found);
+	return found;
+}
+
+/* Compare diename and tname */
+static int die_compare_name(Dwarf_Die die, const char *tname)
+{
+	char *name;
+	int ret;
+	ret = dwarf_diename(die, &name, &__dw_error);
+	ERR_IF(ret == DW_DLV_ERROR);
+	if (ret == DW_DLV_OK) {
+		ret = strcmp(tname, name);
+		dwarf_dealloc(__dw_debug, name, DW_DLA_STRING);
+	} else
+		ret = -1;
+	return ret;
+}
+
+/* Check the address is in the subprogram(function). */
+static int die_within_subprogram(Dwarf_Die sp_die, Dwarf_Addr addr,
+				 Dwarf_Signed *offs)
+{
+	Dwarf_Addr lopc, hipc;
+	int ret;
+
+	/* TODO: check ranges */
+	ret = dwarf_lowpc(sp_die, &lopc, &__dw_error);
+	ERR_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);
+	if (lopc <= addr && addr < hipc) {
+		*offs = addr - lopc;
+		return 1;
+	} else
+		return 0;
+}
+
+/* Check the die is inlined function */
+static Dwarf_Bool die_inlined_subprogram(Dwarf_Die die)
+{
+	/* TODO: check strictly */
+	Dwarf_Bool inl;
+	int ret;
+
+	ret = dwarf_hasattr(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)
+{
+	Dwarf_Attribute attr;
+	Dwarf_Off cu_offs;
+	int ret;
+
+	ret = dwarf_attr(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);
+	dwarf_dealloc(__dw_debug, attr, DW_DLA_ATTR);
+	return cu_offs;
+}
+
+/* Get entry pc(or low pc, 1st entry of ranges)  of the die */
+static Dwarf_Addr die_get_entrypc(Dwarf_Die die)
+{
+	Dwarf_Attribute attr;
+	Dwarf_Addr addr;
+	Dwarf_Off offs;
+	Dwarf_Ranges *ranges;
+	Dwarf_Signed cnt;
+	int ret;
+
+	/* Try to get entry pc */
+	ret = dwarf_attr(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);
+		ERR_IF(ret != DW_DLV_OK);
+		dwarf_dealloc(__dw_debug, attr, DW_DLA_ATTR);
+		return addr;
+	}
+
+	/* Try to get low pc */
+	ret = dwarf_lowpc(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);
+	ERR_IF(ret != DW_DLV_OK);
+	ret = dwarf_formref(attr, &offs, &__dw_error);
+	ERR_IF(ret != DW_DLV_OK);
+	ret = dwarf_get_ranges(__dw_debug, offs, &ranges, &cnt, NULL,
+				&__dw_error);
+	ERR_IF(ret != DW_DLV_OK);
+	addr = ranges[0].dwr_addr1;
+	dwarf_ranges_dealloc(__dw_debug, ranges, cnt);
+	return addr;
+}
+
+/*
+ * Search a Die from Die tree.
+ * Note: cur_link->die should be deallocated in this function.
+ */
+static int __search_die_tree(struct die_link *cur_link,
+			     int (*die_cb)(struct die_link *, void *),
+			     void *data)
+{
+	Dwarf_Die new_die;
+	struct die_link new_link;
+	int ret;
+
+	if (!die_cb)
+		return 0;
+
+	/* Check current die */
+	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);
+		if (ret == DW_DLV_OK) {
+			new_link.parent = cur_link;
+			new_link.die = new_die;
+			ret = __search_die_tree(&new_link, die_cb, data);
+			if (ret)
+				break;
+		}
+
+		/* Move to next sibling */
+		ret = dwarf_siblingof(__dw_debug, cur_link->die, &new_die,
+				      &__dw_error);
+		ERR_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)
+			return 0;
+	}
+	dwarf_dealloc(__dw_debug, cur_link->die, DW_DLA_DIE);
+	return ret;
+}
+
+/* Search a die in its children's die tree */
+static int search_die_from_children(Dwarf_Die parent_die,
+				    int (*die_cb)(struct die_link *, void *),
+				    void *data)
+{
+	struct die_link new_link;
+	int ret;
+
+	new_link.parent = NULL;
+	ret = dwarf_child(parent_die, &new_link.die, &__dw_error);
+	ERR_IF(ret == DW_DLV_ERROR);
+	if (ret == DW_DLV_OK)
+		return __search_die_tree(&new_link, die_cb, data);
+	else
+		return 0;
+}
+
+/* Find a locdesc corresponding to the address */
+static int attr_get_locdesc(Dwarf_Attribute attr, Dwarf_Locdesc *desc,
+			    Dwarf_Addr addr)
+{
+	Dwarf_Signed lcnt;
+	Dwarf_Locdesc **llbuf;
+	int ret, i;
+
+	ret = dwarf_loclist_n(attr, &llbuf, &lcnt, &__dw_error);
+	ERR_IF(ret != DW_DLV_OK);
+	ret = DW_DLV_NO_ENTRY;
+	for (i = 0; i < lcnt; ++i) {
+		if (llbuf[i]->ld_lopc <= addr &&
+		    llbuf[i]->ld_hipc > addr) {
+			memcpy(desc, llbuf[i], sizeof(Dwarf_Locdesc));
+			desc->ld_s =
+				malloc(sizeof(Dwarf_Loc) * llbuf[i]->ld_cents);
+			ERR_IF(desc->ld_s == NULL);
+			memcpy(desc->ld_s, llbuf[i]->ld_s,
+				sizeof(Dwarf_Loc) * llbuf[i]->ld_cents);
+			ret = DW_DLV_OK;
+			break;
+		}
+		dwarf_dealloc(__dw_debug, llbuf[i]->ld_s, DW_DLA_LOC_BLOCK);
+		dwarf_dealloc(__dw_debug, llbuf[i], DW_DLA_LOCDESC);
+	}
+	/* Releasing loop */
+	for (; i < lcnt; ++i) {
+		dwarf_dealloc(__dw_debug, llbuf[i]->ld_s, DW_DLA_LOC_BLOCK);
+		dwarf_dealloc(__dw_debug, llbuf[i], DW_DLA_LOCDESC);
+	}
+	dwarf_dealloc(__dw_debug, llbuf, DW_DLA_LIST);
+	return ret;
+}
+
+/*
+ * Probe finder related functions
+ */
+
+/* Show a location */
+static void show_location(Dwarf_Loc *loc, struct probe_finder *pf)
+{
+	Dwarf_Small op;
+	Dwarf_Unsigned regn;
+	Dwarf_Signed offs;
+	int deref = 0, ret;
+	const char *regs;
+
+	op = loc->lr_atom;
+
+	/* If this is based on frame buffer, set the offset */
+	if (op == DW_OP_fbreg) {
+		deref = 1;
+		offs = (Dwarf_Signed)loc->lr_number;
+		op = pf->fbloc.ld_s[0].lr_atom;
+		loc = &pf->fbloc.ld_s[0];
+	} else
+		offs = 0;
+
+	if (op >= DW_OP_breg0 && op <= DW_OP_breg31) {
+		regn = op - DW_OP_breg0;
+		offs += (Dwarf_Signed)loc->lr_number;
+		deref = 1;
+	} else if (op >= DW_OP_reg0 && op <= DW_OP_reg31) {
+		regn = op - DW_OP_reg0;
+	} else if (op == DW_OP_bregx) {
+		regn = loc->lr_number;
+		offs += (Dwarf_Signed)loc->lr_number2;
+		deref = 1;
+	} else if (op == DW_OP_regx) {
+		regn = loc->lr_number;
+	} else
+		msg_exit(-EINVAL, "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);
+
+	if (deref)
+		ret = snprintf(pf->buf, pf->len,
+				 " %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);
+}
+
+/* Show a variables in kprobe event format */
+static void show_variable(Dwarf_Die vr_die, struct probe_finder *pf)
+{
+	Dwarf_Attribute attr;
+	Dwarf_Locdesc ld;
+	int ret;
+
+	ret = dwarf_attr(vr_die, DW_AT_location, &attr, &__dw_error);
+	if (ret != DW_DLV_OK)
+		goto error;
+	ret = attr_get_locdesc(attr, &ld, (pf->addr - pf->cu_base));
+	if (ret != DW_DLV_OK)
+		goto error;
+	/* TODO? */
+	ERR_IF(ld.ld_cents != 1);
+	show_location(&ld.ld_s[0], pf);
+	free(ld.ld_s);
+	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);
+}
+
+static int variable_callback(struct die_link *dlink, void *data)
+{
+	struct probe_finder *pf = (struct probe_finder *)data;
+	Dwarf_Half tag;
+	int ret;
+
+	ret = dwarf_tag(dlink->die, &tag, &__dw_error);
+	ERR_IF(ret == DW_DLV_ERROR);
+	if ((tag == DW_TAG_formal_parameter ||
+	     tag == DW_TAG_variable) &&
+	    (die_compare_name(dlink->die, pf->var) == 0)) {
+		show_variable(dlink->die, pf);
+		return 1;
+	}
+	/* TODO: Support struct members and arrays */
+	return 0;
+}
+
+/* Find a variable in a subprogram die */
+static void find_variable(Dwarf_Die sp_die, struct probe_finder *pf)
+{
+	int ret;
+
+	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);
+		return ;
+	}
+
+	debug("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)
+		msg_exit(-1, "Failed to find '%s' in this function.\n",
+			 pf->var);
+}
+
+/* Get a frame base on the address */
+static void get_current_frame_base(Dwarf_Die sp_die, struct probe_finder *pf)
+{
+	Dwarf_Attribute attr;
+	int ret;
+
+	ret = dwarf_attr(sp_die, DW_AT_frame_base, &attr, &__dw_error);
+	ERR_IF(ret != DW_DLV_OK);
+	ret = attr_get_locdesc(attr, &pf->fbloc, (pf->addr - pf->cu_base));
+	ERR_IF(ret != DW_DLV_OK);
+	dwarf_dealloc(__dw_debug, attr, DW_DLA_ATTR);
+}
+
+static void free_current_frame_base(struct probe_finder *pf)
+{
+	free(pf->fbloc.ld_s);
+	memset(&pf->fbloc, 0, sizeof(Dwarf_Locdesc));
+}
+
+/* Show a probe point to output buffer */
+static void show_probepoint(Dwarf_Die sp_die, Dwarf_Signed offs,
+			    struct probe_finder *pf)
+{
+	struct probe_point *pp = pf->pp;
+	char *name;
+	char tmp[MAX_PROBE_BUFFER];
+	int ret, i, len;
+
+	/* Output name of probe point */
+	ret = dwarf_diename(sp_die, &name, &__dw_error);
+	ERR_IF(ret == DW_DLV_ERROR);
+	if (ret == DW_DLV_OK) {
+		ret = snprintf(tmp, MAX_PROBE_BUFFER, "%s+%u", name,
+				(unsigned int)offs);
+		dwarf_dealloc(__dw_debug, name, DW_DLA_STRING);
+	} else {
+		/* 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);
+	len = ret;
+
+	/* Find each argument */
+	get_current_frame_base(sp_die, pf);
+	for (i = 0; i < pp->nr_args; i++) {
+		pf->var = pp->args[i];
+		pf->buf = &tmp[len];
+		pf->len = MAX_PROBE_BUFFER - len;
+		find_variable(sp_die, pf);
+		len += strlen(pf->buf);
+	}
+	free_current_frame_base(pf);
+
+	pp->probes[pp->found] = strdup(tmp);
+	pp->found++;
+}
+
+static int probeaddr_callback(struct die_link *dlink, void *data)
+{
+	struct probe_finder *pf = (struct probe_finder *)data;
+	Dwarf_Half tag;
+	Dwarf_Signed offs;
+	int ret;
+
+	ret = dwarf_tag(dlink->die, &tag, &__dw_error);
+	ERR_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)) {
+		show_probepoint(dlink->die, offs, pf);
+		return 1;
+	}
+	return 0;
+}
+
+/* Find probe point from its line number */
+static void find_by_line(Dwarf_Die cu_die, struct probe_finder *pf)
+{
+	struct probe_point *pp = pf->pp;
+	Dwarf_Signed cnt, i;
+	Dwarf_Line *lines;
+	Dwarf_Unsigned lineno = 0;
+	Dwarf_Addr addr;
+	Dwarf_Unsigned fno;
+	int ret;
+
+	ret = dwarf_srclines(cu_die, &lines, &cnt, &__dw_error);
+	ERR_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);
+		if (fno != pf->fno)
+			continue;
+
+		ret = dwarf_lineno(lines[i], &lineno, &__dw_error);
+		ERR_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);
+		debug("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);
+		if (ret == 0)
+			msg_exit(-1,
+				 "Probe point is not found in subprograms.\n");
+		/* Continuing, because target line might be inlined. */
+	}
+	dwarf_srclines_dealloc(__dw_debug, lines, cnt);
+}
+
+/* Search function from function name */
+static int probefunc_callback(struct die_link *dlink, void *data)
+{
+	struct probe_finder *pf = (struct probe_finder *)data;
+	struct probe_point *pp = pf->pp;
+	struct die_link *lk;
+	Dwarf_Signed offs;
+	Dwarf_Half tag;
+	int ret;
+
+	ret = dwarf_tag(dlink->die, &tag, &__dw_error);
+	ERR_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)) {
+				/* Inlined function, save it. */
+				ret = dwarf_die_CU_offset(dlink->die,
+							  &pf->inl_offs,
+							  &__dw_error);
+				ERR_IF(ret != DW_DLV_OK);
+				debug("inline definition offset %lld\n",
+					pf->inl_offs);
+				return 0;
+			}
+			/* Get probe address */
+			pf->addr = die_get_entrypc(dlink->die);
+			pf->addr += pp->offset;
+			/* TODO: Check the address in this function */
+			show_probepoint(dlink->die, pp->offset, pf);
+			/* Continue to search */
+		}
+	} else if (tag == DW_TAG_inlined_subroutine && pf->inl_offs) {
+		if (die_get_abstract_origin(dlink->die) == pf->inl_offs) {
+			/* Get probe address */
+			pf->addr = die_get_entrypc(dlink->die);
+			pf->addr += pp->offset;
+			debug("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;
+				dwarf_tag(lk->die, &tag, &__dw_error);
+				ERR_IF(ret == DW_DLV_ERROR);
+				if (tag == DW_TAG_subprogram &&
+				    !die_inlined_subprogram(lk->die))
+					goto found;
+			}
+			msg_exit(-1, "Failed to find real subprogram.\n");
+found:
+			/* Get offset from subprogram */
+			ret = die_within_subprogram(lk->die, pf->addr, &offs);
+			ERR_IF(!ret);
+			show_probepoint(lk->die, offs, pf);
+			/* Continue to search */
+		}
+	}
+	return 0;
+}
+
+static void find_by_func(Dwarf_Die cu_die, struct probe_finder *pf)
+{
+	search_die_from_children(cu_die, probefunc_callback, pf);
+}
+
+/* Find a probe point */
+int find_probepoint(int fd, struct probe_point *pp)
+{
+	Dwarf_Half addr_size = 0;
+	Dwarf_Unsigned next_cuh = 0;
+	Dwarf_Die cu_die = 0;
+	int cu_number = 0, ret;
+	struct probe_finder pf = {.pp = 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");
+
+	pp->found = 0;
+	while (++cu_number) {
+		/* 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);
+		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);
+
+		/* Check if target file is included. */
+		if (pp->file)
+			pf.fno = die_get_fileno(cu_die, pp->file);
+
+		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);
+			if (ret == DW_DLV_NO_ENTRY)
+				pf.cu_base = 0;
+			if (pp->line)
+				find_by_line(cu_die, &pf);
+			if (pp->function)
+				find_by_func(cu_die, &pf);
+		}
+		dwarf_dealloc(__dw_debug, cu_die, DW_DLA_DIE);
+	}
+	ret = dwarf_finish(__dw_debug, &__dw_error);
+	ERR_IF(ret != DW_DLV_OK);
+
+	return pp->found;
+}
+
diff --git a/tools/perf/util/probe-finder.h b/tools/perf/util/probe-finder.h
new file mode 100644
index 0000000..af920de
--- /dev/null
+++ b/tools/perf/util/probe-finder.h
@@ -0,0 +1,68 @@
+#ifndef _PROBE_FINDER_H
+#define _PROBE_FINDER_H
+
+#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__) \
+			"): " stringify(cnd) "\n");			\
+		exit(1);						\
+	} } while (0)
+
+#define MAX_PATH_LEN 256
+#define MAX_PROBE_BUFFER 1024
+#define MAX_PROBES 128
+
+static inline int is_c_varname(const char *name)
+{
+	/* TODO */
+	return isalpha(name[0]) || name[0] == '_';
+}
+
+struct probe_point {
+	/* Inputs */
+	char	*file;		/* File name */
+	int	line;		/* Line number */
+
+	char	*function;	/* Function name */
+	int	offset;		/* Offset bytes */
+
+	int	nr_args;	/* Number of arguments */
+	char	**args;		/* Arguments */
+
+	/* Output */
+	int	found;		/* Number of found probe points */
+	char	*probes[MAX_PROBES];	/* Output buffers (will be allocated)*/
+};
+
+extern int find_probepoint(int fd, struct probe_point *pp);
+
+#include <libdwarf/dwarf.h>
+#include <libdwarf/libdwarf.h>
+
+struct probe_finder {
+	struct probe_point	*pp;	/* Target probe point */
+
+	/* For function searching */
+	Dwarf_Addr	addr;		/* Address */
+	Dwarf_Unsigned	fno;		/* File number */
+	Dwarf_Off	inl_offs;	/* Inline offset */
+
+	/* For variable searching */
+	Dwarf_Addr	cu_base;	/* Current CU base address */
+	Dwarf_Locdesc	fbloc;		/* Location of Current Frame Base */
+	const char	*var;		/* Current variable name */
+	char		*buf;		/* Current output buffer */
+	int		len;		/* Length of output buffer */
+};
+
+#endif /*_PROBE_FINDER_H */


-- 
Masami Hiramatsu

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

e-mail: mhiramat@redhat.com

[PATCH tracing/kprobes v2 5/5] perf: kprobe command supports without libdwarf

[Masami Hiramatsu]

Enables perf-kprobe even if libdwarf is installed. If libdwarf is not found,
perf-kprobe just disables dwarf support. Users can use perf-kprobe to set
up new events by using kprobe_events format.

Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Christoph Hellwig <hch@infradead.org>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Jim Keniston <jkenisto@us.ibm.com>
Cc: Frank Ch. Eigler <fche@redhat.com>
---

 tools/perf/Makefile            |    6 +++---
 tools/perf/builtin-probe.c     |   42 +++++++++++++++++++++++++++++++++-------
 tools/perf/perf.c              |    2 --
 tools/perf/util/probe-finder.h |    2 ++
 4 files changed, 40 insertions(+), 12 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 6dabcf1..52b1f43 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -385,6 +385,7 @@ BUILTIN_OBJS += builtin-stat.o
 BUILTIN_OBJS += builtin-timechart.o
 BUILTIN_OBJS += builtin-top.o
 BUILTIN_OBJS += builtin-trace.o
+BUILTIN_OBJS += builtin-probe.o
 
 PERFLIBS = $(LIB_FILE)
 
@@ -420,13 +421,12 @@ ifneq ($(shell sh -c "(echo '\#include <libelf.h>'; echo 'int main(void) { Elf *
 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 probe subcommand. Please install libdwarf-dev/libdwarf-devel);
+	msg := $(warning No libdwarf.h found, disables dwarf support. Please install libdwarf-dev/libdwarf-devel);
+	BASIC_CFLAGS += -DNO_LIBDWARF
 else
 	EXTLIBS += -lelf -ldwarf
 	LIB_H += util/probe-finder.h
 	LIB_OBJS += util/probe-finder.o
-	BUILTIN_OBJS += builtin-probe.o
-	BASIC_CFLAGS += -DSUPPORT_DWARF
 endif
 
 ifdef NO_DEMANGLE
diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index 47c5fb8..0c39b07 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -53,6 +53,7 @@ const char *default_search_path[NR_SEARCH_PATH] = {
 static struct {
 	char *vmlinux;
 	char *release;
+	int need_dwarf;
 	int nr_probe;
 	struct probe_point probes[MAX_PROBES];
 	char *events[MAX_PROBES];
@@ -161,6 +162,8 @@ static int parse_probepoint(const struct option *opt __used,
 		      pp->function, pp->file, pp->offset);
 	}
 	free(argv[1]);
+	if (pp->file)
+		session.need_dwarf = 1;
 
 	/* Copy arguments */
 	pp->nr_args = argc - 2;
@@ -170,15 +173,19 @@ static int parse_probepoint(const struct option *opt __used,
 	}
 
 	/* Ensure return probe has no C argument */
-	if (retp)
-		for (i = 0; i < pp->nr_args; i++)
-			if (is_c_varname(pp->args[i]))
+	for (i = 0; i < pp->nr_args; i++)
+		if (is_c_varname(pp->args[i])) {
+			if (retp)
 				semantic_error("You can't specify local"
 						" variable for kretprobe");
+			session.need_dwarf = 1;
+		}
+
 	debug("%d arguments\n", pp->nr_args);
 	return 0;
 }
 
+#ifndef NO_LIBDWARF
 static int open_default_vmlinux(void)
 {
 	struct utsname uts;
@@ -208,6 +215,7 @@ static int open_default_vmlinux(void)
 	}
 	return fd;
 }
+#endif
 
 static const char * const probe_usage[] = {
 	"perf probe [<options>] -P 'PROBEDEF' [-P 'PROBEDEF' ...]",
@@ -215,11 +223,17 @@ static const char * const probe_usage[] = {
 };
 
 static const struct option options[] = {
+#ifndef NO_LIBDWARF
 	OPT_STRING('k', "vmlinux", &session.vmlinux, "file",
 		"vmlinux/module pathname"),
 	OPT_STRING('r', "release", &session.release, "rel", "kernel release"),
+#endif
 	OPT_CALLBACK('P', "probe", NULL,
+#ifdef NO_LIBDWARF
+		"p|r:[GRP/]NAME FUNC[+OFFS] [ARG ...]",
+#else
 		"p|r:[GRP/]NAME FUNC[+OFFS][@SRC]|@SRC:LINE [ARG ...]",
+#endif
 		"probe point definition, where\n"
 		"\t\tp:\tkprobe probe\n"
 		"\t\tr:\tkretprobe probe\n"
@@ -227,9 +241,13 @@ static const struct option options[] = {
 		"\t\tNAME:\tEvent name\n"
 		"\t\tFUNC:\tFunction name\n"
 		"\t\tOFFS:\tOffset from function entry (in byte)\n"
+#ifdef NO_LIBDWARF
+		"\t\tARG:\tProbe argument (only \n"
+#else
 		"\t\tSRC:\tSource code path\n"
 		"\t\tLINE:\tLine number\n"
 		"\t\tARG:\tProbe argument (local variable name or\n"
+#endif
 		"\t\t\tkprobe-tracer argument format is supported.)\n",
 		parse_probepoint),
 	OPT_END()
@@ -277,7 +295,7 @@ error:
 
 int cmd_probe(int argc, const char **argv, const char *prefix __used)
 {
-	int i, j, fd, ret, need_dwarf = 0;
+	int i, j, fd, ret;
 	struct probe_point *pp;
 	char buf[MAX_CMDLEN];
 
@@ -286,12 +304,19 @@ int cmd_probe(int argc, const char **argv, const char *prefix __used)
 	if (argc || session.nr_probe == 0)
 		usage_with_options(probe_usage, options);
 
-	/* Synthesize return probes */
+#ifdef NO_LIBDWARF
+	if (session.need_dwarf)
+		semantic_error("Dwarf-analysis is not supported");
+#endif
+
+	/* Synthesize probes without dwarf */
 	for (j = 0; j < session.nr_probe; j++) {
+#ifndef NO_LIBDWARF
 		if (session.events[j][0] != 'r') {
-			need_dwarf = 1;
+			session.need_dwarf = 1;
 			continue;
 		}
+#endif
 		ret = synthesize_probepoint(&session.probes[j]);
 		if (ret == -E2BIG)
 			semantic_error("probe point is too long.");
@@ -301,7 +326,8 @@ int cmd_probe(int argc, const char **argv, const char *prefix __used)
 		}
 	}
 
-	if (!need_dwarf)
+#ifndef NO_LIBDWARF
+	if (!session.need_dwarf)
 		goto setup_probes;
 
 	if (session.vmlinux)
@@ -330,6 +356,8 @@ int cmd_probe(int argc, const char **argv, const char *prefix __used)
 	close(fd);
 
 setup_probes:
+#endif /* !NO_LIBDWARF */
+
 	/* Settng up probe points */
 	snprintf(buf, MAX_CMDLEN, "%s/../kprobe_events", debugfs_path);
 	fd = open(buf, O_WRONLY, O_APPEND);
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index f598120..c570d17 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -295,9 +295,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "version", cmd_version, 0 },
 		{ "trace", cmd_trace, 0 },
 		{ "sched", cmd_sched, 0 },
-#ifdef SUPPORT_DWARF
 		{ "probe", cmd_probe, 0 },
-#endif
 	};
 	unsigned int i;
 	static const char ext[] = STRIP_EXTENSION;
diff --git a/tools/perf/util/probe-finder.h b/tools/perf/util/probe-finder.h
index af920de..306810c 100644
--- a/tools/perf/util/probe-finder.h
+++ b/tools/perf/util/probe-finder.h
@@ -44,6 +44,7 @@ struct probe_point {
 	char	*probes[MAX_PROBES];	/* Output buffers (will be allocated)*/
 };
 
+#ifndef NO_LIBDWARF
 extern int find_probepoint(int fd, struct probe_point *pp);
 
 #include <libdwarf/dwarf.h>
@@ -64,5 +65,6 @@ struct probe_finder {
 	char		*buf;		/* Current output buffer */
 	int		len;		/* Length of output buffer */
 };
+#endif /* NO_LIBDWARF */
 
 #endif /*_PROBE_FINDER_H */


-- 
Masami Hiramatsu

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

e-mail: mhiramat@redhat.com

Re: [PATCH tracing/kprobes v2 0/5] tracing/kprobes, perf: perf probe support take 2

[Frederic Weisbecker]

On Fri, Oct 02, 2009 at 05:48:34PM -0400, Masami Hiramatsu wrote:
> Hi,
> 
> These patches introduce 'perf probe' command and update kprobe-tracer.
> perf probe command allows you to add new probe points by C line number
> and local variable names.
> 
> This version fixes some bugs, changes subcommand name from kprobe to
> probe and use spaces for separator instead of ',' for visibility (this
> also make it easy to support probe list from stdin).
> 
> Usage
> -----
>  perf probe [<options>] -P 'PROBEDEF' [-P 'PROBEDEF' ...]
> 
>     -k, --vmlinux <file>  vmlinux/module pathname
>     -r, --release <rel>   kernel release
>     -P, --probe <p|r:[GRP/]NAME FUNC[+OFFS][@SRC]|@SRC:LINE [ARG ...]>
>                           probe point definition, where
> 		p:	kprobe probe
> 		r:	kretprobe probe
> 		GRP:	Group name (optional)
> 		NAME:	Event name
> 		FUNC:	Function name
> 		OFFS:	Offset from function entry (in byte)
> 		SRC:	Source code path
> 		LINE:	Line number
> 		ARG:	Probe argument (local variable name or
> 			kprobe-tracer argument format is supported.)
> 
> Examples
> --------
> 1) Add a new kprobe probe on a line of C source code.
> ./perf probe -P 'p:myprobe @fs/read_write.c:285 file buf count'
> Adding new event: p:myprobe vfs_read+57 file=%bx buf=%si count=%ax


Nice! Great thing.

One neat, at a first glance, file=%bx buf=%si look like a format
definition.

How about using file=bx ? Or does that introduce any ambiguities
with kprobes definitions syntax?



> 
> 2) Add a new kretprobe probe on a function return.
> ./perf probe -P 'r:myretprobe vfs_read $rv'
> Adding new event: r:myretprobe vfs_read+0 $rv


The '$' character may perhaps also confuse bash scripts that create
perf probe.



> 3) Check it in the perf list.
> ./perf list
> ...
>   rNNN                                       [raw hardware event descriptor]
> 
>   kprobes:myprobe                            [Tracepoint event]
>   kprobes:myretprobe                         [Tracepoint event]
>   skb:kfree_skb                              [Tracepoint event]
> ...
> 
> 4) Record the event by perf
> ./perf record -f -e kprobes:myprobe:record  -F 1 -a ls
> ...
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.081 MB perf.data (~3540 samples) ]
> 
> 5) Trace the event
> ./perf trace
>             perf-11445 [000] 95862.048894383: myprobe: (c04bbed5) file=dae15e80 buf=b78b2000 count=400
>             perf-11445 [000] 95862.049066533: myprobe: (c04bbed5) file=dae15d80 buf=b78b2000 count=400
>             perf-11445 [000] 95862.049134394: myprobe: (c04bbed5) file=dae15d80 buf=b78b2000 count=400
>             perf-11445 [000] 95862.049171495: myprobe: (c04bbed5) file=dae15a80 buf=b78b2000 count=400
> 
> NOTE
> ----
>  perf still fails to parse format if arguments have special charactors
> (e.g. $rv, +10($sp) etc.) So, tracing myretprobe will fail with this
> version. This will be solved by naming arguments automatically if it
> doesn't have C-language name.
> 
> TODO
> ----
>  - Support sys_perf_counter_open (non-root)


Hmm, we really want it to be only usable by the root
(except if the policy is tuned to allow that, but that's
managed by perf already).


>  - Input from stdin/output to stdout
>  - Non-auto static variable
>  - Fields of data structures (var->field)
>  - Type support
>    - Bit fields
>  - Array (var[N])
>  - Dynamic array indexing (var[var2])
>  - String/dynamic arrays (var:string, var[N..M])
>  - Force Type casting ((type)var)
>  - Non-inline search
>  - libdw, libdwfl
>  - etc.
> 
> Thank you,
> 


Cool, I'm reviewing/testing it and if no rough problem
arise I'll apply it.

Thanks a lot!

Re: [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Frederic Weisbecker]

On Fri, Oct 02, 2009 at 05:48:42PM -0400, Masami Hiramatsu wrote:
> Add $ prefix to the special variables(e.g. sa, rv) of kprobe-tracer.
> This resolves consistency issue between kprobe_events and perf-kprobe.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Mike Galbraith <efault@gmx.de>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> Cc: Jim Keniston <jkenisto@us.ibm.com>
> Cc: Frank Ch. Eigler <fche@redhat.com>
> ---
> 
>  Documentation/trace/kprobetrace.txt |   10 +++---
>  kernel/trace/trace_kprobe.c         |   60 ++++++++++++++++++++++-------------
>  2 files changed, 42 insertions(+), 28 deletions(-)
> 
> diff --git a/Documentation/trace/kprobetrace.txt b/Documentation/trace/kprobetrace.txt
> index 9b8f7c6..40caef0 100644
> --- a/Documentation/trace/kprobetrace.txt
> +++ b/Documentation/trace/kprobetrace.txt
> @@ -36,13 +36,13 @@ Synopsis of kprobe_events
>  
>   FETCHARGS	: Arguments. Each probe can have up to 128 args.
>    %REG	: Fetch register REG
> -  sN	: Fetch Nth entry of stack (N >= 0)
> -  sa	: Fetch stack address.
>    @ADDR	: Fetch memory at ADDR (ADDR should be in kernel)
>    @SYM[+|-offs]	: Fetch memory at SYM +|- offs (SYM should be a data symbol)
> -  aN	: Fetch function argument. (N >= 0)(*)
> -  rv	: Fetch return value.(**)
> -  ra	: Fetch return address.(**)
> +  $sN	: Fetch Nth entry of stack (N >= 0)
> +  $sa	: Fetch stack address.
> +  $aN	: Fetch function argument. (N >= 0)(*)
> +  $rv	: Fetch return value.(**)
> +  $ra	: Fetch return address.(**)



I feel uncomfortable with that, because of bash scripts that
may use it and always need to escape it with antislashes or
use single quotes for it to not be replaced by a random variable
value. If one uses double quotes without antislashes to protect
the command line, the result is unpredictable, depending of
the current set of variables...

May be we can use # instead of $ ? That's a kind of pity because
$ suggested a variable whereas # suggests a constant, we are then
losing this self-explainable characteristic for kprobes
"specific variable fetchs". But that will work without ambiguity.

I hope someone else has a better idea...

Re: [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Masami Hiramatsu]

Frederic Weisbecker wrote:
> On Fri, Oct 02, 2009 at 05:48:42PM -0400, Masami Hiramatsu wrote:
>> Add $ prefix to the special variables(e.g. sa, rv) of kprobe-tracer.
>> This resolves consistency issue between kprobe_events and perf-kprobe.
>>
>> Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
>> Cc: Frederic Weisbecker <fweisbec@gmail.com>
>> Cc: Ingo Molnar <mingo@elte.hu>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
>> Cc: Steven Rostedt <rostedt@goodmis.org>
>> Cc: Mike Galbraith <efault@gmx.de>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
>> Cc: Christoph Hellwig <hch@infradead.org>
>> Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
>> Cc: Jim Keniston <jkenisto@us.ibm.com>
>> Cc: Frank Ch. Eigler <fche@redhat.com>
>> ---
>>
>>  Documentation/trace/kprobetrace.txt |   10 +++---
>>  kernel/trace/trace_kprobe.c         |   60 ++++++++++++++++++++++-------------
>>  2 files changed, 42 insertions(+), 28 deletions(-)
>>
>> diff --git a/Documentation/trace/kprobetrace.txt b/Documentation/trace/kprobetrace.txt
>> index 9b8f7c6..40caef0 100644
>> --- a/Documentation/trace/kprobetrace.txt
>> +++ b/Documentation/trace/kprobetrace.txt
>> @@ -36,13 +36,13 @@ Synopsis of kprobe_events
>>  
>>   FETCHARGS	: Arguments. Each probe can have up to 128 args.
>>    %REG	: Fetch register REG
>> -  sN	: Fetch Nth entry of stack (N >= 0)
>> -  sa	: Fetch stack address.
>>    @ADDR	: Fetch memory at ADDR (ADDR should be in kernel)
>>    @SYM[+|-offs]	: Fetch memory at SYM +|- offs (SYM should be a data symbol)
>> -  aN	: Fetch function argument. (N >= 0)(*)
>> -  rv	: Fetch return value.(**)
>> -  ra	: Fetch return address.(**)
>> +  $sN	: Fetch Nth entry of stack (N >= 0)
>> +  $sa	: Fetch stack address.
>> +  $aN	: Fetch function argument. (N >= 0)(*)
>> +  $rv	: Fetch return value.(**)
>> +  $ra	: Fetch return address.(**)
> 
> 
> 
> I feel uncomfortable with that, because of bash scripts that
> may use it and always need to escape it with antislashes or
> use single quotes for it to not be replaced by a random variable
> value. If one uses double quotes without antislashes to protect
> the command line, the result is unpredictable, depending of
> the current set of variables...
> 
> May be we can use # instead of $ ? That's a kind of pity because
> $ suggested a variable whereas # suggests a constant, we are then
> losing this self-explainable characteristic for kprobes
> "specific variable fetchs". But that will work without ambiguity.

Hmm, # is widely used for comment, including some kernel pseudo
file interfaces, kprobe_events too. Comments are useful if a
probe list is restored from a file.

For accessing local variables, kprobe-tracer needs to support *at least*
below variables:
- Registers
- Stack address (if a register points stack address, this isn't needed)

Below special vars are complementary aliases.
- Function arguments
- Return value
- Return address

and I'd like perf-probe to have a transparent syntax with kprobe-tracer.

This means, if we can remove special vars except registers, or rename it
non-conflictable name with registers, we just need to separate name spaces
of
- Regsiters
- Local variables

Here, local variables will support fields of data structs, and it will
use '->' expression. Since '>' means redirection in bash, local variables
need to be *escaped* in this case. Thus, I think we can use '$' prefix
for it. (I'm OK, because this is similar syntax to systemtap:-).

So, if you don't like %regs, $svars and locals, we can use regs and $locals :-)

Thank you,


-- 
Masami Hiramatsu

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

e-mail: mhiramat@redhat.com

Re: [PATCH tracing/kprobes v2 0/5] tracing/kprobes, perf: perf probe support take 2

[Frank Ch. Eigler]

Masami Hiramatsu <mhiramat@redhat.com> writes:

> [...]
> These patches introduce 'perf probe' command and update kprobe-tracer. [...]
> Usage
> -----
>  perf probe [<options>] -P 'PROBEDEF' [-P 'PROBEDEF' ...]
> [...]
>     -k, --vmlinux <file>  vmlinux/module pathname
>     -r, --release <rel>   kernel release
> [...]

Can you outline a reason for the -r flag?  We use it in systemtap for
cross-compiling purposes, but I didn't think perf was interested in that.

- FChE

Re: [PATCH tracing/kprobes v2 0/5] tracing/kprobes, perf: perf probe support take 2

[Masami Hiramatsu]

Frank Ch. Eigler wrote:
> Masami Hiramatsu<mhiramat@redhat.com>  writes:
>
>> [...]
>> These patches introduce 'perf probe' command and update kprobe-tracer. [...]
>> Usage
>> -----
>>   perf probe [<options>] -P 'PROBEDEF' [-P 'PROBEDEF' ...]
>> [...]
>>      -k, --vmlinux<file>   vmlinux/module pathname
>>      -r, --release<rel>    kernel release
>> [...]
>
> Can you outline a reason for the -r flag?  We use it in systemtap for
> cross-compiling purposes, but I didn't think perf was interested in that.

Indeed, thanks!
I just forgot to remove -r option from the code of c2kpe.c :(

-- 
Masami Hiramatsu

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

e-mail: mhiramat@redhat.com

Re: [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Masami Hiramatsu]

Masami Hiramatsu wrote:
> Frederic Weisbecker wrote:
>> On Fri, Oct 02, 2009 at 05:48:42PM -0400, Masami Hiramatsu wrote:
>>>    FETCHARGS	: Arguments. Each probe can have up to 128 args.
>>>     %REG	: Fetch register REG
>>> -  sN	: Fetch Nth entry of stack (N>= 0)
>>> -  sa	: Fetch stack address.
>>>     @ADDR	: Fetch memory at ADDR (ADDR should be in kernel)
>>>     @SYM[+|-offs]	: Fetch memory at SYM +|- offs (SYM should be a data symbol)
>>> -  aN	: Fetch function argument. (N>= 0)(*)
>>> -  rv	: Fetch return value.(**)
>>> -  ra	: Fetch return address.(**)
>>> +  $sN	: Fetch Nth entry of stack (N>= 0)
>>> +  $sa	: Fetch stack address.
>>> +  $aN	: Fetch function argument. (N>= 0)(*)
>>> +  $rv	: Fetch return value.(**)
>>> +  $ra	: Fetch return address.(**)
>>
>>
>>
>> I feel uncomfortable with that, because of bash scripts that
>> may use it and always need to escape it with antislashes or
>> use single quotes for it to not be replaced by a random variable
>> value. If one uses double quotes without antislashes to protect
>> the command line, the result is unpredictable, depending of
>> the current set of variables...
>>
>> May be we can use # instead of $ ? That's a kind of pity because
>> $ suggested a variable whereas # suggests a constant, we are then
>> losing this self-explainable characteristic for kprobes
>> "specific variable fetchs". But that will work without ambiguity.
>
> Hmm, # is widely used for comment, including some kernel pseudo
> file interfaces, kprobe_events too. Comments are useful if a
> probe list is restored from a file.
>
> For accessing local variables, kprobe-tracer needs to support *at least*
> below variables:
> - Registers
> - Stack address (if a register points stack address, this isn't needed)
>
> Below special vars are complementary aliases.
> - Function arguments
> - Return value
> - Return address
>
> and I'd like perf-probe to have a transparent syntax with kprobe-tracer.
>
> This means, if we can remove special vars except registers, or rename it
> non-conflictable name with registers, we just need to separate name spaces
> of
> - Regsiters
> - Local variables
>
> Here, local variables will support fields of data structs, and it will
> use '->' expression. Since'>' means redirection in bash, local variables
> need to be *escaped* in this case. Thus, I think we can use '$' prefix
> for it. (I'm OK, because this is similar syntax to systemtap:-).
>
> So, if you don't like %regs, $svars and locals, we can use regs and $locals :-)

As far as I can see in arch/*/include/asm/ptrace.h, all registers start with
alphabets :-). So, I'd like to suggest renaming sp-vars to '_sp-vars'.

Then, we will have;
- $local-vars
- @global-symbol
- regs
- _sp-vars
- +|-Offs(ARG)

Thank you,

-- 
Masami Hiramatsu

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

e-mail: mhiramat@redhat.com

Re: [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Frederic Weisbecker]

On Sun, Oct 04, 2009 at 01:21:52AM -0400, Masami Hiramatsu wrote:
> Hmm, # is widely used for comment, including some kernel pseudo
> file interfaces, kprobe_events too. Comments are useful if a
> probe list is restored from a file.
>


Right, let's think about something else.


> For accessing local variables, kprobe-tracer needs to support *at least*
> below variables:
> - Registers
> - Stack address (if a register points stack address, this isn't needed)


Ok.
Well, thinking more about the % sign, we shouldn't worry about
format confusion, since it's a commonly used character for registers,
we can take it for them: %rax, %rbx, etc. (is that what you did
in this patch? I don't remember exactly...)

And for addresses: @addr


> Below special vars are complementary aliases.
> - Function arguments



For the function arguments, I guess we don't need to worry
anymore about r0, r1, etc... but we can deal with the true var
name, without any kind of prefixes.


> - Return value



What about %return ?

As return values are usually stored in a register (at least in Arm
and x86, I don't know about the others), the % prefix fits well for
that.


> - Return address


What about @return :-) ?

That said we shouldn't worry about that in perf, since we can
take snapshots of the backtraces, like in some other perf events.


> and I'd like perf-probe to have a transparent syntax with kprobe-tracer.


That's feasible. But think about the fact that perf probe benefits
from a higher level of code view. Now that we have global and local
variables resolution, we can't anymore expect using r1, r2, rax, 
a1, rv, ra without risking to collide with variable names.

But this tracer hasn't been merged yet, so it's still time
to update its interface.

 
> This means, if we can remove special vars except registers, or rename it
> non-conflictable name with registers, we just need to separate name spaces
> of
> - Regsiters
> - Local variables


Yeah.


> Here, local variables will support fields of data structs, and it will
> use '->' expression. Since '>' means redirection in bash, local variables
> need to be *escaped* in this case. Thus, I think we can use '$' prefix
> for it. (I'm OK, because this is similar syntax to systemtap:-).
> 
> So, if you don't like %regs, $svars and locals, we can use regs and $locals :-)


'>' means redirection, but at least inside quotes it's not interpreted.

I'm fine with %regs actually. But I really don't like the "$" because
I really worry about shell substitutions.

Most people delimit their strings with double quotes.

What if we take the following:

[Ftrace and perf probe side]

%reg = registers, we can also play with deref and offsets like (%esp), 8(%esp), etc.
%return = return value
@return = return addr
arg(n) = arg number, where n is the number


[Perf probe only]

var = variable name, global or local, we can deal with shadow issues later
      through variable scope: func_name:var, filename:var, whatever for now
      it's not a problem. Local also includes argument names.

Hmm?

Re: [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Frederic Weisbecker]

On Mon, Oct 05, 2009 at 12:59:01PM -0400, Masami Hiramatsu wrote:
> As far as I can see in arch/*/include/asm/ptrace.h, all registers start with
> alphabets :-). So, I'd like to suggest renaming sp-vars to '_sp-vars'.
>
> Then, we will have;
> - $local-vars


There is a risk of bash collision.


> - @global-symbol


We could use global-symbol as is. Shadowing between global
and local vars could be dealt with scope resolution:

function:var
file:var
file:line:var

And throw errors while submitting a shadowed var name, crying until
the user defines the scope, only if needed of course (if there are
no shadowing detected, we can submit a naked variable name).


> - regs


That can conflict with variable names


> - _sp-vars


That too.


> - +|-Offs(ARG)

You mean for arg numbers?
So we would have +1 for argument 1?

arg(1) looks more easy to remember and to understand, no?

Thanks.

Re: [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Frederic Weisbecker]

On Mon, Oct 05, 2009 at 09:18:31PM +0200, Frederic Weisbecker wrote:
> On Sun, Oct 04, 2009 at 01:21:52AM -0400, Masami Hiramatsu wrote:
> > Hmm, # is widely used for comment, including some kernel pseudo
> > file interfaces, kprobe_events too. Comments are useful if a
> > probe list is restored from a file.
> >
> 
> 
> Right, let's think about something else.
> 
> 
> > For accessing local variables, kprobe-tracer needs to support *at least*
> > below variables:
> > - Registers
> > - Stack address (if a register points stack address, this isn't needed)
> 
> 
> Ok.
> Well, thinking more about the % sign, we shouldn't worry about
> format confusion, since it's a commonly used character for registers,
> we can take it for them: %rax, %rbx, etc. (is that what you did
> in this patch? I don't remember exactly...)
> 
> And for addresses: @addr
> 
> 
> > Below special vars are complementary aliases.
> > - Function arguments
> 
> 
> 
> For the function arguments, I guess we don't need to worry
> anymore about r0, r1, etc... but we can deal with the true var
> name, without any kind of prefixes.


Sorry, as you pointed out, it's better to keep the same syntax for
both perf probe and ftrace, but having perf probe able to support
variable name.

Hence the following that don't collide: varname, arg(n)


> 
> > - Return value
> 
> 
> 
> What about %return ?
> 
> As return values are usually stored in a register (at least in Arm
> and x86, I don't know about the others), the % prefix fits well for
> that.
> 
> 
> > - Return address
> 
> 
> What about @return :-) ?
> 
> That said we shouldn't worry about that in perf, since we can
> take snapshots of the backtraces, like in some other perf events.
> 



But we need to worry about it because we want to share the ftrace syntax.
So yeah, why not @return ?

Re: [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Masami Hiramatsu]

Frederic Weisbecker wrote:
> On Sun, Oct 04, 2009 at 01:21:52AM -0400, Masami Hiramatsu wrote:
>> Hmm, # is widely used for comment, including some kernel pseudo
>> file interfaces, kprobe_events too. Comments are useful if a
>> probe list is restored from a file.
>>
>
>
> Right, let's think about something else.
>
>
>> For accessing local variables, kprobe-tracer needs to support *at least*
>> below variables:
>> - Registers
>> - Stack address (if a register points stack address, this isn't needed)
>
>
> Ok.
> Well, thinking more about the % sign, we shouldn't worry about
> format confusion, since it's a commonly used character for registers,
> we can take it for them: %rax, %rbx, etc. (is that what you did
> in this patch? I don't remember exactly...)

Yeah, that's what it does currently.

> And for addresses: @addr
>
>
>> Below special vars are complementary aliases.
>> - Function arguments
>
>
>
> For the function arguments, I guess we don't need to worry
> anymore about r0, r1, etc... but we can deal with the true var
> name, without any kind of prefixes.

This depends on ABI, function argument from ABI doesn't need
debuginfo, but it will be unstable on some arch, e.g. x86-32
with/without asmlinkage.

Thus, I think that we can just describe where function arguments
will be(e.g. arg0 is ax) as a note for each architecture
in Documents/trace/kprobetrace.txt.

>> - Return value
>
> What about %return ?
>
> As return values are usually stored in a register (at least in Arm
> and x86, I don't know about the others), the % prefix fits well for
> that.

Sure, that's what I

>> - Return address
>
>
> What about @return :-) ?

Hmm, it might conflict with global symbol... Maybe, we can remove this
because retprobe already shows return address in the head of entry.

(BTW, ideally, I think 'IP' should be the return address in kretprobe.
  But current implementation isn't.)

> That said we shouldn't worry about that in perf, since we can
> take snapshots of the backtraces, like in some other perf events.

Agreed.

>> and I'd like perf-probe to have a transparent syntax with kprobe-tracer.
>
>
> That's feasible. But think about the fact that perf probe benefits
> from a higher level of code view. Now that we have global and local
> variables resolution, we can't anymore expect using r1, r2, rax,
> a1, rv, ra without risking to collide with variable names.
>
> But this tracer hasn't been merged yet, so it's still time
> to update its interface.

Sure.

>> This means, if we can remove special vars except registers, or rename it
>> non-conflictable name with registers, we just need to separate name spaces
>> of
>> - Regsiters
>> - Local variables
>
>
> Yeah.
>
>
>> Here, local variables will support fields of data structs, and it will
>> use '->' expression. Since'>' means redirection in bash, local variables
>> need to be *escaped* in this case. Thus, I think we can use '$' prefix
>> for it. (I'm OK, because this is similar syntax to systemtap:-).
>>
>> So, if you don't like %regs, $svars and locals, we can use regs and $locals :-)
>
>
> '>' means redirection, but at least inside quotes it's not interpreted.

Ah, indeed.

> I'm fine with %regs actually. But I really don't like the "$" because
> I really worry about shell substitutions.
>
> Most people delimit their strings with double quotes.

Sure, especially C programmers :-)

> What if we take the following:
>
> [Ftrace and perf probe side]
>
> %reg = registers, we can also play with deref and offsets like (%esp), 8(%esp), etc.

Hmm, on x86-32, sp at intr context is not pointing the top of stack. actually &sp is
the real address of the stack :(
Perhaps, on x86-32, we can translate %sp to stack address in kprobe-tracer.

 > %return = return value

or %retval? :)

> @return = return addr

I'd like to remove it, because it's already shown.

> arg(n) = arg number, where n is the number

How about %N? or just adds a note in documents.


> [Perf probe only]
>
> var = variable name, global or local, we can deal with shadow issues later
>        through variable scope: func_name:var, filename:var, whatever for now
>        it's not a problem. Local also includes argument names.

That's fine to me. :-)

Thank you!

-- 
Masami Hiramatsu

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

e-mail: mhiramat@redhat.com

Re: [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Masami Hiramatsu]

Frederic Weisbecker wrote:
>> - @global-symbol
>
>
> We could use global-symbol as is. Shadowing between global
> and local vars could be dealt with scope resolution:
>
> function:var
> file:var
> file:line:var
>
> And throw errors while submitting a shadowed var name, crying until
> the user defines the scope, only if needed of course (if there are
> no shadowing detected, we can submit a naked variable name).

Sure, via perf-probe, global/local symbols will be translated into
@address by debuginfo.
But without debuginfo, we still need @symbol syntax for accessing
global defined symbols.

>> - +|-Offs(ARG)
>
> You mean for arg numbers?
> So we would have +1 for argument 1?

Sorry for confusing you, it is for deref syntax.

> arg(1) looks more easy to remember and to understand, no?

I think arg(N) seems a bit different from other parts.
Perhaps, %argumentN is possible ? :-)

Thank you,

-- 
Masami Hiramatsu

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

e-mail: mhiramat@redhat.com

Re: [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Frederic Weisbecker]

On Mon, Oct 05, 2009 at 04:18:39PM -0400, Masami Hiramatsu wrote:
> Frederic Weisbecker wrote:
>> For the function arguments, I guess we don't need to worry
>> anymore about r0, r1, etc... but we can deal with the true var
>> name, without any kind of prefixes.
>
> This depends on ABI, function argument from ABI doesn't need
> debuginfo, but it will be unstable on some arch, e.g. x86-32
> with/without asmlinkage.
>
> Thus, I think that we can just describe where function arguments
> will be(e.g. arg0 is ax) as a note for each architecture
> in Documents/trace/kprobetrace.txt.


Yeah that may help. Although everyone can look at the calling convention
ABI for a given arch but that would still help.


>> What about @return :-) ?
>
> Hmm, it might conflict with global symbol... Maybe, we can remove this
> because retprobe already shows return address in the head of entry.


It won't conflict since "return" is a reserved word and can't then be
used as a symbol.

But yeah, if it's an embeded field, we should remove it.


>> What if we take the following:
>>
>> [Ftrace and perf probe side]
>>
>> %reg = registers, we can also play with deref and offsets like (%esp), 8(%esp), etc.
>
> Hmm, on x86-32, sp at intr context is not pointing the top of stack. actually &sp is
> the real address of the stack :(
> Perhaps, on x86-32, we can translate %sp to stack address in kprobe-tracer.



Oh? You mean in the saved registers while triggering an int 3?



> > %return = return value
>
> or %retval? :)


Yeah, better!


>
>> @return = return addr
>
> I'd like to remove it, because it's already shown.


Ok.



>> arg(n) = arg number, where n is the number
>
> How about %N? or just adds a note in documents.
>


Hmm, the problem is that %1, %2, etc. is not very self-explainable.

May be %arg1, %arg2, etc.. But would that sound confusing since we
have % for registers?



>> [Perf probe only]
>>
>> var = variable name, global or local, we can deal with shadow issues later
>>        through variable scope: func_name:var, filename:var, whatever for now
>>        it's not a problem. Local also includes argument names.
>
> That's fine to me. :-)
>


Great :)
Thanks!

Re: [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Masami Hiramatsu]

Frederic Weisbecker wrote:
> On Mon, Oct 05, 2009 at 04:18:39PM -0400, Masami Hiramatsu wrote:
>> Frederic Weisbecker wrote:
>>> For the function arguments, I guess we don't need to worry
>>> anymore about r0, r1, etc... but we can deal with the true var
>>> name, without any kind of prefixes.
>>
>> This depends on ABI, function argument from ABI doesn't need
>> debuginfo, but it will be unstable on some arch, e.g. x86-32
>> with/without asmlinkage.
>>
>> Thus, I think that we can just describe where function arguments
>> will be(e.g. arg0 is ax) as a note for each architecture
>> in Documents/trace/kprobetrace.txt.
>
>
> Yeah that may help. Although everyone can look at the calling convention
> ABI for a given arch but that would still help.
>
>
>>> What about @return :-) ?
>>
>> Hmm, it might conflict with global symbol... Maybe, we can remove this
>> because retprobe already shows return address in the head of entry.
>
>
> It won't conflict since "return" is a reserved word and can't then be
> used as a symbol.
>
> But yeah, if it's an embeded field, we should remove it.
>
>
>>> What if we take the following:
>>>
>>> [Ftrace and perf probe side]
>>>
>>> %reg = registers, we can also play with deref and offsets like (%esp), 8(%esp), etc.
>>
>> Hmm, on x86-32, sp at intr context is not pointing the top of stack. actually&sp is
>> the real address of the stack :(
>> Perhaps, on x86-32, we can translate %sp to stack address in kprobe-tracer.
>
>
> Oh? You mean in the saved registers while triggering an int 3?

Yes, interrupt/exception handlers don't save sp on x86-32.

>>> arg(n) = arg number, where n is the number
>>
>> How about %N? or just adds a note in documents.
>>
>
>
> Hmm, the problem is that %1, %2, etc. is not very self-explainable.
>
> May be %arg1, %arg2, etc.. But would that sound confusing since we
> have % for registers?

As I sent right now, how about %argumentN ? it will not conflict with
register names...

Thank you,

-- 
Masami Hiramatsu

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

e-mail: mhiramat@redhat.com

Re: [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Frederic Weisbecker]

On Mon, Oct 05, 2009 at 05:05:32PM -0400, Masami Hiramatsu wrote:
> Frederic Weisbecker wrote:
>>> - @global-symbol
>>
>>
>> We could use global-symbol as is. Shadowing between global
>> and local vars could be dealt with scope resolution:
>>
>> function:var
>> file:var
>> file:line:var
>>
>> And throw errors while submitting a shadowed var name, crying until
>> the user defines the scope, only if needed of course (if there are
>> no shadowing detected, we can submit a naked variable name).
>
> Sure, via perf-probe, global/local symbols will be translated into
> @address by debuginfo.
> But without debuginfo, we still need @symbol syntax for accessing
> global defined symbols.



Ah right.


>>> - +|-Offs(ARG)
>>
>> You mean for arg numbers?
>> So we would have +1 for argument 1?
>
> Sorry for confusing you, it is for deref syntax.


Ok :)

Re: [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Frederic Weisbecker]

On Mon, Oct 05, 2009 at 05:11:05PM -0400, Masami Hiramatsu wrote:
> Frederic Weisbecker wrote:
>> Hmm, the problem is that %1, %2, etc. is not very self-explainable.
>>
>> May be %arg1, %arg2, etc.. But would that sound confusing since we
>> have % for registers?
>
> As I sent right now, how about %argumentN ? it will not conflict with
> register names...
>

There are archs that have %arg0 %arg1, ... as register names?

Well, arg(n) looks shorter but I won't personnally mind if
we eventually chose %argumentN. It's also clear, self-explainable
and it won't collide.

Re: [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Masami Hiramatsu]

Frederic Weisbecker wrote:
> On Mon, Oct 05, 2009 at 05:11:05PM -0400, Masami Hiramatsu wrote:
>> Frederic Weisbecker wrote:
>>> Hmm, the problem is that %1, %2, etc. is not very self-explainable.
>>>
>>> May be %arg1, %arg2, etc.. But would that sound confusing since we
>>> have % for registers?
>>
>> As I sent right now, how about %argumentN ? it will not conflict with
>> register names...
>>
>
> There are archs that have %arg0 %arg1, ... as register names?
>
> Well, arg(n) looks shorter but I won't personnally mind if
> we eventually chose %argumentN. It's also clear, self-explainable
> and it won't collide.

Hmm, one idea hits me, how about this? :)
- %register
- %%spvars (%%retval, %%arg0)

Thank you,

-- 
Masami Hiramatsu

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

e-mail: mhiramat@redhat.com

Re: [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Frederic Weisbecker]

On Mon, Oct 05, 2009 at 05:34:24PM -0400, Masami Hiramatsu wrote:
> Hmm, one idea hits me, how about this? :)
> - %register
> - %%spvars (%%retval, %%arg0)


The problem is that such % or %% symbols have a specific
mean in some other well known areas.

If we borrow the % from the AT&T assembly syntax style
to use register names, that we can retrieve in gcc inline
assembly, then one may expect %% to have a meaning inspired
from the same area. %% has its sense in gcc inline assembly,
but applied there, it looks confusing.

I mean, I'm trying to think like someone reading a perf probe
command line without any documentation. The more this person
can understand this command line without documentation, the better.
We know that % is used for register names, some people know that %%
is used for register names too but when we are in gcc inline assembly
with var to reg resolution and need true registers name.
Then if I try to mirror this sense from gcc to perf probe use,
I feel confused, especially in the case of %%arg1.

In this case, we should rather have %%register and %arg0 :)

Hm, %register is a clear pattern.

Somehow, %retval looks clear too, retval is verbose enough and
% is still logical as return values are most of the time (always?)
put in a register.

But %%arg0 looks confusing.

Re: [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Frederic Weisbecker]

On Mon, Oct 05, 2009 at 11:55:50PM +0200, Frederic Weisbecker wrote:
> In this case, we should rather have %%register and %arg0 :)

And the above construct would be so much not obvious in its self
meaning, wrt its gcc inline inspiration.

/me should stop quibbling...

Re: [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Masami Hiramatsu]

Frederic Weisbecker wrote:
> On Mon, Oct 05, 2009 at 05:34:24PM -0400, Masami Hiramatsu wrote:
>> Hmm, one idea hits me, how about this? :)
>> - %register
>> - %%spvars (%%retval, %%arg0)
>
>
> The problem is that such % or %% symbols have a specific
> mean in some other well known areas.
>
> If we borrow the % from the AT&T assembly syntax style
> to use register names, that we can retrieve in gcc inline
> assembly, then one may expect %% to have a meaning inspired
> from the same area. %% has its sense in gcc inline assembly,
> but applied there, it looks confusing.
>
> I mean, I'm trying to think like someone reading a perf probe
> command line without any documentation. The more this person
> can understand this command line without documentation, the better.
> We know that % is used for register names, some people know that %%
> is used for register names too but when we are in gcc inline assembly
> with var to reg resolution and need true registers name.

Hmm, but %%reg syntax is only for the special case of gcc-inline
assembly (e.g. assembler template, see
http://www.ibiblio.org/gferg/ldp/GCC-Inline-Assembly-HOWTO.html#s3).
So, I guess it will not be so confusing.

> Then if I try to mirror this sense from gcc to perf probe use,
> I feel confused, especially in the case of %%arg1.
>
> In this case, we should rather have %%register and %arg0 :)
>
> Hm, %register is a clear pattern.
>
> Somehow, %retval looks clear too, retval is verbose enough and
> % is still logical as return values are most of the time (always?)
> put in a register.
>
> But %%arg0 looks confusing.

Then, can we use @@ for prefix of special variables?? :-)

I'm so anxious about collision between register name and
those vars.

Thank you,

-- 
Masami Hiramatsu

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

e-mail: mhiramat@redhat.com

Re: [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Masami Hiramatsu]

Masami Hiramatsu wrote:
> Then, can we use @@ for prefix of special variables?? :-)

Otherwise, %@vars(means register_of-alias-vars) is another candidate.

-- 
Masami Hiramatsu

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

e-mail: mhiramat@redhat.com

Re: [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Steven Rostedt]

On Mon, 2009-10-05 at 21:26 +0200, Frederic Weisbecker wrote:
> On Mon, Oct 05, 2009 at 12:59:01PM -0400, Masami Hiramatsu wrote:
> > As far as I can see in arch/*/include/asm/ptrace.h, all registers start with
> > alphabets :-). So, I'd like to suggest renaming sp-vars to '_sp-vars'.
> >
> > Then, we will have;
> > - $local-vars
> 
> 
> There is a risk of bash collision.

I actually prefer the "$" notation. As for bash collision, it is common
for shell script writers to be able to distinguish a variable from bash.
Yes we can backslash it, or quote it. But when I see a $var it sticks
out to me that it is a variable. It's not hard to get around. For
example, type:

$ echo "hello $DISPLAY"' or $DISPLAY'

and see what you get.

Makefiles and Perl use '$' for variables those that need to handle it
with bash can easily cope with it.

So my vote is to keep the '$'. It is the most intuitive to what it
means.

Just my 0.02€

-- Steve

Re: [PATCH tracing/kprobes v2 2/5] tracing/kprobes: Avoid field name confliction

[Steven Rostedt]

On Fri, 2009-10-02 at 17:48 -0400, Masami Hiramatsu wrote:
> Check whether the argument name is conflict with other field names.
> 
> Changes in v2:
>  - Add common_lock_depth to reserved name list.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Mike Galbraith <efault@gmx.de>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> Cc: Jim Keniston <jkenisto@us.ibm.com>
> Cc: Frank Ch. Eigler <fche@redhat.com>
> ---
> 
>  kernel/trace/trace_kprobe.c |   65 +++++++++++++++++++++++++++++++++++--------
>  1 files changed, 53 insertions(+), 12 deletions(-)
> 
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index f63ead0..eb1fa0f 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -38,6 +38,25 @@
>  #define MAX_EVENT_NAME_LEN 64
>  #define KPROBE_EVENT_SYSTEM "kprobes"
>  
> +/* Reserved field names */
> +#define FIELD_STRING_IP "ip"
> +#define FIELD_STRING_NARGS "nargs"
> +#define FIELD_STRING_RETIP "ret_ip"
> +#define FIELD_STRING_FUNC "func"
> +
> +const char *reserved_field_names[] = {
> +	"common_type",
> +	"common_flags",
> +	"common_preempt_count",
> +	"common_pid",
> +	"common_tgid",
> +	"common_lock_depth",
> +	FIELD_STRING_IP,
> +	FIELD_STRING_NARGS,
> +	FIELD_STRING_RETIP,
> +	FIELD_STRING_FUNC,
> +};
> +
>  /* currently, trace_kprobe only supports X86. */
>  
>  struct fetch_func {
> @@ -551,6 +570,20 @@ static int parse_probe_arg(char *arg, struct fetch_func *ff, int is_return)
>  	return ret;
>  }
>  
> +/* Return 1 if name is reserved or already used by another argument */
> +static int conflict_field_name(const char *name,
> +			       struct probe_arg *args, int narg)
> +{
> +	int i;
> +	for (i = 0; i < ARRAY_SIZE(reserved_field_names); i++)
> +		if (!strcmp(reserved_field_names[i], name))
> +			return 1;
> +	for (i = 0; i < narg; i++)
> +		if (!strcmp(args[i].name, name))
> +			return 1;

Just a coding preference, but still, I've seen too many mistakes (made
them myself too).

	if (strcmp(args[i].name, name) == 0)

Looks better as a match then

	if (!strcmp(args[i].name, name))

That stands out to me as a miss match. But this is still just a
preference and not something to make me argue the patch.

-- Steve


> +	return 0;
> +}
> +

Re: [PATCH tracing/kprobes v2 4/5] perf: Add perf probe subcommand for kprobe-event setup helper

[Steven Rostedt]

On Fri, 2009-10-02 at 17:49 -0400, Masami Hiramatsu wrote:
> Add perf probe subcommand for kprobe-event setup helper to perf command.
> This allows user to define kprobe events by C expressions (C line numbers,
> C function names, and C local variables).
> 
> Usage
> -----
>  perf probe [<options>] -P 'PROBEDEF' [-P 'PROBEDEF' ...]
> 
>     -k, --vmlinux <file>  vmlinux/module pathname
>     -r, --release <rel>   kernel release
>     -P, --probe <p|r:[GRP/]NAME FUNC[+OFFS][@SRC]|@SRC:LINE [ARG ...]>
>                           probe point definition, where
> 		p:	kprobe probe
> 		r:	kretprobe probe
> 		GRP:	Group name (optional)
> 		NAME:	Event name
> 		FUNC:	Function name
> 		OFFS:	Offset from function entry (in byte)
> 		SRC:	Source code path
> 		LINE:	Line number
> 		ARG:	Probe argument (local variable name or
> 			kprobe-tracer argument format is supported.)
> 
> Changes in v2:
>  - Check synthesized string length.
>  - Rename perf kprobe to perf probe.
>  - Use spaces for separator and update usage comment.
>  - Check error paths in parse_probepoint().
>  - Check optimized-out variables.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Mike Galbraith <efault@gmx.de>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> Cc: Jim Keniston <jkenisto@us.ibm.com>
> Cc: Frank Ch. Eigler <fche@redhat.com>
> ---
> 
>  tools/perf/Makefile            |   10 +
>  tools/perf/builtin-probe.c     |  356 +++++++++++++++++++++
>  tools/perf/builtin.h           |    1 
>  tools/perf/perf.c              |    3 
>  tools/perf/util/probe-finder.c |  690 ++++++++++++++++++++++++++++++++++++++++
>  tools/perf/util/probe-finder.h |   68 ++++
>  6 files changed, 1128 insertions(+), 0 deletions(-)
>  create mode 100644 tools/perf/builtin-probe.c
>  create mode 100644 tools/perf/util/probe-finder.c
>  create mode 100644 tools/perf/util/probe-finder.h
> 
> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> index b5f1953..6dabcf1 100644
> --- a/tools/perf/Makefile
> +++ b/tools/perf/Makefile
> @@ -419,6 +419,16 @@ 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 probe subcommand. Please install libdwarf-dev/libdwarf-devel);

Wow! And I thought my macros were ugly ;-)

> +else
> +	EXTLIBS += -lelf -ldwarf
> +	LIB_H += util/probe-finder.h
> +	LIB_OBJS += util/probe-finder.o
> +	BUILTIN_OBJS += builtin-probe.o
> +	BASIC_CFLAGS += -DSUPPORT_DWARF
> +endif
> +
>  ifdef NO_DEMANGLE
>  	BASIC_CFLAGS += -DNO_DEMANGLE
>  else
> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> new file mode 100644
> index 0000000..47c5fb8
> --- /dev/null
> +++ b/tools/perf/builtin-probe.c
> @@ -0,0 +1,356 @@
> +/*
> + * builtin-probe.c
> + *
> + * Builtin probe command: Set up probe events by C expression
> + *
> + * Written by Masami Hiramatsu <mhiramat@redhat.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
> + *
> + */
> +
> +#include <sys/utsname.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +#include <errno.h>
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <stdlib.h>
> +#include <string.h>
> +
> +#include "perf.h"
> +#include "builtin.h"
> +#include "util/util.h"
> +#include "util/parse-options.h"
> +#include "util/parse-events.h"	/* For debugfs_path */
> +#include "util/probe-finder.h"
> +
> +/* Default vmlinux search paths */
> +#define NR_SEARCH_PATH 3
> +const char *default_search_path[NR_SEARCH_PATH] = {
> +"/lib/modules/%s/build/vmlinux",		/* Custom build kernel */
> +"/usr/lib/debug/lib/modules/%s/vmlinux",	/* Red Hat debuginfo */
> +"/boot/vmlinux-debug-%s",			/* Ubuntu */
> +};
> +
> +#define MAX_PATH_LEN 256
> +#define MAX_PROBES 128
> +
> +/* Session management structure */
> +static struct {
> +	char *vmlinux;
> +	char *release;
> +	int nr_probe;
> +	struct probe_point probes[MAX_PROBES];
> +	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
> +
> +static int parse_probepoint(const struct option *opt __used,
> +			    const char *str, int unset __used)
> +{
> +	char *argv[MAX_PROBE_ARGS + 2];	/* Event + probe + args */
> +	int argc, i;
> +	char *arg, *ptr;
> +	struct probe_point *pp = &session.probes[session.nr_probe];
> +	char **event = &session.events[session.nr_probe];
> +	int retp = 0;
> +
> +	if (!str)	/* The end of probe points */
> +		return 0;
> +
> +	debug("Probe-define(%d): %s\n", session.nr_probe, str);
> +	if (++session.nr_probe == MAX_PROBES)
> +		semantic_error("Too many probes");
> +
> +	/* Separate arguments, similar to argv_split */
> +	argc = 0;
> +	do {
> +		/* Skip separators */
> +		while (isspace(*str))
> +			str++;
> +
> +		/* Add an argument */
> +		if (*str != '\0') {
> +			const char *s = str;
> +
> +			/* Skip the argument */
> +			while (!isspace(*str) && *str != '\0')
> +				str++;
> +
> +			/* Duplicate the argument */
> +			argv[argc] = strndup(s, str - s);
> +			if (argv[argc] == NULL)
> +				perror_exit("strndup");
> +			if (++argc == MAX_PROBE_ARGS)
> +				semantic_error("Too many arguments");
> +			debug("argv[%d]=%s\n", argc, argv[argc - 1]);
> +		}
> +	} while (*str != '\0');
> +	if (argc < 2)
> +		semantic_error("Need event-name and probe-point at least.");
> +
> +	/* Parse the event name */
> +	if (argv[0][0] == 'r')
> +		retp = 1;
> +	else if (argv[0][0] != 'p')
> +		semantic_error("You must specify 'p'(kprobe) or"
> +				" 'r'(kretprobe) first.");
> +	/* TODO: check event name */
> +	*event = argv[0];
> +
> +	/* Parse probe point */
> +	arg = argv[1];
> +	if (arg[0] == '@') {
> +		/* Source Line */
> +		arg++;
> +		ptr = strchr(arg, ':');
> +		if (!ptr || !isdigit(ptr[1]))
> +			semantic_error("Line number is required.");
> +		*ptr++ = '\0';
> +		if (strlen(arg) == 0)
> +			semantic_error("No file name.");
> +		pp->file = strdup(arg);
> +		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);
> +	} else {
> +		/* Function name */
> +		ptr = strchr(arg, '+');
> +		if (ptr) {
> +			if (!isdigit(ptr[1]))
> +				semantic_error("Offset is required.");
> +			*ptr++ = '\0';
> +			pp->offset = atoi(ptr);
> +		} else
> +			ptr = arg;
> +		ptr = strchr(ptr, '@');
> +		if (ptr) {
> +			*ptr++ = '\0';
> +			pp->file = strdup(ptr);
> +		}
> +		pp->function = strdup(arg);
> +		debug("symbol:%s file:%s offset:%d\n",
> +		      pp->function, pp->file, pp->offset);
> +	}
> +	free(argv[1]);
> +
> +	/* Copy arguments */
> +	pp->nr_args = argc - 2;
> +	if (pp->nr_args > 0) {
> +		pp->args = (char **)malloc(sizeof(char *) * pp->nr_args);

Hmm, you don't check for failed malloc here?

> +		memcpy(pp->args, &argv[2], sizeof(char *) * pp->nr_args);
> +	}
> +
> +	/* Ensure return probe has no C argument */
> +	if (retp)
> +		for (i = 0; i < pp->nr_args; i++)
> +			if (is_c_varname(pp->args[i]))
> +				semantic_error("You can't specify local"
> +						" variable for kretprobe");
> +	debug("%d arguments\n", pp->nr_args);
> +	return 0;
> +}
> +
> +static int open_default_vmlinux(void)
> +{
> +	struct utsname uts;
> +	char fname[MAX_PATH_LEN];
> +	int fd, ret, i;
> +
> +	if (!session.release) {
> +		ret = uname(&uts);
> +		if (ret) {
> +			debug("uname() failed.\n");
> +			return -errno;
> +		}
> +		session.release = uts.release;
> +	}
> +	for (i = 0; i < NR_SEARCH_PATH; i++) {
> +		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);
> +			errno = E2BIG;
> +			return -E2BIG;
> +		}
> +		debug("try to open %s\n", fname);
> +		fd = open(fname, O_RDONLY);
> +		if (fd >= 0)
> +			break;
> +	}
> +	return fd;
> +}
> +
> +static const char * const probe_usage[] = {
> +	"perf probe [<options>] -P 'PROBEDEF' [-P 'PROBEDEF' ...]",
> +	NULL
> +};
> +
> +static const struct option options[] = {
> +	OPT_STRING('k', "vmlinux", &session.vmlinux, "file",
> +		"vmlinux/module pathname"),
> +	OPT_STRING('r', "release", &session.release, "rel", "kernel release"),
> +	OPT_CALLBACK('P', "probe", NULL,
> +		"p|r:[GRP/]NAME FUNC[+OFFS][@SRC]|@SRC:LINE [ARG ...]",
> +		"probe point definition, where\n"
> +		"\t\tp:\tkprobe probe\n"
> +		"\t\tr:\tkretprobe probe\n"
> +		"\t\tGRP:\tGroup name (optional)\n"
> +		"\t\tNAME:\tEvent name\n"
> +		"\t\tFUNC:\tFunction name\n"
> +		"\t\tOFFS:\tOffset from function entry (in byte)\n"
> +		"\t\tSRC:\tSource code path\n"
> +		"\t\tLINE:\tLine number\n"
> +		"\t\tARG:\tProbe argument (local variable name or\n"
> +		"\t\t\tkprobe-tracer argument format is supported.)\n",
> +		parse_probepoint),
> +	OPT_END()
> +};
> +
> +static int write_new_event(int fd, const char *buf)
> +{
> +	int ret;
> +
> +	printf("Adding new event: %s\n", buf);
> +	ret = write(fd, buf, strlen(buf));
> +	if (ret <= 0)
> +		perror("Error: Failed to create event");
> +
> +	return ret;
> +}
> +
> +#define MAX_CMDLEN 256
> +
> +static int synthesize_probepoint(struct probe_point *pp)
> +{
> +	char *buf;
> +	int i, len, ret;
> +	pp->probes[0] = buf = (char *)calloc(MAX_CMDLEN, sizeof(char));

Again no check for failed calloc?

> +	ret = snprintf(buf, MAX_CMDLEN, "%s+%d", pp->function, pp->offset);
> +	if (ret <= 0 || ret >= MAX_CMDLEN)
> +		goto error;
> +	len = ret;
> +
> +	for (i = 0; i < pp->nr_args; i++) {
> +		ret = snprintf(&buf[len], MAX_CMDLEN - len, " %s",
> +			       pp->args[i]);
> +		if (ret <= 0 || ret >= MAX_CMDLEN - len)
> +			goto error;
> +		len += ret;
> +	}
> +	pp->found = 1;
> +	return pp->found;
> +error:
> +	free(pp->probes[0]);
> +	if (ret > 0)
> +		ret = -E2BIG;
> +	return ret;
> +}
> +
> +int cmd_probe(int argc, const char **argv, const char *prefix __used)
> +{
> +	int i, j, fd, ret, need_dwarf = 0;
> +	struct probe_point *pp;
> +	char buf[MAX_CMDLEN];
> +
> +	argc = parse_options(argc, argv, options, probe_usage,
> +		PARSE_OPT_STOP_AT_NON_OPTION);
> +	if (argc || session.nr_probe == 0)
> +		usage_with_options(probe_usage, options);
> +
> +	/* Synthesize return probes */
> +	for (j = 0; j < session.nr_probe; j++) {
> +		if (session.events[j][0] != 'r') {
> +			need_dwarf = 1;
> +			continue;
> +		}
> +		ret = synthesize_probepoint(&session.probes[j]);
> +		if (ret == -E2BIG)
> +			semantic_error("probe point is too long.");
> +		else if (ret < 0) {
> +			perror("snprintf");
> +			return -1;
> +		}
> +	}
> +
> +	if (!need_dwarf)
> +		goto setup_probes;
> +
> +	if (session.vmlinux)
> +		fd = open(session.vmlinux, O_RDONLY);
> +	else
> +		fd = open_default_vmlinux();
> +	if (fd < 0) {
> +		perror("vmlinux/module file open");
> +		return -1;
> +	}
> +
> +	/* Searching probe points */
> +	for (j = 0; j < session.nr_probe; j++) {
> +		pp = &session.probes[j];
> +		if (pp->found)
> +			continue;
> +
> +		lseek(fd, SEEK_SET, 0);
> +		ret = find_probepoint(fd, pp);
> +		if (ret <= 0) {
> +			fprintf(stderr, "Error: No probe point found.\n");
> +			return -1;
> +		}
> +		debug("probe event %s found\n", session.events[j]);
> +	}
> +	close(fd);
> +
> +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;
> +	}
> +	for (j = 0; j < session.nr_probe; j++) {
> +		pp = &session.probes[j];
> +		if (pp->found == 1) {
> +			snprintf(buf, MAX_CMDLEN, "%s %s\n",
> +				session.events[j], pp->probes[0]);
> +			write_new_event(fd, buf);
> +		} else
> +			for (i = 0; i < pp->found; i++) {
> +				snprintf(buf, MAX_CMDLEN, "%s%d %s\n",
> +					session.events[j], i, pp->probes[i]);
> +				write_new_event(fd, buf);
> +			}
> +	}
> +	close(fd);
> +	return 0;
> +}
> +
> diff --git a/tools/perf/builtin.h b/tools/perf/builtin.h
> index e11d8d2..ad5f0f4 100644
> --- a/tools/perf/builtin.h
> +++ b/tools/perf/builtin.h

The rest looks fine (for what I can tell, it is all dwarf magic to
me ;-)

-- Steve

Re: [PATCH tracing/kprobes v2 4/5] perf: Add perf probe subcommand for kprobe-event setup helper

[Masami Hiramatsu]

Steven Rostedt wrote:
> On Fri, 2009-10-02 at 17:49 -0400, Masami Hiramatsu wrote:
>> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
>> index b5f1953..6dabcf1 100644
>> --- a/tools/perf/Makefile
>> +++ b/tools/perf/Makefile
>> @@ -419,6 +419,16 @@ 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 probe subcommand. Please install libdwarf-dev/libdwarf-devel);
> 
> Wow! And I thought my macros were ugly ;-)

:-)
Maybe, would I better make a separate c file to check this?
Like "autoconf-checkdwarf.c".

>> +
>> +	/* Copy arguments */
>> +	pp->nr_args = argc - 2;
>> +	if (pp->nr_args > 0) {
>> +		pp->args = (char **)malloc(sizeof(char *) * pp->nr_args);
> 
> Hmm, you don't check for failed malloc here?

Oops...

>> +static int synthesize_probepoint(struct probe_point *pp)
>> +{
>> +	char *buf;
>> +	int i, len, ret;
>> +	pp->probes[0] = buf = (char *)calloc(MAX_CMDLEN, sizeof(char));
> 
> Again no check for failed calloc?

Oops again, I'll check it...


>> diff --git a/tools/perf/builtin.h b/tools/perf/builtin.h
>> index e11d8d2..ad5f0f4 100644
>> --- a/tools/perf/builtin.h
>> +++ b/tools/perf/builtin.h
> 
> The rest looks fine (for what I can tell, it is all dwarf magic to
> me ;-)

:-)

Thank you!

-- 
Masami Hiramatsu

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

e-mail: mhiramat@redhat.com

Re: [PATCH tracing/kprobes v2 2/5] tracing/kprobes: Avoid field name confliction

[Masami Hiramatsu]

Steven Rostedt wrote:
> On Fri, 2009-10-02 at 17:48 -0400, Masami Hiramatsu wrote:
>> Check whether the argument name is conflict with other field names.
>>
>> Changes in v2:
>>  - Add common_lock_depth to reserved name list.
>>
>> Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
>> Cc: Frederic Weisbecker <fweisbec@gmail.com>
>> Cc: Ingo Molnar <mingo@elte.hu>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
>> Cc: Steven Rostedt <rostedt@goodmis.org>
>> Cc: Mike Galbraith <efault@gmx.de>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
>> Cc: Christoph Hellwig <hch@infradead.org>
>> Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
>> Cc: Jim Keniston <jkenisto@us.ibm.com>
>> Cc: Frank Ch. Eigler <fche@redhat.com>
>> ---
>>
>>  kernel/trace/trace_kprobe.c |   65 +++++++++++++++++++++++++++++++++++--------
>>  1 files changed, 53 insertions(+), 12 deletions(-)
>>
>> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
>> index f63ead0..eb1fa0f 100644
>> --- a/kernel/trace/trace_kprobe.c
>> +++ b/kernel/trace/trace_kprobe.c
>> @@ -38,6 +38,25 @@
>>  #define MAX_EVENT_NAME_LEN 64
>>  #define KPROBE_EVENT_SYSTEM "kprobes"
>>  
>> +/* Reserved field names */
>> +#define FIELD_STRING_IP "ip"
>> +#define FIELD_STRING_NARGS "nargs"
>> +#define FIELD_STRING_RETIP "ret_ip"
>> +#define FIELD_STRING_FUNC "func"
>> +
>> +const char *reserved_field_names[] = {
>> +	"common_type",
>> +	"common_flags",
>> +	"common_preempt_count",
>> +	"common_pid",
>> +	"common_tgid",
>> +	"common_lock_depth",
>> +	FIELD_STRING_IP,
>> +	FIELD_STRING_NARGS,
>> +	FIELD_STRING_RETIP,
>> +	FIELD_STRING_FUNC,
>> +};
>> +
>>  /* currently, trace_kprobe only supports X86. */
>>  
>>  struct fetch_func {
>> @@ -551,6 +570,20 @@ static int parse_probe_arg(char *arg, struct fetch_func *ff, int is_return)
>>  	return ret;
>>  }
>>  
>> +/* Return 1 if name is reserved or already used by another argument */
>> +static int conflict_field_name(const char *name,
>> +			       struct probe_arg *args, int narg)
>> +{
>> +	int i;
>> +	for (i = 0; i < ARRAY_SIZE(reserved_field_names); i++)
>> +		if (!strcmp(reserved_field_names[i], name))
>> +			return 1;
>> +	for (i = 0; i < narg; i++)
>> +		if (!strcmp(args[i].name, name))
>> +			return 1;
> 
> Just a coding preference, but still, I've seen too many mistakes (made
> them myself too).
> 
> 	if (strcmp(args[i].name, name) == 0)
> 
> Looks better as a match then
> 
> 	if (!strcmp(args[i].name, name))
> 
> That stands out to me as a miss match. But this is still just a
> preference and not something to make me argue the patch.

Agreed, !strcmp() pattern would better be warned by checkpatch.pl :-)
I'll fix that.

Thank you,

-- 
Masami Hiramatsu

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

e-mail: mhiramat@redhat.com

Re: [PATCH tracing/kprobes v2 4/5] perf: Add perf probe subcommand for kprobe-event setup helper

[Steven Rostedt]

On Mon, 2009-10-05 at 20:57 -0400, Masami Hiramatsu wrote:
> Steven Rostedt wrote:
> > On Fri, 2009-10-02 at 17:49 -0400, Masami Hiramatsu wrote:
> >> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> >> index b5f1953..6dabcf1 100644
> >> --- a/tools/perf/Makefile
> >> +++ b/tools/perf/Makefile
> >> @@ -419,6 +419,16 @@ 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 probe subcommand. Please install libdwarf-dev/libdwarf-devel);
> > 
> > Wow! And I thought my macros were ugly ;-)
> 
> :-)
> Maybe, would I better make a separate c file to check this?
> Like "autoconf-checkdwarf.c".


Nah, I think it is pretty obvious to what it is doing. But then again, I
like ugly hacks like the above.

-- Steve

Re: [PATCH tracing/kprobes v2 4/5] perf: Add perf probe subcommand for kprobe-event setup helper

[Arnaldo Carvalho de Melo]

Em Mon, Oct 05, 2009 at 08:57:28PM -0400, Masami Hiramatsu escreveu:
> Steven Rostedt wrote:
> > On Fri, 2009-10-02 at 17:49 -0400, Masami Hiramatsu wrote:
> >> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> >> index b5f1953..6dabcf1 100644
> >> --- a/tools/perf/Makefile
> >> +++ b/tools/perf/Makefile
> >> @@ -419,6 +419,16 @@ 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 probe subcommand. Please install libdwarf-dev/libdwarf-devel);
> > 
> > Wow! And I thought my macros were ugly ;-)
> 
> :-)
> Maybe, would I better make a separate c file to check this?
> Like "autoconf-checkdwarf.c".

I saw that coming, I'd say leave it as is... :)
 
> >> +
> >> +	/* Copy arguments */
> >> +	pp->nr_args = argc - 2;
> >> +	if (pp->nr_args > 0) {
> >> +		pp->args = (char **)malloc(sizeof(char *) * pp->nr_args);
> > 
> > Hmm, you don't check for failed malloc here?
> 
> Oops...

Also no need for cast
 

- Arnaldo

Re: [PATCH tracing/kprobes v2 4/5] perf: Add perf probe subcommand for kprobe-event setup helper

[Peter Zijlstra]

On Mon, 2009-10-05 at 20:57 -0400, Masami Hiramatsu wrote:
> >> +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 probe subcommand. Please install libdwarf-dev/libdwarf-devel);
> > 
> > Wow! And I thought my macros were ugly ;-)
> 
> :-)
> Maybe, would I better make a separate c file to check this?
> Like "autoconf-checkdwarf.c".

I'm the one who started this trend.. Maybe putting all those tests in a
static .c file makes sense now, put them in a tests/ or checks/
directory though, so it doesn't litter the regular files.

Then again,.. the actual .c file is only half that line, so it doesn't
really clean up that much.

Re: [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Masami Hiramatsu]

Steven Rostedt wrote:
> On Mon, 2009-10-05 at 21:26 +0200, Frederic Weisbecker wrote:
>> On Mon, Oct 05, 2009 at 12:59:01PM -0400, Masami Hiramatsu wrote:
>>> As far as I can see in arch/*/include/asm/ptrace.h, all registers start with
>>> alphabets :-). So, I'd like to suggest renaming sp-vars to '_sp-vars'.
>>>
>>> Then, we will have;
>>> - $local-vars
>>
>>
>> There is a risk of bash collision.
> 
> I actually prefer the "$" notation. As for bash collision, it is common
> for shell script writers to be able to distinguish a variable from bash.
> Yes we can backslash it, or quote it. But when I see a $var it sticks
> out to me that it is a variable. It's not hard to get around. For
> example, type:
> 
> $ echo "hello $DISPLAY"' or $DISPLAY'
> 
> and see what you get.
> 
> Makefiles and Perl use '$' for variables those that need to handle it
> with bash can easily cope with it.
> 
> So my vote is to keep the '$'. It is the most intuitive to what it
> means.
> 
> Just my 0.02€

OK, so here are syntax ideas

* current syntax
<Ftrace>
- $sp_var
- %regs
- @symbol
<Perf>
- local_var

* $local syntax
<Ftrace>
- sp_var
- %regs
- @symbol
<Perf>
- $local_var

* non $ syntax 1
<Ftrace>
- %sp_var
- %regs
- @symbol
<Perf>
- local_var

* non $ syntax 2
<Ftrace>
- %%sp_var or %@sp_var
- %regs
- @symbol
<Perf>
- local_var

So, which one do you like to use? :-)

Thank you,

-- 
Masami Hiramatsu

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

e-mail: mhiramat@redhat.com

Re: [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Frederic Weisbecker]

On Mon, Oct 05, 2009 at 08:12:53PM -0400, Steven Rostedt wrote:
> On Mon, 2009-10-05 at 21:26 +0200, Frederic Weisbecker wrote:
> > On Mon, Oct 05, 2009 at 12:59:01PM -0400, Masami Hiramatsu wrote:
> > > As far as I can see in arch/*/include/asm/ptrace.h, all registers start with
> > > alphabets :-). So, I'd like to suggest renaming sp-vars to '_sp-vars'.
> > >
> > > Then, we will have;
> > > - $local-vars
> > 
> > 
> > There is a risk of bash collision.
> 
> I actually prefer the "$" notation. As for bash collision, it is common
> for shell script writers to be able to distinguish a variable from bash.
> Yes we can backslash it, or quote it. But when I see a $var it sticks
> out to me that it is a variable. It's not hard to get around. For
> example, type:
> 
> $ echo "hello $DISPLAY"' or $DISPLAY'
> 
> and see what you get.
> 
> Makefiles and Perl use '$' for variables those that need to handle it
> with bash can easily cope with it.
> 
> So my vote is to keep the '$'. It is the most intuitive to what it
> means.


Hrrmm...

I fear about future complains, but I may be somehow biased in
that my usual usecases of bash don't involve '$' characters to
protect, so it's not something I'm used to.

Whatever choice we make, there are either downsides in the prefix
self meaning, the collisions or the shell intrpretation.

Now you are two who prefer that, let's pick this one :)

Re: [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Frederic Weisbecker]

On Tue, Oct 06, 2009 at 10:23:50AM -0400, Masami Hiramatsu wrote:
> OK, so here are syntax ideas
> 
> * current syntax
> <Ftrace>
> - $sp_var
> - %regs
> - @symbol
> <Perf>
> - local_var


This one looks good.
And why not turning @symbol into symbol?
Then it's up to perf probe to find the right
target.

And @ can be used for raw addresses?

Re: [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Steven Rostedt]

On Tue, 2009-10-06 at 10:23 -0400, Masami Hiramatsu wrote:
> Steven Rostedt wrote:
> > 
> 
> OK, so here are syntax ideas
> 
> * current syntax
> <Ftrace>
> - $sp_var
> - %regs
> - @symbol
> <Perf>
> - local_var
> 
> * $local syntax
> <Ftrace>
> - sp_var
> - %regs
> - @symbol
> <Perf>
> - $local_var
> 

I'm fine with either of the above. I don't like either of the below.

-- Steve

> * non $ syntax 1
> <Ftrace>
> - %sp_var
> - %regs
> - @symbol
> <Perf>
> - local_var
> 
> * non $ syntax 2
> <Ftrace>
> - %%sp_var or %@sp_var
> - %regs
> - @symbol
> <Perf>
> - local_var
> 
> So, which one do you like to use? :-)
> 
> Thank you,
> 

Re: [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Masami Hiramatsu]

Frederic Weisbecker wrote:
> On Tue, Oct 06, 2009 at 10:23:50AM -0400, Masami Hiramatsu wrote:
>> OK, so here are syntax ideas
>>
>> * current syntax
>> <Ftrace>
>> - $sp_var
>> - %regs
>> - @symbol
>> <Perf>
>> - local_var
> 
> 
> This one looks good.
> And why not turning @symbol into symbol?
> Then it's up to perf probe to find the right
> target.

Hmm, one big reason for separating local_vars
and others is that I'd like to check whether
perf-probe needs debuginfo or not.
If there is no local_vars, we don't need to
parse debuginfo. Sometimes, it is useful just
for setting up probe points.

> And @ can be used for raw addresses?

Yeah, but @ can be shared with symbols and raw addresses.
If the first char is digit, it's not a symbol.:-)

And also, @symbol actually has its syntax rule.:-)
its real syntax is:
@[MODULE:]SYMBOL[+OFFSET]
or
@ADDRESS (0xNN (base 16) or NN (base 10))

Perhaps, since perf-probe has another syntax rule,
I don't want to confuse users. :-P

Thank you,

-- 
Masami Hiramatsu

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

e-mail: mhiramat@redhat.com

Re: [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Masami Hiramatsu]

Steven Rostedt wrote:
> On Tue, 2009-10-06 at 10:23 -0400, Masami Hiramatsu wrote:
>> Steven Rostedt wrote:
>>>
>>
>> OK, so here are syntax ideas
>>
>> * current syntax
>> <Ftrace>
>> - $sp_var
>> - %regs
>> - @symbol
>> <Perf>
>> - local_var
>>
>> * $local syntax
>> <Ftrace>
>> - sp_var
>> - %regs
>> - @symbol
>> <Perf>
>> - $local_var
>>
> 
> I'm fine with either of the above. I don't like either of the below.

OK, I'm fine with current syntax. :-)

Thank you,

-- 
Masami Hiramatsu

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

e-mail: mhiramat@redhat.com

Re: [PATCH tracing/kprobes v2 4/5] perf: Add perf probe subcommand for kprobe-event setup helper

[Masami Hiramatsu]

Peter Zijlstra wrote:
> On Mon, 2009-10-05 at 20:57 -0400, Masami Hiramatsu wrote:
>>>> +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 probe subcommand. Please install libdwarf-dev/libdwarf-devel);
>>>
>>> Wow! And I thought my macros were ugly ;-)
>>
>> :-)
>> Maybe, would I better make a separate c file to check this?
>> Like "autoconf-checkdwarf.c".
> 
> I'm the one who started this trend.. Maybe putting all those tests in a
> static .c file makes sense now, put them in a tests/ or checks/
> directory though, so it doesn't litter the regular files.
> 
> Then again,.. the actual .c file is only half that line, so it doesn't
> really clean up that much.

Yeah, ok, then I'll leave this part in this series.
It's better to be done in another series.

Thank you,

-- 
Masami Hiramatsu

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

e-mail: mhiramat@redhat.com

Re: [PATCH tracing/kprobes v2 1/5] tracing/kprobes: Rename special variables syntax

[Frederic Weisbecker]

On Tue, Oct 06, 2009 at 09:13:50PM -0400, Masami Hiramatsu wrote:
> Frederic Weisbecker wrote:
> > On Tue, Oct 06, 2009 at 10:23:50AM -0400, Masami Hiramatsu wrote:
> >> OK, so here are syntax ideas
> >>
> >> * current syntax
> >> <Ftrace>
> >> - $sp_var
> >> - %regs
> >> - @symbol
> >> <Perf>
> >> - local_var
> > 
> > 
> > This one looks good.
> > And why not turning @symbol into symbol?
> > Then it's up to perf probe to find the right
> > target.
> 
> Hmm, one big reason for separating local_vars
> and others is that I'd like to check whether
> perf-probe needs debuginfo or not.
> If there is no local_vars, we don't need to
> parse debuginfo. Sometimes, it is useful just
> for setting up probe points.
> 
> > And @ can be used for raw addresses?
> 
> Yeah, but @ can be shared with symbols and raw addresses.
> If the first char is digit, it's not a symbol.:-)
> 
> And also, @symbol actually has its syntax rule.:-)
> its real syntax is:
> @[MODULE:]SYMBOL[+OFFSET]
> or
> @ADDRESS (0xNN (base 16) or NN (base 10))
> 
> Perhaps, since perf-probe has another syntax rule,
> I don't want to confuse users. :-P
> 
> Thank you,
> 

Ok let's pick @ for symbols then.

Thanks.