[PATCH -tip 0/7] perf-probe updates - data-structure support improvements, etc.

[PATCH -tip 0/7] perf-probe updates - data-structure support improvements, etc.

[Masami Hiramatsu]

Hi Ingo,

Here are several updates of perf-probe. This series improves
data structure accessing.

- Set the name of argument which traces a data structure member
  as the last member of the data structure reference (e.g. f_mode
  of file->f_mode). This allows us to use perf-trace for tracin
  data-structure members.

- Add the basic type support. This allows us to fetch the memory
  with specified bitwidth. Usually, data-structure members are
  packed on the memory, this means if we want to read a member
  from memory, we have to access it with type casting.
  kprobe-tracer now support tracing argument with basic types
  (u8,u16,u32,u64,s8,s16,s32,s64), and perf-probe decodes the type
  information of the members.

- Support canonical frame address, which is used for refering
  frame-base on the kernel built without CONFIG_FRAME_POINTER.

TODOs (possible features):
  - Support array element (var[N])
  - Support string/dynamic arrays (*var, var[N..M])
  - Support tracing static variables (non global)
  - Support dynamic array-indexing (var[var2])
  - Support force type-casting ((type)var)
  - Show what deta-structure member is assigned to each argument.
  - Better support for probes on modules
  - More debugger like enhancements(%next, --disasm, etc.)

I decided to drop non-root user support (for --add/--del),
because this can set probes for getting sensitive data...

Thank you,

---

Masami Hiramatsu (7):
      perf probe: Support DW_OP_call_frame_cfa in debuginfo
      perf probe: Support basic type casting
      perf probe: Query basic types from debuginfo
      trace/kprobes: Support basic types
      perf probe: Use the last field name as the argument name
      perf probe: Support argument name
      [BUGFIX] perf probe: Fix to close dwarf when failing to analyze it


 Documentation/trace/kprobetrace.txt     |    4 
 kernel/trace/trace.h                    |   16 -
 kernel/trace/trace_kprobe.c             |  535 +++++++++++++++++++------------
 tools/perf/Documentation/perf-probe.txt |   11 +
 tools/perf/builtin-probe.c              |    4 
 tools/perf/util/probe-event.c           |   70 +++-
 tools/perf/util/probe-event.h           |    3 
 tools/perf/util/probe-finder.c          |  131 ++++++--
 tools/perf/util/probe-finder.h          |    1 
 9 files changed, 516 insertions(+), 259 deletions(-)

-- 
Masami Hiramatsu
e-mail: mhiramat@redhat.com

[PATCH -tip 1/7] [BUGFIX] perf probe: Fix to close dwarf when failing to analyze it

[Masami Hiramatsu]

Fix to close libdw routine when failing to analyze it in
find_perf_probe_point().

Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
---

 tools/perf/util/probe-finder.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index db52ec2..5f6cecc 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -812,8 +812,10 @@ int find_perf_probe_point(int fd, unsigned long addr,
 		return -ENOENT;
 
 	/* Find cu die */
-	if (!dwarf_addrdie(dbg, (Dwarf_Addr)addr, &cudie))
-		return -EINVAL;
+	if (!dwarf_addrdie(dbg, (Dwarf_Addr)addr, &cudie)) {
+		ret = -EINVAL;
+		goto end;
+	}
 
 	/* Find a corresponding line */
 	line = dwarf_getsrc_die(&cudie, (Dwarf_Addr)addr);


-- 
Masami Hiramatsu
e-mail: mhiramat@redhat.com

[PATCH -tip 2/7] perf probe: Support argument name

[Masami Hiramatsu]

Set given names to event arguments. The syntax is same as kprobe-tracer,
you can add 'NAME=' right before each argument.

e.g.
  ./perf probe vfs_read foo=file

 then, 'foo' is set to the argument name as below.

  ./perf probe -l
  probe:vfs_read       (on vfs_read@linux-2.6-tip/fs/read_write.c with foo)


Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
---

 tools/perf/Documentation/perf-probe.txt |   10 ++++++++-
 tools/perf/builtin-probe.c              |    4 ++--
 tools/perf/util/probe-event.c           |   34 ++++++++++++++++++++++---------
 tools/perf/util/probe-event.h           |    1 +
 tools/perf/util/probe-finder.c          |   27 +++++++++++++++----------
 5 files changed, 52 insertions(+), 24 deletions(-)

diff --git a/tools/perf/Documentation/perf-probe.txt b/tools/perf/Documentation/perf-probe.txt
index 0f944b3..0657cf4 100644
--- a/tools/perf/Documentation/perf-probe.txt
+++ b/tools/perf/Documentation/perf-probe.txt
@@ -79,7 +79,15 @@ Probe points are defined by following syntax.
 'EVENT' specifies the name of new event, if omitted, it will be set the name of the probed function. Currently, event group name is set as 'probe'.
 'FUNC' specifies a probed function name, and it may have one of the following options; '+OFFS' is the offset from function entry address in bytes, ':RLN' is the relative-line number from function entry line, and '%return' means that it probes function return. And ';PTN' means lazy matching pattern (see LAZY MATCHING). Note that ';PTN' must be the end of the probe point definition.  In addition, '@SRC' specifies a source file which has that function.
 It is also possible to specify a probe point by the source line number or lazy matching by using 'SRC:ALN' or 'SRC;PTN' syntax, where 'SRC' is the source file path, ':ALN' is the line number and ';PTN' is the lazy matching pattern.
-'ARG' specifies the arguments of this probe point. You can use the name of local variable, or kprobe-tracer argument format (e.g. $retval, %ax, etc).
+'ARG' specifies the arguments of this probe point, (see PROBE ARGUMENT).
+
+PROBE ARGUMENT
+--------------
+Each probe argument follows below syntax.
+
+ [NAME=]LOCALVAR|$retval|%REG|@SYMBOL
+
+'NAME' specifies the name of this argument (optional). You can use the name of local variable, local data structure member (e.g. var->field, var.field2), or kprobe-tracer argument format (e.g. $retval, %ax, etc).
 
 LINE SYNTAX
 -----------
diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
index cf2ffa5..23933c1 100644
--- a/tools/perf/builtin-probe.c
+++ b/tools/perf/builtin-probe.c
@@ -142,9 +142,9 @@ static const struct option options[] = {
 	OPT_CALLBACK('a', "add", NULL,
 #ifdef DWARF_SUPPORT
 		"[EVENT=]FUNC[@SRC][+OFF|%return|:RL|;PT]|SRC:AL|SRC;PT"
-		" [ARG ...]",
+		" [[NAME=]ARG ...]",
 #else
-		"[EVENT=]FUNC[+OFF|%return] [ARG ...]",
+		"[EVENT=]FUNC[+OFF|%return] [[NAME=]ARG ...]",
 #endif
 		"probe point definition, where\n"
 		"\t\tGROUP:\tGroup name (optional)\n"
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 3fc0be7..ab6f53d 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -437,22 +437,28 @@ static void parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
 /* Parse perf-probe event argument */
 static void parse_perf_probe_arg(const char *str, struct perf_probe_arg *arg)
 {
-	const char *tmp;
+	char *tmp;
 	struct perf_probe_arg_field **fieldp;
 
 	pr_debug("parsing arg: %s into ", str);
 
+	tmp = strchr(str, '=');
+	if (tmp) {
+		arg->name = xstrndup(str, tmp - str);
+		str = tmp + 1;
+	}
+
 	tmp = strpbrk(str, "-.");
 	if (!is_c_varname(str) || !tmp) {
 		/* A variable, register, symbol or special value */
-		arg->name = xstrdup(str);
-		pr_debug("%s\n", arg->name);
+		arg->var = xstrdup(str);
+		pr_debug("%s\n", arg->var);
 		return;
 	}
 
 	/* Structure fields */
-	arg->name = xstrndup(str, tmp - str);
-	pr_debug("%s, ", arg->name);
+	arg->var = xstrndup(str, tmp - str);
+	pr_debug("%s, ", arg->var);
 	fieldp = &arg->field;
 
 	do {
@@ -497,7 +503,7 @@ void parse_perf_probe_command(const char *cmd, struct perf_probe_event *pev)
 	pev->args = xzalloc(sizeof(struct perf_probe_arg) * pev->nargs);
 	for (i = 0; i < pev->nargs; i++) {
 		parse_perf_probe_arg(argv[i + 1], &pev->args[i]);
-		if (is_c_varname(pev->args[i].name) && pev->point.retprobe)
+		if (is_c_varname(pev->args[i].var) && pev->point.retprobe)
 			semantic_error("You can't specify local variable for"
 				       " kretprobe");
 	}
@@ -514,7 +520,7 @@ bool perf_probe_event_need_dwarf(struct perf_probe_event *pev)
 		return true;
 
 	for (i = 0; i < pev->nargs; i++)
-		if (is_c_varname(pev->args[i].name))
+		if (is_c_varname(pev->args[i].var))
 			return true;
 
 	return false;
@@ -575,7 +581,10 @@ int synthesize_perf_probe_arg(struct perf_probe_arg *pa, char *buf, size_t len)
 	int ret;
 	char *tmp = buf;
 
-	ret = e_snprintf(tmp, len, "%s", pa->name);
+	if (pa->name && pa->var)
+		ret = e_snprintf(tmp, len, "%s=%s", pa->name, pa->var);
+	else
+		ret = e_snprintf(tmp, len, "%s", pa->name ? pa->name : pa->var);
 	if (ret <= 0)
 		goto error;
 	tmp += ret;
@@ -803,6 +812,8 @@ void clear_perf_probe_event(struct perf_probe_event *pev)
 	for (i = 0; i < pev->nargs; i++) {
 		if (pev->args[i].name)
 			free(pev->args[i].name);
+		if (pev->args[i].var)
+			free(pev->args[i].var);
 		field = pev->args[i].field;
 		while (field) {
 			next = field->next;
@@ -1117,8 +1128,11 @@ static int convert_to_kprobe_trace_events(struct perf_probe_event *pev,
 	if (tev->nargs) {
 		tev->args = xzalloc(sizeof(struct kprobe_trace_arg)
 				    * tev->nargs);
-		for (i = 0; i < tev->nargs; i++)
-			tev->args[i].value = xstrdup(pev->args[i].name);
+		for (i = 0; i < tev->nargs; i++) {
+			if (pev->args[i].name)
+				tev->args[i].name = xstrdup(pev->args[i].name);
+			tev->args[i].value = xstrdup(pev->args[i].var);
+		}
 	}
 
 	/* Currently just checking function name from symbol map */
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index cd308b0..a73ede6 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -55,6 +55,7 @@ struct perf_probe_arg_field {
 /* Perf probe probing argument */
 struct perf_probe_arg {
 	char				*name;	/* Argument name */
+	char				*var;	/* Variable name */
 	struct perf_probe_arg_field	*field;	/* Structure fields */
 };
 
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 5f6cecc..043140f 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -478,35 +478,40 @@ static void convert_variable(Dwarf_Die *vr_die, struct probe_finder *pf)
 	convert_location(expr, pf);
 
 	if (pf->pvar->field)
-		convert_variable_fields(vr_die, pf->pvar->name,
+		convert_variable_fields(vr_die, pf->pvar->var,
 					pf->pvar->field, &pf->tvar->ref);
 	/* *expr will be cached in libdw. Don't free it. */
 	return ;
 error:
 	/* TODO: Support const_value */
 	die("Failed to find the location of %s at this address.\n"
-	    " Perhaps, it has been optimized out.", pf->pvar->name);
+	    " Perhaps, it has been optimized out.", pf->pvar->var);
 }
 
 /* Find a variable in a subprogram die */
 static void find_variable(Dwarf_Die *sp_die, struct probe_finder *pf)
 {
 	Dwarf_Die vr_die;
-	char buf[128];
+	char buf[32];
 
-	/* TODO: Support struct members and arrays */
-	if (!is_c_varname(pf->pvar->name)) {
+	/* TODO: Support arrays */
+	if (pf->pvar->name)
+		pf->tvar->name = xstrdup(pf->pvar->name);
+	else {
+		synthesize_perf_probe_arg(pf->pvar, buf, 32);
+		pf->tvar->name = xstrdup(buf);
+	}
+
+	if (!is_c_varname(pf->pvar->var)) {
 		/* Copy raw parameters */
-		pf->tvar->value = xstrdup(pf->pvar->name);
+		pf->tvar->value = xstrdup(pf->pvar->var);
 	} else {
-		synthesize_perf_probe_arg(pf->pvar, buf, 128);
-		pf->tvar->name = xstrdup(buf);
 		pr_debug("Searching '%s' variable in context.\n",
-			 pf->pvar->name);
+			 pf->pvar->var);
 		/* Search child die for local variables and parameters. */
-		if (!die_find_variable(sp_die, pf->pvar->name, &vr_die))
+		if (!die_find_variable(sp_die, pf->pvar->var, &vr_die))
 			die("Failed to find '%s' in this function.",
-			    pf->pvar->name);
+			    pf->pvar->var);
 		convert_variable(&vr_die, pf);
 	}
 }


-- 
Masami Hiramatsu
e-mail: mhiramat@redhat.com

[PATCH -tip 3/7] perf probe: Use the last field name as the argument name

[Masami Hiramatsu]

Set the last field name to the argument name when the argument
is refering a data-structure member.

e.g.
 ./perf probe --add 'vfs_read file->f_mode'
 Add new event:
   probe:vfs_read       (on vfs_read with f_mode=file->f_mode)

 This probe records file->f_mode, but the argument name becomes "f_mode".

This enables perf-trace command to parse trace event format correctly.

Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
---

 tools/perf/Documentation/perf-probe.txt |    2 +-
 tools/perf/util/probe-event.c           |    4 ++++
 2 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/tools/perf/Documentation/perf-probe.txt b/tools/perf/Documentation/perf-probe.txt
index 0657cf4..d781fc5 100644
--- a/tools/perf/Documentation/perf-probe.txt
+++ b/tools/perf/Documentation/perf-probe.txt
@@ -87,7 +87,7 @@ Each probe argument follows below syntax.
 
  [NAME=]LOCALVAR|$retval|%REG|@SYMBOL
 
-'NAME' specifies the name of this argument (optional). You can use the name of local variable, local data structure member (e.g. var->field, var.field2), or kprobe-tracer argument format (e.g. $retval, %ax, etc).
+'NAME' specifies the name of this argument (optional). You can use the name of local variable, local data structure member (e.g. var->field, var.field2), or kprobe-tracer argument format (e.g. $retval, %ax, etc). Note that the name of this argument will be set as the last member name if you specify a local data structure member (e.g. field2 for 'var->field1.field2'.)
 
 LINE SYNTAX
 -----------
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index ab6f53d..19de8b7 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -481,6 +481,10 @@ static void parse_perf_probe_arg(const char *str, struct perf_probe_arg *arg)
 	} while (tmp);
 	(*fieldp)->name = xstrdup(str);
 	pr_debug("%s(%d)\n", (*fieldp)->name, (*fieldp)->ref);
+
+	/* If no name is specified, set the last field name */
+	if (!arg->name)
+		arg->name = xstrdup((*fieldp)->name);
 }
 
 /* Parse perf-probe event command */


-- 
Masami Hiramatsu
e-mail: mhiramat@redhat.com

[PATCH -tip 4/7] trace/kprobes: Support basic types

[Masami Hiramatsu]

Support basic types of integer (u8, u16, u32, u64, s8, s16, s32, s64) in
kprobe tracer. With this patch, users can specify above basic types on
each arguments after ':'. If omitted, the argument type is set as
unsigned long (u32 or u64, arch-dependent).

 e.g.
  echo 'p account_system_time+0 hardirq_offset=%si:s32' > kprobe_events

  adds a probe recording hardirq_offset in signed-32bits value on the
  entry of account_system_time.

Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
---

 Documentation/trace/kprobetrace.txt |    4 
 kernel/trace/trace.h                |   16 -
 kernel/trace/trace_kprobe.c         |  535 ++++++++++++++++++++++-------------
 3 files changed, 334 insertions(+), 221 deletions(-)

diff --git a/Documentation/trace/kprobetrace.txt b/Documentation/trace/kprobetrace.txt
index a9100b2..ec94748 100644
--- a/Documentation/trace/kprobetrace.txt
+++ b/Documentation/trace/kprobetrace.txt
@@ -40,7 +40,9 @@ Synopsis of kprobe_events
   $stack	: Fetch stack address.
   $retval	: Fetch return value.(*)
   +|-offs(FETCHARG) : Fetch memory at FETCHARG +|- offs address.(**)
-  NAME=FETCHARG: Set NAME as the argument name of FETCHARG.
+  NAME=FETCHARG : Set NAME as the argument name of FETCHARG.
+  FETCHARG:TYPE : Set TYPE as the type of FETCHARG. Currently, basic types
+		  (u8/u16/u32/u64/s8/s16/s32/s64) are supported.
 
   (*) only for return probe.
   (**) this is useful for fetching a field of data structures.
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index bec2c97..3ebdb6b 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -102,29 +102,17 @@ struct syscall_trace_exit {
 	long			ret;
 };
 
-struct kprobe_trace_entry {
+struct kprobe_trace_entry_head {
 	struct trace_entry	ent;
 	unsigned long		ip;
-	int			nargs;
-	unsigned long		args[];
 };
 
-#define SIZEOF_KPROBE_TRACE_ENTRY(n)			\
-	(offsetof(struct kprobe_trace_entry, args) +	\
-	(sizeof(unsigned long) * (n)))
-
-struct kretprobe_trace_entry {
+struct kretprobe_trace_entry_head {
 	struct trace_entry	ent;
 	unsigned long		func;
 	unsigned long		ret_ip;
-	int			nargs;
-	unsigned long		args[];
 };
 
-#define SIZEOF_KRETPROBE_TRACE_ENTRY(n)			\
-	(offsetof(struct kretprobe_trace_entry, args) +	\
-	(sizeof(unsigned long) * (n)))
-
 /*
  * trace_flag_type is an enumeration that holds different
  * states when a trace occurs. These are:
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 1251e36..a751432 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -29,6 +29,8 @@
 #include <linux/ctype.h>
 #include <linux/ptrace.h>
 #include <linux/perf_event.h>
+#include <linux/stringify.h>
+#include <asm/bitsperlong.h>
 
 #include "trace.h"
 #include "trace_output.h"
@@ -40,7 +42,6 @@
 
 /* Reserved field names */
 #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"
 
@@ -52,56 +53,102 @@ const char *reserved_field_names[] = {
 	"common_tgid",
 	"common_lock_depth",
 	FIELD_STRING_IP,
-	FIELD_STRING_NARGS,
 	FIELD_STRING_RETIP,
 	FIELD_STRING_FUNC,
 };
 
-struct fetch_func {
-	unsigned long (*func)(struct pt_regs *, void *);
+/* Printing function type */
+typedef int (*print_type_func_t)(struct trace_seq *, const char *, void *);
+#define PRINT_TYPE_FUNC_NAME(type)	print_type_##type
+#define PRINT_TYPE_FMT_NAME(type)	print_type_format_##type
+
+/* Printing  in basic type function template */
+#define DEFINE_BASIC_PRINT_TYPE_FUNC(type, fmt, cast)			\
+static __kprobes int PRINT_TYPE_FUNC_NAME(type)(struct trace_seq *s,	\
+						const char *name, void *data)\
+{									\
+	return trace_seq_printf(s, " %s=" fmt, name, (cast)*(type *)data);\
+}									\
+static const char PRINT_TYPE_FMT_NAME(type)[] = fmt;
+
+DEFINE_BASIC_PRINT_TYPE_FUNC(u8, "%x", unsigned int)
+DEFINE_BASIC_PRINT_TYPE_FUNC(u16, "%x", unsigned int)
+DEFINE_BASIC_PRINT_TYPE_FUNC(u32, "%lx", unsigned long)
+DEFINE_BASIC_PRINT_TYPE_FUNC(u64, "%llx", unsigned long long)
+DEFINE_BASIC_PRINT_TYPE_FUNC(s8, "%d", int)
+DEFINE_BASIC_PRINT_TYPE_FUNC(s16, "%d", int)
+DEFINE_BASIC_PRINT_TYPE_FUNC(s32, "%ld", long)
+DEFINE_BASIC_PRINT_TYPE_FUNC(s64, "%lld", long long)
+
+/* Data fetch function type */
+typedef	void (*fetch_func_t)(struct pt_regs *, void *, void *);
+
+struct fetch_param {
+	fetch_func_t	fn;
 	void *data;
 };
 
-static __kprobes unsigned long call_fetch(struct fetch_func *f,
-					  struct pt_regs *regs)
+static __kprobes void call_fetch(struct fetch_param *fprm,
+				 struct pt_regs *regs, void *dest)
 {
-	return f->func(regs, f->data);
+	return fprm->fn(regs, fprm->data, dest);
 }
 
-/* fetch handlers */
-static __kprobes unsigned long fetch_register(struct pt_regs *regs,
-					      void *offset)
-{
-	return regs_get_register(regs, (unsigned int)((unsigned long)offset));
+#define FETCH_FUNC_NAME(kind, type)	fetch_##kind##_##type
+/*
+ * Define macro for basic types - we don't need to define s* types, because
+ * we have to care only about bitwidth at recording time.
+ */
+#define DEFINE_BASIC_FETCH_FUNCS(kind)  \
+DEFINE_FETCH_##kind(u8)			\
+DEFINE_FETCH_##kind(u16)		\
+DEFINE_FETCH_##kind(u32)		\
+DEFINE_FETCH_##kind(u64)
+
+#define CHECK_BASIC_FETCH_FUNCS(kind, fn)	\
+	((FETCH_FUNC_NAME(kind, u8) == fn) ||	\
+	 (FETCH_FUNC_NAME(kind, u16) == fn) ||	\
+	 (FETCH_FUNC_NAME(kind, u32) == fn) ||	\
+	 (FETCH_FUNC_NAME(kind, u64) == fn))
+
+/* Data fetch function templates */
+#define DEFINE_FETCH_reg(type)						\
+static __kprobes void FETCH_FUNC_NAME(reg, type)(struct pt_regs *regs,	\
+					  void *offset, void *dest)	\
+{									\
+	*(type *)dest = (type)regs_get_register(regs,			\
+				(unsigned int)((unsigned long)offset));	\
 }
-
-static __kprobes unsigned long fetch_stack(struct pt_regs *regs,
-					   void *num)
-{
-	return regs_get_kernel_stack_nth(regs,
-					 (unsigned int)((unsigned long)num));
+DEFINE_BASIC_FETCH_FUNCS(reg)
+
+#define DEFINE_FETCH_stack(type)					\
+static __kprobes void FETCH_FUNC_NAME(stack, type)(struct pt_regs *regs,\
+					  void *offset, void *dest)	\
+{									\
+	*(type *)dest = (type)regs_get_kernel_stack_nth(regs,		\
+				(unsigned int)((unsigned long)offset));	\
 }
+DEFINE_BASIC_FETCH_FUNCS(stack)
 
-static __kprobes unsigned long fetch_memory(struct pt_regs *regs, void *addr)
-{
-	unsigned long retval;
-
-	if (probe_kernel_address(addr, retval))
-		return 0;
-	return retval;
+#define DEFINE_FETCH_retval(type)					\
+static __kprobes void FETCH_FUNC_NAME(retval, type)(struct pt_regs *regs,\
+					  void *dummy, void *dest)	\
+{									\
+	*(type *)dest = (type)regs_return_value(regs);			\
 }
-
-static __kprobes unsigned long fetch_retvalue(struct pt_regs *regs,
-					      void *dummy)
-{
-	return regs_return_value(regs);
-}
-
-static __kprobes unsigned long fetch_stack_address(struct pt_regs *regs,
-						   void *dummy)
-{
-	return kernel_stack_pointer(regs);
+DEFINE_BASIC_FETCH_FUNCS(retval)
+
+#define DEFINE_FETCH_memory(type)					\
+static __kprobes void FETCH_FUNC_NAME(memory, type)(struct pt_regs *regs,\
+					  void *addr, void *dest)	\
+{									\
+	type retval;							\
+	if (probe_kernel_address(addr, retval))				\
+		*(type *)dest = 0;					\
+	else								\
+		*(type *)dest = retval;					\
 }
+DEFINE_BASIC_FETCH_FUNCS(memory)
 
 /* Memory fetching by symbol */
 struct symbol_cache {
@@ -145,51 +192,126 @@ static struct symbol_cache *alloc_symbol_cache(const char *sym, long offset)
 	return sc;
 }
 
-static __kprobes unsigned long fetch_symbol(struct pt_regs *regs, void *data)
-{
-	struct symbol_cache *sc = data;
-
-	if (sc->addr)
-		return fetch_memory(regs, (void *)sc->addr);
-	else
-		return 0;
+#define DEFINE_FETCH_symbol(type)					\
+static __kprobes void FETCH_FUNC_NAME(symbol, type)(struct pt_regs *regs,\
+					  void *data, void *dest)	\
+{									\
+	struct symbol_cache *sc = data;					\
+	if (sc->addr)							\
+		fetch_memory_##type(regs, (void *)sc->addr, dest);	\
+	else								\
+		*(type *)dest = 0;					\
 }
+DEFINE_BASIC_FETCH_FUNCS(symbol)
 
-/* Special indirect memory access interface */
-struct indirect_fetch_data {
-	struct fetch_func orig;
+/* Dereference memory access function */
+struct deref_fetch_param {
+	struct fetch_param orig;
 	long offset;
 };
 
-static __kprobes unsigned long fetch_indirect(struct pt_regs *regs, void *data)
-{
-	struct indirect_fetch_data *ind = data;
-	unsigned long addr;
-
-	addr = call_fetch(&ind->orig, regs);
-	if (addr) {
-		addr += ind->offset;
-		return fetch_memory(regs, (void *)addr);
-	} else
-		return 0;
+#define DEFINE_FETCH_deref(type)					\
+static __kprobes void FETCH_FUNC_NAME(deref, type)(struct pt_regs *regs,\
+					    void *data, void *dest)	\
+{									\
+	struct deref_fetch_param *dprm = data;				\
+	unsigned long addr;						\
+	call_fetch(&dprm->orig, regs, &addr);				\
+	if (addr) {							\
+		addr += dprm->offset;					\
+		fetch_memory_##type(regs, (void *)addr, dest);		\
+	} else								\
+		*(type *)dest = 0;					\
 }
+DEFINE_BASIC_FETCH_FUNCS(deref)
 
-static __kprobes void free_indirect_fetch_data(struct indirect_fetch_data *data)
+static __kprobes void free_deref_fetch_param(struct deref_fetch_param *data)
 {
-	if (data->orig.func == fetch_indirect)
-		free_indirect_fetch_data(data->orig.data);
-	else if (data->orig.func == fetch_symbol)
+	if (CHECK_BASIC_FETCH_FUNCS(deref, data->orig.fn))
+		free_deref_fetch_param(data->orig.data);
+	else if (CHECK_BASIC_FETCH_FUNCS(symbol, data->orig.fn))
 		free_symbol_cache(data->orig.data);
 	kfree(data);
 }
 
+/* Default (unsigned long) fetch type */
+#define __DEFAULT_FETCH_TYPE(t) u##t
+#define _DEFAULT_FETCH_TYPE(t) __DEFAULT_FETCH_TYPE(t)
+#define DEFAULT_FETCH_TYPE _DEFAULT_FETCH_TYPE(BITS_PER_LONG)
+#define DEFAULT_FETCH_TYPE_STR __stringify(DEFAULT_FETCH_TYPE)
+
+#define ASSIGN_FETCH_FUNC(kind, type)	\
+	.kind = FETCH_FUNC_NAME(kind, type)
+
+#define ASSIGN_FETCH_TYPE(ptype, ftype, sign)	\
+	{.name = #ptype,			\
+	 .size = sizeof(ftype),			\
+	 .is_signed = sign,			\
+	 .print = PRINT_TYPE_FUNC_NAME(ptype),	\
+	 .fmt = PRINT_TYPE_FMT_NAME(ptype),	\
+ASSIGN_FETCH_FUNC(reg, ftype),			\
+ASSIGN_FETCH_FUNC(stack, ftype),		\
+ASSIGN_FETCH_FUNC(retval, ftype),		\
+ASSIGN_FETCH_FUNC(memory, ftype),		\
+ASSIGN_FETCH_FUNC(symbol, ftype),		\
+ASSIGN_FETCH_FUNC(deref, ftype),		\
+	}
+
+/* Fetch type information table */
+static const struct fetch_type {
+	const char	*name;		/* Name of type */
+	size_t		size;		/* Byte size of type */
+	int		is_signed;	/* Signed flag */
+	print_type_func_t	print;	/* Print functions */
+	const char	*fmt;		/* Fromat string */
+	/* Fetch functions */
+	fetch_func_t	reg;
+	fetch_func_t	stack;
+	fetch_func_t	retval;
+	fetch_func_t	memory;
+	fetch_func_t	symbol;
+	fetch_func_t	deref;
+} fetch_type_table[] = {
+	ASSIGN_FETCH_TYPE(u8,  u8,  0),
+	ASSIGN_FETCH_TYPE(u16, u16, 0),
+	ASSIGN_FETCH_TYPE(u32, u32, 0),
+	ASSIGN_FETCH_TYPE(u64, u64, 0),
+	ASSIGN_FETCH_TYPE(s8,  u8,  1),
+	ASSIGN_FETCH_TYPE(s16, u16, 1),
+	ASSIGN_FETCH_TYPE(s32, u32, 1),
+	ASSIGN_FETCH_TYPE(s64, u64, 1),
+};
+
+static const struct fetch_type *find_fetch_type(const char *type)
+{
+	int i;
+
+	if (!type)
+		type = DEFAULT_FETCH_TYPE_STR;
+
+	for (i = 0; i < ARRAY_SIZE(fetch_type_table); i++)
+		if (strcmp(type, fetch_type_table[i].name) == 0)
+			return &fetch_type_table[i];
+	return NULL;
+}
+
+/* Special function : only accept unsigned long */
+static __kprobes void fetch_stack_address(struct pt_regs *regs,
+					  void *dummy, void *dest)
+{
+	*(unsigned long *)dest = kernel_stack_pointer(regs);
+}
+
 /**
  * Kprobe event core functions
  */
 
 struct probe_arg {
-	struct fetch_func	fetch;
-	const char		*name;
+	struct fetch_param	fetch;
+	unsigned int		offset;	/* Offset from argument entry */
+	const char		*name;	/* Name of this argument */
+	const char		*comm;	/* Command of this argument */
+	const struct fetch_type	*type;	/* Type of this argument */
 };
 
 /* Flags for trace_probe */
@@ -204,6 +326,7 @@ struct trace_probe {
 	const char		*symbol;	/* symbol name */
 	struct ftrace_event_call	call;
 	struct trace_event		event;
+	ssize_t			size;		/* trace entry size */
 	unsigned int		nr_args;
 	struct probe_arg	args[];
 };
@@ -212,6 +335,7 @@ struct trace_probe {
 	(offsetof(struct trace_probe, args) +	\
 	(sizeof(struct probe_arg) * (n)))
 
+
 static __kprobes int probe_is_return(struct trace_probe *tp)
 {
 	return tp->rp.handler != NULL;
@@ -222,49 +346,6 @@ static __kprobes const char *probe_symbol(struct trace_probe *tp)
 	return tp->symbol ? tp->symbol : "unknown";
 }
 
-static int probe_arg_string(char *buf, size_t n, struct fetch_func *ff)
-{
-	int ret = -EINVAL;
-
-	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, "$stack%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;
-		if (sc->offset)
-			ret = snprintf(buf, n, "@%s%+ld", sc->symbol,
-					sc->offset);
-		else
-			ret = snprintf(buf, n, "@%s", sc->symbol);
-	} else if (ff->func == fetch_retvalue)
-		ret = snprintf(buf, n, "$retval");
-	else if (ff->func == fetch_stack_address)
-		ret = snprintf(buf, n, "$stack");
-	else if (ff->func == fetch_indirect) {
-		struct indirect_fetch_data *id = ff->data;
-		size_t l = 0;
-		ret = snprintf(buf, n, "%+ld(", id->offset);
-		if (ret >= n)
-			goto end;
-		l += ret;
-		ret = probe_arg_string(buf + l, n - l, &id->orig);
-		if (ret < 0)
-			goto end;
-		l += ret;
-		ret = snprintf(buf + l, n - l, ")");
-		ret += l;
-	}
-end:
-	if (ret >= n)
-		return -ENOSPC;
-	return ret;
-}
-
 static int register_probe_event(struct trace_probe *tp);
 static void unregister_probe_event(struct trace_probe *tp);
 
@@ -347,11 +428,12 @@ error:
 
 static void free_probe_arg(struct probe_arg *arg)
 {
-	if (arg->fetch.func == fetch_symbol)
+	if (CHECK_BASIC_FETCH_FUNCS(deref, arg->fetch.fn))
+		free_deref_fetch_param(arg->fetch.data);
+	else if (CHECK_BASIC_FETCH_FUNCS(symbol, arg->fetch.fn))
 		free_symbol_cache(arg->fetch.data);
-	else if (arg->fetch.func == fetch_indirect)
-		free_indirect_fetch_data(arg->fetch.data);
 	kfree(arg->name);
+	kfree(arg->comm);
 }
 
 static void free_trace_probe(struct trace_probe *tp)
@@ -457,28 +539,30 @@ 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_vars(char *arg, struct fetch_func *ff, int is_return)
+static int parse_probe_vars(char *arg, const struct fetch_type *t,
+			    struct fetch_param *f, int is_return)
 {
 	int ret = 0;
 	unsigned long param;
 
 	if (strcmp(arg, "retval") == 0) {
-		if (is_return) {
-			ff->func = fetch_retvalue;
-			ff->data = NULL;
-		} else
+		if (is_return)
+			f->fn = t->retval;
+		else
 			ret = -EINVAL;
 	} else if (strncmp(arg, "stack", 5) == 0) {
 		if (arg[5] == '\0') {
-			ff->func = fetch_stack_address;
-			ff->data = NULL;
+			if (strcmp(t->name, DEFAULT_FETCH_TYPE_STR) == 0)
+				f->fn = fetch_stack_address;
+			else
+				ret = -EINVAL;
 		} else if (isdigit(arg[5])) {
 			ret = strict_strtoul(arg + 5, 10, &param);
 			if (ret || param > PARAM_MAX_STACK)
 				ret = -EINVAL;
 			else {
-				ff->func = fetch_stack;
-				ff->data = (void *)param;
+				f->fn = t->stack;
+				f->data = (void *)param;
 			}
 		} else
 			ret = -EINVAL;
@@ -488,7 +572,8 @@ static int parse_probe_vars(char *arg, struct fetch_func *ff, int is_return)
 }
 
 /* Recursive argument parser */
-static int __parse_probe_arg(char *arg, struct fetch_func *ff, int is_return)
+static int __parse_probe_arg(char *arg, const struct fetch_type *t,
+			     struct fetch_param *f, int is_return)
 {
 	int ret = 0;
 	unsigned long param;
@@ -497,13 +582,13 @@ static int __parse_probe_arg(char *arg, struct fetch_func *ff, int is_return)
 
 	switch (arg[0]) {
 	case '$':
-		ret = parse_probe_vars(arg + 1, ff, is_return);
+		ret = parse_probe_vars(arg + 1, t, f, 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;
+			f->fn = t->reg;
+			f->data = (void *)(unsigned long)ret;
 			ret = 0;
 		}
 		break;
@@ -512,26 +597,22 @@ static int __parse_probe_arg(char *arg, struct fetch_func *ff, int is_return)
 			ret = strict_strtoul(arg + 1, 0, &param);
 			if (ret)
 				break;
-			ff->func = fetch_memory;
-			ff->data = (void *)param;
+			f->fn = t->memory;
+			f->data = (void *)param;
 		} else {
 			ret = split_symbol_offset(arg + 1, &offset);
 			if (ret)
 				break;
-			ff->data = alloc_symbol_cache(arg + 1, offset);
-			if (ff->data)
-				ff->func = fetch_symbol;
-			else
-				ret = -EINVAL;
+			f->data = alloc_symbol_cache(arg + 1, offset);
+			if (f->data)
+				f->fn = t->symbol;
 		}
 		break;
-	case '+':	/* indirect memory */
+	case '+':	/* deref memory */
 	case '-':
 		tmp = strchr(arg, '(');
-		if (!tmp) {
-			ret = -EINVAL;
+		if (!tmp)
 			break;
-		}
 		*tmp = '\0';
 		ret = strict_strtol(arg + 1, 0, &offset);
 		if (ret)
@@ -541,38 +622,58 @@ static int __parse_probe_arg(char *arg, struct fetch_func *ff, int is_return)
 		arg = tmp + 1;
 		tmp = strrchr(arg, ')');
 		if (tmp) {
-			struct indirect_fetch_data *id;
+			struct deref_fetch_param *dprm;
+			const struct fetch_type *t2 = find_fetch_type(NULL);
 			*tmp = '\0';
-			id = kzalloc(sizeof(struct indirect_fetch_data),
-				     GFP_KERNEL);
-			if (!id)
+			dprm = kzalloc(sizeof(struct deref_fetch_param),
+				       GFP_KERNEL);
+			if (!dprm)
 				return -ENOMEM;
-			id->offset = offset;
-			ret = __parse_probe_arg(arg, &id->orig, is_return);
+			dprm->offset = offset;
+			ret = __parse_probe_arg(arg, t2, &dprm->orig,
+						is_return);
 			if (ret)
-				kfree(id);
+				kfree(dprm);
 			else {
-				ff->func = fetch_indirect;
-				ff->data = (void *)id;
+				f->fn = t->deref;
+				f->data = (void *)dprm;
 			}
-		} else
-			ret = -EINVAL;
+		}
 		break;
-	default:
-		/* TODO: support custom handler */
-		ret = -EINVAL;
 	}
+	if (!ret && !f->fn)
+		ret = -EINVAL;
 	return ret;
 }
 
 /* String length checking wrapper */
-static int parse_probe_arg(char *arg, struct fetch_func *ff, int is_return)
+static int parse_probe_arg(char *arg, struct trace_probe *tp,
+			   struct probe_arg *parg, int is_return)
 {
+	const char *t;
+
 	if (strlen(arg) > MAX_ARGSTR_LEN) {
 		pr_info("Argument is too long.: %s\n",  arg);
 		return -ENOSPC;
 	}
-	return __parse_probe_arg(arg, ff, is_return);
+	parg->comm = kstrdup(arg, GFP_KERNEL);
+	if (!parg->comm) {
+		pr_info("Failed to allocate memory for command '%s'.\n", arg);
+		return -ENOMEM;
+	}
+	t = strchr(parg->comm, ':');
+	if (t) {
+		arg[t - parg->comm] = '\0';
+		t++;
+	}
+	parg->type = find_fetch_type(t);
+	if (!parg->type) {
+		pr_info("Unsupported type: %s\n", t);
+		return -EINVAL;
+	}
+	parg->offset = tp->size;
+	tp->size += parg->type->size;
+	return __parse_probe_arg(arg, parg->type, &parg->fetch, is_return);
 }
 
 /* Return 1 if name is reserved or already used by another argument */
@@ -602,15 +703,18 @@ static int create_trace_probe(int argc, char **argv)
 	 *  @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
-	 * Indirect memory fetch:
+	 * Dereferencing memory fetch:
 	 *  +|-offs(ARG) : fetch memory at ARG +|- offs address.
 	 * Alias name of args:
 	 *  NAME=FETCHARG : set NAME as alias of FETCHARG.
+	 * Type of args:
+	 *  FETCHARG:TYPE : use TYPE instead of unsigned long.
 	 */
 	struct trace_probe *tp;
 	int i, ret = 0;
 	int is_return = 0, is_delete = 0;
-	char *symbol = NULL, *event = NULL, *arg = NULL, *group = NULL;
+	char *symbol = NULL, *event = NULL, *group = NULL;
+	char *arg, *tmp;
 	unsigned long offset = 0;
 	void *addr = NULL;
 	char buf[MAX_EVENT_NAME_LEN];
@@ -723,13 +827,6 @@ static int create_trace_probe(int argc, char **argv)
 		else
 			arg = argv[i];
 
-		if (conflict_field_name(argv[i], tp->args, i)) {
-			pr_info("Argument%d name '%s' conflicts with "
-				"another field.\n", i, argv[i]);
-			ret = -EINVAL;
-			goto error;
-		}
-
 		tp->args[i].name = kstrdup(argv[i], GFP_KERNEL);
 		if (!tp->args[i].name) {
 			pr_info("Failed to allocate argument%d name '%s'.\n",
@@ -737,9 +834,19 @@ static int create_trace_probe(int argc, char **argv)
 			ret = -ENOMEM;
 			goto error;
 		}
+		tmp = strchr(tp->args[i].name, ':');
+		if (tmp)
+			*tmp = '_';	/* convert : to _ */
+
+		if (conflict_field_name(tp->args[i].name, tp->args, i)) {
+			pr_info("Argument%d name '%s' conflicts with "
+				"another field.\n", i, argv[i]);
+			ret = -EINVAL;
+			goto error;
+		}
 
 		/* Parse fetch argument */
-		ret = parse_probe_arg(arg, &tp->args[i].fetch, is_return);
+		ret = parse_probe_arg(arg, tp, &tp->args[i], is_return);
 		if (ret) {
 			pr_info("Parse error at argument%d. (%d)\n", i, ret);
 			kfree(tp->args[i].name);
@@ -794,8 +901,7 @@ static void probes_seq_stop(struct seq_file *m, void *v)
 static int probes_seq_show(struct seq_file *m, void *v)
 {
 	struct trace_probe *tp = v;
-	int i, ret;
-	char buf[MAX_ARGSTR_LEN + 1];
+	int i;
 
 	seq_printf(m, "%c", probe_is_return(tp) ? 'r' : 'p');
 	seq_printf(m, ":%s/%s", tp->call.system, tp->call.name);
@@ -807,15 +913,10 @@ static int probes_seq_show(struct seq_file *m, void *v)
 	else
 		seq_printf(m, " %s", probe_symbol(tp));
 
-	for (i = 0; i < tp->nr_args; i++) {
-		ret = probe_arg_string(buf, MAX_ARGSTR_LEN, &tp->args[i].fetch);
-		if (ret < 0) {
-			pr_warning("Argument%d decoding error(%d).\n", i, ret);
-			return ret;
-		}
-		seq_printf(m, " %s=%s", tp->args[i].name, buf);
-	}
+	for (i = 0; i < tp->nr_args; i++)
+		seq_printf(m, " %s=%s", tp->args[i].name, tp->args[i].comm);
 	seq_printf(m, "\n");
+
 	return 0;
 }
 
@@ -945,9 +1046,10 @@ static const struct file_operations kprobe_profile_ops = {
 static __kprobes void kprobe_trace_func(struct kprobe *kp, struct pt_regs *regs)
 {
 	struct trace_probe *tp = container_of(kp, struct trace_probe, rp.kp);
-	struct kprobe_trace_entry *entry;
+	struct kprobe_trace_entry_head *entry;
 	struct ring_buffer_event *event;
 	struct ring_buffer *buffer;
+	u8 *data;
 	int size, i, pc;
 	unsigned long irq_flags;
 	struct ftrace_event_call *call = &tp->call;
@@ -957,7 +1059,7 @@ static __kprobes void kprobe_trace_func(struct kprobe *kp, struct pt_regs *regs)
 	local_save_flags(irq_flags);
 	pc = preempt_count();
 
-	size = SIZEOF_KPROBE_TRACE_ENTRY(tp->nr_args);
+	size = sizeof(*entry) + tp->size;
 
 	event = trace_current_buffer_lock_reserve(&buffer, call->id, size,
 						  irq_flags, pc);
@@ -965,10 +1067,10 @@ static __kprobes void kprobe_trace_func(struct kprobe *kp, struct pt_regs *regs)
 		return;
 
 	entry = ring_buffer_event_data(event);
-	entry->nargs = tp->nr_args;
 	entry->ip = (unsigned long)kp->addr;
+	data = (u8 *)&entry[1];
 	for (i = 0; i < tp->nr_args; i++)
-		entry->args[i] = call_fetch(&tp->args[i].fetch, regs);
+		call_fetch(&tp->args[i].fetch, regs, data + tp->args[i].offset);
 
 	if (!filter_current_check_discard(buffer, call, entry, event))
 		trace_nowake_buffer_unlock_commit(buffer, event, irq_flags, pc);
@@ -979,9 +1081,10 @@ static __kprobes void kretprobe_trace_func(struct kretprobe_instance *ri,
 					  struct pt_regs *regs)
 {
 	struct trace_probe *tp = container_of(ri->rp, struct trace_probe, rp);
-	struct kretprobe_trace_entry *entry;
+	struct kretprobe_trace_entry_head *entry;
 	struct ring_buffer_event *event;
 	struct ring_buffer *buffer;
+	u8 *data;
 	int size, i, pc;
 	unsigned long irq_flags;
 	struct ftrace_event_call *call = &tp->call;
@@ -989,7 +1092,7 @@ static __kprobes void kretprobe_trace_func(struct kretprobe_instance *ri,
 	local_save_flags(irq_flags);
 	pc = preempt_count();
 
-	size = SIZEOF_KRETPROBE_TRACE_ENTRY(tp->nr_args);
+	size = sizeof(*entry) + tp->size;
 
 	event = trace_current_buffer_lock_reserve(&buffer, call->id, size,
 						  irq_flags, pc);
@@ -997,11 +1100,11 @@ static __kprobes void kretprobe_trace_func(struct kretprobe_instance *ri,
 		return;
 
 	entry = ring_buffer_event_data(event);
-	entry->nargs = tp->nr_args;
 	entry->func = (unsigned long)tp->rp.kp.addr;
 	entry->ret_ip = (unsigned long)ri->ret_addr;
+	data = (u8 *)&entry[1];
 	for (i = 0; i < tp->nr_args; i++)
-		entry->args[i] = call_fetch(&tp->args[i].fetch, regs);
+		call_fetch(&tp->args[i].fetch, regs, data + tp->args[i].offset);
 
 	if (!filter_current_check_discard(buffer, call, entry, event))
 		trace_nowake_buffer_unlock_commit(buffer, event, irq_flags, pc);
@@ -1011,13 +1114,14 @@ static __kprobes void kretprobe_trace_func(struct kretprobe_instance *ri,
 enum print_line_t
 print_kprobe_event(struct trace_iterator *iter, int flags)
 {
-	struct kprobe_trace_entry *field;
+	struct kprobe_trace_entry_head *field;
 	struct trace_seq *s = &iter->seq;
 	struct trace_event *event;
 	struct trace_probe *tp;
+	u8 *data;
 	int i;
 
-	field = (struct kprobe_trace_entry *)iter->ent;
+	field = (struct kprobe_trace_entry_head *)iter->ent;
 	event = ftrace_find_event(field->ent.type);
 	tp = container_of(event, struct trace_probe, event);
 
@@ -1030,9 +1134,10 @@ print_kprobe_event(struct trace_iterator *iter, int flags)
 	if (!trace_seq_puts(s, ")"))
 		goto partial;
 
-	for (i = 0; i < field->nargs; i++)
-		if (!trace_seq_printf(s, " %s=%lx",
-				      tp->args[i].name, field->args[i]))
+	data = (u8 *)&field[1];
+	for (i = 0; i < tp->nr_args; i++)
+		if (!tp->args[i].type->print(s, tp->args[i].name,
+					     data + tp->args[i].offset))
 			goto partial;
 
 	if (!trace_seq_puts(s, "\n"))
@@ -1046,13 +1151,14 @@ partial:
 enum print_line_t
 print_kretprobe_event(struct trace_iterator *iter, int flags)
 {
-	struct kretprobe_trace_entry *field;
+	struct kretprobe_trace_entry_head *field;
 	struct trace_seq *s = &iter->seq;
 	struct trace_event *event;
 	struct trace_probe *tp;
+	u8 *data;
 	int i;
 
-	field = (struct kretprobe_trace_entry *)iter->ent;
+	field = (struct kretprobe_trace_entry_head *)iter->ent;
 	event = ftrace_find_event(field->ent.type);
 	tp = container_of(event, struct trace_probe, event);
 
@@ -1071,9 +1177,10 @@ print_kretprobe_event(struct trace_iterator *iter, int flags)
 	if (!trace_seq_puts(s, ")"))
 		goto partial;
 
-	for (i = 0; i < field->nargs; i++)
-		if (!trace_seq_printf(s, " %s=%lx",
-				      tp->args[i].name, field->args[i]))
+	data = (u8 *)&field[1];
+	for (i = 0; i < tp->nr_args; i++)
+		if (!tp->args[i].type->print(s, tp->args[i].name,
+					     data + tp->args[i].offset))
 			goto partial;
 
 	if (!trace_seq_puts(s, "\n"))
@@ -1129,29 +1236,43 @@ static int probe_event_raw_init(struct ftrace_event_call *event_call)
 static int kprobe_event_define_fields(struct ftrace_event_call *event_call)
 {
 	int ret, i;
-	struct kprobe_trace_entry field;
+	struct kprobe_trace_entry_head field;
 	struct trace_probe *tp = (struct trace_probe *)event_call->data;
 
 	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);
+	for (i = 0; i < tp->nr_args; i++) {
+		ret = trace_define_field(event_call, tp->args[i].type->name,
+					 tp->args[i].name,
+					 sizeof(field) + tp->args[i].offset,
+					 tp->args[i].type->size,
+					 tp->args[i].type->is_signed,
+					 FILTER_OTHER);
+		if (ret)
+			return ret;
+	}
 	return 0;
 }
 
 static int kretprobe_event_define_fields(struct ftrace_event_call *event_call)
 {
 	int ret, i;
-	struct kretprobe_trace_entry field;
+	struct kretprobe_trace_entry_head field;
 	struct trace_probe *tp = (struct trace_probe *)event_call->data;
 
 	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);
+	for (i = 0; i < tp->nr_args; i++) {
+		ret = trace_define_field(event_call, tp->args[i].type->name,
+					 tp->args[i].name,
+					 sizeof(field) + tp->args[i].offset,
+					 tp->args[i].type->size,
+					 tp->args[i].type->is_signed,
+					 FILTER_OTHER);
+		if (ret)
+			return ret;
+	}
 	return 0;
 }
 
@@ -1176,8 +1297,8 @@ static int __set_print_fmt(struct trace_probe *tp, char *buf, int len)
 	pos += snprintf(buf + pos, LEN_OR_ZERO, "\"%s", fmt);
 
 	for (i = 0; i < tp->nr_args; i++) {
-		pos += snprintf(buf + pos, LEN_OR_ZERO, " %s=%%lx",
-				tp->args[i].name);
+		pos += snprintf(buf + pos, LEN_OR_ZERO, " %s=%s",
+				tp->args[i].name, tp->args[i].type->fmt);
 	}
 
 	pos += snprintf(buf + pos, LEN_OR_ZERO, "\", %s", arg);
@@ -1219,12 +1340,13 @@ static __kprobes void kprobe_perf_func(struct kprobe *kp,
 {
 	struct trace_probe *tp = container_of(kp, struct trace_probe, rp.kp);
 	struct ftrace_event_call *call = &tp->call;
-	struct kprobe_trace_entry *entry;
+	struct kprobe_trace_entry_head *entry;
+	u8 *data;
 	int size, __size, i;
 	unsigned long irq_flags;
 	int rctx;
 
-	__size = SIZEOF_KPROBE_TRACE_ENTRY(tp->nr_args);
+	__size = sizeof(*entry) + tp->size;
 	size = ALIGN(__size + sizeof(u32), sizeof(u64));
 	size -= sizeof(u32);
 	if (WARN_ONCE(size > PERF_MAX_TRACE_SIZE,
@@ -1235,10 +1357,10 @@ static __kprobes void kprobe_perf_func(struct kprobe *kp,
 	if (!entry)
 		return;
 
-	entry->nargs = tp->nr_args;
 	entry->ip = (unsigned long)kp->addr;
+	data = (u8 *)&entry[1];
 	for (i = 0; i < tp->nr_args; i++)
-		entry->args[i] = call_fetch(&tp->args[i].fetch, regs);
+		call_fetch(&tp->args[i].fetch, regs, data + tp->args[i].offset);
 
 	perf_trace_buf_submit(entry, size, rctx, entry->ip, 1, irq_flags, regs);
 }
@@ -1249,12 +1371,13 @@ static __kprobes void kretprobe_perf_func(struct kretprobe_instance *ri,
 {
 	struct trace_probe *tp = container_of(ri->rp, struct trace_probe, rp);
 	struct ftrace_event_call *call = &tp->call;
-	struct kretprobe_trace_entry *entry;
+	struct kretprobe_trace_entry_head *entry;
+	u8 *data;
 	int size, __size, i;
 	unsigned long irq_flags;
 	int rctx;
 
-	__size = SIZEOF_KRETPROBE_TRACE_ENTRY(tp->nr_args);
+	__size = sizeof(*entry) + tp->size;
 	size = ALIGN(__size + sizeof(u32), sizeof(u64));
 	size -= sizeof(u32);
 	if (WARN_ONCE(size > PERF_MAX_TRACE_SIZE,
@@ -1265,11 +1388,11 @@ static __kprobes void kretprobe_perf_func(struct kretprobe_instance *ri,
 	if (!entry)
 		return;
 
-	entry->nargs = tp->nr_args;
 	entry->func = (unsigned long)tp->rp.kp.addr;
 	entry->ret_ip = (unsigned long)ri->ret_addr;
+	data = (u8 *)&entry[1];
 	for (i = 0; i < tp->nr_args; i++)
-		entry->args[i] = call_fetch(&tp->args[i].fetch, regs);
+		call_fetch(&tp->args[i].fetch, regs, data + tp->args[i].offset);
 
 	perf_trace_buf_submit(entry, size, rctx, entry->ret_ip, 1,
 			       irq_flags, regs);


-- 
Masami Hiramatsu
e-mail: mhiramat@redhat.com

[PATCH -tip 5/7] perf probe: Query basic types from debuginfo

[Masami Hiramatsu]

Query the basic type information (byte-size and signed-flag) from
debuginfo and pass that to kprobe-tracer. This is especially useful
for tracing the members of data structure, because each member has
different byte-size on the memory.

Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
---

 tools/perf/util/probe-event.c  |    9 +++++
 tools/perf/util/probe-event.h  |    1 +
 tools/perf/util/probe-finder.c |   78 ++++++++++++++++++++++++++++++++++++----
 3 files changed, 80 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 19de8b7..05ca4a9 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -740,6 +740,13 @@ static int synthesize_kprobe_trace_arg(struct kprobe_trace_arg *arg,
 		buf += ret;
 		buflen -= ret;
 	}
+	/* Print argument type */
+	if (arg->type) {
+		ret = e_snprintf(buf, buflen, ":%s", arg->type);
+		if (ret <= 0)
+			return ret;
+		buf += ret;
+	}
 
 	return buf - tmp;
 }
@@ -848,6 +855,8 @@ void clear_kprobe_trace_event(struct kprobe_trace_event *tev)
 			free(tev->args[i].name);
 		if (tev->args[i].value)
 			free(tev->args[i].value);
+		if (tev->args[i].type)
+			free(tev->args[i].type);
 		ref = tev->args[i].ref;
 		while (ref) {
 			next = ref->next;
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index a73ede6..0759db6 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -23,6 +23,7 @@ struct kprobe_trace_arg_ref {
 struct kprobe_trace_arg {
 	char				*name;	/* Argument name */
 	char				*value;	/* Base value */
+	char				*type;	/* Type name */
 	struct kprobe_trace_arg_ref	*ref;	/* Referencing offset */
 };
 
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 043140f..f3c2706 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -84,6 +84,9 @@ const char *x86_64_regs_table[X86_64_MAX_REGS] = {
 #define arch_regs_table x86_32_regs_table
 #endif
 
+/* Kprobe tracer basic type is up to u64 */
+#define MAX_BASIC_TYPE_BITS	64
+
 /* Return architecture dependent register string (for kprobe-tracer) */
 static const char *get_arch_regstr(unsigned int n)
 {
@@ -228,6 +231,31 @@ static Dwarf_Die *die_get_real_type(Dwarf_Die *vr_die, Dwarf_Die *die_mem)
 	return die_mem;
 }
 
+static bool die_is_signed_type(Dwarf_Die *tp_die)
+{
+	Dwarf_Attribute attr;
+	Dwarf_Word ret;
+
+	if (dwarf_attr(tp_die, DW_AT_encoding, &attr) == NULL ||
+	    dwarf_formudata(&attr, &ret) != 0)
+		return false;
+
+	return (ret == DW_ATE_signed_char || ret == DW_ATE_signed ||
+		ret == DW_ATE_signed_fixed);
+}
+
+static int die_get_byte_size(Dwarf_Die *tp_die)
+{
+	Dwarf_Attribute attr;
+	Dwarf_Word ret;
+
+	if (dwarf_attr(tp_die, DW_AT_byte_size, &attr) == NULL ||
+	    dwarf_formudata(&attr, &ret) != 0)
+		return 0;
+
+	return (int)ret;
+}
+
 /* Return values for die_find callbacks */
 enum {
 	DIE_FIND_CB_FOUND = 0,		/* End of Search */
@@ -404,13 +432,42 @@ static void convert_location(Dwarf_Op *op, struct probe_finder *pf)
 	}
 }
 
+static void convert_variable_type(Dwarf_Die *vr_die,
+				  struct kprobe_trace_arg *targ)
+{
+	Dwarf_Die type;
+	char buf[16];
+	int ret;
+
+	if (die_get_real_type(vr_die, &type) == NULL)
+		die("Failed to get a type information of %s.",
+		    dwarf_diename(vr_die));
+
+	ret = die_get_byte_size(&type) * 8;
+	if (ret) {
+		/* Check the bitwidth */
+		if (ret > MAX_BASIC_TYPE_BITS) {
+			pr_warning("  Warning: %s exceeds max-bitwidth."
+				   " Cut down to %d bits.\n",
+				   dwarf_diename(&type), MAX_BASIC_TYPE_BITS);
+			ret = MAX_BASIC_TYPE_BITS;
+		}
+
+		ret = snprintf(buf, 16, "%c%d",
+			       die_is_signed_type(&type) ? 's' : 'u', ret);
+		if (ret < 0 || ret >= 16)
+			die("Failed to convert variable type.");
+		targ->type = xstrdup(buf);
+	}
+}
+
 static void convert_variable_fields(Dwarf_Die *vr_die, const char *varname,
 				    struct perf_probe_arg_field *field,
-				    struct kprobe_trace_arg_ref **ref_ptr)
+				    struct kprobe_trace_arg_ref **ref_ptr,
+				    Dwarf_Die *die_mem)
 {
 	struct kprobe_trace_arg_ref *ref = *ref_ptr;
 	Dwarf_Attribute attr;
-	Dwarf_Die member;
 	Dwarf_Die type;
 	Dwarf_Word offs;
 
@@ -444,26 +501,27 @@ static void convert_variable_fields(Dwarf_Die *vr_die, const char *varname,
 	if (dwarf_tag(&type) != DW_TAG_structure_type)
 		die("%s is not a data structure.", varname);
 
-	if (die_find_member(&type, field->name, &member) == NULL)
+	if (die_find_member(&type, field->name, die_mem) == NULL)
 		die("%s(tyep:%s) has no member %s.", varname,
 		    dwarf_diename(&type), field->name);
 
 	/* Get the offset of the field */
-	if (dwarf_attr(&member, DW_AT_data_member_location, &attr) == NULL ||
+	if (dwarf_attr(die_mem, DW_AT_data_member_location, &attr) == NULL ||
 	    dwarf_formudata(&attr, &offs) != 0)
 		die("Failed to get the offset of %s.", field->name);
 	ref->offset += (long)offs;
 
 	/* Converting next field */
 	if (field->next)
-		convert_variable_fields(&member, field->name, field->next,
-					&ref);
+		convert_variable_fields(die_mem, field->name, field->next,
+					&ref, die_mem);
 }
 
 /* Show a variables in kprobe event format */
 static void convert_variable(Dwarf_Die *vr_die, struct probe_finder *pf)
 {
 	Dwarf_Attribute attr;
+	Dwarf_Die die_mem;
 	Dwarf_Op *expr;
 	size_t nexpr;
 	int ret;
@@ -477,9 +535,13 @@ static void convert_variable(Dwarf_Die *vr_die, struct probe_finder *pf)
 
 	convert_location(expr, pf);
 
-	if (pf->pvar->field)
+	if (pf->pvar->field) {
 		convert_variable_fields(vr_die, pf->pvar->var,
-					pf->pvar->field, &pf->tvar->ref);
+					pf->pvar->field, &pf->tvar->ref,
+					&die_mem);
+		vr_die = &die_mem;
+	}
+	convert_variable_type(vr_die, pf->tvar);
 	/* *expr will be cached in libdw. Don't free it. */
 	return ;
 error:


-- 
Masami Hiramatsu
e-mail: mhiramat@redhat.com

[PATCH -tip 6/7] perf probe: Support basic type casting

[Masami Hiramatsu]

Add basic type casting for arguments to perf probe. This allows
users to specify the actual type of arguments. Of course, if
user sets invalid types, kprobe-tracer rejects that.

Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
---

 tools/perf/Documentation/perf-probe.txt |    3 ++-
 tools/perf/util/probe-event.c           |   23 ++++++++++++++++++++++-
 tools/perf/util/probe-event.h           |    1 +
 tools/perf/util/probe-finder.c          |   10 ++++++++--
 4 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/tools/perf/Documentation/perf-probe.txt b/tools/perf/Documentation/perf-probe.txt
index d781fc5..21820c3 100644
--- a/tools/perf/Documentation/perf-probe.txt
+++ b/tools/perf/Documentation/perf-probe.txt
@@ -85,9 +85,10 @@ PROBE ARGUMENT
 --------------
 Each probe argument follows below syntax.
 
- [NAME=]LOCALVAR|$retval|%REG|@SYMBOL
+ [NAME=]LOCALVAR|$retval|%REG|@SYMBOL[:TYPE]
 
 'NAME' specifies the name of this argument (optional). You can use the name of local variable, local data structure member (e.g. var->field, var.field2), or kprobe-tracer argument format (e.g. $retval, %ax, etc). Note that the name of this argument will be set as the last member name if you specify a local data structure member (e.g. field2 for 'var->field1.field2'.)
+'TYPE' casts the type of this argument (optional). If omitted, perf probe automatically set the type based on debuginfo.
 
 LINE SYNTAX
 -----------
diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
index 05ca4a9..bef2805 100644
--- a/tools/perf/util/probe-event.c
+++ b/tools/perf/util/probe-event.c
@@ -435,7 +435,7 @@ static void parse_perf_probe_point(char *arg, struct perf_probe_event *pev)
 }
 
 /* Parse perf-probe event argument */
-static void parse_perf_probe_arg(const char *str, struct perf_probe_arg *arg)
+static void parse_perf_probe_arg(char *str, struct perf_probe_arg *arg)
 {
 	char *tmp;
 	struct perf_probe_arg_field **fieldp;
@@ -445,9 +445,17 @@ static void parse_perf_probe_arg(const char *str, struct perf_probe_arg *arg)
 	tmp = strchr(str, '=');
 	if (tmp) {
 		arg->name = xstrndup(str, tmp - str);
+		pr_debug("name:%s ", arg->name);
 		str = tmp + 1;
 	}
 
+	tmp = strchr(str, ':');
+	if (tmp) {	/* Type setting */
+		*tmp = '\0';
+		arg->type = xstrdup(tmp + 1);
+		pr_debug("type:%s ", arg->type);
+	}
+
 	tmp = strpbrk(str, "-.");
 	if (!is_c_varname(str) || !tmp) {
 		/* A variable, register, symbol or special value */
@@ -603,6 +611,15 @@ int synthesize_perf_probe_arg(struct perf_probe_arg *pa, char *buf, size_t len)
 		len -= ret;
 		field = field->next;
 	}
+
+	if (pa->type) {
+		ret = e_snprintf(tmp, len, ":%s", pa->type);
+		if (ret <= 0)
+			goto error;
+		tmp += ret;
+		len -= ret;
+	}
+
 	return tmp - buf;
 error:
 	die("Failed to synthesize perf probe argument: %s", strerror(-ret));
@@ -825,6 +842,8 @@ void clear_perf_probe_event(struct perf_probe_event *pev)
 			free(pev->args[i].name);
 		if (pev->args[i].var)
 			free(pev->args[i].var);
+		if (pev->args[i].type)
+			free(pev->args[i].type);
 		field = pev->args[i].field;
 		while (field) {
 			next = field->next;
@@ -1145,6 +1164,8 @@ static int convert_to_kprobe_trace_events(struct perf_probe_event *pev,
 			if (pev->args[i].name)
 				tev->args[i].name = xstrdup(pev->args[i].name);
 			tev->args[i].value = xstrdup(pev->args[i].var);
+			if (pev->args[i].type)
+				tev->args[i].type = xstrdup(pev->args[i].type);
 		}
 	}
 
diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
index 0759db6..5a9837c 100644
--- a/tools/perf/util/probe-event.h
+++ b/tools/perf/util/probe-event.h
@@ -57,6 +57,7 @@ struct perf_probe_arg_field {
 struct perf_probe_arg {
 	char				*name;	/* Argument name */
 	char				*var;	/* Variable name */
+	char				*type;	/* Type name */
 	struct perf_probe_arg_field	*field;	/* Structure fields */
 };
 
diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index f3c2706..11d6592 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -541,7 +541,10 @@ static void convert_variable(Dwarf_Die *vr_die, struct probe_finder *pf)
 					&die_mem);
 		vr_die = &die_mem;
 	}
-	convert_variable_type(vr_die, pf->tvar);
+	if (pf->pvar->type)
+		pf->tvar->type = xstrdup(pf->pvar->type);
+	else
+		convert_variable_type(vr_die, pf->tvar);
 	/* *expr will be cached in libdw. Don't free it. */
 	return ;
 error:
@@ -554,13 +557,16 @@ error:
 static void find_variable(Dwarf_Die *sp_die, struct probe_finder *pf)
 {
 	Dwarf_Die vr_die;
-	char buf[32];
+	char buf[32], *ptr;
 
 	/* TODO: Support arrays */
 	if (pf->pvar->name)
 		pf->tvar->name = xstrdup(pf->pvar->name);
 	else {
 		synthesize_perf_probe_arg(pf->pvar, buf, 32);
+		ptr = strchr(buf, ':');	/* Change type separator to _ */
+		if (ptr)
+			*ptr = '_';
 		pf->tvar->name = xstrdup(buf);
 	}
 


-- 
Masami Hiramatsu
e-mail: mhiramat@redhat.com

[PATCH -tip 7/7] perf probe: Support DW_OP_call_frame_cfa in debuginfo

[Masami Hiramatsu]

When building kernel without CONFIG_FRAME_POINTER, gcc uses
CFA (canonical frame address) for frame base. With this patch,
perf probe just gets CFI (call frame information) from debuginfo
and search corresponding CFA from the CFI. IOW, this allows
perf probe works correctly on the kernel without CONFIG_FRAME_POINTER.

<Before>
 ./perf probe -fn sched_slice:12 lw.weight
  Fatal: DW_OP 156 is not supported.
              (^^^ DW_OP_call_frame_cfa)

<After>
./perf probe -fn sched_slice:12 lw.weight
Add new event:
  probe:sched_slice    (on sched_slice:12 with weight=lw.weight)

Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
---

 tools/perf/util/probe-finder.c |   14 +++++++++++---
 tools/perf/util/probe-finder.h |    1 +
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/probe-finder.c b/tools/perf/util/probe-finder.c
index 11d6592..26f02b9 100644
--- a/tools/perf/util/probe-finder.c
+++ b/tools/perf/util/probe-finder.c
@@ -396,7 +396,6 @@ static void convert_location(Dwarf_Op *op, struct probe_finder *pf)
 	const char *regs;
 	struct kprobe_trace_arg *tvar = pf->tvar;
 
-	/* TODO: support CFA */
 	/* If this is based on frame buffer, set the offset */
 	if (op->atom == DW_OP_fbreg) {
 		if (pf->fb_ops == NULL)
@@ -623,11 +622,17 @@ static void convert_probe_point(Dwarf_Die *sp_die, struct probe_finder *pf)
 	/* Get the frame base attribute/ops */
 	dwarf_attr(sp_die, DW_AT_frame_base, &fb_attr);
 	ret = dwarf_getlocation_addr(&fb_attr, pf->addr, &pf->fb_ops, &nops, 1);
-	if (ret <= 0 || nops == 0)
+	if (ret <= 0 || nops == 0) {
 		pf->fb_ops = NULL;
+	} else if (nops == 1 && pf->fb_ops[0].atom == DW_OP_call_frame_cfa &&
+		   pf->cfi != NULL) {
+		Dwarf_Frame *frame;
+		ret = dwarf_cfi_addrframe(pf->cfi, pf->addr, &frame);
+		DIE_IF(ret != 0);
+		dwarf_frame_cfa(frame, &pf->fb_ops, &nops);
+	}
 
 	/* Find each argument */
-	/* TODO: use dwarf_cfi_addrframe */
 	tev->nargs = pf->pev->nargs;
 	tev->args = xzalloc(sizeof(struct kprobe_trace_arg) * tev->nargs);
 	for (i = 0; i < pf->pev->nargs; i++) {
@@ -836,6 +841,9 @@ int find_kprobe_trace_events(int fd, struct perf_probe_event *pev,
 	if (!dbg)
 		return -ENOENT;
 
+	/* Get the call frame information from this dwarf */
+	pf.cfi = dwarf_getcfi(dbg);
+
 	off = 0;
 	line_list__init(&pf.lcache);
 	/* Loop on CUs (Compilation Unit) */
diff --git a/tools/perf/util/probe-finder.h b/tools/perf/util/probe-finder.h
index 3564f22..ddaa2a7 100644
--- a/tools/perf/util/probe-finder.h
+++ b/tools/perf/util/probe-finder.h
@@ -42,6 +42,7 @@ struct probe_finder {
 	struct list_head	lcache;		/* Line cache for lazy match */
 
 	/* For variable searching */
+	Dwarf_CFI		*cfi;		/* Call Frame Information */
 	Dwarf_Op		*fb_ops;	/* Frame base attribute */
 	struct perf_probe_arg	*pvar;		/* Current target variable */
 	struct kprobe_trace_arg	*tvar;		/* Current result variable */


-- 
Masami Hiramatsu
e-mail: mhiramat@redhat.com

Re: [PATCH -tip 5/7] perf probe: Query basic types from debuginfo

[Arnaldo Carvalho de Melo]

Em Mon, Mar 29, 2010 at 04:38:29PM -0400, Masami Hiramatsu escreveu:
> +static void convert_variable_type(Dwarf_Die *vr_die,
> +				  struct kprobe_trace_arg *targ)
> +{
> +	Dwarf_Die type;
> +	char buf[16];
> +	int ret;
> +
> +	if (die_get_real_type(vr_die, &type) == NULL)
> +		die("Failed to get a type information of %s.",
> +		    dwarf_diename(vr_die));
> +
> +	ret = die_get_byte_size(&type) * 8;
> +	if (ret) {
> +		/* Check the bitwidth */
> +		if (ret > MAX_BASIC_TYPE_BITS) {
> +			pr_warning("  Warning: %s exceeds max-bitwidth."
> +				   " Cut down to %d bits.\n",
> +				   dwarf_diename(&type), MAX_BASIC_TYPE_BITS);
> +			ret = MAX_BASIC_TYPE_BITS;
> +		}
> +
> +		ret = snprintf(buf, 16, "%c%d",
> +			       die_is_signed_type(&type) ? 's' : 'u', ret);
> +		if (ret < 0 || ret >= 16)
> +			die("Failed to convert variable type.");
> +		targ->type = xstrdup(buf);

Question, should we use the equivalent to panic'ing the kernel in the
userspace bits in tools/?

I tend to see all code there as potentially part of a library, i.e. to
be callable by some unantecipated new tool or library that would rather
receive a return value telling it that the operation can't be performed
for some reason so that it can inform the user instead of having the
whole tool exit to the command line.

It may well be that some specific operation needs lots of resources but
many other don't, panic'c because the one that requires lots of
resources can't be performed, bringing down a gui/tui is really nasty.

Perhaps the answer will be the same as for when people added panic calls
in the kernel in the past, "you're screwed up anyway if that happens",
but it just feels wrong :-\

In the DWARF bits it has even the added twist namespace collapse with
DIE 8-)

- Arnaldo

Re: [PATCH -tip 5/7] perf probe: Query basic types from debuginfo

[Masami Hiramatsu]

Arnaldo Carvalho de Melo wrote:
> Em Mon, Mar 29, 2010 at 04:38:29PM -0400, Masami Hiramatsu escreveu:
>> +static void convert_variable_type(Dwarf_Die *vr_die,
>> +				  struct kprobe_trace_arg *targ)
>> +{
>> +	Dwarf_Die type;
>> +	char buf[16];
>> +	int ret;
>> +
>> +	if (die_get_real_type(vr_die, &type) == NULL)
>> +		die("Failed to get a type information of %s.",
>> +		    dwarf_diename(vr_die));
>> +
>> +	ret = die_get_byte_size(&type) * 8;
>> +	if (ret) {
>> +		/* Check the bitwidth */
>> +		if (ret > MAX_BASIC_TYPE_BITS) {
>> +			pr_warning("  Warning: %s exceeds max-bitwidth."
>> +				   " Cut down to %d bits.\n",
>> +				   dwarf_diename(&type), MAX_BASIC_TYPE_BITS);
>> +			ret = MAX_BASIC_TYPE_BITS;
>> +		}
>> +
>> +		ret = snprintf(buf, 16, "%c%d",
>> +			       die_is_signed_type(&type) ? 's' : 'u', ret);
>> +		if (ret < 0 || ret >= 16)
>> +			die("Failed to convert variable type.");
>> +		targ->type = xstrdup(buf);
> 
> Question, should we use the equivalent to panic'ing the kernel in the
> userspace bits in tools/?
> 
> I tend to see all code there as potentially part of a library, i.e. to
> be callable by some unantecipated new tool or library that would rather
> receive a return value telling it that the operation can't be performed
> for some reason so that it can inform the user instead of having the
> whole tool exit to the command line.

OK, so that you want to see 

ret = library_func();
if (ret < 0)	/* Something wrong happened */
	return ret;

instead of 

library_func();
hopefully_executed_next_func();



> It may well be that some specific operation needs lots of resources but
> many other don't, panic'c because the one that requires lots of
> resources can't be performed, bringing down a gui/tui is really nasty.

Hmm, agreed. But I'd like to have some API for storing dying^H^H^H^H^Herror
message. (maybe can we use setjump/longjump approach - like try/cache - for die()?)

Thank you,

> 
> Perhaps the answer will be the same as for when people added panic calls
> in the kernel in the past, "you're screwed up anyway if that happens",
> but it just feels wrong :-\
> 
> In the DWARF bits it has even the added twist namespace collapse with
> DIE 8-)
> 
> - Arnaldo
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
Masami Hiramatsu
e-mail: mhiramat@redhat.com

Re: [PATCH -tip 5/7] perf probe: Query basic types from debuginfo

[Arnaldo Carvalho de Melo]

Em Mon, Mar 29, 2010 at 05:22:00PM -0400, Masami Hiramatsu escreveu:
> Arnaldo Carvalho de Melo wrote:
> > Em Mon, Mar 29, 2010 at 04:38:29PM -0400, Masami Hiramatsu escreveu:
> > Question, should we use the equivalent to panic'ing the kernel in the
> > userspace bits in tools/?

> > I tend to see all code there as potentially part of a library, i.e. to
> > be callable by some unantecipated new tool or library that would rather
> > receive a return value telling it that the operation can't be performed
> > for some reason so that it can inform the user instead of having the
> > whole tool exit to the command line.
> 
> OK, so that you want to see 
> 
> ret = library_func();
> if (ret < 0)	/* Something wrong happened */
> 	return ret;
> 
> instead of 
> 
> library_func();
> hopefully_executed_next_func();

Exactly. That was my _suggestion_ :-)
 
> > It may well be that some specific operation needs lots of resources but
> > many other don't, panic'c because the one that requires lots of
> > resources can't be performed, bringing down a gui/tui is really nasty.
> 
> Hmm, agreed. But I'd like to have some API for storing dying^H^H^H^H^Herror
> message. (maybe can we use setjump/longjump approach - like try/cache - for die()?)

Can't we do just as we do in the kernel and propagate the error back to
callers? All the way to userspace, that in this case would be the user
instead of an app started by a user?  :-)

Wrt api for storing messages, we have pr_{warning,error,debug}, etc, in
the TUI mode it is even redirected to the bottom line and I plan to have
them get into something browseable if the user wants to see the last pr_
messages.

IOW, just act like you're writing kernel code, that will make it more
likely that people that are used to writing code like that will feel at
ease while hacking tools/.

- Arnaldo

Re: [PATCH -tip 5/7] perf probe: Query basic types from debuginfo

[Masami Hiramatsu]

Arnaldo Carvalho de Melo wrote:
> Em Mon, Mar 29, 2010 at 05:22:00PM -0400, Masami Hiramatsu escreveu:
>> Arnaldo Carvalho de Melo wrote:
>>> Em Mon, Mar 29, 2010 at 04:38:29PM -0400, Masami Hiramatsu escreveu:
>>> Question, should we use the equivalent to panic'ing the kernel in the
>>> userspace bits in tools/?
> 
>>> I tend to see all code there as potentially part of a library, i.e. to
>>> be callable by some unantecipated new tool or library that would rather
>>> receive a return value telling it that the operation can't be performed
>>> for some reason so that it can inform the user instead of having the
>>> whole tool exit to the command line.
>>
>> OK, so that you want to see 
>>
>> ret = library_func();
>> if (ret < 0)	/* Something wrong happened */
>> 	return ret;
>>
>> instead of 
>>
>> library_func();
>> hopefully_executed_next_func();
> 
> Exactly. That was my _suggestion_ :-)
>  
>>> It may well be that some specific operation needs lots of resources but
>>> many other don't, panic'c because the one that requires lots of
>>> resources can't be performed, bringing down a gui/tui is really nasty.
>>
>> Hmm, agreed. But I'd like to have some API for storing dying^H^H^H^H^Herror
>> message. (maybe can we use setjump/longjump approach - like try/cache - for die()?)
> 
> Can't we do just as we do in the kernel and propagate the error back to
> callers? All the way to userspace, that in this case would be the user
> instead of an app started by a user?  :-)
> 
> Wrt api for storing messages, we have pr_{warning,error,debug}, etc, in
> the TUI mode it is even redirected to the bottom line and I plan to have
> them get into something browseable if the user wants to see the last pr_
> messages.

Ah, that's very nice:)
OK, I'll try to use it and remove all 'die()'s.

Thank you!

> 
> IOW, just act like you're writing kernel code, that will make it more
> likely that people that are used to writing code like that will feel at
> ease while hacking tools/.
> 
> - Arnaldo
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

-- 
Masami Hiramatsu
e-mail: mhiramat@redhat.com

Re: [PATCH -tip 5/7] perf probe: Query basic types from debuginfo

[Arnaldo Carvalho de Melo]

Em Mon, Mar 29, 2010 at 06:05:03PM -0400, Masami Hiramatsu escreveu:
> Arnaldo Carvalho de Melo wrote:
> > Can't we do just as we do in the kernel and propagate the error back to
> > callers? All the way to userspace, that in this case would be the user
> > instead of an app started by a user?  :-)
> > 
> > Wrt api for storing messages, we have pr_{warning,error,debug}, etc, in
> > the TUI mode it is even redirected to the bottom line and I plan to have
> > them get into something browseable if the user wants to see the last pr_
> > messages.
> 
> Ah, that's very nice:)
> OK, I'll try to use it and remove all 'die()'s.

Thanks a lot!

- Arnaldo