Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[PATCH 12/17] tracing/probes: Implement 'memory' fetch method for uprobes

2 views
Skip to first unread message

Namhyung Kim

unread,
Dec 15, 2013, 11:40:02 PM12/15/13
to
From: Namhyung Kim <namhyu...@lge.com>

Use separate method to fetch from memory. Move existing functions to
trace_kprobe.c and make them static. Also add new memory fetch
implementation for uprobes.

Acked-by: Masami Hiramatsu <masami.hi...@hitachi.com>
Cc: Srikar Dronamraju <sri...@linux.vnet.ibm.com>
Cc: Oleg Nesterov <ol...@redhat.com>
Cc: zhangwei(Jovi) <jovi.z...@huawei.com>
Cc: Arnaldo Carvalho de Melo <ac...@ghostprotocols.net>
Signed-off-by: Namhyung Kim <namh...@kernel.org>
---
kernel/trace/trace_kprobe.c | 77 +++++++++++++++++++++++++++++++++++++++++++++
kernel/trace/trace_probe.c | 77 ---------------------------------------------
kernel/trace/trace_probe.h | 4 ---
kernel/trace/trace_uprobe.c | 52 ++++++++++++++++++++++++++++++
4 files changed, 129 insertions(+), 81 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 1c38f3f47d0d..d1d3d8982345 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -148,6 +148,83 @@ DEFINE_BASIC_FETCH_FUNCS(stack)
#define fetch_stack_string NULL
#define fetch_stack_string_size NULL

+#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)
+/*
+ * Fetch a null-terminated string. Caller MUST set *(u32 *)dest with max
+ * length and relative data location.
+ */
+static __kprobes void FETCH_FUNC_NAME(memory, string)(struct pt_regs *regs,
+ void *addr, void *dest)
+{
+ long ret;
+ int maxlen = get_rloc_len(*(u32 *)dest);
+ u8 *dst = get_rloc_data(dest);
+ u8 *src = addr;
+ mm_segment_t old_fs = get_fs();
+
+ if (!maxlen)
+ return;
+
+ /*
+ * Try to get string again, since the string can be changed while
+ * probing.
+ */
+ set_fs(KERNEL_DS);
+ pagefault_disable();
+
+ do
+ ret = __copy_from_user_inatomic(dst++, src++, 1);
+ while (dst[-1] && ret == 0 && src - (u8 *)addr < maxlen);
+
+ dst[-1] = '\0';
+ pagefault_enable();
+ set_fs(old_fs);
+
+ if (ret < 0) { /* Failed to fetch string */
+ ((u8 *)get_rloc_data(dest))[0] = '\0';
+ *(u32 *)dest = make_data_rloc(0, get_rloc_offs(*(u32 *)dest));
+ } else {
+ *(u32 *)dest = make_data_rloc(src - (u8 *)addr,
+ get_rloc_offs(*(u32 *)dest));
+ }
+}
+
+/* Return the length of string -- including null terminal byte */
+static __kprobes void FETCH_FUNC_NAME(memory, string_size)(struct pt_regs *regs,
+ void *addr, void *dest)
+{
+ mm_segment_t old_fs;
+ int ret, len = 0;
+ u8 c;
+
+ old_fs = get_fs();
+ set_fs(KERNEL_DS);
+ pagefault_disable();
+
+ do {
+ ret = __copy_from_user_inatomic(&c, (u8 *)addr + len, 1);
+ len++;
+ } while (c && ret == 0 && len < MAX_STRING_SIZE);
+
+ pagefault_enable();
+ set_fs(old_fs);
+
+ if (ret < 0) /* Failed to check the length */
+ *(u32 *)dest = 0;
+ else
+ *(u32 *)dest = len;
+}
+
#define DEFINE_FETCH_symbol(type) \
__kprobes void FETCH_FUNC_NAME(symbol, type)(struct pt_regs *regs, \
void *data, void *dest) \
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 8d7231d436da..8f7a2b6d389d 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -103,83 +103,6 @@ DEFINE_BASIC_FETCH_FUNCS(retval)
#define fetch_retval_string NULL
#define fetch_retval_string_size NULL

-#define DEFINE_FETCH_memory(type) \
-__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)
-/*
- * Fetch a null-terminated string. Caller MUST set *(u32 *)dest with max
- * length and relative data location.
- */
-__kprobes void FETCH_FUNC_NAME(memory, string)(struct pt_regs *regs,
- void *addr, void *dest)
-{
- long ret;
- int maxlen = get_rloc_len(*(u32 *)dest);
- u8 *dst = get_rloc_data(dest);
- u8 *src = addr;
- mm_segment_t old_fs = get_fs();
-
- if (!maxlen)
- return;
-
- /*
- * Try to get string again, since the string can be changed while
- * probing.
- */
- set_fs(KERNEL_DS);
- pagefault_disable();
-
- do
- ret = __copy_from_user_inatomic(dst++, src++, 1);
- while (dst[-1] && ret == 0 && src - (u8 *)addr < maxlen);
-
- dst[-1] = '\0';
- pagefault_enable();
- set_fs(old_fs);
-
- if (ret < 0) { /* Failed to fetch string */
- ((u8 *)get_rloc_data(dest))[0] = '\0';
- *(u32 *)dest = make_data_rloc(0, get_rloc_offs(*(u32 *)dest));
- } else {
- *(u32 *)dest = make_data_rloc(src - (u8 *)addr,
- get_rloc_offs(*(u32 *)dest));
- }
-}
-
-/* Return the length of string -- including null terminal byte */
-__kprobes void FETCH_FUNC_NAME(memory, string_size)(struct pt_regs *regs,
- void *addr, void *dest)
-{
- mm_segment_t old_fs;
- int ret, len = 0;
- u8 c;
-
- old_fs = get_fs();
- set_fs(KERNEL_DS);
- pagefault_disable();
-
- do {
- ret = __copy_from_user_inatomic(&c, (u8 *)addr + len, 1);
- len++;
- } while (c && ret == 0 && len < MAX_STRING_SIZE);
-
- pagefault_enable();
- set_fs(old_fs);
-
- if (ret < 0) /* Failed to check the length */
- *(u32 *)dest = 0;
- else
- *(u32 *)dest = len;
-}
-
/* Dereference memory access function */
struct deref_fetch_param {
struct fetch_param orig;
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 8be84550ceb3..2d5b8f5f5310 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -171,10 +171,6 @@ DECLARE_BASIC_FETCH_FUNCS(retval);
#define fetch_retval_string NULL
#define fetch_retval_string_size NULL

-DECLARE_BASIC_FETCH_FUNCS(memory);
-DECLARE_FETCH_FUNC(memory, string);
-DECLARE_FETCH_FUNC(memory, string_size);
-
DECLARE_BASIC_FETCH_FUNCS(symbol);
DECLARE_FETCH_FUNC(symbol, string);
DECLARE_FETCH_FUNC(symbol, string_size);
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index adc9ac70fd3c..7eb1b3244c94 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -114,6 +114,58 @@ DEFINE_BASIC_FETCH_FUNCS(stack)
#define fetch_stack_string NULL
#define fetch_stack_string_size NULL

+#define DEFINE_FETCH_memory(type) \
+static __kprobes void FETCH_FUNC_NAME(memory, type)(struct pt_regs *regs,\
+ void *addr, void *dest) \
+{ \
+ type retval; \
+ void __user *vaddr = (void __force __user *) addr; \
+ \
+ if (copy_from_user(&retval, vaddr, sizeof(type))) \
+ *(type *)dest = 0; \
+ else \
+ *(type *) dest = retval; \
+}
+DEFINE_BASIC_FETCH_FUNCS(memory)
+/*
+ * Fetch a null-terminated string. Caller MUST set *(u32 *)dest with max
+ * length and relative data location.
+ */
+static __kprobes void FETCH_FUNC_NAME(memory, string)(struct pt_regs *regs,
+ void *addr, void *dest)
+{
+ long ret;
+ u32 rloc = *(u32 *)dest;
+ int maxlen = get_rloc_len(rloc);
+ u8 *dst = get_rloc_data(dest);
+ void __user *src = (void __force __user *) addr;
+
+ if (!maxlen)
+ return;
+
+ ret = strncpy_from_user(dst, src, maxlen);
+
+ if (ret < 0) { /* Failed to fetch string */
+ ((u8 *)get_rloc_data(dest))[0] = '\0';
+ *(u32 *)dest = make_data_rloc(0, get_rloc_offs(rloc));
+ } else {
+ *(u32 *)dest = make_data_rloc(ret, get_rloc_offs(rloc));
+ }
+}
+
+static __kprobes void FETCH_FUNC_NAME(memory, string_size)(struct pt_regs *regs,
+ void *addr, void *dest)
+{
+ int len;
+ void __user *vaddr = (void __force __user *) addr;
+
+ len = strnlen_user(vaddr, MAX_STRING_SIZE);
+
+ if (len == 0 || len > MAX_STRING_SIZE) /* Failed to check length */
+ *(u32 *)dest = 0;
+ else
+ *(u32 *)dest = len;
+}

/* uprobes don't support symbol fetch methods */
#define fetch_symbol_u8 NULL
--
1.7.11.7

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

Namhyung Kim

unread,
Dec 15, 2013, 11:40:02 PM12/15/13
to
From: Namhyung Kim <namhyu...@lge.com>

Move existing functions to trace_kprobe.c and add NULL entries to the
uprobes fetch type table. I don't make them static since some generic
routines like update/free_XXX_fetch_param() require pointers to the
functions.

Cc: Masami Hiramatsu <masami.hi...@hitachi.com>
Cc: Srikar Dronamraju <sri...@linux.vnet.ibm.com>
Cc: Oleg Nesterov <ol...@redhat.com>
Cc: zhangwei(Jovi) <jovi.z...@huawei.com>
Cc: Arnaldo Carvalho de Melo <ac...@ghostprotocols.net>
Signed-off-by: Namhyung Kim <namh...@kernel.org>
---
kernel/trace/trace_kprobe.c | 59 +++++++++++++++++++++++++++++++++++++++++++++
kernel/trace/trace_probe.c | 59 ---------------------------------------------
kernel/trace/trace_probe.h | 24 ++++++++++++++++++
kernel/trace/trace_uprobe.c | 8 ++++++
4 files changed, 91 insertions(+), 59 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 11478020f76c..1c38f3f47d0d 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -88,6 +88,51 @@ static int kprobe_dispatcher(struct kprobe *kp, struct pt_regs *regs);
static int kretprobe_dispatcher(struct kretprobe_instance *ri,
struct pt_regs *regs);

+/* Memory fetching by symbol */
+struct symbol_cache {
+ char *symbol;
+ long offset;
+ unsigned long addr;
+};
+
+unsigned long update_symbol_cache(struct symbol_cache *sc)
+{
+ sc->addr = (unsigned long)kallsyms_lookup_name(sc->symbol);
+
+ if (sc->addr)
+ sc->addr += sc->offset;
+
+ return sc->addr;
+}
+
+void free_symbol_cache(struct symbol_cache *sc)
+{
+ kfree(sc->symbol);
+ kfree(sc);
+}
+
+struct symbol_cache *alloc_symbol_cache(const char *sym, long offset)
+{
+ struct symbol_cache *sc;
+
+ if (!sym || strlen(sym) == 0)
+ return NULL;
+
+ sc = kzalloc(sizeof(struct symbol_cache), GFP_KERNEL);
+ if (!sc)
+ return NULL;
+
+ sc->symbol = kstrdup(sym, GFP_KERNEL);
+ if (!sc->symbol) {
+ kfree(sc);
+ return NULL;
+ }
+ sc->offset = offset;
+ update_symbol_cache(sc);
+
+ return sc;
+}
+
/*
* Kprobes-specific fetch functions
*/
@@ -103,6 +148,20 @@ DEFINE_BASIC_FETCH_FUNCS(stack)
#define fetch_stack_string NULL
#define fetch_stack_string_size NULL

+#define DEFINE_FETCH_symbol(type) \
+__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)
+DEFINE_FETCH_symbol(string)
+DEFINE_FETCH_symbol(string_size)
+
/* Fetch type information table */
const struct fetch_type kprobes_fetch_type_table[] = {
/* Special types */
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 77aa7d18821e..a31ad478b7f6 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -180,65 +180,6 @@ __kprobes void FETCH_FUNC_NAME(memory, string_size)(struct pt_regs *regs,
*(u32 *)dest = len;
}

-/* Memory fetching by symbol */
-struct symbol_cache {
- char *symbol;
- long offset;
- unsigned long addr;
-};
-
-static unsigned long update_symbol_cache(struct symbol_cache *sc)
-{
- sc->addr = (unsigned long)kallsyms_lookup_name(sc->symbol);
-
- if (sc->addr)
- sc->addr += sc->offset;
-
- return sc->addr;
-}
-
-static void free_symbol_cache(struct symbol_cache *sc)
-{
- kfree(sc->symbol);
- kfree(sc);
-}
-
-static struct symbol_cache *alloc_symbol_cache(const char *sym, long offset)
-{
- struct symbol_cache *sc;
-
- if (!sym || strlen(sym) == 0)
- return NULL;
-
- sc = kzalloc(sizeof(struct symbol_cache), GFP_KERNEL);
- if (!sc)
- return NULL;
-
- sc->symbol = kstrdup(sym, GFP_KERNEL);
- if (!sc->symbol) {
- kfree(sc);
- return NULL;
- }
- sc->offset = offset;
- update_symbol_cache(sc);
-
- return sc;
-}
-
-#define DEFINE_FETCH_symbol(type) \
-__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)
-DEFINE_FETCH_symbol(string)
-DEFINE_FETCH_symbol(string_size)
-
/* Dereference memory access function */
struct deref_fetch_param {
struct fetch_param orig;
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 8211dd674ab6..8be84550ceb3 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -239,6 +239,30 @@ ASSIGN_FETCH_FUNC(bitfield, ftype), \
extern __weak const struct fetch_type kprobes_fetch_type_table[];
extern __weak const struct fetch_type uprobes_fetch_type_table[];

+#ifdef CONFIG_KPROBE_EVENT
+struct symbol_cache;
+unsigned long update_symbol_cache(struct symbol_cache *sc);
+void free_symbol_cache(struct symbol_cache *sc);
+struct symbol_cache *alloc_symbol_cache(const char *sym, long offset);
+#else
+struct symbol_cache {
+};
+static inline unsigned long __used update_symbol_cache(struct symbol_cache *sc)
+{
+ return 0;
+}
+
+static inline void __used free_symbol_cache(struct symbol_cache *sc)
+{
+}
+
+static inline struct symbol_cache * __used
+alloc_symbol_cache(const char *sym, long offset)
+{
+ return NULL;
+}
+#endif /* CONFIG_KPROBE_EVENT */
+
struct probe_arg {
struct fetch_param fetch;
struct fetch_param fetch_size;
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 5395d37e5e72..adc9ac70fd3c 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -115,6 +115,14 @@ DEFINE_BASIC_FETCH_FUNCS(stack)
#define fetch_stack_string_size NULL


+/* uprobes don't support symbol fetch methods */
+#define fetch_symbol_u8 NULL
+#define fetch_symbol_u16 NULL
+#define fetch_symbol_u32 NULL
+#define fetch_symbol_u64 NULL
+#define fetch_symbol_string NULL
+#define fetch_symbol_string_size NULL
+
/* Fetch type information table */
const struct fetch_type uprobes_fetch_type_table[] = {
/* Special types */

Namhyung Kim

unread,
Dec 15, 2013, 11:40:02 PM12/15/13
to
From: Namhyung Kim <namhyu...@lge.com>

Enable to fetch data from a file offset. Currently it only supports
fetching from same binary uprobe set. It'll translate the file offset
to a proper virtual address in the process.

The syntax is "@+OFFSET" as it does similar to normal memory fetching
(@ADDR) which does no address translation.

Suggested-by: Oleg Nesterov <ol...@redhat.com>
Acked-by: Masami Hiramatsu <masami.hi...@hitachi.com>
Cc: Srikar Dronamraju <sri...@linux.vnet.ibm.com>
Cc: Oleg Nesterov <ol...@redhat.com>
Cc: zhangwei(Jovi) <jovi.z...@huawei.com>
Cc: Arnaldo Carvalho de Melo <ac...@ghostprotocols.net>
Signed-off-by: Namhyung Kim <namh...@kernel.org>
---
Documentation/trace/uprobetracer.txt | 1 +
kernel/trace/trace_kprobe.c | 8 ++++++++
kernel/trace/trace_probe.c | 13 +++++++++++-
kernel/trace/trace_probe.h | 2 ++
kernel/trace/trace_uprobe.c | 40 ++++++++++++++++++++++++++++++++++++
5 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/Documentation/trace/uprobetracer.txt b/Documentation/trace/uprobetracer.txt
index 6e5cff263e2b..f1cf9a34ad9d 100644
--- a/Documentation/trace/uprobetracer.txt
+++ b/Documentation/trace/uprobetracer.txt
@@ -32,6 +32,7 @@ Synopsis of uprobe_tracer
FETCHARGS : Arguments. Each probe can have up to 128 args.
%REG : Fetch register REG
@ADDR : Fetch memory at ADDR (ADDR should be in userspace)
+ @+OFFSET : Fetch memory at OFFSET (OFFSET from same file as PATH)
$stackN : Fetch Nth entry of stack (N >= 0)
$stack : Fetch stack address.
$retval : Fetch return value.(*)
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index d1d3d8982345..cc6bb8531f2f 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -239,6 +239,14 @@ DEFINE_BASIC_FETCH_FUNCS(symbol)
DEFINE_FETCH_symbol(string)
DEFINE_FETCH_symbol(string_size)

+/* kprobes don't support file_offset fetch methods */
+#define fetch_file_offset_u8 NULL
+#define fetch_file_offset_u16 NULL
+#define fetch_file_offset_u32 NULL
+#define fetch_file_offset_u64 NULL
+#define fetch_file_offset_string NULL
+#define fetch_file_offset_string_size NULL
+
/* Fetch type information table */
const struct fetch_type kprobes_fetch_type_table[] = {
/* Special types */
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index a130d612e705..8364a421b4df 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -374,7 +374,7 @@ static int parse_probe_arg(char *arg, const struct fetch_type *t,
}
break;

- case '@': /* memory or symbol */
+ case '@': /* memory, file-offset or symbol */
if (isdigit(arg[1])) {
ret = kstrtoul(arg + 1, 0, &param);
if (ret)
@@ -382,6 +382,17 @@ static int parse_probe_arg(char *arg, const struct fetch_type *t,

f->fn = t->fetch[FETCH_MTD_memory];
f->data = (void *)param;
+ } else if (arg[1] == '+') {
+ /* kprobes don't support file offsets */
+ if (is_kprobe)
+ return -EINVAL;
+
+ ret = kstrtol(arg + 2, 0, &offset);
+ if (ret)
+ break;
+
+ f->fn = t->fetch[FETCH_MTD_file_offset];
+ f->data = (void *)offset;
} else {
/* uprobes don't support symbols */
if (!is_kprobe)
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 2d5b8f5f5310..e29d743fef5d 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -106,6 +106,7 @@ enum {
FETCH_MTD_symbol,
FETCH_MTD_deref,
FETCH_MTD_bitfield,
+ FETCH_MTD_file_offset,
FETCH_MTD_END,
};

@@ -217,6 +218,7 @@ ASSIGN_FETCH_FUNC(memory, ftype), \
ASSIGN_FETCH_FUNC(symbol, ftype), \
ASSIGN_FETCH_FUNC(deref, ftype), \
ASSIGN_FETCH_FUNC(bitfield, ftype), \
+ASSIGN_FETCH_FUNC(file_offset, ftype), \
} \
}

diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 6ed319be5a84..0f7681c7b757 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -70,6 +70,11 @@ static int unregister_uprobe_event(struct trace_uprobe *tu);
static DEFINE_MUTEX(uprobe_lock);
static LIST_HEAD(uprobe_list);

+struct uprobe_dispatch_data {
+ struct trace_uprobe *tu;
+ unsigned long bp_addr;
+};
+
static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs *regs);
static int uretprobe_dispatcher(struct uprobe_consumer *con,
unsigned long func, struct pt_regs *regs);
@@ -175,6 +180,29 @@ static __kprobes void FETCH_FUNC_NAME(memory, string_size)(struct pt_regs *regs,
#define fetch_symbol_string NULL
#define fetch_symbol_string_size NULL

+static unsigned long translate_user_vaddr(void *file_offset)
+{
+ unsigned long base_addr;
+ struct uprobe_dispatch_data *udd;
+
+ udd = (void *) current->utask->vaddr;
+
+ base_addr = udd->bp_addr - udd->tu->offset;
+ return base_addr + (unsigned long)file_offset;
+}
+
+#define DEFINE_FETCH_file_offset(type) \
+static __kprobes void FETCH_FUNC_NAME(file_offset, type)(struct pt_regs *regs,\
+ void *offset, void *dest) \
+{ \
+ void *vaddr = (void *)translate_user_vaddr(offset); \
+ \
+ FETCH_FUNC_NAME(memory, type)(regs, vaddr, dest); \
+}
+DEFINE_BASIC_FETCH_FUNCS(file_offset)
+DEFINE_FETCH_file_offset(string)
+DEFINE_FETCH_file_offset(string_size)
+
/* Fetch type information table */
const struct fetch_type uprobes_fetch_type_table[] = {
/* Special types */
@@ -1106,11 +1134,17 @@ int trace_uprobe_register(struct ftrace_event_call *event, enum trace_reg type,
static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs *regs)
{
struct trace_uprobe *tu;
+ struct uprobe_dispatch_data udd;
int ret = 0;

tu = container_of(con, struct trace_uprobe, consumer);
tu->nhit++;

+ udd.tu = tu;
+ udd.bp_addr = instruction_pointer(regs);
+
+ current->utask->vaddr = (unsigned long) &udd;
+
if (tu->tp.flags & TP_FLAG_TRACE)
ret |= uprobe_trace_func(tu, regs);

@@ -1125,9 +1159,15 @@ static int uretprobe_dispatcher(struct uprobe_consumer *con,
unsigned long func, struct pt_regs *regs)
{
struct trace_uprobe *tu;
+ struct uprobe_dispatch_data udd;

tu = container_of(con, struct trace_uprobe, consumer);

+ udd.tu = tu;
+ udd.bp_addr = func;
+
+ current->utask->vaddr = (unsigned long) &udd;
+
if (tu->tp.flags & TP_FLAG_TRACE)
uretprobe_trace_func(tu, func, regs);

Namhyung Kim

unread,
Dec 15, 2013, 11:40:02 PM12/15/13
to
From: Namhyung Kim <namhyu...@lge.com>

Use separate method to fetch from stack. Move existing functions to
trace_kprobe.c and make them static. Also add new stack fetch
implementation for uprobes.

Cc: Masami Hiramatsu <masami.hi...@hitachi.com>
Cc: Srikar Dronamraju <sri...@linux.vnet.ibm.com>
Cc: Oleg Nesterov <ol...@redhat.com>
Cc: zhangwei(Jovi) <jovi.z...@huawei.com>
Cc: Arnaldo Carvalho de Melo <ac...@ghostprotocols.net>
Signed-off-by: Namhyung Kim <namh...@kernel.org>
---
kernel/trace/trace_kprobe.c | 15 +++++++++++++++
kernel/trace/trace_probe.c | 22 ----------------------
kernel/trace/trace_probe.h | 14 ++++++++++----
kernel/trace/trace_uprobe.c | 41 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 66 insertions(+), 26 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 23c6c3feff82..11478020f76c 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -88,6 +88,21 @@ static int kprobe_dispatcher(struct kprobe *kp, struct pt_regs *regs);
static int kretprobe_dispatcher(struct kretprobe_instance *ri,
struct pt_regs *regs);

+/*
+ * Kprobes-specific fetch functions
+ */
+#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)
+/* No string on the stack entry */
+#define fetch_stack_string NULL
+#define fetch_stack_string_size NULL
+
/* Fetch type information table */
const struct fetch_type kprobes_fetch_type_table[] = {
/* Special types */
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 541036ec7392..77aa7d18821e 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -70,16 +70,6 @@ __kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq *s,

const char PRINT_TYPE_FMT_NAME(string)[] = "\\\"%s\\\"";

-/*
- * 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(method) \
-DEFINE_FETCH_##method(u8) \
-DEFINE_FETCH_##method(u16) \
-DEFINE_FETCH_##method(u32) \
-DEFINE_FETCH_##method(u64)
-
#define CHECK_FETCH_FUNCS(method, fn) \
(((FETCH_FUNC_NAME(method, u8) == fn) || \
(FETCH_FUNC_NAME(method, u16) == fn) || \
@@ -102,18 +92,6 @@ DEFINE_BASIC_FETCH_FUNCS(reg)
#define fetch_reg_string NULL
#define fetch_reg_string_size NULL

-#define DEFINE_FETCH_stack(type) \
-__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)
-/* No string on the stack entry */
-#define fetch_stack_string NULL
-#define fetch_stack_string_size NULL
-
#define DEFINE_FETCH_retval(type) \
__kprobes void FETCH_FUNC_NAME(retval, type)(struct pt_regs *regs, \
void *dummy, void *dest) \
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 5b77798d1130..8211dd674ab6 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -167,10 +167,6 @@ DECLARE_BASIC_FETCH_FUNCS(reg);
#define fetch_reg_string NULL
#define fetch_reg_string_size NULL

-DECLARE_BASIC_FETCH_FUNCS(stack);
-#define fetch_stack_string NULL
-#define fetch_stack_string_size NULL
-
DECLARE_BASIC_FETCH_FUNCS(retval);
#define fetch_retval_string NULL
#define fetch_retval_string_size NULL
@@ -191,6 +187,16 @@ DECLARE_BASIC_FETCH_FUNCS(bitfield);
#define fetch_bitfield_string NULL
#define fetch_bitfield_string_size NULL

+/*
+ * 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(method) \
+DEFINE_FETCH_##method(u8) \
+DEFINE_FETCH_##method(u16) \
+DEFINE_FETCH_##method(u32) \
+DEFINE_FETCH_##method(u64)
+
/* Default (unsigned long) fetch type */
#define __DEFAULT_FETCH_TYPE(t) u##t
#define _DEFAULT_FETCH_TYPE(t) __DEFAULT_FETCH_TYPE(t)
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 2c60925ea073..5395d37e5e72 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -74,6 +74,47 @@ static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs *regs);
static int uretprobe_dispatcher(struct uprobe_consumer *con,
unsigned long func, struct pt_regs *regs);

+#ifdef CONFIG_STACK_GROWSUP
+static unsigned long adjust_stack_addr(unsigned long addr, unsigned int n)
+{
+ return addr - (n * sizeof(long));
+}
+#else
+static unsigned long adjust_stack_addr(unsigned long addr, unsigned int n)
+{
+ return addr + (n * sizeof(long));
+}
+#endif
+
+static unsigned long get_user_stack_nth(struct pt_regs *regs, unsigned int n)
+{
+ unsigned long ret;
+ unsigned long addr = user_stack_pointer(regs);
+
+ addr = adjust_stack_addr(addr, n);
+
+ if (copy_from_user(&ret, (void __force __user *) addr, sizeof(ret)))
+ return 0;
+
+ return ret;
+}
+
+/*
+ * Uprobes-specific fetch functions
+ */
+#define DEFINE_FETCH_stack(type) \
+static __kprobes void FETCH_FUNC_NAME(stack, type)(struct pt_regs *regs,\
+ void *offset, void *dest) \
+{ \
+ *(type *)dest = (type)get_user_stack_nth(regs, \
+ ((unsigned long)offset)); \
+}
+DEFINE_BASIC_FETCH_FUNCS(stack)
+/* No string on the stack entry */
+#define fetch_stack_string NULL
+#define fetch_stack_string_size NULL
+
+
/* Fetch type information table */
const struct fetch_type uprobes_fetch_type_table[] = {
/* Special types */

Namhyung Kim

unread,
Dec 15, 2013, 11:40:02 PM12/15/13
to
From: Namhyung Kim <namhyu...@lge.com>

Use separate fetch_type_table for kprobes and uprobes. It currently
shares all fetch methods but some of them will be implemented
differently later.

This is not to break build if [ku]probes is configured alone (like
!CONFIG_KPROBE_EVENT and CONFIG_UPROBE_EVENT). So I added '__weak'
to the table declaration so that it can be safely omitted when it
configured out.

Acked-by: Masami Hiramatsu <masami.hi...@hitachi.com>
Cc: Srikar Dronamraju <sri...@linux.vnet.ibm.com>
Cc: Oleg Nesterov <ol...@redhat.com>
Cc: zhangwei(Jovi) <jovi.z...@huawei.com>
Cc: Arnaldo Carvalho de Melo <ac...@ghostprotocols.net>
Signed-off-by: Namhyung Kim <namh...@kernel.org>
---
kernel/trace/trace_kprobe.c | 20 ++++++++++++++
kernel/trace/trace_probe.c | 65 ++++++++++++++++++---------------------------
kernel/trace/trace_probe.h | 53 ++++++++++++++++++++++++++++++++++++
kernel/trace/trace_uprobe.c | 20 ++++++++++++++
4 files changed, 119 insertions(+), 39 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 62d6c961bbce..23c6c3feff82 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -88,6 +88,26 @@ static int kprobe_dispatcher(struct kprobe *kp, struct pt_regs *regs);
static int kretprobe_dispatcher(struct kretprobe_instance *ri,
struct pt_regs *regs);

+/* Fetch type information table */
+const struct fetch_type kprobes_fetch_type_table[] = {
+ /* Special types */
+ [FETCH_TYPE_STRING] = __ASSIGN_FETCH_TYPE("string", string, string,
+ sizeof(u32), 1, "__data_loc char[]"),
+ [FETCH_TYPE_STRSIZE] = __ASSIGN_FETCH_TYPE("string_size", u32,
+ string_size, sizeof(u32), 0, "u32"),
+ /* Basic types */
+ 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),
+
+ ASSIGN_FETCH_TYPE_END
+};
+
/*
* Allocate new trace_probe and initialize it (including kprobes).
*/
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index c26bc9eaa2ac..541036ec7392 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -54,10 +54,6 @@ DEFINE_BASIC_PRINT_TYPE_FUNC(s16, "%d")
DEFINE_BASIC_PRINT_TYPE_FUNC(s32, "%d")
DEFINE_BASIC_PRINT_TYPE_FUNC(s64, "%Ld")

-/* For defining macros, define string/string_size types */
-typedef u32 string;
-typedef u32 string_size;
-
/* Print type function for string type */
__kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq *s,
const char *name,
@@ -74,7 +70,6 @@ __kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq *s,

const char PRINT_TYPE_FMT_NAME(string)[] = "\\\"%s\\\"";

-#define FETCH_FUNC_NAME(method, type) fetch_##method##_##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.
@@ -359,25 +354,8 @@ free_bitfield_fetch_param(struct bitfield_fetch_param *data)
kfree(data);
}

-/* Fetch type information table */
-static const struct fetch_type fetch_type_table[] = {
- /* Special types */
- [FETCH_TYPE_STRING] = __ASSIGN_FETCH_TYPE("string", string, string,
- sizeof(u32), 1, "__data_loc char[]"),
- [FETCH_TYPE_STRSIZE] = __ASSIGN_FETCH_TYPE("string_size", u32,
- string_size, sizeof(u32), 0, "u32"),
- /* Basic types */
- 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)
+static const struct fetch_type *find_fetch_type(const char *type,
+ const struct fetch_type *ftbl)
{
int i;

@@ -398,21 +376,22 @@ static const struct fetch_type *find_fetch_type(const char *type)

switch (bs) {
case 8:
- return find_fetch_type("u8");
+ return find_fetch_type("u8", ftbl);
case 16:
- return find_fetch_type("u16");
+ return find_fetch_type("u16", ftbl);
case 32:
- return find_fetch_type("u32");
+ return find_fetch_type("u32", ftbl);
case 64:
- return find_fetch_type("u64");
+ return find_fetch_type("u64", ftbl);
default:
goto fail;
}
}

- for (i = 0; i < ARRAY_SIZE(fetch_type_table); i++)
- if (strcmp(type, fetch_type_table[i].name) == 0)
- return &fetch_type_table[i];
+ for (i = 0; ftbl[i].name; i++) {
+ if (strcmp(type, ftbl[i].name) == 0)
+ return &ftbl[i];
+ }

fail:
return NULL;
@@ -426,16 +405,17 @@ static __kprobes void fetch_stack_address(struct pt_regs *regs,
}

static fetch_func_t get_fetch_size_function(const struct fetch_type *type,
- fetch_func_t orig_fn)
+ fetch_func_t orig_fn,
+ const struct fetch_type *ftbl)
{
int i;

- if (type != &fetch_type_table[FETCH_TYPE_STRING])
+ if (type != &ftbl[FETCH_TYPE_STRING])
return NULL; /* Only string type needs size function */

for (i = 0; i < FETCH_MTD_END; i++)
if (type->fetch[i] == orig_fn)
- return fetch_type_table[FETCH_TYPE_STRSIZE].fetch[i];
+ return ftbl[FETCH_TYPE_STRSIZE].fetch[i];

WARN_ON(1); /* This should not happen */

@@ -504,12 +484,14 @@ static int parse_probe_vars(char *arg, const struct fetch_type *t,
static int parse_probe_arg(char *arg, const struct fetch_type *t,
struct fetch_param *f, bool is_return, bool is_kprobe)
{
+ const struct fetch_type *ftbl;
unsigned long param;
long offset;
char *tmp;
- int ret;
+ int ret = 0;

- ret = 0;
+ ftbl = is_kprobe ? kprobes_fetch_type_table : uprobes_fetch_type_table;
+ BUG_ON(ftbl == NULL);

/* Until uprobe_events supports only reg arguments */
if (!is_kprobe && arg[0] != '%')
@@ -568,7 +550,7 @@ static int parse_probe_arg(char *arg, const struct fetch_type *t,
struct deref_fetch_param *dprm;
const struct fetch_type *t2;

- t2 = find_fetch_type(NULL);
+ t2 = find_fetch_type(NULL, ftbl);
*tmp = '\0';
dprm = kzalloc(sizeof(struct deref_fetch_param), GFP_KERNEL);

@@ -637,9 +619,13 @@ static int __parse_bitfield_probe_arg(const char *bf,
int traceprobe_parse_probe_arg(char *arg, ssize_t *size,
struct probe_arg *parg, bool is_return, bool is_kprobe)
{
+ const struct fetch_type *ftbl;
const char *t;
int ret;

+ ftbl = is_kprobe ? kprobes_fetch_type_table : uprobes_fetch_type_table;
+ BUG_ON(ftbl == NULL);
+
if (strlen(arg) > MAX_ARGSTR_LEN) {
pr_info("Argument is too long.: %s\n", arg);
return -ENOSPC;
@@ -654,7 +640,7 @@ int traceprobe_parse_probe_arg(char *arg, ssize_t *size,
arg[t - parg->comm] = '\0';
t++;
}
- parg->type = find_fetch_type(t);
+ parg->type = find_fetch_type(t, ftbl);
if (!parg->type) {
pr_info("Unsupported type: %s\n", t);
return -EINVAL;
@@ -668,7 +654,8 @@ int traceprobe_parse_probe_arg(char *arg, ssize_t *size,

if (ret >= 0) {
parg->fetch_size.fn = get_fetch_size_function(parg->type,
- parg->fetch.fn);
+ parg->fetch.fn,
+ ftbl);
parg->fetch_size.data = parg->fetch.data;
}

diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index bd621c08b6c6..5b77798d1130 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -126,6 +126,10 @@ struct fetch_param {
void *data;
};

+/* For defining macros, define string/string_size types */
+typedef u32 string;
+typedef u32 string_size;
+
#define PRINT_TYPE_FUNC_NAME(type) print_type_##type
#define PRINT_TYPE_FMT_NAME(type) print_type_format_##type

@@ -146,6 +150,47 @@ DECLARE_BASIC_PRINT_TYPE_FUNC(s32);
DECLARE_BASIC_PRINT_TYPE_FUNC(s64);
DECLARE_BASIC_PRINT_TYPE_FUNC(string);

+#define FETCH_FUNC_NAME(method, type) fetch_##method##_##type
+
+/* Declare macro for basic types */
+#define DECLARE_FETCH_FUNC(method, type) \
+extern void FETCH_FUNC_NAME(method, type)(struct pt_regs *regs, \
+ void *data, void *dest)
+
+#define DECLARE_BASIC_FETCH_FUNCS(method) \
+DECLARE_FETCH_FUNC(method, u8); \
+DECLARE_FETCH_FUNC(method, u16); \
+DECLARE_FETCH_FUNC(method, u32); \
+DECLARE_FETCH_FUNC(method, u64)
+
+DECLARE_BASIC_FETCH_FUNCS(reg);
+#define fetch_reg_string NULL
+#define fetch_reg_string_size NULL
+
+DECLARE_BASIC_FETCH_FUNCS(stack);
+#define fetch_stack_string NULL
+#define fetch_stack_string_size NULL
+
+DECLARE_BASIC_FETCH_FUNCS(retval);
+#define fetch_retval_string NULL
+#define fetch_retval_string_size NULL
+
+DECLARE_BASIC_FETCH_FUNCS(memory);
+DECLARE_FETCH_FUNC(memory, string);
+DECLARE_FETCH_FUNC(memory, string_size);
+
+DECLARE_BASIC_FETCH_FUNCS(symbol);
+DECLARE_FETCH_FUNC(symbol, string);
+DECLARE_FETCH_FUNC(symbol, string_size);
+
+DECLARE_BASIC_FETCH_FUNCS(deref);
+DECLARE_FETCH_FUNC(deref, string);
+DECLARE_FETCH_FUNC(deref, string_size);
+
+DECLARE_BASIC_FETCH_FUNCS(bitfield);
+#define fetch_bitfield_string NULL
+#define fetch_bitfield_string_size NULL
+
/* Default (unsigned long) fetch type */
#define __DEFAULT_FETCH_TYPE(t) u##t
#define _DEFAULT_FETCH_TYPE(t) __DEFAULT_FETCH_TYPE(t)
@@ -176,9 +221,17 @@ ASSIGN_FETCH_FUNC(bitfield, ftype), \
#define ASSIGN_FETCH_TYPE(ptype, ftype, sign) \
__ASSIGN_FETCH_TYPE(#ptype, ptype, ftype, sizeof(ftype), sign, #ptype)

+#define ASSIGN_FETCH_TYPE_END {}
+
#define FETCH_TYPE_STRING 0
#define FETCH_TYPE_STRSIZE 1

+/*
+ * Fetch type information table.
+ * It's declared as a weak symbol due to conditional compilation.
+ */
+extern __weak const struct fetch_type kprobes_fetch_type_table[];
+extern __weak const struct fetch_type uprobes_fetch_type_table[];

struct probe_arg {
struct fetch_param fetch;
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index b233d9cb1216..2c60925ea073 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -74,6 +74,26 @@ static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs *regs);
static int uretprobe_dispatcher(struct uprobe_consumer *con,
unsigned long func, struct pt_regs *regs);

+/* Fetch type information table */
+const struct fetch_type uprobes_fetch_type_table[] = {
+ /* Special types */
+ [FETCH_TYPE_STRING] = __ASSIGN_FETCH_TYPE("string", string, string,
+ sizeof(u32), 1, "__data_loc char[]"),
+ [FETCH_TYPE_STRSIZE] = __ASSIGN_FETCH_TYPE("string_size", u32,
+ string_size, sizeof(u32), 0, "u32"),
+ /* Basic types */
+ 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),
+
+ ASSIGN_FETCH_TYPE_END
+};
+
static inline void init_trace_uprobe_filter(struct trace_uprobe_filter *filter)
{
rwlock_init(&filter->rwlock);

Namhyung Kim

unread,
Dec 15, 2013, 11:40:02 PM12/15/13
to
Hello,

This patchset implements memory (address), stack[N], deference,
bitfield, retval (it needs uretprobe tho) and file_offset fetch
methods for uprobes. It's based on the previous work [1] done by
Hyeoncheol Lee.

Now kprobes and uprobes have their own fetch_type_tables and, in turn,
memory and stack access methods. The symbol and file_offset fetch
methods are only available to kprobes and uprobes, respectively.
Other fetch methods are shared.

For the file_offset method, it translates the offset argument to a
virtual address in a process. To do that, it calculates base mapping
address using probe address (utask->vaddr) and probe offset
(tu->offset) and then adds the argument offset. Those info are
carried via utask and a new fetch parameter.

The syntax is '@+offset' where offset are relative address to the base
address. For shared libraries, it'd be simply the st_value of symbol
in ELF format. But for executable, it needs to subtract base load
address (e.g. 0x40000 for x86_64) from the symbol value. Please see
previous discussion for an example [2] - Note that the syntax changed
to '@+' from plain '@'. The plain '@addr' syntax is used for
accessing absolute memory address if you already know the exact address.

Many thanks to Oleg who provides valuable feedbacks and suggestions.

The patch 1-2 are bug fixes and can be applied independently.
The patch 16 is a preparation for patch 17 which implements the
file_offset fetch method.


* v9 changes:
- [ku]probes_fetch_type_table have NULL terminator (Masami)
- make symbol fetch methods static inline for !CONFIG_KPROBE_EVENT (Masami)
- add more Ack's from Masami

* v8 changes:
- rename tk, tu and tp more consistently (Srikar)
- change prefix format specifier: %#x -> 0x%x (Masami)
- convert file_offset_param to uprobe_dispatch_data (Oleg)
- add more Ack's from Srikar and Masami

* v7 changes:
- restructure patches not to break build with !CONFIG_[KU]PROBE_EVENT
- print 0x prefix for unsigned types
- add @+file_offset fetch method (Oleg)
- get rid of uprobe_buffer_mutex (Oleg)
- pass 'is_return' to uprobes argument parser


[1] https://lkml.org/lkml/2012/11/14/84
[2] https://lkml.org/lkml/2013/11/5/25

A simple example:

# cat foo.c
int glob = -1;
char str[] = "hello uprobe.";

struct foo {
unsigned int unused: 2;
unsigned int foo: 20;
unsigned int bar: 10;
} foo = {
.foo = 5,
};

int main(int argc, char *argv[])
{
long local = 0x1234;

return 127;
}

# gcc -o foo -g foo.c

# objdump -d foo | grep -A9 -F '<main>'
00000000004004b0 <main>:
4004b0: 55 push %rbp
4004b1: 48 89 e5 mov %rsp,%rbp
4004b4: 89 7d ec mov %edi,-0x14(%rbp)
4004b7: 48 89 75 e0 mov %rsi,-0x20(%rbp)
4004bb: 48 c7 45 f8 34 12 00 movq $0x1234,-0x8(%rbp)
4004c2: 00
4004c3: b8 7f 00 00 00 mov $0x7f,%eax
4004c8: 5d pop %rbp
4004c9: c3 retq

# nm foo | grep -e glob$ -e str -e foo
00000000006008bc D foo
00000000006008a8 D glob
00000000006008ac D str

# perf probe -x /home/namhyung/tmp/foo -a 'foo=main+0x13 glob=@0x6008a8:s32 \
> str=@+0x2008ac:string bit=@+0x2008bc:b10@2/32 argc=%di:s32 local=-0x8(%bp)'
Added new event:
probe_foo:foo (on 0x4c3 with glob=@0x6008a8:s32 str=@+0x2008ac:string
bit=@+0x2008bc:b10@2/32 argc=%di:s32 local=-0x8(%bp))

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

perf record -e probe_foo:foo -aR sleep 1

# perf record -e probe_foo:foo ./foo
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.001 MB perf.data (~33 samples) ]

# perf script | grep -v ^#
foo 2008 [002 2199.867154: probe_foo:foo (4004c3)
glob=-1 str="hello uprobe." bit=0x5 argc=1 local=0x1234


This patchset is based on the current for-next branch of the Steven
Rostedt's linux-trace tree. I also put this on my 'uprobe/fetch-v9'
branch in my tree:

git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git


Any comments are welcome, thanks.
Namhyung


Cc: Masami Hiramatsu <masami.hi...@hitachi.com>
Cc: Srikar Dronamraju <sri...@linux.vnet.ibm.com>
Cc: Oleg Nesterov <ol...@redhat.com>
Cc: zhangwei(Jovi) <jovi.z...@huawei.com>
Cc: Arnaldo Carvalho de Melo <ac...@ghostprotocols.net>
Cc: Hemant Kumar <hks...@linux.vnet.ibm.com>


Hyeoncheol Lee (1):
tracing/probes: Add fetch{,_size} member into deref fetch method

Namhyung Kim (15):
tracing/uprobes: Fix documentation of uprobe registration syntax
tracing/probes: Fix basic print type functions
tracing/kprobes: Factor out struct trace_probe
tracing/uprobes: Convert to struct trace_probe
tracing/kprobes: Move common functions to trace_probe.h
tracing/probes: Integrate duplicate set_print_fmt()
tracing/probes: Move fetch function helpers to trace_probe.h
tracing/probes: Split [ku]probes_fetch_type_table
tracing/probes: Implement 'stack' fetch method for uprobes
tracing/probes: Move 'symbol' fetch method to kprobes
tracing/probes: Implement 'memory' fetch method for uprobes
tracing/uprobes: Pass 'is_return' to traceprobe_parse_probe_arg()
tracing/uprobes: Fetch args before reserving a ring buffer
tracing/uprobes: Add support for full argument access methods
tracing/uprobes: Add @+file_offset fetch method

Oleg Nesterov (1):
uprobes: Allocate ->utask before handler_chain() for tracing handlers

Documentation/trace/uprobetracer.txt | 36 +-
kernel/events/uprobes.c | 4 +
kernel/trace/trace_kprobe.c | 812 +++++++++++++++++++----------------
kernel/trace/trace_probe.c | 440 +++++++------------
kernel/trace/trace_probe.h | 216 ++++++++++
kernel/trace/trace_uprobe.c | 495 +++++++++++++++------
6 files changed, 1208 insertions(+), 795 deletions(-)

Namhyung Kim

unread,
Dec 15, 2013, 11:40:02 PM12/15/13
to
From: Hyeoncheol Lee <cheo...@lge.com>

The deref fetch methods access a memory region but it assumes that
it's a kernel memory since uprobes does not support them.

Add ->fetch and ->fetch_size member in order to provide a proper
access methods for supporting uprobes.

Acked-by: Masami Hiramatsu <masami.hi...@hitachi.com>
Cc: Srikar Dronamraju <sri...@linux.vnet.ibm.com>
Cc: Oleg Nesterov <ol...@redhat.com>
Cc: zhangwei(Jovi) <jovi.z...@huawei.com>
Cc: Arnaldo Carvalho de Melo <ac...@ghostprotocols.net>
Signed-off-by: Hyeoncheol Lee <cheo...@lge.com>
[namh...@kernel.org: Split original patch into pieces as requested]
Signed-off-by: Namhyung Kim <namh...@kernel.org>
---
kernel/trace/trace_probe.c | 22 ++++++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index a31ad478b7f6..8d7231d436da 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -184,6 +184,8 @@ __kprobes void FETCH_FUNC_NAME(memory, string_size)(struct pt_regs *regs,
struct deref_fetch_param {
struct fetch_param orig;
long offset;
+ fetch_func_t fetch;
+ fetch_func_t fetch_size;
};

#define DEFINE_FETCH_deref(type) \
@@ -195,13 +197,26 @@ __kprobes void FETCH_FUNC_NAME(deref, type)(struct pt_regs *regs, \
call_fetch(&dprm->orig, regs, &addr); \
if (addr) { \
addr += dprm->offset; \
- fetch_memory_##type(regs, (void *)addr, dest); \
+ dprm->fetch(regs, (void *)addr, dest); \
} else \
*(type *)dest = 0; \
}
DEFINE_BASIC_FETCH_FUNCS(deref)
DEFINE_FETCH_deref(string)
-DEFINE_FETCH_deref(string_size)
+
+__kprobes void FETCH_FUNC_NAME(deref, string_size)(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 && dprm->fetch_size) {
+ addr += dprm->offset;
+ dprm->fetch_size(regs, (void *)addr, dest);
+ } else
+ *(string_size *)dest = 0;
+}

static __kprobes void update_deref_fetch_param(struct deref_fetch_param *data)
{
@@ -477,6 +492,9 @@ static int parse_probe_arg(char *arg, const struct fetch_type *t,
return -ENOMEM;

dprm->offset = offset;
+ dprm->fetch = t->fetch[FETCH_MTD_memory];
+ dprm->fetch_size = get_fetch_size_function(t,
+ dprm->fetch, ftbl);
ret = parse_probe_arg(arg, t2, &dprm->orig, is_return,
is_kprobe);
if (ret)

Namhyung Kim

unread,
Dec 15, 2013, 11:40:02 PM12/15/13
to
From: Namhyung Kim <namhyu...@lge.com>

The uprobe syntax requires an offset after a file path not a symbol.

Reviewed-by: Masami Hiramatsu <masami.hi...@hitachi.com>
Acked-by: Oleg Nesterov <ol...@redhat.com>
Acked-by: Srikar Dronamraju <sri...@linux.vnet.ibm.com>
Cc: zhangwei(Jovi) <jovi.z...@huawei.com>
Cc: Arnaldo Carvalho de Melo <ac...@ghostprotocols.net>
Signed-off-by: Namhyung Kim <namh...@kernel.org>
---
Documentation/trace/uprobetracer.txt | 10 +++++-----
kernel/trace/trace_uprobe.c | 2 +-
2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/Documentation/trace/uprobetracer.txt b/Documentation/trace/uprobetracer.txt
index d9c3e682312c..8f1a8b8956fc 100644
--- a/Documentation/trace/uprobetracer.txt
+++ b/Documentation/trace/uprobetracer.txt
@@ -19,15 +19,15 @@ user to calculate the offset of the probepoint in the object.

Synopsis of uprobe_tracer
-------------------------
- p[:[GRP/]EVENT] PATH:SYMBOL[+offs] [FETCHARGS] : Set a uprobe
- r[:[GRP/]EVENT] PATH:SYMBOL[+offs] [FETCHARGS] : Set a return uprobe (uretprobe)
- -:[GRP/]EVENT : Clear uprobe or uretprobe event
+ p[:[GRP/]EVENT] PATH:OFFSET [FETCHARGS] : Set a uprobe
+ r[:[GRP/]EVENT] PATH:OFFSET [FETCHARGS] : Set a return uprobe (uretprobe)
+ -:[GRP/]EVENT : Clear uprobe or uretprobe event

GRP : Group name. If omitted, "uprobes" is the default value.
EVENT : Event name. If omitted, the event name is generated based
- on SYMBOL+offs.
+ on PATH+OFFSET.
PATH : Path to an executable or a library.
- SYMBOL[+offs] : Symbol+offset where the probe is inserted.
+ OFFSET : Offset where the probe is inserted.

FETCHARGS : Arguments. Each probe can have up to 128 args.
%REG : Fetch register REG
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index b6dcc42ef7f5..c77b92d61551 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -211,7 +211,7 @@ end:

/*
* Argument syntax:
- * - Add uprobe: p|r[:[GRP/]EVENT] PATH:SYMBOL [FETCHARGS]
+ * - Add uprobe: p|r[:[GRP/]EVENT] PATH:OFFSET [FETCHARGS]
*
* - Remove uprobe: -:[GRP/]EVENT
*/

Namhyung Kim

unread,
Dec 15, 2013, 11:40:02 PM12/15/13
to
From: Namhyung Kim <namhyu...@lge.com>

Convert struct trace_uprobe to make use of the common trace_probe
structure.

Reviewed-by: Masami Hiramatsu <masami.hi...@hitachi.com>
Acked-by: Srikar Dronamraju <sri...@linux.vnet.ibm.com>
Cc: Oleg Nesterov <ol...@redhat.com>
Cc: zhangwei(Jovi) <jovi.z...@huawei.com>
Cc: Arnaldo Carvalho de Melo <ac...@ghostprotocols.net>
Signed-off-by: Namhyung Kim <namh...@kernel.org>
---
kernel/trace/trace_uprobe.c | 159 ++++++++++++++++++++++----------------------
1 file changed, 79 insertions(+), 80 deletions(-)

diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index c77b92d61551..afda3726f288 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -51,22 +51,17 @@ struct trace_uprobe_filter {
*/
struct trace_uprobe {
struct list_head list;
- struct ftrace_event_class class;
- struct ftrace_event_call call;
struct trace_uprobe_filter filter;
struct uprobe_consumer consumer;
struct inode *inode;
char *filename;
unsigned long offset;
unsigned long nhit;
- unsigned int flags; /* For TP_FLAG_* */
- ssize_t size; /* trace entry size */
- unsigned int nr_args;
- struct probe_arg args[];
+ struct trace_probe tp;
};

-#define SIZEOF_TRACE_UPROBE(n) \
- (offsetof(struct trace_uprobe, args) + \
+#define SIZEOF_TRACE_UPROBE(n) \
+ (offsetof(struct trace_uprobe, tp.args) + \
(sizeof(struct probe_arg) * (n)))

static int register_uprobe_event(struct trace_uprobe *tu);
@@ -114,13 +109,13 @@ alloc_trace_uprobe(const char *group, const char *event, int nargs, bool is_ret)
if (!tu)
return ERR_PTR(-ENOMEM);

- tu->call.class = &tu->class;
- tu->call.name = kstrdup(event, GFP_KERNEL);
- if (!tu->call.name)
+ tu->tp.call.class = &tu->tp.class;
+ tu->tp.call.name = kstrdup(event, GFP_KERNEL);
+ if (!tu->tp.call.name)
goto error;

- tu->class.system = kstrdup(group, GFP_KERNEL);
- if (!tu->class.system)
+ tu->tp.class.system = kstrdup(group, GFP_KERNEL);
+ if (!tu->tp.class.system)
goto error;

INIT_LIST_HEAD(&tu->list);
@@ -128,11 +123,11 @@ alloc_trace_uprobe(const char *group, const char *event, int nargs, bool is_ret)
if (is_ret)
tu->consumer.ret_handler = uretprobe_dispatcher;
init_trace_uprobe_filter(&tu->filter);
- tu->call.flags |= TRACE_EVENT_FL_USE_CALL_FILTER;
+ tu->tp.call.flags |= TRACE_EVENT_FL_USE_CALL_FILTER;
return tu;

error:
- kfree(tu->call.name);
+ kfree(tu->tp.call.name);
kfree(tu);

return ERR_PTR(-ENOMEM);
@@ -142,12 +137,12 @@ static void free_trace_uprobe(struct trace_uprobe *tu)
{
int i;

- for (i = 0; i < tu->nr_args; i++)
- traceprobe_free_probe_arg(&tu->args[i]);
+ for (i = 0; i < tu->tp.nr_args; i++)
+ traceprobe_free_probe_arg(&tu->tp.args[i]);

iput(tu->inode);
- kfree(tu->call.class->system);
- kfree(tu->call.name);
+ kfree(tu->tp.call.class->system);
+ kfree(tu->tp.call.name);
kfree(tu->filename);
kfree(tu);
}
@@ -157,8 +152,8 @@ static struct trace_uprobe *find_probe_event(const char *event, const char *grou
struct trace_uprobe *tu;

list_for_each_entry(tu, &uprobe_list, list)
- if (strcmp(tu->call.name, event) == 0 &&
- strcmp(tu->call.class->system, group) == 0)
+ if (strcmp(tu->tp.call.name, event) == 0 &&
+ strcmp(tu->tp.call.class->system, group) == 0)
return tu;

return NULL;
@@ -181,16 +176,16 @@ static int unregister_trace_uprobe(struct trace_uprobe *tu)
/* Register a trace_uprobe and probe_event */
static int register_trace_uprobe(struct trace_uprobe *tu)
{
- struct trace_uprobe *old_tp;
+ struct trace_uprobe *old_tu;
int ret;

mutex_lock(&uprobe_lock);

/* register as an event */
- old_tp = find_probe_event(tu->call.name, tu->call.class->system);
- if (old_tp) {
+ old_tu = find_probe_event(tu->tp.call.name, tu->tp.call.class->system);
+ if (old_tu) {
/* delete old event */
- ret = unregister_trace_uprobe(old_tp);
+ ret = unregister_trace_uprobe(old_tu);
if (ret)
goto end;
}
@@ -360,34 +355,36 @@ static int create_trace_uprobe(int argc, char **argv)
/* parse arguments */
ret = 0;
for (i = 0; i < argc && i < MAX_TRACE_ARGS; i++) {
+ struct probe_arg *parg = &tu->tp.args[i];
+
/* Increment count for freeing args in error case */
- tu->nr_args++;
+ tu->tp.nr_args++;

/* Parse argument name */
arg = strchr(argv[i], '=');
if (arg) {
*arg++ = '\0';
- tu->args[i].name = kstrdup(argv[i], GFP_KERNEL);
+ parg->name = kstrdup(argv[i], GFP_KERNEL);
} else {
arg = argv[i];
/* If argument name is omitted, set "argN" */
snprintf(buf, MAX_EVENT_NAME_LEN, "arg%d", i + 1);
- tu->args[i].name = kstrdup(buf, GFP_KERNEL);
+ parg->name = kstrdup(buf, GFP_KERNEL);
}

- if (!tu->args[i].name) {
+ if (!parg->name) {
pr_info("Failed to allocate argument[%d] name.\n", i);
ret = -ENOMEM;
goto error;
}

- if (!is_good_name(tu->args[i].name)) {
- pr_info("Invalid argument[%d] name: %s\n", i, tu->args[i].name);
+ if (!is_good_name(parg->name)) {
+ pr_info("Invalid argument[%d] name: %s\n", i, parg->name);
ret = -EINVAL;
goto error;
}

- if (traceprobe_conflict_field_name(tu->args[i].name, tu->args, i)) {
+ if (traceprobe_conflict_field_name(parg->name, tu->tp.args, i)) {
pr_info("Argument[%d] name '%s' conflicts with "
"another field.\n", i, argv[i]);
ret = -EINVAL;
@@ -395,7 +392,8 @@ static int create_trace_uprobe(int argc, char **argv)
}

/* Parse fetch argument */
- ret = traceprobe_parse_probe_arg(arg, &tu->size, &tu->args[i], false, false);
+ ret = traceprobe_parse_probe_arg(arg, &tu->tp.size, parg,
+ false, false);
if (ret) {
pr_info("Parse error at argument[%d]. (%d)\n", i, ret);
goto error;
@@ -459,11 +457,11 @@ static int probes_seq_show(struct seq_file *m, void *v)
char c = is_ret_probe(tu) ? 'r' : 'p';
int i;

- seq_printf(m, "%c:%s/%s", c, tu->call.class->system, tu->call.name);
+ seq_printf(m, "%c:%s/%s", c, tu->tp.call.class->system, tu->tp.call.name);
seq_printf(m, " %s:0x%p", tu->filename, (void *)tu->offset);

- for (i = 0; i < tu->nr_args; i++)
- seq_printf(m, " %s=%s", tu->args[i].name, tu->args[i].comm);
+ for (i = 0; i < tu->tp.nr_args; i++)
+ seq_printf(m, " %s=%s", tu->tp.args[i].name, tu->tp.args[i].comm);

seq_printf(m, "\n");
return 0;
@@ -509,7 +507,7 @@ static int probes_profile_seq_show(struct seq_file *m, void *v)
{
struct trace_uprobe *tu = v;

- seq_printf(m, " %s %-44s %15lu\n", tu->filename, tu->call.name, tu->nhit);
+ seq_printf(m, " %s %-44s %15lu\n", tu->filename, tu->tp.call.name, tu->nhit);
return 0;
}

@@ -541,11 +539,11 @@ static void uprobe_trace_print(struct trace_uprobe *tu,
struct ring_buffer *buffer;
void *data;
int size, i;
- struct ftrace_event_call *call = &tu->call;
+ struct ftrace_event_call *call = &tu->tp.call;

size = SIZEOF_TRACE_ENTRY(is_ret_probe(tu));
event = trace_current_buffer_lock_reserve(&buffer, call->event.type,
- size + tu->size, 0, 0);
+ size + tu->tp.size, 0, 0);
if (!event)
return;

@@ -559,8 +557,10 @@ static void uprobe_trace_print(struct trace_uprobe *tu,
data = DATAOF_TRACE_ENTRY(entry, false);
}

- for (i = 0; i < tu->nr_args; i++)
- call_fetch(&tu->args[i].fetch, regs, data + tu->args[i].offset);
+ for (i = 0; i < tu->tp.nr_args; i++) {
+ call_fetch(&tu->tp.args[i].fetch, regs,
+ data + tu->tp.args[i].offset);
+ }

if (!call_filter_check_discard(call, entry, buffer, event))
trace_buffer_unlock_commit(buffer, event, 0, 0);
@@ -591,23 +591,24 @@ print_uprobe_event(struct trace_iterator *iter, int flags, struct trace_event *e
int i;

entry = (struct uprobe_trace_entry_head *)iter->ent;
- tu = container_of(event, struct trace_uprobe, call.event);
+ tu = container_of(event, struct trace_uprobe, tp.call.event);

if (is_ret_probe(tu)) {
- if (!trace_seq_printf(s, "%s: (0x%lx <- 0x%lx)", tu->call.name,
+ if (!trace_seq_printf(s, "%s: (0x%lx <- 0x%lx)", tu->tp.call.name,
entry->vaddr[1], entry->vaddr[0]))
goto partial;
data = DATAOF_TRACE_ENTRY(entry, true);
} else {
- if (!trace_seq_printf(s, "%s: (0x%lx)", tu->call.name,
+ if (!trace_seq_printf(s, "%s: (0x%lx)", tu->tp.call.name,
entry->vaddr[0]))
goto partial;
data = DATAOF_TRACE_ENTRY(entry, false);
}

- for (i = 0; i < tu->nr_args; i++) {
- if (!tu->args[i].type->print(s, tu->args[i].name,
- data + tu->args[i].offset, entry))
+ for (i = 0; i < tu->tp.nr_args; i++) {
+ struct probe_arg *parg = &tu->tp.args[i];
+
+ if (!parg->type->print(s, parg->name, data + parg->offset, entry))
goto partial;
}

@@ -618,11 +619,6 @@ partial:
return TRACE_TYPE_PARTIAL_LINE;
}

-static inline bool is_trace_uprobe_enabled(struct trace_uprobe *tu)
-{
- return tu->flags & (TP_FLAG_TRACE | TP_FLAG_PROFILE);
-}
-
typedef bool (*filter_func_t)(struct uprobe_consumer *self,
enum uprobe_filter_ctx ctx,
struct mm_struct *mm);
@@ -632,29 +628,29 @@ probe_event_enable(struct trace_uprobe *tu, int flag, filter_func_t filter)
{
int ret = 0;

- if (is_trace_uprobe_enabled(tu))
+ if (trace_probe_is_enabled(&tu->tp))
return -EINTR;

WARN_ON(!uprobe_filter_is_empty(&tu->filter));

- tu->flags |= flag;
+ tu->tp.flags |= flag;
tu->consumer.filter = filter;
ret = uprobe_register(tu->inode, tu->offset, &tu->consumer);
if (ret)
- tu->flags &= ~flag;
+ tu->tp.flags &= ~flag;

return ret;
}

static void probe_event_disable(struct trace_uprobe *tu, int flag)
{
- if (!is_trace_uprobe_enabled(tu))
+ if (!trace_probe_is_enabled(&tu->tp))
return;

WARN_ON(!uprobe_filter_is_empty(&tu->filter));

uprobe_unregister(tu->inode, tu->offset, &tu->consumer);
- tu->flags &= ~flag;
+ tu->tp.flags &= ~flag;
}

static int uprobe_event_define_fields(struct ftrace_event_call *event_call)
@@ -672,12 +668,12 @@ static int uprobe_event_define_fields(struct ftrace_event_call *event_call)
size = SIZEOF_TRACE_ENTRY(false);
}
/* Set argument names as fields */
- for (i = 0; i < tu->nr_args; i++) {
- ret = trace_define_field(event_call, tu->args[i].type->fmttype,
- tu->args[i].name,
- size + tu->args[i].offset,
- tu->args[i].type->size,
- tu->args[i].type->is_signed,
+ for (i = 0; i < tu->tp.nr_args; i++) {
+ struct probe_arg *parg = &tu->tp.args[i];
+
+ ret = trace_define_field(event_call, parg->type->fmttype,
+ parg->name, size + parg->offset,
+ parg->type->size, parg->type->is_signed,
FILTER_OTHER);

if (ret)
@@ -705,16 +701,16 @@ static int __set_print_fmt(struct trace_uprobe *tu, char *buf, int len)

pos += snprintf(buf + pos, LEN_OR_ZERO, "\"%s", fmt);

- for (i = 0; i < tu->nr_args; i++) {
+ for (i = 0; i < tu->tp.nr_args; i++) {
pos += snprintf(buf + pos, LEN_OR_ZERO, " %s=%s",
- tu->args[i].name, tu->args[i].type->fmt);
+ tu->tp.args[i].name, tu->tp.args[i].type->fmt);
}

pos += snprintf(buf + pos, LEN_OR_ZERO, "\", %s", arg);

- for (i = 0; i < tu->nr_args; i++) {
+ for (i = 0; i < tu->tp.nr_args; i++) {
pos += snprintf(buf + pos, LEN_OR_ZERO, ", REC->%s",
- tu->args[i].name);
+ tu->tp.args[i].name);
}

return pos; /* return the length of print_fmt */
@@ -734,7 +730,7 @@ static int set_print_fmt(struct trace_uprobe *tu)

/* Second: actually write the @print_fmt */
__set_print_fmt(tu, print_fmt, len + 1);
- tu->call.print_fmt = print_fmt;
+ tu->tp.call.print_fmt = print_fmt;

return 0;
}
@@ -831,14 +827,14 @@ static bool uprobe_perf_filter(struct uprobe_consumer *uc,
static void uprobe_perf_print(struct trace_uprobe *tu,
unsigned long func, struct pt_regs *regs)
{
- struct ftrace_event_call *call = &tu->call;
+ struct ftrace_event_call *call = &tu->tp.call;
struct uprobe_trace_entry_head *entry;
struct hlist_head *head;
void *data;
int size, rctx, i;

size = SIZEOF_TRACE_ENTRY(is_ret_probe(tu));
- size = ALIGN(size + tu->size + sizeof(u32), sizeof(u64)) - sizeof(u32);
+ size = ALIGN(size + tu->tp.size + sizeof(u32), sizeof(u64)) - sizeof(u32);

preempt_disable();
head = this_cpu_ptr(call->perf_events);
@@ -858,8 +854,11 @@ static void uprobe_perf_print(struct trace_uprobe *tu,
data = DATAOF_TRACE_ENTRY(entry, false);
}

- for (i = 0; i < tu->nr_args; i++)
- call_fetch(&tu->args[i].fetch, regs, data + tu->args[i].offset);
+ for (i = 0; i < tu->tp.nr_args; i++) {
+ struct probe_arg *parg = &tu->tp.args[i];
+
+ call_fetch(&parg->fetch, regs, data + parg->offset);
+ }

perf_trace_buf_submit(entry, size, rctx, 0, 1, regs, head, NULL);
out:
@@ -926,11 +925,11 @@ static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs *regs)
tu = container_of(con, struct trace_uprobe, consumer);
tu->nhit++;

- if (tu->flags & TP_FLAG_TRACE)
+ if (tu->tp.flags & TP_FLAG_TRACE)
ret |= uprobe_trace_func(tu, regs);

#ifdef CONFIG_PERF_EVENTS
- if (tu->flags & TP_FLAG_PROFILE)
+ if (tu->tp.flags & TP_FLAG_PROFILE)
ret |= uprobe_perf_func(tu, regs);
#endif
return ret;
@@ -943,11 +942,11 @@ static int uretprobe_dispatcher(struct uprobe_consumer *con,

tu = container_of(con, struct trace_uprobe, consumer);

- if (tu->flags & TP_FLAG_TRACE)
+ if (tu->tp.flags & TP_FLAG_TRACE)
uretprobe_trace_func(tu, func, regs);

#ifdef CONFIG_PERF_EVENTS
- if (tu->flags & TP_FLAG_PROFILE)
+ if (tu->tp.flags & TP_FLAG_PROFILE)
uretprobe_perf_func(tu, func, regs);
#endif
return 0;
@@ -959,7 +958,7 @@ static struct trace_event_functions uprobe_funcs = {

static int register_uprobe_event(struct trace_uprobe *tu)
{
- struct ftrace_event_call *call = &tu->call;
+ struct ftrace_event_call *call = &tu->tp.call;
int ret;

/* Initialize ftrace_event_call */
@@ -994,11 +993,11 @@ static int unregister_uprobe_event(struct trace_uprobe *tu)
int ret;

/* tu->event is unregistered in trace_remove_event_call() */
- ret = trace_remove_event_call(&tu->call);
+ ret = trace_remove_event_call(&tu->tp.call);
if (ret)
return ret;
- kfree(tu->call.print_fmt);
- tu->call.print_fmt = NULL;
+ kfree(tu->tp.call.print_fmt);
+ tu->tp.call.print_fmt = NULL;
return 0;

Namhyung Kim

unread,
Dec 15, 2013, 11:40:02 PM12/15/13
to
From: Oleg Nesterov <ol...@redhat.com>

uprobe_trace_print() and uprobe_perf_print() need to pass the additional
info to call_fetch() methods, currently there is no simple way to do this.

current->utask looks like a natural place to hold this info, but we need
to allocate it before handler_chain().

This is a bit unfortunate, perhaps we will find a better solution later,
but this is simple and should work right now.

Signed-off-by: Oleg Nesterov <ol...@redhat.com>
Acked-by: Masami Hiramatsu <masami.hi...@hitachi.com>
Cc: Srikar Dronamraju <sri...@linux.vnet.ibm.com>
Signed-off-by: Namhyung Kim <namh...@kernel.org>
---
kernel/events/uprobes.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 24b7d6ca871b..3cc8e0bb8acf 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -1828,6 +1828,10 @@ static void handle_swbp(struct pt_regs *regs)
if (unlikely(!test_bit(UPROBE_COPY_INSN, &uprobe->flags)))
goto out;

+ /* Tracing handlers use ->utask to communicate with fetch methods */
+ if (!get_utask())
+ goto out;
+
handler_chain(uprobe, regs);
if (can_skip_sstep(uprobe, regs))
goto out;

Namhyung Kim

unread,
Dec 15, 2013, 11:40:02 PM12/15/13
to
From: Namhyung Kim <namhyu...@lge.com>

Currently uprobes don't pass is_return to the argument parser so that
it cannot make use of "$retval" fetch method since it only works for
return probes.

Reviewed-by: Masami Hiramatsu <masami.hi...@hitachi.com>
Cc: Srikar Dronamraju <sri...@linux.vnet.ibm.com>
Cc: Oleg Nesterov <ol...@redhat.com>
Cc: zhangwei(Jovi) <jovi.z...@huawei.com>
Cc: Arnaldo Carvalho de Melo <ac...@ghostprotocols.net>
Signed-off-by: Namhyung Kim <namh...@kernel.org>
---
kernel/trace/trace_uprobe.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 7eb1b3244c94..39dfc9a01a4a 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -514,7 +514,7 @@ static int create_trace_uprobe(int argc, char **argv)

/* Parse fetch argument */
ret = traceprobe_parse_probe_arg(arg, &tu->tp.size, parg,
- false, false);
+ is_return, false);
if (ret) {
pr_info("Parse error at argument[%d]. (%d)\n", i, ret);
goto error;

Namhyung Kim

unread,
Dec 15, 2013, 11:40:02 PM12/15/13
to
From: Namhyung Kim <namhyu...@lge.com>

Fetching from user space should be done in a non-atomic context. So
use a per-cpu buffer and copy its content to the ring buffer
atomically. Note that we can migrate during accessing user memory
thus use a per-cpu mutex to protect concurrent accesses.

This is needed since we'll be able to fetch args from an user memory
which can be swapped out. Before that uprobes could fetch args from
registers only which saved in a kernel space.

While at it, use __get_data_size() and store_trace_args() to reduce
code duplication. And add struct uprobe_cpu_buffer and its helpers as
suggested by Oleg.

Reviewed-by: Masami Hiramatsu <masami.hi...@hitachi.com>
Cc: Srikar Dronamraju <sri...@linux.vnet.ibm.com>
Cc: Oleg Nesterov <ol...@redhat.com>
Cc: zhangwei(Jovi) <jovi.z...@huawei.com>
Cc: Arnaldo Carvalho de Melo <ac...@ghostprotocols.net>
Signed-off-by: Namhyung Kim <namh...@kernel.org>
---
kernel/trace/trace_uprobe.c | 146 +++++++++++++++++++++++++++++++++++++++-----
1 file changed, 132 insertions(+), 14 deletions(-)

diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 39dfc9a01a4a..6ed319be5a84 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -652,21 +652,117 @@ static const struct file_operations uprobe_profile_ops = {
.release = seq_release,
};

+struct uprobe_cpu_buffer {
+ struct mutex mutex;
+ void *buf;
+};
+static struct uprobe_cpu_buffer __percpu *uprobe_cpu_buffer;
+static int uprobe_buffer_refcnt;
+
+static int uprobe_buffer_init(void)
+{
+ int cpu, err_cpu;
+
+ uprobe_cpu_buffer = alloc_percpu(struct uprobe_cpu_buffer);
+ if (uprobe_cpu_buffer == NULL)
+ return -ENOMEM;
+
+ for_each_possible_cpu(cpu) {
+ struct page *p = alloc_pages_node(cpu_to_node(cpu),
+ GFP_KERNEL, 0);
+ if (p == NULL) {
+ err_cpu = cpu;
+ goto err;
+ }
+ per_cpu_ptr(uprobe_cpu_buffer, cpu)->buf = page_address(p);
+ mutex_init(&per_cpu_ptr(uprobe_cpu_buffer, cpu)->mutex);
+ }
+
+ return 0;
+
+err:
+ for_each_possible_cpu(cpu) {
+ if (cpu == err_cpu)
+ break;
+ free_page((unsigned long)per_cpu_ptr(uprobe_cpu_buffer, cpu)->buf);
+ }
+
+ free_percpu(uprobe_cpu_buffer);
+ return -ENOMEM;
+}
+
+static int uprobe_buffer_enable(void)
+{
+ int ret = 0;
+
+ BUG_ON(!mutex_is_locked(&event_mutex));
+
+ if (uprobe_buffer_refcnt++ == 0) {
+ ret = uprobe_buffer_init();
+ if (ret < 0)
+ uprobe_buffer_refcnt--;
+ }
+
+ return ret;
+}
+
+static void uprobe_buffer_disable(void)
+{
+ BUG_ON(!mutex_is_locked(&event_mutex));
+
+ if (--uprobe_buffer_refcnt == 0) {
+ free_percpu(uprobe_cpu_buffer);
+ uprobe_cpu_buffer = NULL;
+ }
+}
+
+static struct uprobe_cpu_buffer *uprobe_buffer_get(void)
+{
+ struct uprobe_cpu_buffer *ucb;
+ int cpu;
+
+ cpu = raw_smp_processor_id();
+ ucb = per_cpu_ptr(uprobe_cpu_buffer, cpu);
+
+ /*
+ * Use per-cpu buffers for fastest access, but we might migrate
+ * so the mutex makes sure we have sole access to it.
+ */
+ mutex_lock(&ucb->mutex);
+
+ return ucb;
+}
+
+static void uprobe_buffer_put(struct uprobe_cpu_buffer *ucb)
+{
+ mutex_unlock(&ucb->mutex);
+}
+
static void uprobe_trace_print(struct trace_uprobe *tu,
unsigned long func, struct pt_regs *regs)
{
struct uprobe_trace_entry_head *entry;
struct ring_buffer_event *event;
struct ring_buffer *buffer;
+ struct uprobe_cpu_buffer *ucb;
void *data;
- int size, i;
+ int size, dsize, esize;
struct ftrace_event_call *call = &tu->tp.call;

- size = SIZEOF_TRACE_ENTRY(is_ret_probe(tu));
+ dsize = __get_data_size(&tu->tp, regs);
+ esize = SIZEOF_TRACE_ENTRY(is_ret_probe(tu));
+
+ if (WARN_ON_ONCE(!uprobe_cpu_buffer || tu->tp.size + dsize > PAGE_SIZE))
+ return;
+
+ ucb = uprobe_buffer_get();
+ store_trace_args(esize, &tu->tp, regs, ucb->buf, dsize);
+
+ size = esize + tu->tp.size + dsize;
event = trace_current_buffer_lock_reserve(&buffer, call->event.type,
- size + tu->tp.size, 0, 0);
+ size, 0, 0);
if (!event)
- return;
+ goto out;

entry = ring_buffer_event_data(event);
if (is_ret_probe(tu)) {
@@ -678,13 +774,13 @@ static void uprobe_trace_print(struct trace_uprobe *tu,
data = DATAOF_TRACE_ENTRY(entry, false);
}

- for (i = 0; i < tu->tp.nr_args; i++) {
- call_fetch(&tu->tp.args[i].fetch, regs,
- data + tu->tp.args[i].offset);
- }
+ memcpy(data, ucb->buf, tu->tp.size + dsize);

if (!call_filter_check_discard(call, entry, buffer, event))
trace_buffer_unlock_commit(buffer, event, 0, 0);
+
+out:
+ uprobe_buffer_put(ucb);
}

/* uprobe handler */
@@ -752,6 +848,10 @@ probe_event_enable(struct trace_uprobe *tu, int flag, filter_func_t filter)
if (trace_probe_is_enabled(&tu->tp))
return -EINTR;

+ ret = uprobe_buffer_enable();
+ if (ret < 0)
+ return ret;
+
WARN_ON(!uprobe_filter_is_empty(&tu->filter));

tu->tp.flags |= flag;
@@ -772,6 +872,8 @@ static void probe_event_disable(struct trace_uprobe *tu, int flag)

uprobe_unregister(tu->inode, tu->offset, &tu->consumer);
tu->tp.flags &= ~flag;
+
+ uprobe_buffer_disable();
}

static int uprobe_event_define_fields(struct ftrace_event_call *event_call)
@@ -898,11 +1000,24 @@ static void uprobe_perf_print(struct trace_uprobe *tu,
struct ftrace_event_call *call = &tu->tp.call;
struct uprobe_trace_entry_head *entry;
struct hlist_head *head;
+ struct uprobe_cpu_buffer *ucb;
void *data;
- int size, rctx, i;
+ int size, dsize, esize;
+ int rctx;
+
+ dsize = __get_data_size(&tu->tp, regs);
+ esize = SIZEOF_TRACE_ENTRY(is_ret_probe(tu));

- size = SIZEOF_TRACE_ENTRY(is_ret_probe(tu));
- size = ALIGN(size + tu->tp.size + sizeof(u32), sizeof(u64)) - sizeof(u32);
+ if (WARN_ON_ONCE(!uprobe_cpu_buffer))
+ return;
+
+ size = esize + tu->tp.size + dsize;
+ size = ALIGN(size + sizeof(u32), sizeof(u64)) - sizeof(u32);
+ if (WARN_ONCE(size > PERF_MAX_TRACE_SIZE, "profile buffer not large enough"))
+ return;
+
+ ucb = uprobe_buffer_get();
+ store_trace_args(esize, &tu->tp, regs, ucb->buf, dsize);

preempt_disable();
head = this_cpu_ptr(call->perf_events);
@@ -922,15 +1037,18 @@ static void uprobe_perf_print(struct trace_uprobe *tu,
data = DATAOF_TRACE_ENTRY(entry, false);
}

- for (i = 0; i < tu->tp.nr_args; i++) {
- struct probe_arg *parg = &tu->tp.args[i];
+ memcpy(data, ucb->buf, tu->tp.size + dsize);
+
+ if (size - esize > tu->tp.size + dsize) {
+ int len = tu->tp.size + dsize;

- call_fetch(&parg->fetch, regs, data + parg->offset);
+ memset(data + len, 0, size - esize - len);
}

perf_trace_buf_submit(entry, size, rctx, 0, 1, regs, head, NULL);
out:
preempt_enable();
+ uprobe_buffer_put(ucb);
}

/* uprobe profile handler */

Namhyung Kim

unread,
Dec 15, 2013, 11:40:03 PM12/15/13
to
From: Namhyung Kim <namhyu...@lge.com>

The set_print_fmt() functions are implemented almost same for
[ku]probes. Move it to a common place and get rid of the duplication.

Acked-by: Masami Hiramatsu <masami.hi...@hitachi.com>
Cc: Srikar Dronamraju <sri...@linux.vnet.ibm.com>
Cc: Oleg Nesterov <ol...@redhat.com>
Cc: zhangwei(Jovi) <jovi.z...@huawei.com>
Cc: Arnaldo Carvalho de Melo <ac...@ghostprotocols.net>
Signed-off-by: Namhyung Kim <namh...@kernel.org>
---
kernel/trace/trace_kprobe.c | 63 +--------------------------------------------
kernel/trace/trace_probe.c | 62 ++++++++++++++++++++++++++++++++++++++++++++
kernel/trace/trace_probe.h | 2 ++
kernel/trace/trace_uprobe.c | 55 +--------------------------------------
4 files changed, 66 insertions(+), 116 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 0167d4b92431..62d6c961bbce 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -964,67 +964,6 @@ static int kretprobe_event_define_fields(struct ftrace_event_call *event_call)
return 0;
}

-static int __set_print_fmt(struct trace_kprobe *tk, char *buf, int len)
-{
- int i;
- int pos = 0;
-
- const char *fmt, *arg;
-
- if (!trace_kprobe_is_return(tk)) {
- fmt = "(%lx)";
- arg = "REC->" FIELD_STRING_IP;
- } else {
- fmt = "(%lx <- %lx)";
- arg = "REC->" FIELD_STRING_FUNC ", REC->" FIELD_STRING_RETIP;
- }
-
- /* When len=0, we just calculate the needed length */
-#define LEN_OR_ZERO (len ? len - pos : 0)
-
- pos += snprintf(buf + pos, LEN_OR_ZERO, "\"%s", fmt);
-
- for (i = 0; i < tk->tp.nr_args; i++) {
- pos += snprintf(buf + pos, LEN_OR_ZERO, " %s=%s",
- tk->tp.args[i].name, tk->tp.args[i].type->fmt);
- }
-
- pos += snprintf(buf + pos, LEN_OR_ZERO, "\", %s", arg);
-
- for (i = 0; i < tk->tp.nr_args; i++) {
- if (strcmp(tk->tp.args[i].type->name, "string") == 0)
- pos += snprintf(buf + pos, LEN_OR_ZERO,
- ", __get_str(%s)",
- tk->tp.args[i].name);
- else
- pos += snprintf(buf + pos, LEN_OR_ZERO, ", REC->%s",
- tk->tp.args[i].name);
- }
-
-#undef LEN_OR_ZERO
-
- /* return the length of print_fmt */
- return pos;
-}
-
-static int set_print_fmt(struct trace_kprobe *tk)
-{
- int len;
- char *print_fmt;
-
- /* First: called with 0 length to calculate the needed length */
- len = __set_print_fmt(tk, NULL, 0);
- print_fmt = kmalloc(len + 1, GFP_KERNEL);
- if (!print_fmt)
- return -ENOMEM;
-
- /* Second: actually write the @print_fmt */
- __set_print_fmt(tk, print_fmt, len + 1);
- tk->tp.call.print_fmt = print_fmt;
-
- return 0;
-}
-
#ifdef CONFIG_PERF_EVENTS

/* Kprobe profile handler */
@@ -1175,7 +1114,7 @@ static int register_kprobe_event(struct trace_kprobe *tk)
call->event.funcs = &kprobe_funcs;
call->class->define_fields = kprobe_event_define_fields;
}
- if (set_print_fmt(tk) < 0)
+ if (set_print_fmt(&tk->tp, trace_kprobe_is_return(tk)) < 0)
return -ENOMEM;
ret = register_ftrace_event(&call->event);
if (!ret) {
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 430505b08a6f..d8347b01ce89 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -837,3 +837,65 @@ out:

return ret;
}
+
+static int __set_print_fmt(struct trace_probe *tp, char *buf, int len,
+ bool is_return)
+{
+ int i;
+ int pos = 0;
+
+ const char *fmt, *arg;
+
+ if (!is_return) {
+ fmt = "(%lx)";
+ arg = "REC->" FIELD_STRING_IP;
+ } else {
+ fmt = "(%lx <- %lx)";
+ arg = "REC->" FIELD_STRING_FUNC ", REC->" FIELD_STRING_RETIP;
+ }
+
+ /* When len=0, we just calculate the needed length */
+#define LEN_OR_ZERO (len ? len - pos : 0)
+
+ 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=%s",
+ tp->args[i].name, tp->args[i].type->fmt);
+ }
+
+ pos += snprintf(buf + pos, LEN_OR_ZERO, "\", %s", arg);
+
+ for (i = 0; i < tp->nr_args; i++) {
+ if (strcmp(tp->args[i].type->name, "string") == 0)
+ pos += snprintf(buf + pos, LEN_OR_ZERO,
+ ", __get_str(%s)",
+ tp->args[i].name);
+ else
+ pos += snprintf(buf + pos, LEN_OR_ZERO, ", REC->%s",
+ tp->args[i].name);
+ }
+
+#undef LEN_OR_ZERO
+
+ /* return the length of print_fmt */
+ return pos;
+}
+
+int set_print_fmt(struct trace_probe *tp, bool is_return)
+{
+ int len;
+ char *print_fmt;
+
+ /* First: called with 0 length to calculate the needed length */
+ len = __set_print_fmt(tp, NULL, 0, is_return);
+ print_fmt = kmalloc(len + 1, GFP_KERNEL);
+ if (!print_fmt)
+ return -ENOMEM;
+
+ /* Second: actually write the @print_fmt */
+ __set_print_fmt(tp, print_fmt, len + 1, is_return);
+ tp->call.print_fmt = print_fmt;
+
+ return 0;
+}
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index d384fbd4025c..2c979cb66367 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -226,3 +226,5 @@ store_trace_args(int ent_size, struct trace_probe *tp, struct pt_regs *regs,
data + tp->args[i].offset);
}
}
+
+extern int set_print_fmt(struct trace_probe *tp, bool is_return);
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index afda3726f288..b233d9cb1216 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -682,59 +682,6 @@ static int uprobe_event_define_fields(struct ftrace_event_call *event_call)
return 0;
}

-#define LEN_OR_ZERO (len ? len - pos : 0)
-static int __set_print_fmt(struct trace_uprobe *tu, char *buf, int len)
-{
- const char *fmt, *arg;
- int i;
- int pos = 0;
-
- if (is_ret_probe(tu)) {
- fmt = "(%lx <- %lx)";
- arg = "REC->" FIELD_STRING_FUNC ", REC->" FIELD_STRING_RETIP;
- } else {
- fmt = "(%lx)";
- arg = "REC->" FIELD_STRING_IP;
- }
-
- /* When len=0, we just calculate the needed length */
-
- pos += snprintf(buf + pos, LEN_OR_ZERO, "\"%s", fmt);
-
- for (i = 0; i < tu->tp.nr_args; i++) {
- pos += snprintf(buf + pos, LEN_OR_ZERO, " %s=%s",
- tu->tp.args[i].name, tu->tp.args[i].type->fmt);
- }
-
- pos += snprintf(buf + pos, LEN_OR_ZERO, "\", %s", arg);
-
- for (i = 0; i < tu->tp.nr_args; i++) {
- pos += snprintf(buf + pos, LEN_OR_ZERO, ", REC->%s",
- tu->tp.args[i].name);
- }
-
- return pos; /* return the length of print_fmt */
-}
-#undef LEN_OR_ZERO
-
-static int set_print_fmt(struct trace_uprobe *tu)
-{
- char *print_fmt;
- int len;
-
- /* First: called with 0 length to calculate the needed length */
- len = __set_print_fmt(tu, NULL, 0);
- print_fmt = kmalloc(len + 1, GFP_KERNEL);
- if (!print_fmt)
- return -ENOMEM;
-
- /* Second: actually write the @print_fmt */
- __set_print_fmt(tu, print_fmt, len + 1);
- tu->tp.call.print_fmt = print_fmt;
-
- return 0;
-}
-
#ifdef CONFIG_PERF_EVENTS
static bool
__uprobe_perf_filter(struct trace_uprobe_filter *filter, struct mm_struct *mm)
@@ -966,7 +913,7 @@ static int register_uprobe_event(struct trace_uprobe *tu)
call->event.funcs = &uprobe_funcs;
call->class->define_fields = uprobe_event_define_fields;

- if (set_print_fmt(tu) < 0)
+ if (set_print_fmt(&tu->tp, is_ret_probe(tu)) < 0)
return -ENOMEM;

ret = register_ftrace_event(&call->event);

Namhyung Kim

unread,
Dec 15, 2013, 11:40:03 PM12/15/13
to
From: Namhyung Kim <namhyu...@lge.com>

The __get_data_size() and store_trace_args() will be used by uprobes
too. Move them to a common location.

Acked-by: Masami Hiramatsu <masami.hi...@hitachi.com>
Cc: Srikar Dronamraju <sri...@linux.vnet.ibm.com>
Cc: Oleg Nesterov <ol...@redhat.com>
Cc: zhangwei(Jovi) <jovi.z...@huawei.com>
Cc: Arnaldo Carvalho de Melo <ac...@ghostprotocols.net>
Signed-off-by: Namhyung Kim <namh...@kernel.org>
---
kernel/trace/trace_kprobe.c | 48 ---------------------------------------------
kernel/trace/trace_probe.h | 48 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 48 insertions(+), 48 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index db922fd74b05..0167d4b92431 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -740,54 +740,6 @@ static const struct file_operations kprobe_profile_ops = {
.release = seq_release,
};

-/* Sum up total data length for dynamic arraies (strings) */
-static __kprobes int __get_data_size(struct trace_probe *tp,
- struct pt_regs *regs)
-{
- int i, ret = 0;
- u32 len;
-
- for (i = 0; i < tp->nr_args; i++)
- if (unlikely(tp->args[i].fetch_size.fn)) {
- call_fetch(&tp->args[i].fetch_size, regs, &len);
- ret += len;
- }
-
- return ret;
-}
-
-/* Store the value of each argument */
-static __kprobes void store_trace_args(int ent_size, struct trace_probe *tp,
- struct pt_regs *regs,
- u8 *data, int maxlen)
-{
- int i;
- u32 end = tp->size;
- u32 *dl; /* Data (relative) location */
-
- for (i = 0; i < tp->nr_args; i++) {
- if (unlikely(tp->args[i].fetch_size.fn)) {
- /*
- * First, we set the relative location and
- * maximum data length to *dl
- */
- dl = (u32 *)(data + tp->args[i].offset);
- *dl = make_data_rloc(maxlen, end - tp->args[i].offset);
- /* Then try to fetch string or dynamic array data */
- call_fetch(&tp->args[i].fetch, regs, dl);
- /* Reduce maximum length */
- end += get_rloc_len(*dl);
- maxlen -= get_rloc_len(*dl);
- /* Trick here, convert data_rloc to data_loc */
- *dl = convert_rloc_to_loc(*dl,
- ent_size + tp->args[i].offset);
- } else
- /* Just fetching data normally */
- call_fetch(&tp->args[i].fetch, regs,
- data + tp->args[i].offset);
- }
-}
-
/* Kprobe handler */
static __kprobes void
__kprobe_trace_func(struct trace_kprobe *tk, struct pt_regs *regs,
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 984e91ed8a44..d384fbd4025c 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -178,3 +178,51 @@ extern ssize_t traceprobe_probes_write(struct file *file,
int (*createfn)(int, char**));

extern int traceprobe_command(const char *buf, int (*createfn)(int, char**));
+
+/* Sum up total data length for dynamic arraies (strings) */
+static inline __kprobes int
+__get_data_size(struct trace_probe *tp, struct pt_regs *regs)
+{
+ int i, ret = 0;
+ u32 len;
+
+ for (i = 0; i < tp->nr_args; i++)
+ if (unlikely(tp->args[i].fetch_size.fn)) {
+ call_fetch(&tp->args[i].fetch_size, regs, &len);
+ ret += len;
+ }
+
+ return ret;
+}
+
+/* Store the value of each argument */
+static inline __kprobes void
+store_trace_args(int ent_size, struct trace_probe *tp, struct pt_regs *regs,
+ u8 *data, int maxlen)
+{
+ int i;
+ u32 end = tp->size;
+ u32 *dl; /* Data (relative) location */
+
+ for (i = 0; i < tp->nr_args; i++) {
+ if (unlikely(tp->args[i].fetch_size.fn)) {
+ /*
+ * First, we set the relative location and
+ * maximum data length to *dl
+ */
+ dl = (u32 *)(data + tp->args[i].offset);
+ *dl = make_data_rloc(maxlen, end - tp->args[i].offset);
+ /* Then try to fetch string or dynamic array data */
+ call_fetch(&tp->args[i].fetch, regs, dl);
+ /* Reduce maximum length */
+ end += get_rloc_len(*dl);
+ maxlen -= get_rloc_len(*dl);
+ /* Trick here, convert data_rloc to data_loc */
+ *dl = convert_rloc_to_loc(*dl,
+ ent_size + tp->args[i].offset);
+ } else
+ /* Just fetching data normally */
+ call_fetch(&tp->args[i].fetch, regs,
+ data + tp->args[i].offset);
+ }
+}

Namhyung Kim

unread,
Dec 15, 2013, 11:40:03 PM12/15/13
to
From: Namhyung Kim <namhyu...@lge.com>

Move fetch function helper macros/functions to the header file and
make them external. This is preparation of supporting uprobe fetch
table in next patch.

Acked-by: Masami Hiramatsu <masami.hi...@hitachi.com>
Cc: Srikar Dronamraju <sri...@linux.vnet.ibm.com>
Cc: Oleg Nesterov <ol...@redhat.com>
Cc: zhangwei(Jovi) <jovi.z...@huawei.com>
Cc: Arnaldo Carvalho de Melo <ac...@ghostprotocols.net>
Signed-off-by: Namhyung Kim <namh...@kernel.org>
---
kernel/trace/trace_probe.c | 74 ++++++++--------------------------------------
kernel/trace/trace_probe.h | 65 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 78 insertions(+), 61 deletions(-)

diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index d8347b01ce89..c26bc9eaa2ac 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -35,19 +35,15 @@ const char *reserved_field_names[] = {
FIELD_STRING_FUNC,
};

-/* Printing function type */
-#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) \
-static __kprobes int PRINT_TYPE_FUNC_NAME(type)(struct trace_seq *s, \
+__kprobes int PRINT_TYPE_FUNC_NAME(type)(struct trace_seq *s, \
const char *name, \
void *data, void *ent) \
{ \
return trace_seq_printf(s, " %s=" fmt, name, *(type *)data); \
} \
-static const char PRINT_TYPE_FMT_NAME(type)[] = fmt;
+const char PRINT_TYPE_FMT_NAME(type)[] = fmt;

DEFINE_BASIC_PRINT_TYPE_FUNC(u8 , "0x%x")
DEFINE_BASIC_PRINT_TYPE_FUNC(u16, "0x%x")
@@ -58,23 +54,12 @@ DEFINE_BASIC_PRINT_TYPE_FUNC(s16, "%d")
DEFINE_BASIC_PRINT_TYPE_FUNC(s32, "%d")
DEFINE_BASIC_PRINT_TYPE_FUNC(s64, "%Ld")

-static inline void *get_rloc_data(u32 *dl)
-{
- return (u8 *)dl + get_rloc_offs(*dl);
-}
-
-/* For data_loc conversion */
-static inline void *get_loc_data(u32 *dl, void *ent)
-{
- return (u8 *)ent + get_rloc_offs(*dl);
-}
-
/* For defining macros, define string/string_size types */
typedef u32 string;
typedef u32 string_size;

/* Print type function for string type */
-static __kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq *s,
+__kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq *s,
const char *name,
void *data, void *ent)
{
@@ -87,7 +72,7 @@ static __kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq *s,
(const char *)get_loc_data(data, ent));
}

-static const char PRINT_TYPE_FMT_NAME(string)[] = "\\\"%s\\\"";
+const char PRINT_TYPE_FMT_NAME(string)[] = "\\\"%s\\\"";

#define FETCH_FUNC_NAME(method, type) fetch_##method##_##type
/*
@@ -111,7 +96,7 @@ DEFINE_FETCH_##method(u64)

/* Data fetch function templates */
#define DEFINE_FETCH_reg(type) \
-static __kprobes void FETCH_FUNC_NAME(reg, type)(struct pt_regs *regs, \
+__kprobes void FETCH_FUNC_NAME(reg, type)(struct pt_regs *regs, \
void *offset, void *dest) \
{ \
*(type *)dest = (type)regs_get_register(regs, \
@@ -123,7 +108,7 @@ DEFINE_BASIC_FETCH_FUNCS(reg)
#define fetch_reg_string_size NULL

#define DEFINE_FETCH_stack(type) \
-static __kprobes void FETCH_FUNC_NAME(stack, type)(struct pt_regs *regs,\
+__kprobes void FETCH_FUNC_NAME(stack, type)(struct pt_regs *regs, \
void *offset, void *dest) \
{ \
*(type *)dest = (type)regs_get_kernel_stack_nth(regs, \
@@ -135,7 +120,7 @@ DEFINE_BASIC_FETCH_FUNCS(stack)
#define fetch_stack_string_size NULL

#define DEFINE_FETCH_retval(type) \
-static __kprobes void FETCH_FUNC_NAME(retval, type)(struct pt_regs *regs,\
+__kprobes void FETCH_FUNC_NAME(retval, type)(struct pt_regs *regs, \
void *dummy, void *dest) \
{ \
*(type *)dest = (type)regs_return_value(regs); \
@@ -146,7 +131,7 @@ DEFINE_BASIC_FETCH_FUNCS(retval)
#define fetch_retval_string_size NULL

#define DEFINE_FETCH_memory(type) \
-static __kprobes void FETCH_FUNC_NAME(memory, type)(struct pt_regs *regs,\
+__kprobes void FETCH_FUNC_NAME(memory, type)(struct pt_regs *regs, \
void *addr, void *dest) \
{ \
type retval; \
@@ -160,7 +145,7 @@ DEFINE_BASIC_FETCH_FUNCS(memory)
* Fetch a null-terminated string. Caller MUST set *(u32 *)dest with max
* length and relative data location.
*/
-static __kprobes void FETCH_FUNC_NAME(memory, string)(struct pt_regs *regs,
+__kprobes void FETCH_FUNC_NAME(memory, string)(struct pt_regs *regs,
void *addr, void *dest)
{
long ret;
@@ -197,7 +182,7 @@ static __kprobes void FETCH_FUNC_NAME(memory, string)(struct pt_regs *regs,
}

/* Return the length of string -- including null terminal byte */
-static __kprobes void FETCH_FUNC_NAME(memory, string_size)(struct pt_regs *regs,
+__kprobes void FETCH_FUNC_NAME(memory, string_size)(struct pt_regs *regs,
void *addr, void *dest)
{
mm_segment_t old_fs;
@@ -268,7 +253,7 @@ static struct symbol_cache *alloc_symbol_cache(const char *sym, long offset)
}

#define DEFINE_FETCH_symbol(type) \
-static __kprobes void FETCH_FUNC_NAME(symbol, type)(struct pt_regs *regs,\
+__kprobes void FETCH_FUNC_NAME(symbol, type)(struct pt_regs *regs, \
void *data, void *dest) \
{ \
struct symbol_cache *sc = data; \
@@ -288,7 +273,7 @@ struct deref_fetch_param {
};

#define DEFINE_FETCH_deref(type) \
-static __kprobes void FETCH_FUNC_NAME(deref, type)(struct pt_regs *regs,\
+__kprobes void FETCH_FUNC_NAME(deref, type)(struct pt_regs *regs, \
void *data, void *dest) \
{ \
struct deref_fetch_param *dprm = data; \
@@ -329,7 +314,7 @@ struct bitfield_fetch_param {
};

#define DEFINE_FETCH_bitfield(type) \
-static __kprobes void FETCH_FUNC_NAME(bitfield, type)(struct pt_regs *regs,\
+__kprobes void FETCH_FUNC_NAME(bitfield, type)(struct pt_regs *regs, \
void *data, void *dest) \
{ \
struct bitfield_fetch_param *bprm = data; \
@@ -374,39 +359,6 @@ free_bitfield_fetch_param(struct bitfield_fetch_param *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(method, type) \
- [FETCH_MTD_##method] = FETCH_FUNC_NAME(method, type)
-
-#define __ASSIGN_FETCH_TYPE(_name, ptype, ftype, _size, sign, _fmttype) \
- {.name = _name, \
- .size = _size, \
- .is_signed = sign, \
- .print = PRINT_TYPE_FUNC_NAME(ptype), \
- .fmt = PRINT_TYPE_FMT_NAME(ptype), \
- .fmttype = _fmttype, \
- .fetch = { \
-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), \
-ASSIGN_FETCH_FUNC(bitfield, ftype), \
- } \
- }
-
-#define ASSIGN_FETCH_TYPE(ptype, ftype, sign) \
- __ASSIGN_FETCH_TYPE(#ptype, ptype, ftype, sizeof(ftype), sign, #ptype)
-
-#define FETCH_TYPE_STRING 0
-#define FETCH_TYPE_STRSIZE 1
-
/* Fetch type information table */
static const struct fetch_type fetch_type_table[] = {
/* Special types */
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 2c979cb66367..bd621c08b6c6 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -81,6 +81,17 @@
*/
#define convert_rloc_to_loc(dl, offs) ((u32)(dl) + (offs))

+static inline void *get_rloc_data(u32 *dl)
+{
+ return (u8 *)dl + get_rloc_offs(*dl);
+}
+
+/* For data_loc conversion */
+static inline void *get_loc_data(u32 *dl, void *ent)
+{
+ return (u8 *)ent + get_rloc_offs(*dl);
+}
+
/* Data fetch function type */
typedef void (*fetch_func_t)(struct pt_regs *, void *, void *);
/* Printing function type */
@@ -115,6 +126,60 @@ struct fetch_param {
void *data;
};

+#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 DECLARE_BASIC_PRINT_TYPE_FUNC(type) \
+__kprobes int PRINT_TYPE_FUNC_NAME(type)(struct trace_seq *s, \
+ const char *name, \
+ void *data, void *ent); \
+extern const char PRINT_TYPE_FMT_NAME(type)[]
+
+DECLARE_BASIC_PRINT_TYPE_FUNC(u8);
+DECLARE_BASIC_PRINT_TYPE_FUNC(u16);
+DECLARE_BASIC_PRINT_TYPE_FUNC(u32);
+DECLARE_BASIC_PRINT_TYPE_FUNC(u64);
+DECLARE_BASIC_PRINT_TYPE_FUNC(s8);
+DECLARE_BASIC_PRINT_TYPE_FUNC(s16);
+DECLARE_BASIC_PRINT_TYPE_FUNC(s32);
+DECLARE_BASIC_PRINT_TYPE_FUNC(s64);
+DECLARE_BASIC_PRINT_TYPE_FUNC(string);
+
+/* 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(method, type) \
+ [FETCH_MTD_##method] = FETCH_FUNC_NAME(method, type)
+
+#define __ASSIGN_FETCH_TYPE(_name, ptype, ftype, _size, sign, _fmttype) \
+ {.name = _name, \
+ .size = _size, \
+ .is_signed = sign, \
+ .print = PRINT_TYPE_FUNC_NAME(ptype), \
+ .fmt = PRINT_TYPE_FMT_NAME(ptype), \
+ .fmttype = _fmttype, \
+ .fetch = { \
+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), \
+ASSIGN_FETCH_FUNC(bitfield, ftype), \
+ } \
+ }
+
+#define ASSIGN_FETCH_TYPE(ptype, ftype, sign) \
+ __ASSIGN_FETCH_TYPE(#ptype, ptype, ftype, sizeof(ftype), sign, #ptype)
+
+#define FETCH_TYPE_STRING 0
+#define FETCH_TYPE_STRSIZE 1
+
+
struct probe_arg {
struct fetch_param fetch;
struct fetch_param fetch_size;

Namhyung Kim

unread,
Dec 15, 2013, 11:40:03 PM12/15/13
to
From: Namhyung Kim <namhyu...@lge.com>

Enable to fetch other types of argument for the uprobes. IOW, we can
access stack, memory, deref, bitfield and retval from uprobes now.

The format for the argument types are same as kprobes (but @SYMBOL
type is not supported for uprobes), i.e:

@ADDR : Fetch memory at ADDR
$stackN : Fetch Nth entry of stack (N >= 0)
$stack : Fetch stack address
$retval : Fetch return value
+|-offs(FETCHARG) : Fetch memory at FETCHARG +|- offs address

Note that the retval only can be used with uretprobes.

Original-patch-by: Hyeoncheol Lee <cheo...@lge.com>
Acked-by: Masami Hiramatsu <masami.hi...@hitachi.com>
Cc: Srikar Dronamraju <sri...@linux.vnet.ibm.com>
Cc: Oleg Nesterov <ol...@redhat.com>
Cc: zhangwei(Jovi) <jovi.z...@huawei.com>
Cc: Arnaldo Carvalho de Melo <ac...@ghostprotocols.net>
Signed-off-by: Namhyung Kim <namh...@kernel.org>
---
Documentation/trace/uprobetracer.txt | 25 +++++++++++++++++++++++++
kernel/trace/trace_probe.c | 34 ++++++++++++++++++++++------------
2 files changed, 47 insertions(+), 12 deletions(-)

diff --git a/Documentation/trace/uprobetracer.txt b/Documentation/trace/uprobetracer.txt
index 8f1a8b8956fc..6e5cff263e2b 100644
--- a/Documentation/trace/uprobetracer.txt
+++ b/Documentation/trace/uprobetracer.txt
@@ -31,6 +31,31 @@ Synopsis of uprobe_tracer

FETCHARGS : Arguments. Each probe can have up to 128 args.
%REG : Fetch register REG
+ @ADDR : Fetch memory at ADDR (ADDR should be in userspace)
+ $stackN : Fetch Nth entry of stack (N >= 0)
+ $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.
+ FETCHARG:TYPE : Set TYPE as the type of FETCHARG. Currently, basic types
+ (u8/u16/u32/u64/s8/s16/s32/s64), "string" and bitfield
+ are supported.
+
+ (*) only for return probe.
+ (**) this is useful for fetching a field of data structures.
+
+Types
+-----
+Several types are supported for fetch-args. Uprobe tracer will access memory
+by given type. Prefix 's' and 'u' means those types are signed and unsigned
+respectively. Traced arguments are shown in decimal (signed) or hex (unsigned).
+String type is a special type, which fetches a "null-terminated" string from
+user space.
+Bitfield is another special type, which takes 3 parameters, bit-width, bit-
+offset, and container-size (usually 32). The syntax is;
+
+ b<bit-width>@<bit-offset>/<container-size>
+

Event Profiling
---------------
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 8f7a2b6d389d..a130d612e705 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -255,12 +255,18 @@ fail:
}

/* Special function : only accept unsigned long */
-static __kprobes void fetch_stack_address(struct pt_regs *regs,
- void *dummy, void *dest)
+static __kprobes void fetch_kernel_stack_address(struct pt_regs *regs,
+ void *dummy, void *dest)
{
*(unsigned long *)dest = kernel_stack_pointer(regs);
}

+static __kprobes void fetch_user_stack_address(struct pt_regs *regs,
+ void *dummy, void *dest)
+{
+ *(unsigned long *)dest = user_stack_pointer(regs);
+}
+
static fetch_func_t get_fetch_size_function(const struct fetch_type *type,
fetch_func_t orig_fn,
const struct fetch_type *ftbl)
@@ -305,7 +311,8 @@ int traceprobe_split_symbol_offset(char *symbol, unsigned long *offset)
#define PARAM_MAX_STACK (THREAD_SIZE / sizeof(unsigned long))

static int parse_probe_vars(char *arg, const struct fetch_type *t,
- struct fetch_param *f, bool is_return)
+ struct fetch_param *f, bool is_return,
+ bool is_kprobe)
{
int ret = 0;
unsigned long param;
@@ -317,13 +324,16 @@ static int parse_probe_vars(char *arg, const struct fetch_type *t,
ret = -EINVAL;
} else if (strncmp(arg, "stack", 5) == 0) {
if (arg[5] == '\0') {
- if (strcmp(t->name, DEFAULT_FETCH_TYPE_STR) == 0)
- f->fn = fetch_stack_address;
+ if (strcmp(t->name, DEFAULT_FETCH_TYPE_STR))
+ return -EINVAL;
+
+ if (is_kprobe)
+ f->fn = fetch_kernel_stack_address;
else
- ret = -EINVAL;
+ f->fn = fetch_user_stack_address;
} else if (isdigit(arg[5])) {
ret = kstrtoul(arg + 5, 10, &param);
- if (ret || param > PARAM_MAX_STACK)
+ if (ret || (is_kprobe && param > PARAM_MAX_STACK))
ret = -EINVAL;
else {
f->fn = t->fetch[FETCH_MTD_stack];
@@ -350,13 +360,9 @@ static int parse_probe_arg(char *arg, const struct fetch_type *t,
ftbl = is_kprobe ? kprobes_fetch_type_table : uprobes_fetch_type_table;
BUG_ON(ftbl == NULL);

- /* Until uprobe_events supports only reg arguments */
- if (!is_kprobe && arg[0] != '%')
- return -EINVAL;
-
switch (arg[0]) {
case '$':
- ret = parse_probe_vars(arg + 1, t, f, is_return);
+ ret = parse_probe_vars(arg + 1, t, f, is_return, is_kprobe);
break;

case '%': /* named register */
@@ -377,6 +383,10 @@ static int parse_probe_arg(char *arg, const struct fetch_type *t,
f->fn = t->fetch[FETCH_MTD_memory];
f->data = (void *)param;
} else {
+ /* uprobes don't support symbols */
+ if (!is_kprobe)
+ return -EINVAL;
+
ret = traceprobe_split_symbol_offset(arg + 1, &offset);
if (ret)
break;

Namhyung Kim

unread,
Dec 15, 2013, 11:40:03 PM12/15/13
to
From: Namhyung Kim <namhyu...@lge.com>

The print format of s32 type was "ld" and it's casted to "long". So
it turned out to print 4294967295 for "-1" on 64-bit systems. Not
sure whether it worked well on 32-bit systems.

Anyway, it doesn't need to have cast argument at all since it already
casted using type pointer - just get rid of it. Thanks to Oleg for
pointing that out.

And print 0x prefix for unsigned type as it shows hex numbers.

Suggested-by: Oleg Nesterov <ol...@redhat.com>
Acked-by: Masami Hiramatsu <masami.hi...@hitachi.com>
Cc: Srikar Dronamraju <sri...@linux.vnet.ibm.com>
Cc: Oleg Nesterov <ol...@redhat.com>
Cc: zhangwei(Jovi) <jovi.z...@huawei.com>
Cc: Arnaldo Carvalho de Melo <ac...@ghostprotocols.net>
Signed-off-by: Namhyung Kim <namh...@kernel.org>
---
kernel/trace/trace_probe.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 412e959709b4..430505b08a6f 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -40,23 +40,23 @@ const char *reserved_field_names[] = {
#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) \
+#define DEFINE_BASIC_PRINT_TYPE_FUNC(type, fmt) \
static __kprobes int PRINT_TYPE_FUNC_NAME(type)(struct trace_seq *s, \
const char *name, \
- void *data, void *ent)\
+ void *data, void *ent) \
{ \
- return trace_seq_printf(s, " %s=" fmt, name, (cast)*(type *)data);\
+ return trace_seq_printf(s, " %s=" fmt, name, *(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)
+DEFINE_BASIC_PRINT_TYPE_FUNC(u8 , "0x%x")
+DEFINE_BASIC_PRINT_TYPE_FUNC(u16, "0x%x")
+DEFINE_BASIC_PRINT_TYPE_FUNC(u32, "0x%x")
+DEFINE_BASIC_PRINT_TYPE_FUNC(u64, "0x%Lx")
+DEFINE_BASIC_PRINT_TYPE_FUNC(s8, "%d")
+DEFINE_BASIC_PRINT_TYPE_FUNC(s16, "%d")
+DEFINE_BASIC_PRINT_TYPE_FUNC(s32, "%d")
+DEFINE_BASIC_PRINT_TYPE_FUNC(s64, "%Ld")

static inline void *get_rloc_data(u32 *dl)
{

Namhyung Kim

unread,
Dec 15, 2013, 11:40:03 PM12/15/13
to
From: Namhyung Kim <namhyu...@lge.com>

There are functions that can be shared to both of kprobes and uprobes.
Separate common data structure to struct trace_probe and use it from
the shared functions.

Acked-by: Masami Hiramatsu <masami.hi...@hitachi.com>
Cc: Srikar Dronamraju <sri...@linux.vnet.ibm.com>
Cc: Oleg Nesterov <ol...@redhat.com>
Cc: zhangwei(Jovi) <jovi.z...@huawei.com>
Cc: Arnaldo Carvalho de Melo <ac...@ghostprotocols.net>
Signed-off-by: Namhyung Kim <namh...@kernel.org>
---
kernel/trace/trace_kprobe.c | 548 ++++++++++++++++++++++----------------------
kernel/trace/trace_probe.h | 20 ++
2 files changed, 289 insertions(+), 279 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index dae9541ada9e..db922fd74b05 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -27,18 +27,12 @@
/**
* Kprobe event core functions
*/
-struct trace_probe {
+struct trace_kprobe {
struct list_head list;
struct kretprobe rp; /* Use rp.kp for kprobe use */
unsigned long nhit;
- unsigned int flags; /* For TP_FLAG_* */
const char *symbol; /* symbol name */
- struct ftrace_event_class class;
- struct ftrace_event_call call;
- struct list_head files;
- ssize_t size; /* trace entry size */
- unsigned int nr_args;
- struct probe_arg args[];
+ struct trace_probe tp;
};

struct event_file_link {
@@ -46,56 +40,46 @@ struct event_file_link {
struct list_head list;
};

-#define SIZEOF_TRACE_PROBE(n) \
- (offsetof(struct trace_probe, args) + \
+#define SIZEOF_TRACE_KPROBE(n) \
+ (offsetof(struct trace_kprobe, tp.args) + \
(sizeof(struct probe_arg) * (n)))


-static __kprobes bool trace_probe_is_return(struct trace_probe *tp)
+static __kprobes bool trace_kprobe_is_return(struct trace_kprobe *tk)
{
- return tp->rp.handler != NULL;
+ return tk->rp.handler != NULL;
}

-static __kprobes const char *trace_probe_symbol(struct trace_probe *tp)
+static __kprobes const char *trace_kprobe_symbol(struct trace_kprobe *tk)
{
- return tp->symbol ? tp->symbol : "unknown";
+ return tk->symbol ? tk->symbol : "unknown";
}

-static __kprobes unsigned long trace_probe_offset(struct trace_probe *tp)
+static __kprobes unsigned long trace_kprobe_offset(struct trace_kprobe *tk)
{
- return tp->rp.kp.offset;
+ return tk->rp.kp.offset;
}

-static __kprobes bool trace_probe_is_enabled(struct trace_probe *tp)
+static __kprobes bool trace_kprobe_has_gone(struct trace_kprobe *tk)
{
- return !!(tp->flags & (TP_FLAG_TRACE | TP_FLAG_PROFILE));
+ return !!(kprobe_gone(&tk->rp.kp));
}

-static __kprobes bool trace_probe_is_registered(struct trace_probe *tp)
-{
- return !!(tp->flags & TP_FLAG_REGISTERED);
-}
-
-static __kprobes bool trace_probe_has_gone(struct trace_probe *tp)
-{
- return !!(kprobe_gone(&tp->rp.kp));
-}
-
-static __kprobes bool trace_probe_within_module(struct trace_probe *tp,
- struct module *mod)
+static __kprobes bool trace_kprobe_within_module(struct trace_kprobe *tk,
+ struct module *mod)
{
int len = strlen(mod->name);
- const char *name = trace_probe_symbol(tp);
+ const char *name = trace_kprobe_symbol(tk);
return strncmp(mod->name, name, len) == 0 && name[len] == ':';
}

-static __kprobes bool trace_probe_is_on_module(struct trace_probe *tp)
+static __kprobes bool trace_kprobe_is_on_module(struct trace_kprobe *tk)
{
- return !!strchr(trace_probe_symbol(tp), ':');
+ return !!strchr(trace_kprobe_symbol(tk), ':');
}

-static int register_probe_event(struct trace_probe *tp);
-static int unregister_probe_event(struct trace_probe *tp);
+static int register_kprobe_event(struct trace_kprobe *tk);
+static int unregister_kprobe_event(struct trace_kprobe *tk);

static DEFINE_MUTEX(probe_lock);
static LIST_HEAD(probe_list);
@@ -107,42 +91,42 @@ static int kretprobe_dispatcher(struct kretprobe_instance *ri,
/*
* Allocate new trace_probe and initialize it (including kprobes).
*/
-static struct trace_probe *alloc_trace_probe(const char *group,
+static struct trace_kprobe *alloc_trace_kprobe(const char *group,
const char *event,
void *addr,
const char *symbol,
unsigned long offs,
int nargs, bool is_return)
{
- struct trace_probe *tp;
+ struct trace_kprobe *tk;
int ret = -ENOMEM;

- tp = kzalloc(SIZEOF_TRACE_PROBE(nargs), GFP_KERNEL);
- if (!tp)
+ tk = kzalloc(SIZEOF_TRACE_KPROBE(nargs), GFP_KERNEL);
+ if (!tk)
return ERR_PTR(ret);

if (symbol) {
- tp->symbol = kstrdup(symbol, GFP_KERNEL);
- if (!tp->symbol)
+ tk->symbol = kstrdup(symbol, GFP_KERNEL);
+ if (!tk->symbol)
goto error;
- tp->rp.kp.symbol_name = tp->symbol;
- tp->rp.kp.offset = offs;
+ tk->rp.kp.symbol_name = tk->symbol;
+ tk->rp.kp.offset = offs;
} else
- tp->rp.kp.addr = addr;
+ tk->rp.kp.addr = addr;

if (is_return)
- tp->rp.handler = kretprobe_dispatcher;
+ tk->rp.handler = kretprobe_dispatcher;
else
- tp->rp.kp.pre_handler = kprobe_dispatcher;
+ tk->rp.kp.pre_handler = kprobe_dispatcher;

if (!event || !is_good_name(event)) {
ret = -EINVAL;
goto error;
}

- tp->call.class = &tp->class;
- tp->call.name = kstrdup(event, GFP_KERNEL);
- if (!tp->call.name)
+ tk->tp.call.class = &tk->tp.class;
+ tk->tp.call.name = kstrdup(event, GFP_KERNEL);
+ if (!tk->tp.call.name)
goto error;

if (!group || !is_good_name(group)) {
@@ -150,42 +134,42 @@ static struct trace_probe *alloc_trace_probe(const char *group,
goto error;
}

- tp->class.system = kstrdup(group, GFP_KERNEL);
- if (!tp->class.system)
+ tk->tp.class.system = kstrdup(group, GFP_KERNEL);
+ if (!tk->tp.class.system)
goto error;

- INIT_LIST_HEAD(&tp->list);
- INIT_LIST_HEAD(&tp->files);
- return tp;
+ INIT_LIST_HEAD(&tk->list);
+ INIT_LIST_HEAD(&tk->tp.files);
+ return tk;
error:
- kfree(tp->call.name);
- kfree(tp->symbol);
- kfree(tp);
+ kfree(tk->tp.call.name);
+ kfree(tk->symbol);
+ kfree(tk);
return ERR_PTR(ret);
}

-static void free_trace_probe(struct trace_probe *tp)
+static void free_trace_kprobe(struct trace_kprobe *tk)
{
int i;

- for (i = 0; i < tp->nr_args; i++)
- traceprobe_free_probe_arg(&tp->args[i]);
+ for (i = 0; i < tk->tp.nr_args; i++)
+ traceprobe_free_probe_arg(&tk->tp.args[i]);

- kfree(tp->call.class->system);
- kfree(tp->call.name);
- kfree(tp->symbol);
- kfree(tp);
+ kfree(tk->tp.call.class->system);
+ kfree(tk->tp.call.name);
+ kfree(tk->symbol);
+ kfree(tk);
}

-static struct trace_probe *find_trace_probe(const char *event,
- const char *group)
+static struct trace_kprobe *find_trace_kprobe(const char *event,
+ const char *group)
{
- struct trace_probe *tp;
+ struct trace_kprobe *tk;

- list_for_each_entry(tp, &probe_list, list)
- if (strcmp(tp->call.name, event) == 0 &&
- strcmp(tp->call.class->system, group) == 0)
- return tp;
+ list_for_each_entry(tk, &probe_list, list)
+ if (strcmp(tk->tp.call.name, event) == 0 &&
+ strcmp(tk->tp.call.class->system, group) == 0)
+ return tk;
return NULL;
}

@@ -194,7 +178,7 @@ static struct trace_probe *find_trace_probe(const char *event,
* if the file is NULL, enable "perf" handler, or enable "trace" handler.
*/
static int
-enable_trace_probe(struct trace_probe *tp, struct ftrace_event_file *file)
+enable_trace_kprobe(struct trace_kprobe *tk, struct ftrace_event_file *file)
{
int ret = 0;

@@ -208,17 +192,17 @@ enable_trace_probe(struct trace_probe *tp, struct ftrace_event_file *file)
}

link->file = file;
- list_add_tail_rcu(&link->list, &tp->files);
+ list_add_tail_rcu(&link->list, &tk->tp.files);

- tp->flags |= TP_FLAG_TRACE;
+ tk->tp.flags |= TP_FLAG_TRACE;
} else
- tp->flags |= TP_FLAG_PROFILE;
+ tk->tp.flags |= TP_FLAG_PROFILE;

- if (trace_probe_is_registered(tp) && !trace_probe_has_gone(tp)) {
- if (trace_probe_is_return(tp))
- ret = enable_kretprobe(&tp->rp);
+ if (trace_probe_is_registered(&tk->tp) && !trace_kprobe_has_gone(tk)) {
+ if (trace_kprobe_is_return(tk))
+ ret = enable_kretprobe(&tk->rp);
else
- ret = enable_kprobe(&tp->rp.kp);
+ ret = enable_kprobe(&tk->rp.kp);
}
out:
return ret;
@@ -241,14 +225,14 @@ find_event_file_link(struct trace_probe *tp, struct ftrace_event_file *file)
* if the file is NULL, disable "perf" handler, or disable "trace" handler.
*/
static int
-disable_trace_probe(struct trace_probe *tp, struct ftrace_event_file *file)
+disable_trace_kprobe(struct trace_kprobe *tk, struct ftrace_event_file *file)
{
struct event_file_link *link = NULL;
int wait = 0;
int ret = 0;

if (file) {
- link = find_event_file_link(tp, file);
+ link = find_event_file_link(&tk->tp, file);
if (!link) {
ret = -EINVAL;
goto out;
@@ -256,18 +240,18 @@ disable_trace_probe(struct trace_probe *tp, struct ftrace_event_file *file)

list_del_rcu(&link->list);
wait = 1;
- if (!list_empty(&tp->files))
+ if (!list_empty(&tk->tp.files))
goto out;

- tp->flags &= ~TP_FLAG_TRACE;
+ tk->tp.flags &= ~TP_FLAG_TRACE;
} else
- tp->flags &= ~TP_FLAG_PROFILE;
+ tk->tp.flags &= ~TP_FLAG_PROFILE;

- if (!trace_probe_is_enabled(tp) && trace_probe_is_registered(tp)) {
- if (trace_probe_is_return(tp))
- disable_kretprobe(&tp->rp);
+ if (!trace_probe_is_enabled(&tk->tp) && trace_probe_is_registered(&tk->tp)) {
+ if (trace_kprobe_is_return(tk))
+ disable_kretprobe(&tk->rp);
else
- disable_kprobe(&tp->rp.kp);
+ disable_kprobe(&tk->rp.kp);
wait = 1;
}
out:
@@ -288,40 +272,40 @@ disable_trace_probe(struct trace_probe *tp, struct ftrace_event_file *file)
}

/* Internal register function - just handle k*probes and flags */
-static int __register_trace_probe(struct trace_probe *tp)
+static int __register_trace_kprobe(struct trace_kprobe *tk)
{
int i, ret;

- if (trace_probe_is_registered(tp))
+ if (trace_probe_is_registered(&tk->tp))
return -EINVAL;

- for (i = 0; i < tp->nr_args; i++)
- traceprobe_update_arg(&tp->args[i]);
+ for (i = 0; i < tk->tp.nr_args; i++)
+ traceprobe_update_arg(&tk->tp.args[i]);

/* Set/clear disabled flag according to tp->flag */
- if (trace_probe_is_enabled(tp))
- tp->rp.kp.flags &= ~KPROBE_FLAG_DISABLED;
+ if (trace_probe_is_enabled(&tk->tp))
+ tk->rp.kp.flags &= ~KPROBE_FLAG_DISABLED;
else
- tp->rp.kp.flags |= KPROBE_FLAG_DISABLED;
+ tk->rp.kp.flags |= KPROBE_FLAG_DISABLED;

- if (trace_probe_is_return(tp))
- ret = register_kretprobe(&tp->rp);
+ if (trace_kprobe_is_return(tk))
+ ret = register_kretprobe(&tk->rp);
else
- ret = register_kprobe(&tp->rp.kp);
+ ret = register_kprobe(&tk->rp.kp);

if (ret == 0)
- tp->flags |= TP_FLAG_REGISTERED;
+ tk->tp.flags |= TP_FLAG_REGISTERED;
else {
pr_warning("Could not insert probe at %s+%lu: %d\n",
- trace_probe_symbol(tp), trace_probe_offset(tp), ret);
- if (ret == -ENOENT && trace_probe_is_on_module(tp)) {
+ trace_kprobe_symbol(tk), trace_kprobe_offset(tk), ret);
+ if (ret == -ENOENT && trace_kprobe_is_on_module(tk)) {
pr_warning("This probe might be able to register after"
"target module is loaded. Continue.\n");
ret = 0;
} else if (ret == -EILSEQ) {
pr_warning("Probing address(0x%p) is not an "
"instruction boundary.\n",
- tp->rp.kp.addr);
+ tk->rp.kp.addr);
ret = -EINVAL;
}
}
@@ -330,67 +314,67 @@ static int __register_trace_probe(struct trace_probe *tp)
}

/* Internal unregister function - just handle k*probes and flags */
-static void __unregister_trace_probe(struct trace_probe *tp)
+static void __unregister_trace_kprobe(struct trace_kprobe *tk)
{
- if (trace_probe_is_registered(tp)) {
- if (trace_probe_is_return(tp))
- unregister_kretprobe(&tp->rp);
+ if (trace_probe_is_registered(&tk->tp)) {
+ if (trace_kprobe_is_return(tk))
+ unregister_kretprobe(&tk->rp);
else
- unregister_kprobe(&tp->rp.kp);
- tp->flags &= ~TP_FLAG_REGISTERED;
+ unregister_kprobe(&tk->rp.kp);
+ tk->tp.flags &= ~TP_FLAG_REGISTERED;
/* Cleanup kprobe for reuse */
- if (tp->rp.kp.symbol_name)
- tp->rp.kp.addr = NULL;
+ if (tk->rp.kp.symbol_name)
+ tk->rp.kp.addr = NULL;
}
}

/* Unregister a trace_probe and probe_event: call with locking probe_lock */
-static int unregister_trace_probe(struct trace_probe *tp)
+static int unregister_trace_kprobe(struct trace_kprobe *tk)
{
/* Enabled event can not be unregistered */
- if (trace_probe_is_enabled(tp))
+ if (trace_probe_is_enabled(&tk->tp))
return -EBUSY;

/* Will fail if probe is being used by ftrace or perf */
- if (unregister_probe_event(tp))
+ if (unregister_kprobe_event(tk))
return -EBUSY;

- __unregister_trace_probe(tp);
- list_del(&tp->list);
+ __unregister_trace_kprobe(tk);
+ list_del(&tk->list);

return 0;
}

/* Register a trace_probe and probe_event */
-static int register_trace_probe(struct trace_probe *tp)
+static int register_trace_kprobe(struct trace_kprobe *tk)
{
- struct trace_probe *old_tp;
+ struct trace_kprobe *old_tk;
int ret;

mutex_lock(&probe_lock);

/* Delete old (same name) event if exist */
- old_tp = find_trace_probe(tp->call.name, tp->call.class->system);
- if (old_tp) {
- ret = unregister_trace_probe(old_tp);
+ old_tk = find_trace_kprobe(tk->tp.call.name, tk->tp.call.class->system);
+ if (old_tk) {
+ ret = unregister_trace_kprobe(old_tk);
if (ret < 0)
goto end;
- free_trace_probe(old_tp);
+ free_trace_kprobe(old_tk);
}

/* Register new event */
- ret = register_probe_event(tp);
+ ret = register_kprobe_event(tk);
if (ret) {
pr_warning("Failed to register probe event(%d)\n", ret);
goto end;
}

/* Register k*probe */
- ret = __register_trace_probe(tp);
+ ret = __register_trace_kprobe(tk);
if (ret < 0)
- unregister_probe_event(tp);
+ unregister_kprobe_event(tk);
else
- list_add_tail(&tp->list, &probe_list);
+ list_add_tail(&tk->list, &probe_list);

end:
mutex_unlock(&probe_lock);
@@ -398,11 +382,11 @@ end:
}

/* Module notifier call back, checking event on the module */
-static int trace_probe_module_callback(struct notifier_block *nb,
+static int trace_kprobe_module_callback(struct notifier_block *nb,
unsigned long val, void *data)
{
struct module *mod = data;
- struct trace_probe *tp;
+ struct trace_kprobe *tk;
int ret;

if (val != MODULE_STATE_COMING)
@@ -410,15 +394,15 @@ static int trace_probe_module_callback(struct notifier_block *nb,

/* Update probes on coming module */
mutex_lock(&probe_lock);
- list_for_each_entry(tp, &probe_list, list) {
- if (trace_probe_within_module(tp, mod)) {
+ list_for_each_entry(tk, &probe_list, list) {
+ if (trace_kprobe_within_module(tk, mod)) {
/* Don't need to check busy - this should have gone. */
- __unregister_trace_probe(tp);
- ret = __register_trace_probe(tp);
+ __unregister_trace_kprobe(tk);
+ ret = __register_trace_kprobe(tk);
if (ret)
pr_warning("Failed to re-register probe %s on"
"%s: %d\n",
- tp->call.name, mod->name, ret);
+ tk->tp.call.name, mod->name, ret);
}
}
mutex_unlock(&probe_lock);
@@ -426,12 +410,12 @@ static int trace_probe_module_callback(struct notifier_block *nb,
return NOTIFY_DONE;
}

-static struct notifier_block trace_probe_module_nb = {
- .notifier_call = trace_probe_module_callback,
+static struct notifier_block trace_kprobe_module_nb = {
+ .notifier_call = trace_kprobe_module_callback,
.priority = 1 /* Invoked after kprobe module callback */
};

-static int create_trace_probe(int argc, char **argv)
+static int create_trace_kprobe(int argc, char **argv)
{
/*
* Argument syntax:
@@ -451,7 +435,7 @@ static int create_trace_probe(int argc, char **argv)
* Type of args:
* FETCHARG:TYPE : use TYPE instead of unsigned long.
*/
- struct trace_probe *tp;
+ struct trace_kprobe *tk;
int i, ret = 0;
bool is_return = false, is_delete = false;
char *symbol = NULL, *event = NULL, *group = NULL;
@@ -498,16 +482,16 @@ static int create_trace_probe(int argc, char **argv)
return -EINVAL;
}
mutex_lock(&probe_lock);
- tp = find_trace_probe(event, group);
- if (!tp) {
+ tk = find_trace_kprobe(event, group);
+ if (!tk) {
mutex_unlock(&probe_lock);
pr_info("Event %s/%s doesn't exist.\n", group, event);
return -ENOENT;
}
/* delete an event */
- ret = unregister_trace_probe(tp);
+ ret = unregister_trace_kprobe(tk);
if (ret == 0)
- free_trace_probe(tp);
+ free_trace_kprobe(tk);
mutex_unlock(&probe_lock);
return ret;
}
@@ -554,47 +538,49 @@ static int create_trace_probe(int argc, char **argv)
is_return ? 'r' : 'p', addr);
event = buf;
}
- tp = alloc_trace_probe(group, event, addr, symbol, offset, argc,
+ tk = alloc_trace_kprobe(group, event, addr, symbol, offset, argc,
is_return);
- if (IS_ERR(tp)) {
+ if (IS_ERR(tk)) {
pr_info("Failed to allocate trace_probe.(%d)\n",
- (int)PTR_ERR(tp));
- return PTR_ERR(tp);
+ (int)PTR_ERR(tk));
+ return PTR_ERR(tk);
}

/* parse arguments */
ret = 0;
for (i = 0; i < argc && i < MAX_TRACE_ARGS; i++) {
+ struct probe_arg *parg = &tk->tp.args[i];
+
/* Increment count for freeing args in error case */
- tp->nr_args++;
+ tk->tp.nr_args++;

/* Parse argument name */
arg = strchr(argv[i], '=');
if (arg) {
*arg++ = '\0';
- tp->args[i].name = kstrdup(argv[i], GFP_KERNEL);
+ parg->name = kstrdup(argv[i], GFP_KERNEL);
} else {
arg = argv[i];
/* If argument name is omitted, set "argN" */
snprintf(buf, MAX_EVENT_NAME_LEN, "arg%d", i + 1);
- tp->args[i].name = kstrdup(buf, GFP_KERNEL);
+ parg->name = kstrdup(buf, GFP_KERNEL);
}

- if (!tp->args[i].name) {
+ if (!parg->name) {
pr_info("Failed to allocate argument[%d] name.\n", i);
ret = -ENOMEM;
goto error;
}

- if (!is_good_name(tp->args[i].name)) {
+ if (!is_good_name(parg->name)) {
pr_info("Invalid argument[%d] name: %s\n",
- i, tp->args[i].name);
+ i, parg->name);
ret = -EINVAL;
goto error;
}

- if (traceprobe_conflict_field_name(tp->args[i].name,
- tp->args, i)) {
+ if (traceprobe_conflict_field_name(parg->name,
+ tk->tp.args, i)) {
pr_info("Argument[%d] name '%s' conflicts with "
"another field.\n", i, argv[i]);
ret = -EINVAL;
@@ -602,7 +588,7 @@ static int create_trace_probe(int argc, char **argv)
}

/* Parse fetch argument */
- ret = traceprobe_parse_probe_arg(arg, &tp->size, &tp->args[i],
+ ret = traceprobe_parse_probe_arg(arg, &tk->tp.size, parg,
is_return, true);
if (ret) {
pr_info("Parse error at argument[%d]. (%d)\n", i, ret);
@@ -610,35 +596,35 @@ static int create_trace_probe(int argc, char **argv)
}
}

- ret = register_trace_probe(tp);
+ ret = register_trace_kprobe(tk);
if (ret)
goto error;
return 0;

error:
- free_trace_probe(tp);
+ free_trace_kprobe(tk);
return ret;
}

-static int release_all_trace_probes(void)
+static int release_all_trace_kprobes(void)
{
- struct trace_probe *tp;
+ struct trace_kprobe *tk;
int ret = 0;

mutex_lock(&probe_lock);
/* Ensure no probe is in use. */
- list_for_each_entry(tp, &probe_list, list)
- if (trace_probe_is_enabled(tp)) {
+ list_for_each_entry(tk, &probe_list, list)
+ if (trace_probe_is_enabled(&tk->tp)) {
ret = -EBUSY;
goto end;
}
/* TODO: Use batch unregistration */
while (!list_empty(&probe_list)) {
- tp = list_entry(probe_list.next, struct trace_probe, list);
- ret = unregister_trace_probe(tp);
+ tk = list_entry(probe_list.next, struct trace_kprobe, list);
+ ret = unregister_trace_kprobe(tk);
if (ret)
goto end;
- free_trace_probe(tp);
+ free_trace_kprobe(tk);
}

end:
@@ -666,22 +652,22 @@ 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;
+ struct trace_kprobe *tk = v;
int i;

- seq_printf(m, "%c", trace_probe_is_return(tp) ? 'r' : 'p');
- seq_printf(m, ":%s/%s", tp->call.class->system, tp->call.name);
+ seq_printf(m, "%c", trace_kprobe_is_return(tk) ? 'r' : 'p');
+ seq_printf(m, ":%s/%s", tk->tp.call.class->system, tk->tp.call.name);

- if (!tp->symbol)
- seq_printf(m, " 0x%p", tp->rp.kp.addr);
- else if (tp->rp.kp.offset)
- seq_printf(m, " %s+%u", trace_probe_symbol(tp),
- tp->rp.kp.offset);
+ if (!tk->symbol)
+ seq_printf(m, " 0x%p", tk->rp.kp.addr);
+ else if (tk->rp.kp.offset)
+ seq_printf(m, " %s+%u", trace_kprobe_symbol(tk),
+ tk->rp.kp.offset);
else
- seq_printf(m, " %s", trace_probe_symbol(tp));
+ seq_printf(m, " %s", trace_kprobe_symbol(tk));

- for (i = 0; i < tp->nr_args; i++)
- seq_printf(m, " %s=%s", tp->args[i].name, tp->args[i].comm);
+ for (i = 0; i < tk->tp.nr_args; i++)
+ seq_printf(m, " %s=%s", tk->tp.args[i].name, tk->tp.args[i].comm);
seq_printf(m, "\n");

return 0;
@@ -699,7 +685,7 @@ static int probes_open(struct inode *inode, struct file *file)
int ret;

if ((file->f_mode & FMODE_WRITE) && (file->f_flags & O_TRUNC)) {
- ret = release_all_trace_probes();
+ ret = release_all_trace_kprobes();
if (ret < 0)
return ret;
}
@@ -711,7 +697,7 @@ static ssize_t probes_write(struct file *file, const char __user *buffer,
size_t count, loff_t *ppos)
{
return traceprobe_probes_write(file, buffer, count, ppos,
- create_trace_probe);
+ create_trace_kprobe);
}

static const struct file_operations kprobe_events_ops = {
@@ -726,10 +712,10 @@ static const struct file_operations kprobe_events_ops = {
/* Probes profiling interfaces */
static int probes_profile_seq_show(struct seq_file *m, void *v)
{
- struct trace_probe *tp = v;
+ struct trace_kprobe *tk = v;

- seq_printf(m, " %-44s %15lu %15lu\n", tp->call.name, tp->nhit,
- tp->rp.kp.nmissed);
+ seq_printf(m, " %-44s %15lu %15lu\n", tk->tp.call.name, tk->nhit,
+ tk->rp.kp.nmissed);

return 0;
}
@@ -804,7 +790,7 @@ static __kprobes void store_trace_args(int ent_size, struct trace_probe *tp,

/* Kprobe handler */
static __kprobes void
-__kprobe_trace_func(struct trace_probe *tp, struct pt_regs *regs,
+__kprobe_trace_func(struct trace_kprobe *tk, struct pt_regs *regs,
struct ftrace_event_file *ftrace_file)
{
struct kprobe_trace_entry_head *entry;
@@ -812,7 +798,7 @@ __kprobe_trace_func(struct trace_probe *tp, struct pt_regs *regs,
struct ring_buffer *buffer;
int size, dsize, pc;
unsigned long irq_flags;
- struct ftrace_event_call *call = &tp->call;
+ struct ftrace_event_call *call = &tk->tp.call;

WARN_ON(call != ftrace_file->event_call);

@@ -822,8 +808,8 @@ __kprobe_trace_func(struct trace_probe *tp, struct pt_regs *regs,
local_save_flags(irq_flags);
pc = preempt_count();

- dsize = __get_data_size(tp, regs);
- size = sizeof(*entry) + tp->size + dsize;
+ dsize = __get_data_size(&tk->tp, regs);
+ size = sizeof(*entry) + tk->tp.size + dsize;

event = trace_event_buffer_lock_reserve(&buffer, ftrace_file,
call->event.type,
@@ -832,8 +818,8 @@ __kprobe_trace_func(struct trace_probe *tp, struct pt_regs *regs,
return;

entry = ring_buffer_event_data(event);
- entry->ip = (unsigned long)tp->rp.kp.addr;
- store_trace_args(sizeof(*entry), tp, regs, (u8 *)&entry[1], dsize);
+ entry->ip = (unsigned long)tk->rp.kp.addr;
+ store_trace_args(sizeof(*entry), &tk->tp, regs, (u8 *)&entry[1], dsize);

if (!filter_check_discard(ftrace_file, entry, buffer, event))
trace_buffer_unlock_commit_regs(buffer, event,
@@ -841,17 +827,17 @@ __kprobe_trace_func(struct trace_probe *tp, struct pt_regs *regs,
}

static __kprobes void
-kprobe_trace_func(struct trace_probe *tp, struct pt_regs *regs)
+kprobe_trace_func(struct trace_kprobe *tk, struct pt_regs *regs)
{
struct event_file_link *link;

- list_for_each_entry_rcu(link, &tp->files, list)
- __kprobe_trace_func(tp, regs, link->file);
+ list_for_each_entry_rcu(link, &tk->tp.files, list)
+ __kprobe_trace_func(tk, regs, link->file);
}

/* Kretprobe handler */
static __kprobes void
-__kretprobe_trace_func(struct trace_probe *tp, struct kretprobe_instance *ri,
+__kretprobe_trace_func(struct trace_kprobe *tk, struct kretprobe_instance *ri,
struct pt_regs *regs,
struct ftrace_event_file *ftrace_file)
{
@@ -860,7 +846,7 @@ __kretprobe_trace_func(struct trace_probe *tp, struct kretprobe_instance *ri,
struct ring_buffer *buffer;
int size, pc, dsize;
unsigned long irq_flags;
- struct ftrace_event_call *call = &tp->call;
+ struct ftrace_event_call *call = &tk->tp.call;

WARN_ON(call != ftrace_file->event_call);

@@ -870,8 +856,8 @@ __kretprobe_trace_func(struct trace_probe *tp, struct kretprobe_instance *ri,
local_save_flags(irq_flags);
pc = preempt_count();

- dsize = __get_data_size(tp, regs);
- size = sizeof(*entry) + tp->size + dsize;
+ dsize = __get_data_size(&tk->tp, regs);
+ size = sizeof(*entry) + tk->tp.size + dsize;

event = trace_event_buffer_lock_reserve(&buffer, ftrace_file,
call->event.type,
@@ -880,9 +866,9 @@ __kretprobe_trace_func(struct trace_probe *tp, struct kretprobe_instance *ri,
return;

entry = ring_buffer_event_data(event);
- entry->func = (unsigned long)tp->rp.kp.addr;
+ entry->func = (unsigned long)tk->rp.kp.addr;
entry->ret_ip = (unsigned long)ri->ret_addr;
- store_trace_args(sizeof(*entry), tp, regs, (u8 *)&entry[1], dsize);
+ store_trace_args(sizeof(*entry), &tk->tp, regs, (u8 *)&entry[1], dsize);

if (!filter_check_discard(ftrace_file, entry, buffer, event))
trace_buffer_unlock_commit_regs(buffer, event,
@@ -890,13 +876,13 @@ __kretprobe_trace_func(struct trace_probe *tp, struct kretprobe_instance *ri,
}

static __kprobes void
-kretprobe_trace_func(struct trace_probe *tp, struct kretprobe_instance *ri,
+kretprobe_trace_func(struct trace_kprobe *tk, struct kretprobe_instance *ri,
struct pt_regs *regs)
{
struct event_file_link *link;

- list_for_each_entry_rcu(link, &tp->files, list)
- __kretprobe_trace_func(tp, ri, regs, link->file);
+ list_for_each_entry_rcu(link, &tk->tp.files, list)
+ __kretprobe_trace_func(tk, ri, regs, link->file);
}

/* Event entry printers */
@@ -983,16 +969,18 @@ static int kprobe_event_define_fields(struct ftrace_event_call *event_call)
{
int ret, i;
struct kprobe_trace_entry_head field;
- struct trace_probe *tp = (struct trace_probe *)event_call->data;
+ struct trace_kprobe *tk = (struct trace_kprobe *)event_call->data;

DEFINE_FIELD(unsigned long, ip, FIELD_STRING_IP, 0);
/* Set argument names as fields */
- for (i = 0; i < tp->nr_args; i++) {
- ret = trace_define_field(event_call, tp->args[i].type->fmttype,
- tp->args[i].name,
- sizeof(field) + tp->args[i].offset,
- tp->args[i].type->size,
- tp->args[i].type->is_signed,
+ for (i = 0; i < tk->tp.nr_args; i++) {
+ struct probe_arg *parg = &tk->tp.args[i];
+
+ ret = trace_define_field(event_call, parg->type->fmttype,
+ parg->name,
+ sizeof(field) + parg->offset,
+ parg->type->size,
+ parg->type->is_signed,
FILTER_OTHER);
if (ret)
return ret;
@@ -1004,17 +992,19 @@ static int kretprobe_event_define_fields(struct ftrace_event_call *event_call)
{
int ret, i;
struct kretprobe_trace_entry_head field;
- struct trace_probe *tp = (struct trace_probe *)event_call->data;
+ struct trace_kprobe *tk = (struct trace_kprobe *)event_call->data;

DEFINE_FIELD(unsigned long, func, FIELD_STRING_FUNC, 0);
DEFINE_FIELD(unsigned long, ret_ip, FIELD_STRING_RETIP, 0);
/* Set argument names as fields */
- for (i = 0; i < tp->nr_args; i++) {
- ret = trace_define_field(event_call, tp->args[i].type->fmttype,
- tp->args[i].name,
- sizeof(field) + tp->args[i].offset,
- tp->args[i].type->size,
- tp->args[i].type->is_signed,
+ for (i = 0; i < tk->tp.nr_args; i++) {
+ struct probe_arg *parg = &tk->tp.args[i];
+
+ ret = trace_define_field(event_call, parg->type->fmttype,
+ parg->name,
+ sizeof(field) + parg->offset,
+ parg->type->size,
+ parg->type->is_signed,
FILTER_OTHER);
if (ret)
return ret;
@@ -1022,14 +1012,14 @@ static int kretprobe_event_define_fields(struct ftrace_event_call *event_call)
return 0;
}

-static int __set_print_fmt(struct trace_probe *tp, char *buf, int len)
+static int __set_print_fmt(struct trace_kprobe *tk, char *buf, int len)
{
int i;
int pos = 0;

const char *fmt, *arg;

- if (!trace_probe_is_return(tp)) {
+ if (!trace_kprobe_is_return(tk)) {
fmt = "(%lx)";
arg = "REC->" FIELD_STRING_IP;
} else {
@@ -1042,21 +1032,21 @@ 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++) {
+ for (i = 0; i < tk->tp.nr_args; i++) {
pos += snprintf(buf + pos, LEN_OR_ZERO, " %s=%s",
- tp->args[i].name, tp->args[i].type->fmt);
+ tk->tp.args[i].name, tk->tp.args[i].type->fmt);
}

pos += snprintf(buf + pos, LEN_OR_ZERO, "\", %s", arg);

- for (i = 0; i < tp->nr_args; i++) {
- if (strcmp(tp->args[i].type->name, "string") == 0)
+ for (i = 0; i < tk->tp.nr_args; i++) {
+ if (strcmp(tk->tp.args[i].type->name, "string") == 0)
pos += snprintf(buf + pos, LEN_OR_ZERO,
", __get_str(%s)",
- tp->args[i].name);
+ tk->tp.args[i].name);
else
pos += snprintf(buf + pos, LEN_OR_ZERO, ", REC->%s",
- tp->args[i].name);
+ tk->tp.args[i].name);
}

#undef LEN_OR_ZERO
@@ -1065,20 +1055,20 @@ static int __set_print_fmt(struct trace_probe *tp, char *buf, int len)
return pos;
}

-static int set_print_fmt(struct trace_probe *tp)
+static int set_print_fmt(struct trace_kprobe *tk)
{
int len;
char *print_fmt;

/* First: called with 0 length to calculate the needed length */
- len = __set_print_fmt(tp, NULL, 0);
+ len = __set_print_fmt(tk, NULL, 0);
print_fmt = kmalloc(len + 1, GFP_KERNEL);
if (!print_fmt)
return -ENOMEM;

/* Second: actually write the @print_fmt */
- __set_print_fmt(tp, print_fmt, len + 1);
- tp->call.print_fmt = print_fmt;
+ __set_print_fmt(tk, print_fmt, len + 1);
+ tk->tp.call.print_fmt = print_fmt;

return 0;
}
@@ -1087,9 +1077,9 @@ static int set_print_fmt(struct trace_probe *tp)

/* Kprobe profile handler */
static __kprobes void
-kprobe_perf_func(struct trace_probe *tp, struct pt_regs *regs)
+kprobe_perf_func(struct trace_kprobe *tk, struct pt_regs *regs)
{
- struct ftrace_event_call *call = &tp->call;
+ struct ftrace_event_call *call = &tk->tp.call;
struct kprobe_trace_entry_head *entry;
struct hlist_head *head;
int size, __size, dsize;
@@ -1099,8 +1089,8 @@ kprobe_perf_func(struct trace_probe *tp, struct pt_regs *regs)
if (hlist_empty(head))
return;

- dsize = __get_data_size(tp, regs);
- __size = sizeof(*entry) + tp->size + dsize;
+ dsize = __get_data_size(&tk->tp, regs);
+ __size = sizeof(*entry) + tk->tp.size + dsize;
size = ALIGN(__size + sizeof(u32), sizeof(u64));
size -= sizeof(u32);

@@ -1108,18 +1098,18 @@ kprobe_perf_func(struct trace_probe *tp, struct pt_regs *regs)
if (!entry)
return;

- entry->ip = (unsigned long)tp->rp.kp.addr;
+ entry->ip = (unsigned long)tk->rp.kp.addr;
memset(&entry[1], 0, dsize);
- store_trace_args(sizeof(*entry), tp, regs, (u8 *)&entry[1], dsize);
+ store_trace_args(sizeof(*entry), &tk->tp, regs, (u8 *)&entry[1], dsize);
perf_trace_buf_submit(entry, size, rctx, 0, 1, regs, head, NULL);
}

/* Kretprobe profile handler */
static __kprobes void
-kretprobe_perf_func(struct trace_probe *tp, struct kretprobe_instance *ri,
+kretprobe_perf_func(struct trace_kprobe *tk, struct kretprobe_instance *ri,
struct pt_regs *regs)
{
- struct ftrace_event_call *call = &tp->call;
+ struct ftrace_event_call *call = &tk->tp.call;
struct kretprobe_trace_entry_head *entry;
struct hlist_head *head;
int size, __size, dsize;
@@ -1129,8 +1119,8 @@ kretprobe_perf_func(struct trace_probe *tp, struct kretprobe_instance *ri,
if (hlist_empty(head))
return;

- dsize = __get_data_size(tp, regs);
- __size = sizeof(*entry) + tp->size + dsize;
+ dsize = __get_data_size(&tk->tp, regs);
+ __size = sizeof(*entry) + tk->tp.size + dsize;
size = ALIGN(__size + sizeof(u32), sizeof(u64));
size -= sizeof(u32);

@@ -1138,9 +1128,9 @@ kretprobe_perf_func(struct trace_probe *tp, struct kretprobe_instance *ri,
if (!entry)
return;

- entry->func = (unsigned long)tp->rp.kp.addr;
+ entry->func = (unsigned long)tk->rp.kp.addr;
entry->ret_ip = (unsigned long)ri->ret_addr;
- store_trace_args(sizeof(*entry), tp, regs, (u8 *)&entry[1], dsize);
+ store_trace_args(sizeof(*entry), &tk->tp, regs, (u8 *)&entry[1], dsize);
perf_trace_buf_submit(entry, size, rctx, 0, 1, regs, head, NULL);
}
#endif /* CONFIG_PERF_EVENTS */
@@ -1155,20 +1145,20 @@ static __kprobes
int kprobe_register(struct ftrace_event_call *event,
enum trace_reg type, void *data)
{
- struct trace_probe *tp = (struct trace_probe *)event->data;
+ struct trace_kprobe *tk = (struct trace_kprobe *)event->data;
struct ftrace_event_file *file = data;

switch (type) {
case TRACE_REG_REGISTER:
- return enable_trace_probe(tp, file);
+ return enable_trace_kprobe(tk, file);
case TRACE_REG_UNREGISTER:
- return disable_trace_probe(tp, file);
+ return disable_trace_kprobe(tk, file);

#ifdef CONFIG_PERF_EVENTS
case TRACE_REG_PERF_REGISTER:
- return enable_trace_probe(tp, NULL);
+ return enable_trace_kprobe(tk, NULL);
case TRACE_REG_PERF_UNREGISTER:
- return disable_trace_probe(tp, NULL);
+ return disable_trace_kprobe(tk, NULL);
case TRACE_REG_PERF_OPEN:
case TRACE_REG_PERF_CLOSE:
case TRACE_REG_PERF_ADD:
@@ -1182,15 +1172,15 @@ int kprobe_register(struct ftrace_event_call *event,
static __kprobes
int kprobe_dispatcher(struct kprobe *kp, struct pt_regs *regs)
{
- struct trace_probe *tp = container_of(kp, struct trace_probe, rp.kp);
+ struct trace_kprobe *tk = container_of(kp, struct trace_kprobe, rp.kp);

- tp->nhit++;
+ tk->nhit++;

- if (tp->flags & TP_FLAG_TRACE)
- kprobe_trace_func(tp, regs);
+ if (tk->tp.flags & TP_FLAG_TRACE)
+ kprobe_trace_func(tk, regs);
#ifdef CONFIG_PERF_EVENTS
- if (tp->flags & TP_FLAG_PROFILE)
- kprobe_perf_func(tp, regs);
+ if (tk->tp.flags & TP_FLAG_PROFILE)
+ kprobe_perf_func(tk, regs);
#endif
return 0; /* We don't tweek kernel, so just return 0 */
}
@@ -1198,15 +1188,15 @@ int kprobe_dispatcher(struct kprobe *kp, struct pt_regs *regs)
static __kprobes
int kretprobe_dispatcher(struct kretprobe_instance *ri, struct pt_regs *regs)
{
- struct trace_probe *tp = container_of(ri->rp, struct trace_probe, rp);
+ struct trace_kprobe *tk = container_of(ri->rp, struct trace_kprobe, rp);

- tp->nhit++;
+ tk->nhit++;

- if (tp->flags & TP_FLAG_TRACE)
- kretprobe_trace_func(tp, ri, regs);
+ if (tk->tp.flags & TP_FLAG_TRACE)
+ kretprobe_trace_func(tk, ri, regs);
#ifdef CONFIG_PERF_EVENTS
- if (tp->flags & TP_FLAG_PROFILE)
- kretprobe_perf_func(tp, ri, regs);
+ if (tk->tp.flags & TP_FLAG_PROFILE)
+ kretprobe_perf_func(tk, ri, regs);
#endif
return 0; /* We don't tweek kernel, so just return 0 */
}
@@ -1219,21 +1209,21 @@ static struct trace_event_functions kprobe_funcs = {
.trace = print_kprobe_event
};

-static int register_probe_event(struct trace_probe *tp)
+static int register_kprobe_event(struct trace_kprobe *tk)
{
- struct ftrace_event_call *call = &tp->call;
+ struct ftrace_event_call *call = &tk->tp.call;
int ret;

/* Initialize ftrace_event_call */
INIT_LIST_HEAD(&call->class->fields);
- if (trace_probe_is_return(tp)) {
+ if (trace_kprobe_is_return(tk)) {
call->event.funcs = &kretprobe_funcs;
call->class->define_fields = kretprobe_event_define_fields;
} else {
call->event.funcs = &kprobe_funcs;
call->class->define_fields = kprobe_event_define_fields;
}
- if (set_print_fmt(tp) < 0)
+ if (set_print_fmt(tk) < 0)
return -ENOMEM;
ret = register_ftrace_event(&call->event);
if (!ret) {
@@ -1242,7 +1232,7 @@ static int register_probe_event(struct trace_probe *tp)
}
call->flags = 0;
call->class->reg = kprobe_register;
- call->data = tp;
+ call->data = tk;
ret = trace_add_event_call(call);
if (ret) {
pr_info("Failed to register kprobe event: %s\n", call->name);
@@ -1252,14 +1242,14 @@ static int register_probe_event(struct trace_probe *tp)
return ret;
}

-static int unregister_probe_event(struct trace_probe *tp)
+static int unregister_kprobe_event(struct trace_kprobe *tk)
{
int ret;

/* tp->event is unregistered in trace_remove_event_call() */
- ret = trace_remove_event_call(&tp->call);
+ ret = trace_remove_event_call(&tk->tp.call);
if (!ret)
- kfree(tp->call.print_fmt);
+ kfree(tk->tp.call.print_fmt);
return ret;
}

@@ -1269,7 +1259,7 @@ static __init int init_kprobe_trace(void)
struct dentry *d_tracer;
struct dentry *entry;

- if (register_module_notifier(&trace_probe_module_nb))
+ if (register_module_notifier(&trace_kprobe_module_nb))
return -EINVAL;

d_tracer = tracing_init_dentry();
@@ -1309,12 +1299,12 @@ static __used int kprobe_trace_selftest_target(int a1, int a2, int a3,
}

static struct ftrace_event_file *
-find_trace_probe_file(struct trace_probe *tp, struct trace_array *tr)
+find_trace_probe_file(struct trace_kprobe *tk, struct trace_array *tr)
{
struct ftrace_event_file *file;

list_for_each_entry(file, &tr->events, list)
- if (file->event_call == &tp->call)
+ if (file->event_call == &tk->tp.call)
return file;

return NULL;
@@ -1328,7 +1318,7 @@ static __init int kprobe_trace_self_tests_init(void)
{
int ret, warn = 0;
int (*target)(int, int, int, int, int, int);
- struct trace_probe *tp;
+ struct trace_kprobe *tk;
struct ftrace_event_file *file;

target = kprobe_trace_selftest_target;
@@ -1343,17 +1333,17 @@ static __init int kprobe_trace_self_tests_init(void)
warn++;
} else {
/* Enable trace point */
- tp = find_trace_probe("testprobe", KPROBE_EVENT_SYSTEM);
- if (WARN_ON_ONCE(tp == NULL)) {
+ tk = find_trace_probe("testprobe", KPROBE_EVENT_SYSTEM);
+ if (WARN_ON_ONCE(tk == NULL)) {
pr_warn("error on getting new probe.\n");
warn++;
} else {
- file = find_trace_probe_file(tp, top_trace_array());
+ file = find_trace_probe_file(tk, top_trace_array());
if (WARN_ON_ONCE(file == NULL)) {
pr_warn("error on getting probe file.\n");
warn++;
} else
- enable_trace_probe(tp, file);
+ enable_trace_probe(tk, file);
}
}

@@ -1364,17 +1354,17 @@ static __init int kprobe_trace_self_tests_init(void)
warn++;
} else {
/* Enable trace point */
- tp = find_trace_probe("testprobe2", KPROBE_EVENT_SYSTEM);
- if (WARN_ON_ONCE(tp == NULL)) {
+ tk = find_trace_probe("testprobe2", KPROBE_EVENT_SYSTEM);
+ if (WARN_ON_ONCE(tk == NULL)) {
pr_warn("error on getting 2nd new probe.\n");
warn++;
} else {
- file = find_trace_probe_file(tp, top_trace_array());
+ file = find_trace_probe_file(tk, top_trace_array());
if (WARN_ON_ONCE(file == NULL)) {
pr_warn("error on getting probe file.\n");
warn++;
} else
- enable_trace_probe(tp, file);
+ enable_trace_probe(tk, file);
}
}

@@ -1384,30 +1374,30 @@ static __init int kprobe_trace_self_tests_init(void)
ret = target(1, 2, 3, 4, 5, 6);

/* Disable trace points before removing it */
- tp = find_trace_probe("testprobe", KPROBE_EVENT_SYSTEM);
- if (WARN_ON_ONCE(tp == NULL)) {
+ tk = find_trace_probe("testprobe", KPROBE_EVENT_SYSTEM);
+ if (WARN_ON_ONCE(tk == NULL)) {
pr_warn("error on getting test probe.\n");
warn++;
} else {
- file = find_trace_probe_file(tp, top_trace_array());
+ file = find_trace_probe_file(tk, top_trace_array());
if (WARN_ON_ONCE(file == NULL)) {
pr_warn("error on getting probe file.\n");
warn++;
} else
- disable_trace_probe(tp, file);
+ disable_trace_probe(tk, file);
}

- tp = find_trace_probe("testprobe2", KPROBE_EVENT_SYSTEM);
- if (WARN_ON_ONCE(tp == NULL)) {
+ tk = find_trace_probe("testprobe2", KPROBE_EVENT_SYSTEM);
+ if (WARN_ON_ONCE(tk == NULL)) {
pr_warn("error on getting 2nd test probe.\n");
warn++;
} else {
- file = find_trace_probe_file(tp, top_trace_array());
+ file = find_trace_probe_file(tk, top_trace_array());
if (WARN_ON_ONCE(file == NULL)) {
pr_warn("error on getting probe file.\n");
warn++;
} else
- disable_trace_probe(tp, file);
+ disable_trace_probe(tk, file);
}

ret = traceprobe_command("-:testprobe", create_trace_probe);
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 5c7e09d10d74..984e91ed8a44 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -124,6 +124,26 @@ struct probe_arg {
const struct fetch_type *type; /* Type of this argument */
};

+struct trace_probe {
+ unsigned int flags; /* For TP_FLAG_* */
+ struct ftrace_event_class class;
+ struct ftrace_event_call call;
+ struct list_head files;
+ ssize_t size; /* trace entry size */
+ unsigned int nr_args;
+ struct probe_arg args[];
+};
+
+static inline bool trace_probe_is_enabled(struct trace_probe *tp)
+{
+ return !!(tp->flags & (TP_FLAG_TRACE | TP_FLAG_PROFILE));
+}
+
+static inline bool trace_probe_is_registered(struct trace_probe *tp)
+{
+ return !!(tp->flags & TP_FLAG_REGISTERED);
+}
+
static inline __kprobes void call_fetch(struct fetch_param *fprm,
struct pt_regs *regs, void *dest)
{

Masami Hiramatsu

unread,
Dec 16, 2013, 3:20:02 AM12/16/13
to
(2013/12/16 13:32), Namhyung Kim wrote:
> From: Namhyung Kim <namhyu...@lge.com>
>
> Use separate method to fetch from stack. Move existing functions to
> trace_kprobe.c and make them static. Also add new stack fetch
> implementation for uprobes.
>
> Cc: Masami Hiramatsu <masami.hi...@hitachi.com>
> Cc: Srikar Dronamraju <sri...@linux.vnet.ibm.com>
> Cc: Oleg Nesterov <ol...@redhat.com>
> Cc: zhangwei(Jovi) <jovi.z...@huawei.com>
> Cc: Arnaldo Carvalho de Melo <ac...@ghostprotocols.net>
> Signed-off-by: Namhyung Kim <namh...@kernel.org>

Looks good for me :)

Acked-by: Masami Hiramatsu <masami.hi...@hitachi.com>

Thank you!
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hi...@hitachi.com

Masami Hiramatsu

unread,
Dec 16, 2013, 3:30:03 AM12/16/13
to
(2013/12/16 13:32), Namhyung Kim wrote:
> From: Namhyung Kim <namhyu...@lge.com>
>
> Move existing functions to trace_kprobe.c and add NULL entries to the
> uprobes fetch type table. I don't make them static since some generic
> routines like update/free_XXX_fetch_param() require pointers to the
> functions.
>
> Cc: Masami Hiramatsu <masami.hi...@hitachi.com>
> Cc: Srikar Dronamraju <sri...@linux.vnet.ibm.com>
> Cc: Oleg Nesterov <ol...@redhat.com>
> Cc: zhangwei(Jovi) <jovi.z...@huawei.com>
> Cc: Arnaldo Carvalho de Melo <ac...@ghostprotocols.net>
> Signed-off-by: Namhyung Kim <namh...@kernel.org>

This looks good for me :)

Acked-by: Masami Hiramatsu <masami.hi...@hitachi.com>

And now all of this series has been reviewed/acked by me!

Thank you!
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hi...@hitachi.com


Namhyung Kim

unread,
Dec 16, 2013, 3:50:01 AM12/16/13
to
Hi Masami,

On Mon, 16 Dec 2013 17:23:25 +0900, Masami Hiramatsu wrote:
> (2013/12/16 13:32), Namhyung Kim wrote:
>> From: Namhyung Kim <namhyu...@lge.com>
>>
>> Move existing functions to trace_kprobe.c and add NULL entries to the
>> uprobes fetch type table. I don't make them static since some generic
>> routines like update/free_XXX_fetch_param() require pointers to the
>> functions.
>>
>> Cc: Masami Hiramatsu <masami.hi...@hitachi.com>
>> Cc: Srikar Dronamraju <sri...@linux.vnet.ibm.com>
>> Cc: Oleg Nesterov <ol...@redhat.com>
>> Cc: zhangwei(Jovi) <jovi.z...@huawei.com>
>> Cc: Arnaldo Carvalho de Melo <ac...@ghostprotocols.net>
>> Signed-off-by: Namhyung Kim <namh...@kernel.org>
>
> This looks good for me :)
>
> Acked-by: Masami Hiramatsu <masami.hi...@hitachi.com>
>
> And now all of this series has been reviewed/acked by me!

Thank you so much for your review! :)

Thanks,
Namhyung

Namhyung Kim

unread,
Dec 16, 2013, 7:20:02 PM12/16/13
to
Hi Steve,

Would you please review this and consider merging it? It seems there's
no more remaining issue and we want to make a progress on it. :)

Oleg, Srikar, it'd be great if you could confirm it though.

Thanks,
Namhyung

Steven Rostedt

unread,
Dec 18, 2013, 3:30:02 PM12/18/13
to
On Mon, 16 Dec 2013 13:32:09 +0900
Namhyung Kim <namh...@kernel.org> wrote:

> index 5395d37e5e72..adc9ac70fd3c 100644
> --- a/kernel/trace/trace_uprobe.c
> +++ b/kernel/trace/trace_uprobe.c
> @@ -115,6 +115,14 @@ DEFINE_BASIC_FETCH_FUNCS(stack)
> #define fetch_stack_string_size NULL
>
>
> +/* uprobes don't support symbol fetch methods */

"uprobes does not support symbol fetch methods"

-- Steve

> +#define fetch_symbol_u8 NULL
> +#define fetch_symbol_u16 NULL
> +#define fetch_symbol_u32 NULL
> +#define fetch_symbol_u64 NULL
> +#define fetch_symbol_string NULL
> +#define fetch_symbol_string_size NULL
> +
> /* Fetch type information table */
> const struct fetch_type uprobes_fetch_type_table[] = {
> /* Special types */

--

Steven Rostedt

unread,
Dec 18, 2013, 3:30:02 PM12/18/13
to
On Tue, 17 Dec 2013 09:16:37 +0900
Namhyung Kim <namh...@kernel.org> wrote:

> Hi Steve,
>
> Would you please review this and consider merging it? It seems there's
> no more remaining issue and we want to make a progress on it. :)
>
> Oleg, Srikar, it'd be great if you could confirm it though.
>

I'd feel better if we got some feedback from Oleg and Srikar too.

I don't have test code for uprobes, and I have minimal test code for
kprobes. How has this been tested?

-- Steve

Oleg Nesterov

unread,
Dec 18, 2013, 3:40:02 PM12/18/13
to
On 12/18, Steven Rostedt wrote:
>
> On Tue, 17 Dec 2013 09:16:37 +0900
> Namhyung Kim <namh...@kernel.org> wrote:
>
> > Hi Steve,
> >
> > Would you please review this and consider merging it? It seems there's
> > no more remaining issue and we want to make a progress on it. :)
> >
> > Oleg, Srikar, it'd be great if you could confirm it though.
> >
>
> I'd feel better if we got some feedback from Oleg and Srikar too.

Sorry, didn't have the time to review.

I'll try very much to do this tomorrow. But I do not expect I will find
something interesting, at first glance this version addresses all concerns
I had.

Oleg.

Namhyung Kim

unread,
Dec 19, 2013, 2:40:01 AM12/19/13
to
Hi Steve,

On Wed, 18 Dec 2013 15:23:38 -0500, Steven Rostedt wrote:
> On Mon, 16 Dec 2013 13:32:09 +0900
> Namhyung Kim <namh...@kernel.org> wrote:
>
>> index 5395d37e5e72..adc9ac70fd3c 100644
>> --- a/kernel/trace/trace_uprobe.c
>> +++ b/kernel/trace/trace_uprobe.c
>> @@ -115,6 +115,14 @@ DEFINE_BASIC_FETCH_FUNCS(stack)
>> #define fetch_stack_string_size NULL
>>
>>
>> +/* uprobes don't support symbol fetch methods */
>
> "uprobes does not support symbol fetch methods"

I was confused since the "uprobes" looks like a plural..

Thanks,
Namhyung

Namhyung Kim

unread,
Dec 19, 2013, 4:00:02 AM12/19/13
to
Hi Oleg,

On Wed, 18 Dec 2013 21:34:24 +0100, Oleg Nesterov wrote:
> On 12/18, Steven Rostedt wrote:
>>
>> On Tue, 17 Dec 2013 09:16:37 +0900
>> Namhyung Kim <namh...@kernel.org> wrote:
>>
>> > Hi Steve,
>> >
>> > Would you please review this and consider merging it? It seems there's
>> > no more remaining issue and we want to make a progress on it. :)
>> >
>> > Oleg, Srikar, it'd be great if you could confirm it though.
>> >
>>
>> I'd feel better if we got some feedback from Oleg and Srikar too.
>
> Sorry, didn't have the time to review.
>
> I'll try very much to do this tomorrow. But I do not expect I will find
> something interesting, at first glance this version addresses all concerns
> I had.

Thanks in advance! Please let me know if you find anything
interesting. :)

Thanks,
Namhyung

Namhyung Kim

unread,
Dec 19, 2013, 4:00:03 AM12/19/13
to
On Wed, 18 Dec 2013 15:25:53 -0500, Steven Rostedt wrote:
> I don't have test code for uprobes, and I have minimal test code for
> kprobes. How has this been tested?

Basically I tested it with a very simple example code like in the
description of this patchset and the code in the link below - note that
the argument format was slightly changed to have a prifix of "@+".

https://lkml.org/lkml/2013/11/5/25

Maybe we need to write more test code. I'll try to find a way to test
it automatically with the perf tools.

Thanks,
Namhyung

Steven Rostedt

unread,
Dec 19, 2013, 9:20:01 AM12/19/13
to
On Thu, 19 Dec 2013 16:34:09 +0900
Namhyung Kim <namh...@kernel.org> wrote:

> Hi Steve,
>
> On Wed, 18 Dec 2013 15:23:38 -0500, Steven Rostedt wrote:
> > On Mon, 16 Dec 2013 13:32:09 +0900
> > Namhyung Kim <namh...@kernel.org> wrote:
> >
> >> index 5395d37e5e72..adc9ac70fd3c 100644
> >> --- a/kernel/trace/trace_uprobe.c
> >> +++ b/kernel/trace/trace_uprobe.c
> >> @@ -115,6 +115,14 @@ DEFINE_BASIC_FETCH_FUNCS(stack)
> >> #define fetch_stack_string_size NULL
> >>
> >>
> >> +/* uprobes don't support symbol fetch methods */
> >
> > "uprobes does not support symbol fetch methods"
>
> I was confused since the "uprobes" looks like a plural..
>

Heh, yeah I guess it can be. I was thinking of it as singular. I was
thinking of ftrace, perf, kprobes, uprobes, all being the same. As in
"kprobes system" or "uprobes system". I implicitly added in my mind
"system" to that sentence.

uprobes (system) does not support fetch methods

But I guess one can think of it as each individual uprobe does not
support fetch methods, so "uprobes do not support fetch methods" would
then be correct.

But still, I would use the full "do not" as the "don't" here just
sounds funny to me (sounds like a boxer talking "Dey don't do dat to
me! Dey don't support fetch methods either!")

-- Steve

Namhyung Kim

unread,
Dec 19, 2013, 8:10:01 PM12/19/13
to
Hi Steve,
Okay, will use "do not" in the next version (if needed :) ).

Thanks,
Namhyung

Namhyung Kim

unread,
Dec 22, 2013, 7:40:02 AM12/22/13
to
Hi Steve,

2013-12-21 (토), 00:22 -0500, Steven Rostedt:
> I applied all the patches and started testing and it failed to build,
> with this error:
>
>
> /home/rostedt/work/git/linux-trace.git/kernel/trace/trace_kprobe.c: In function ‘kprobe_trace_self_tests_init’:
> /home/rostedt/work/git/linux-trace.git/kernel/trace/trace_kprobe.c:1400:7: error: ‘create_trace_probe’ undeclared (first use in this function)
> /home/rostedt/work/git/linux-trace.git/kernel/trace/trace_kprobe.c:1400:7: note: each undeclared identifier is reported only once for each function it appears in
> /home/rostedt/work/git/linux-trace.git/kernel/trace/trace_kprobe.c:1406:3: error: implicit declaration of function ‘find_trace_probe’ [-Werror=implicit-function-declaration]
> /home/rostedt/work/git/linux-trace.git/kernel/trace/trace_kprobe.c:1406:6: warning: assignment makes pointer from integer without a cast [enabled by default]
> /home/rostedt/work/git/linux-trace.git/kernel/trace/trace_kprobe.c:1416:5: error: implicit declaration of function ‘enable_trace_probe’ [-Werror=implicit-function-declaration]
> /home/rostedt/work/git/linux-trace.git/kernel/trace/trace_kprobe.c:1427:6: warning: assignment makes pointer from integer without a cast [enabled by default]
> /home/rostedt/work/git/linux-trace.git/kernel/trace/trace_kprobe.c:1447:5: warning: assignment makes pointer from integer without a cast [enabled by default]
> /home/rostedt/work/git/linux-trace.git/kernel/trace/trace_kprobe.c:1457:4: error: implicit declaration of function ‘disable_trace_probe’ [-Werror=implicit-function-declaration]
> /home/rostedt/work/git/linux-trace.git/kernel/trace/trace_kprobe.c:1460:5: warning: assignment makes pointer from integer without a cast [enabled by default]
> /home/rostedt/work/git/linux-trace.git/kernel/trace/trace_kprobe.c:1486:2: error: implicit declaration of function ‘release_all_trace_probes’ [-Werror=implicit-function-declaration
> ]
> cc1: some warnings being treated as errors
> distcc[17703] ERROR: compile /home/rostedt/work/git/linux-trace.git/kernel/trace/trace_kprobe.c on localhost failed
> make[3]: *** [kernel/trace/trace_kprobe.o] Error 1
> make[3]: *** Waiting for unfinished jobs....
>
> I applied it to my "ftrace/core" branch.
>
> I attached my config.

Oops, sorry. I forgot to enable FTRACE_STARTUP_TEST. I've fixed it,
rebased it onto ftrace/core and pushed to my "uprobe/fetch-v10" branch.
Please test it again when you have time. :)

Thanks,
Namhyung

Oleg Nesterov

unread,
Dec 23, 2013, 1:10:01 PM12/23/13
to
On 12/16, Namhyung Kim wrote:
>
> This patchset implements memory (address), stack[N], deference,
> bitfield, retval (it needs uretprobe tho) and file_offset fetch
> methods for uprobes. It's based on the previous work [1] done by
> Hyeoncheol Lee.

I can't say I understand every change in details, but everything looks
fine to me. And this series addresses all my previous comments.

FWIW,

Acked-by: Oleg Nesterov <ol...@redhat.com>

Namhyung Kim

unread,
Dec 24, 2013, 3:10:01 AM12/24/13
to
Hi Oleg,

On Mon, 23 Dec 2013 19:00:26 +0100, Oleg Nesterov wrote:
> On 12/16, Namhyung Kim wrote:
>>
>> This patchset implements memory (address), stack[N], deference,
>> bitfield, retval (it needs uretprobe tho) and file_offset fetch
>> methods for uprobes. It's based on the previous work [1] done by
>> Hyeoncheol Lee.
>
> I can't say I understand every change in details, but everything looks
> fine to me. And this series addresses all my previous comments.
>
> FWIW,
>
> Acked-by: Oleg Nesterov <ol...@redhat.com>

Thank you so much for your review!! :)
Namhyung

Steven Rostedt

unread,
Jan 2, 2014, 4:00:01 PM1/2/14
to
On Mon, 16 Dec 2013 13:32:14 +0900
Namhyung Kim <namh...@kernel.org> wrote:

> From: Namhyung Kim <namhyu...@lge.com>
>
> Enable to fetch other types of argument for the uprobes. IOW, we can
> access stack, memory, deref, bitfield and retval from uprobes now.
>
> The format for the argument types are same as kprobes (but @SYMBOL
> type is not supported for uprobes), i.e:
>
> @ADDR : Fetch memory at ADDR
> $stackN : Fetch Nth entry of stack (N >= 0)
> $stack : Fetch stack address
> $retval : Fetch return value
> +|-offs(FETCHARG) : Fetch memory at FETCHARG +|- offs address
>
> Note that the retval only can be used with uretprobes.
>
> Original-patch-by: Hyeoncheol Lee <cheo...@lge.com>

Where was the original patch posted? And can we get Hyeoncheol's SOB for
this?

Thanks!

-- Steve

Steven Rostedt

unread,
Jan 2, 2014, 4:10:02 PM1/2/14
to
On Thu, 2 Jan 2014 15:58:27 -0500
Steven Rostedt <ros...@goodmis.org> wrote:

> On Mon, 16 Dec 2013 13:32:14 +0900
> Namhyung Kim <namh...@kernel.org> wrote:
>
> > From: Namhyung Kim <namhyu...@lge.com>
> >
> > Enable to fetch other types of argument for the uprobes. IOW, we can
> > access stack, memory, deref, bitfield and retval from uprobes now.
> >
> > The format for the argument types are same as kprobes (but @SYMBOL
> > type is not supported for uprobes), i.e:
> >
> > @ADDR : Fetch memory at ADDR
> > $stackN : Fetch Nth entry of stack (N >= 0)
> > $stack : Fetch stack address
> > $retval : Fetch return value
> > +|-offs(FETCHARG) : Fetch memory at FETCHARG +|- offs address
> >
> > Note that the retval only can be used with uretprobes.
> >
> > Original-patch-by: Hyeoncheol Lee <cheo...@lge.com>
>
> Where was the original patch posted? And can we get Hyeoncheol's SOB for
> this?
>

Was this the other part of patch 11? Still should have Hyeoncheol's
signed-off-by. If you already had it on the original (before the split)
then we can add it here too, as he already signed off on the code that
this was based on.

-- Steve

Hyeoncheol Lee

unread,
Jan 2, 2014, 8:20:02 PM1/2/14
to
Patches look good to me.

Signed-off-by: Hyeoncheol Lee <cheo...@lge.com>

Steven Rostedt

unread,
Jan 2, 2014, 9:00:02 PM1/2/14
to
On Fri, 3 Jan 2014 10:17:23 +0900
"Hyeoncheol Lee" <cheo...@lge.com> wrote:

> Patches look good to me.
>
> Signed-off-by: Hyeoncheol Lee <cheo...@lge.com>
>

Thanks! I'll add this to my queue.
0 new messages