2014-01-17 08:08:48

by Namhyung Kim

[permalink] [raw]
Subject: [PATCHSET 0/5] tracing/uprobes: Support multi buffer and event trigger

Hello,
(Resending with LKML CC'ed)

This patchset tries to add support for recent multi buffer and event
trigger changes to uprobes. The multi buffer support patch is an
updated version of Zovi's previous patch v6 [1].

Zovi, please tell me if you have any update and/or issues with this.

Masami and Oleg, I kept your Reviewed-by's in the patch since I think
it's just an rebase. Please take a look again to see whether I added
some mistakes.


You can also get it from 'uprobe/trigger-v1' branch in my tree

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

Any comments are welcome, thanks
Namhyung


[1] https://lkml.org/lkml/2013/7/4/165

Cc: Masami Hiramatsu <[email protected]>
Cc: Oleg Nesterov <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: zhangwei(Jovi) <[email protected]>
Cc: Tom Zanussi <[email protected]>


Namhyung Kim (4):
tracing/uprobes: Rename uprobe_{trace,perf}_print() functions
tracing/uprobes: Move argument fetching to uprobe_dispatcher()
tracing/uprobes: Support event triggering
tracing/uprobes: Support mix of ftrace and perf

zhangwei(Jovi) (1):
tracing/uprobes: Support ftrace_event_file base multibuffer

kernel/trace/trace_kprobe.c | 17 ----
kernel/trace/trace_probe.h | 17 ++++
kernel/trace/trace_uprobe.c | 191 +++++++++++++++++++++++++++++++-------------
3 files changed, 151 insertions(+), 74 deletions(-)

--
1.7.11.7


2014-01-17 08:08:51

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 2/5] tracing/uprobes: Move argument fetching to uprobe_dispatcher()

A single uprobe event might serve different users like ftrace and
perf. And this is especially important for upcoming multi buffer
support. But in this case it'll fetch (same) data from userspace
multiple times. So move it to the beginning of the dispatcher
function and reuse it for each users.

Cc: Masami Hiramatsu <[email protected]>
Cc: Oleg Nesterov <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: zhangwei(Jovi) <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
kernel/trace/trace_uprobe.c | 93 +++++++++++++++++++++++++++------------------
1 file changed, 56 insertions(+), 37 deletions(-)

diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index c5d2612bf233..d83155e0da78 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -759,30 +759,25 @@ static void uprobe_buffer_put(struct uprobe_cpu_buffer *ucb)
}

static void __uprobe_trace_func(struct trace_uprobe *tu,
- unsigned long func, struct pt_regs *regs)
+ unsigned long func, struct pt_regs *regs,
+ struct uprobe_cpu_buffer *ucb, int dsize)
{
struct uprobe_trace_entry_head *entry;
struct ring_buffer_event *event;
struct ring_buffer *buffer;
- struct uprobe_cpu_buffer *ucb;
void *data;
- int size, dsize, esize;
+ int size, esize;
struct ftrace_event_call *call = &tu->tp.call;

- 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))
+ if (WARN_ON_ONCE(tu->tp.size + dsize > PAGE_SIZE))
return;

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

entry = ring_buffer_event_data(event);
if (is_ret_probe(tu)) {
@@ -798,23 +793,22 @@ static void __uprobe_trace_func(struct trace_uprobe *tu,

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

/* uprobe handler */
-static int uprobe_trace_func(struct trace_uprobe *tu, struct pt_regs *regs)
+static int uprobe_trace_func(struct trace_uprobe *tu, struct pt_regs *regs,
+ struct uprobe_cpu_buffer *ucb, int dsize)
{
if (!is_ret_probe(tu))
- __uprobe_trace_func(tu, 0, regs);
+ __uprobe_trace_func(tu, 0, regs, ucb, dsize);
return 0;
}

static void uretprobe_trace_func(struct trace_uprobe *tu, unsigned long func,
- struct pt_regs *regs)
+ struct pt_regs *regs,
+ struct uprobe_cpu_buffer *ucb, int dsize)
{
- __uprobe_trace_func(tu, func, regs);
+ __uprobe_trace_func(tu, func, regs, ucb, dsize);
}

/* Event entry printers */
@@ -1015,30 +1009,23 @@ static bool uprobe_perf_filter(struct uprobe_consumer *uc,
}

static void __uprobe_perf_func(struct trace_uprobe *tu,
- unsigned long func, struct pt_regs *regs)
+ unsigned long func, struct pt_regs *regs,
+ struct uprobe_cpu_buffer *ucb, int dsize)
{
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, dsize, esize;
+ int size, esize;
int rctx;

- dsize = __get_data_size(&tu->tp, regs);
esize = SIZEOF_TRACE_ENTRY(is_ret_probe(tu));

- 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);
if (hlist_empty(head))
@@ -1068,24 +1055,25 @@ static void __uprobe_perf_func(struct trace_uprobe *tu,
perf_trace_buf_submit(entry, size, rctx, 0, 1, regs, head, NULL);
out:
preempt_enable();
- uprobe_buffer_put(ucb);
}

/* uprobe profile handler */
-static int uprobe_perf_func(struct trace_uprobe *tu, struct pt_regs *regs)
+static int uprobe_perf_func(struct trace_uprobe *tu, struct pt_regs *regs,
+ struct uprobe_cpu_buffer *ucb, int dsize)
{
if (!uprobe_perf_filter(&tu->consumer, 0, current->mm))
return UPROBE_HANDLER_REMOVE;

if (!is_ret_probe(tu))
- __uprobe_perf_func(tu, 0, regs);
+ __uprobe_perf_func(tu, 0, regs, ucb, dsize);
return 0;
}

static void uretprobe_perf_func(struct trace_uprobe *tu, unsigned long func,
- struct pt_regs *regs)
+ struct pt_regs *regs,
+ struct uprobe_cpu_buffer *ucb, int dsize)
{
- __uprobe_perf_func(tu, func, regs);
+ __uprobe_perf_func(tu, func, regs, ucb, dsize);
}
#endif /* CONFIG_PERF_EVENTS */

@@ -1127,8 +1115,11 @@ static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs *regs)
{
struct trace_uprobe *tu;
struct uprobe_dispatch_data udd;
+ struct uprobe_cpu_buffer *ucb;
+ int dsize, esize;
int ret = 0;

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

@@ -1137,13 +1128,29 @@ static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs *regs)

current->utask->vaddr = (unsigned long) &udd;

+#ifdef CONFIG_PERF_EVENTS
+ if ((tu->tp.flags & TP_FLAG_TRACE) == 0 &&
+ !uprobe_perf_filter(&tu->consumer, 0, current->mm))
+ return UPROBE_HANDLER_REMOVE;
+#endif
+
+ if (WARN_ON_ONCE(!uprobe_cpu_buffer))
+ return 0;
+
+ dsize = __get_data_size(&tu->tp, regs);
+ esize = SIZEOF_TRACE_ENTRY(is_ret_probe(tu));
+
+ ucb = uprobe_buffer_get();
+ store_trace_args(esize, &tu->tp, regs, ucb->buf, dsize);
+
if (tu->tp.flags & TP_FLAG_TRACE)
- ret |= uprobe_trace_func(tu, regs);
+ ret |= uprobe_trace_func(tu, regs, ucb, dsize);

#ifdef CONFIG_PERF_EVENTS
if (tu->tp.flags & TP_FLAG_PROFILE)
- ret |= uprobe_perf_func(tu, regs);
+ ret |= uprobe_perf_func(tu, regs, ucb, dsize);
#endif
+ uprobe_buffer_put(ucb);
return ret;
}

@@ -1152,6 +1159,8 @@ static int uretprobe_dispatcher(struct uprobe_consumer *con,
{
struct trace_uprobe *tu;
struct uprobe_dispatch_data udd;
+ struct uprobe_cpu_buffer *ucb;
+ int dsize, esize;

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

@@ -1160,13 +1169,23 @@ static int uretprobe_dispatcher(struct uprobe_consumer *con,

current->utask->vaddr = (unsigned long) &udd;

+ if (WARN_ON_ONCE(!uprobe_cpu_buffer))
+ return 0;
+
+ dsize = __get_data_size(&tu->tp, regs);
+ esize = SIZEOF_TRACE_ENTRY(is_ret_probe(tu));
+
+ ucb = uprobe_buffer_get();
+ store_trace_args(esize, &tu->tp, regs, ucb->buf, dsize);
+
if (tu->tp.flags & TP_FLAG_TRACE)
- uretprobe_trace_func(tu, func, regs);
+ uretprobe_trace_func(tu, func, regs, ucb, dsize);

#ifdef CONFIG_PERF_EVENTS
if (tu->tp.flags & TP_FLAG_PROFILE)
- uretprobe_perf_func(tu, func, regs);
+ uretprobe_perf_func(tu, func, regs, ucb, dsize);
#endif
+ uprobe_buffer_put(ucb);
return 0;
}

--
1.7.11.7

2014-01-17 08:09:05

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 4/5] tracing/uprobes: Support event triggering

Add support for event triggering to uprobes. This is same as kprobes
support added by Tom (plus cleanup by Steven).

Cc: Masami Hiramatsu <[email protected]>
Cc: Oleg Nesterov <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: zhangwei(Jovi) <[email protected]>
Cc: Tom Zanussi <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
kernel/trace/trace_uprobe.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 349c6df9e332..01fcb0db75cb 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -776,6 +776,9 @@ static void __uprobe_trace_func(struct trace_uprobe *tu,
if (WARN_ON_ONCE(tu->tp.size + dsize > PAGE_SIZE))
return;

+ if (ftrace_trigger_soft_disabled(ftrace_file))
+ return;
+
esize = SIZEOF_TRACE_ENTRY(is_ret_probe(tu));
size = esize + tu->tp.size + dsize;
event = trace_event_buffer_lock_reserve(&buffer, ftrace_file,
@@ -795,8 +798,7 @@ static void __uprobe_trace_func(struct trace_uprobe *tu,

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);
+ event_trigger_unlock_commit(ftrace_file, buffer, event, entry, 0, 0);
}

/* uprobe handler */
--
1.7.11.7

2014-01-17 08:09:03

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 1/5] tracing/uprobes: Rename uprobe_{trace,perf}_print() functions

The uprobe_{trace,perf}_print functions are misnomers since what they
do is not printing. There's also a real print function named
print_uprobe_event() so they'll only increase confusion IMHO.

Rename them with double underscores to follow convention of kprobe.

Cc: Oleg Nesterov <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
kernel/trace/trace_uprobe.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 79e52d93860b..c5d2612bf233 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -758,7 +758,7 @@ static void uprobe_buffer_put(struct uprobe_cpu_buffer *ucb)
mutex_unlock(&ucb->mutex);
}

-static void uprobe_trace_print(struct trace_uprobe *tu,
+static void __uprobe_trace_func(struct trace_uprobe *tu,
unsigned long func, struct pt_regs *regs)
{
struct uprobe_trace_entry_head *entry;
@@ -807,14 +807,14 @@ out:
static int uprobe_trace_func(struct trace_uprobe *tu, struct pt_regs *regs)
{
if (!is_ret_probe(tu))
- uprobe_trace_print(tu, 0, regs);
+ __uprobe_trace_func(tu, 0, regs);
return 0;
}

static void uretprobe_trace_func(struct trace_uprobe *tu, unsigned long func,
struct pt_regs *regs)
{
- uprobe_trace_print(tu, func, regs);
+ __uprobe_trace_func(tu, func, regs);
}

/* Event entry printers */
@@ -1014,7 +1014,7 @@ static bool uprobe_perf_filter(struct uprobe_consumer *uc,
return ret;
}

-static void uprobe_perf_print(struct trace_uprobe *tu,
+static void __uprobe_perf_func(struct trace_uprobe *tu,
unsigned long func, struct pt_regs *regs)
{
struct ftrace_event_call *call = &tu->tp.call;
@@ -1078,14 +1078,14 @@ static int uprobe_perf_func(struct trace_uprobe *tu, struct pt_regs *regs)
return UPROBE_HANDLER_REMOVE;

if (!is_ret_probe(tu))
- uprobe_perf_print(tu, 0, regs);
+ __uprobe_perf_func(tu, 0, regs);
return 0;
}

static void uretprobe_perf_func(struct trace_uprobe *tu, unsigned long func,
struct pt_regs *regs)
{
- uprobe_perf_print(tu, func, regs);
+ __uprobe_perf_func(tu, func, regs);
}
#endif /* CONFIG_PERF_EVENTS */

--
1.7.11.7

2014-01-17 08:09:00

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 3/5] tracing/uprobes: Support ftrace_event_file base multibuffer

From: "zhangwei(Jovi)" <[email protected]>

Support multi-buffer on uprobe-based dynamic events by
using ftrace_event_file.

This patch is based kprobe-based dynamic events multibuffer
support work initially, commited by Masami(commit 41a7dd420c),
but revised as below:

Oleg changed the kprobe-based multibuffer design from
array-pointers of ftrace_event_file into simple list,
so this patch also change to the list design.

rcu_read_lock/unlock added into uprobe_trace_func/uretprobe_trace_func,
to synchronize with ftrace_event_file list add and delete.

Even though we allow multi-uprobes instances now,
but TP_FLAG_PROFILE/TP_FLAG_TRACE are still mutually exclusive
in probe_event_enable currently, this means we cannot allow
one user is using uprobe-tracer, and another user is using
perf-probe on same uprobe concurrently.
(Perhaps this will be fix in future, kprobe don't have this
limitation now)

Signed-off-by: zhangwei(Jovi) <[email protected]>
Reviewed-by: Masami Hiramatsu <[email protected]>
Reviewed-by: Oleg Nesterov <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
kernel/trace/trace_kprobe.c | 17 -------
kernel/trace/trace_probe.h | 17 +++++++
kernel/trace/trace_uprobe.c | 105 +++++++++++++++++++++++++++++++++++---------
3 files changed, 101 insertions(+), 38 deletions(-)

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index bdbae450c13e..d021d21dd150 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -35,11 +35,6 @@ struct trace_kprobe {
struct trace_probe tp;
};

-struct event_file_link {
- struct ftrace_event_file *file;
- struct list_head list;
-};
-
#define SIZEOF_TRACE_KPROBE(n) \
(offsetof(struct trace_kprobe, tp.args) + \
(sizeof(struct probe_arg) * (n)))
@@ -387,18 +382,6 @@ enable_trace_kprobe(struct trace_kprobe *tk, struct ftrace_event_file *file)
return ret;
}

-static struct event_file_link *
-find_event_file_link(struct trace_probe *tp, struct ftrace_event_file *file)
-{
- struct event_file_link *link;
-
- list_for_each_entry(link, &tp->files, list)
- if (link->file == file)
- return link;
-
- return NULL;
-}
-
/*
* Disable trace_probe
* if the file is NULL, disable "perf" handler, or disable "trace" handler.
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index b73574a5f429..fb1ab5dfbd42 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -288,6 +288,11 @@ struct trace_probe {
struct probe_arg args[];
};

+struct event_file_link {
+ struct ftrace_event_file *file;
+ struct list_head list;
+};
+
static inline bool trace_probe_is_enabled(struct trace_probe *tp)
{
return !!(tp->flags & (TP_FLAG_TRACE | TP_FLAG_PROFILE));
@@ -316,6 +321,18 @@ static inline int is_good_name(const char *name)
return 1;
}

+static inline struct event_file_link *
+find_event_file_link(struct trace_probe *tp, struct ftrace_event_file *file)
+{
+ struct event_file_link *link;
+
+ list_for_each_entry(link, &tp->files, list)
+ if (link->file == file)
+ return link;
+
+ return NULL;
+}
+
extern int traceprobe_parse_probe_arg(char *arg, ssize_t *size,
struct probe_arg *parg, bool is_return, bool is_kprobe);

diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index d83155e0da78..349c6df9e332 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -260,6 +260,7 @@ alloc_trace_uprobe(const char *group, const char *event, int nargs, bool is_ret)
goto error;

INIT_LIST_HEAD(&tu->list);
+ INIT_LIST_HEAD(&tu->tp.files);
tu->consumer.handler = uprobe_dispatcher;
if (is_ret)
tu->consumer.ret_handler = uretprobe_dispatcher;
@@ -760,7 +761,8 @@ static void uprobe_buffer_put(struct uprobe_cpu_buffer *ucb)

static void __uprobe_trace_func(struct trace_uprobe *tu,
unsigned long func, struct pt_regs *regs,
- struct uprobe_cpu_buffer *ucb, int dsize)
+ struct uprobe_cpu_buffer *ucb, int dsize,
+ struct ftrace_event_file *ftrace_file)
{
struct uprobe_trace_entry_head *entry;
struct ring_buffer_event *event;
@@ -769,13 +771,15 @@ static void __uprobe_trace_func(struct trace_uprobe *tu,
int size, esize;
struct ftrace_event_call *call = &tu->tp.call;

+ WARN_ON(call != ftrace_file->event_call);
+
if (WARN_ON_ONCE(tu->tp.size + dsize > PAGE_SIZE))
return;

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

@@ -799,8 +803,16 @@ static void __uprobe_trace_func(struct trace_uprobe *tu,
static int uprobe_trace_func(struct trace_uprobe *tu, struct pt_regs *regs,
struct uprobe_cpu_buffer *ucb, int dsize)
{
- if (!is_ret_probe(tu))
- __uprobe_trace_func(tu, 0, regs, ucb, dsize);
+ struct event_file_link *link;
+
+ if (is_ret_probe(tu))
+ return 0;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(link, &tu->tp.files, list)
+ __uprobe_trace_func(tu, 0, regs, ucb, dsize, link->file);
+ rcu_read_unlock();
+
return 0;
}

@@ -808,7 +820,12 @@ static void uretprobe_trace_func(struct trace_uprobe *tu, unsigned long func,
struct pt_regs *regs,
struct uprobe_cpu_buffer *ucb, int dsize)
{
- __uprobe_trace_func(tu, func, regs, ucb, dsize);
+ struct event_file_link *link;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(link, &tu->tp.files, list)
+ __uprobe_trace_func(tu, func, regs, ucb, dsize, link->file);
+ rcu_read_unlock();
}

/* Event entry printers */
@@ -855,12 +872,31 @@ typedef bool (*filter_func_t)(struct uprobe_consumer *self,
struct mm_struct *mm);

static int
-probe_event_enable(struct trace_uprobe *tu, int flag, filter_func_t filter)
+probe_event_enable(struct trace_uprobe *tu, struct ftrace_event_file *file,
+ filter_func_t filter)
{
- int ret = 0;
+ bool enabled = trace_probe_is_enabled(&tu->tp);
+ struct event_file_link *link = NULL;
+ int ret;
+
+ if (file) {
+ if (tu->tp.flags & TP_FLAG_PROFILE)
+ return -EINTR;

- if (trace_probe_is_enabled(&tu->tp))
- return -EINTR;
+ link = kmalloc(sizeof(*link), GFP_KERNEL);
+ if (!link)
+ return -ENOMEM;
+
+ link->file = file;
+ list_add_tail_rcu(&link->list, &tu->tp.files);
+
+ tu->tp.flags |= TP_FLAG_TRACE;
+ } else {
+ if (tu->tp.flags & TP_FLAG_TRACE)
+ return -EINTR;
+
+ tu->tp.flags |= TP_FLAG_PROFILE;
+ }

ret = uprobe_buffer_enable();
if (ret < 0)
@@ -868,24 +904,49 @@ probe_event_enable(struct trace_uprobe *tu, int flag, filter_func_t filter)

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

- tu->tp.flags |= flag;
+ if (enabled)
+ return 0;
+
tu->consumer.filter = filter;
ret = uprobe_register(tu->inode, tu->offset, &tu->consumer);
- if (ret)
- tu->tp.flags &= ~flag;
+ if (ret) {
+ if (file) {
+ list_del(&link->list);
+ kfree(link);
+ tu->tp.flags &= ~TP_FLAG_TRACE;
+ } else
+ tu->tp.flags &= ~TP_FLAG_PROFILE;
+ }

return ret;
}

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

+ if (file) {
+ struct event_file_link *link;
+
+ link = find_event_file_link(&tu->tp, file);
+ if (!link)
+ return;
+
+ list_del_rcu(&link->list);
+ /* synchronize with u{,ret}probe_trace_func */
+ synchronize_sched();
+ kfree(link);
+
+ if (!list_empty(&tu->tp.files))
+ return;
+ }
+
WARN_ON(!uprobe_filter_is_empty(&tu->filter));

uprobe_unregister(tu->inode, tu->offset, &tu->consumer);
- tu->tp.flags &= ~flag;
+ tu->tp.flags &= file ? ~TP_FLAG_TRACE : ~TP_FLAG_PROFILE;

uprobe_buffer_disable();
}
@@ -1077,25 +1138,27 @@ static void uretprobe_perf_func(struct trace_uprobe *tu, unsigned long func,
}
#endif /* CONFIG_PERF_EVENTS */

-static
-int trace_uprobe_register(struct ftrace_event_call *event, enum trace_reg type, void *data)
+static int
+trace_uprobe_register(struct ftrace_event_call *event, enum trace_reg type,
+ void *data)
{
struct trace_uprobe *tu = event->data;
+ struct ftrace_event_file *file = data;

switch (type) {
case TRACE_REG_REGISTER:
- return probe_event_enable(tu, TP_FLAG_TRACE, NULL);
+ return probe_event_enable(tu, file, NULL);

case TRACE_REG_UNREGISTER:
- probe_event_disable(tu, TP_FLAG_TRACE);
+ probe_event_disable(tu, file);
return 0;

#ifdef CONFIG_PERF_EVENTS
case TRACE_REG_PERF_REGISTER:
- return probe_event_enable(tu, TP_FLAG_PROFILE, uprobe_perf_filter);
+ return probe_event_enable(tu, NULL, uprobe_perf_filter);

case TRACE_REG_PERF_UNREGISTER:
- probe_event_disable(tu, TP_FLAG_PROFILE);
+ probe_event_disable(tu, NULL);
return 0;

case TRACE_REG_PERF_OPEN:
--
1.7.11.7

2014-01-17 08:09:56

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 5/5] tracing/uprobes: Support mix of ftrace and perf

It seems there's no reason to prevent mixed used of ftrace and perf
for a single uprobe event. At least the kprobes already support it.

Cc: Masami Hiramatsu <[email protected]>
Cc: Oleg Nesterov <[email protected]>
Cc: Srikar Dronamraju <[email protected]>
Cc: zhangwei(Jovi) <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
kernel/trace/trace_uprobe.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 01fcb0db75cb..e4473367e7a4 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -882,9 +882,6 @@ probe_event_enable(struct trace_uprobe *tu, struct ftrace_event_file *file,
int ret;

if (file) {
- if (tu->tp.flags & TP_FLAG_PROFILE)
- return -EINTR;
-
link = kmalloc(sizeof(*link), GFP_KERNEL);
if (!link)
return -ENOMEM;
@@ -893,12 +890,8 @@ probe_event_enable(struct trace_uprobe *tu, struct ftrace_event_file *file,
list_add_tail_rcu(&link->list, &tu->tp.files);

tu->tp.flags |= TP_FLAG_TRACE;
- } else {
- if (tu->tp.flags & TP_FLAG_TRACE)
- return -EINTR;
-
+ } else
tu->tp.flags |= TP_FLAG_PROFILE;
- }

ret = uprobe_buffer_enable();
if (ret < 0)
--
1.7.11.7

2014-01-20 02:11:57

by zhangwei(Jovi)

[permalink] [raw]
Subject: Re: [PATCHSET 0/5] tracing/uprobes: Support multi buffer and event trigger

Hi Namhyung,

On 2014/1/17 16:08, Namhyung Kim wrote:
> Hello,
> (Resending with LKML CC'ed)
>
> This patchset tries to add support for recent multi buffer and event
> trigger changes to uprobes. The multi buffer support patch is an
> updated version of Zovi's previous patch v6 [1].
>
> Zovi, please tell me if you have any update and/or issues with this.
>
No more update on [Patch 3/5] from my side.
Thank you for resending this patch!

Jovi

2014-01-23 00:00:01

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCHSET 0/5] tracing/uprobes: Support multi buffer and event trigger

Hi Jovi,

On Mon, Jan 20, 2014 at 2:10 AM, zhangwei(Jovi)
<[email protected]> wrote:
> Hi Namhyung,
>
> On 2014/1/17 16:08, Namhyung Kim wrote:
>> Hello,
>> (Resending with LKML CC'ed)
>>
>> This patchset tries to add support for recent multi buffer and event
>> trigger changes to uprobes. The multi buffer support patch is an
>> updated version of Zovi's previous patch v6 [1].
>>
>> Zovi, please tell me if you have any update and/or issues with this.
>>
> No more update on [Patch 3/5] from my side.
> Thank you for resending this patch!

Apologies for misspelling your name again, and thanks for the reply. :)

Thanks,
Namhyung

2014-02-03 05:06:15

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCHSET 0/5] tracing/uprobes: Support multi buffer and event trigger

Ping!

On Fri, 17 Jan 2014 17:08:35 +0900, Namhyung Kim wrote:
> Hello,
> (Resending with LKML CC'ed)
>
> This patchset tries to add support for recent multi buffer and event
> trigger changes to uprobes. The multi buffer support patch is an
> updated version of Zovi's previous patch v6 [1].
>
> Zovi, please tell me if you have any update and/or issues with this.
>
> Masami and Oleg, I kept your Reviewed-by's in the patch since I think
> it's just an rebase. Please take a look again to see whether I added
> some mistakes.
>
>
> You can also get it from 'uprobe/trigger-v1' branch in my tree
>
> git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
>
> Any comments are welcome, thanks
> Namhyung
>
>
> [1] https://lkml.org/lkml/2013/7/4/165
>
> Cc: Masami Hiramatsu <[email protected]>
> Cc: Oleg Nesterov <[email protected]>
> Cc: Srikar Dronamraju <[email protected]>
> Cc: zhangwei(Jovi) <[email protected]>
> Cc: Tom Zanussi <[email protected]>
>
>
> Namhyung Kim (4):
> tracing/uprobes: Rename uprobe_{trace,perf}_print() functions
> tracing/uprobes: Move argument fetching to uprobe_dispatcher()
> tracing/uprobes: Support event triggering
> tracing/uprobes: Support mix of ftrace and perf
>
> zhangwei(Jovi) (1):
> tracing/uprobes: Support ftrace_event_file base multibuffer
>
> kernel/trace/trace_kprobe.c | 17 ----
> kernel/trace/trace_probe.h | 17 ++++
> kernel/trace/trace_uprobe.c | 191 +++++++++++++++++++++++++++++++-------------
> 3 files changed, 151 insertions(+), 74 deletions(-)

2014-02-04 01:58:39

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCHSET 0/5] tracing/uprobes: Support multi buffer and event trigger

On Mon, 03 Feb 2014 14:06:12 +0900
Namhyung Kim <[email protected]> wrote:

> Ping!

Hi Namhyung,

I plan on getting these ready for the 3.15 queue. There was a bit too
much in 3.14 to add these on top of at the last minute.

Currently, I'm working on some bugs at work as well as some things I
found in mainline. I'll be reviewing these when I get a chance.

Thanks,

-- Steve

2014-02-04 23:31:22

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCHSET 0/5] tracing/uprobes: Support multi buffer and event trigger

Hi Steve,

On Mon, 3 Feb 2014 20:58:28 -0500, Steven Rostedt wrote:
> On Mon, 03 Feb 2014 14:06:12 +0900
> Namhyung Kim <[email protected]> wrote:
>
>> Ping!
>
> Hi Namhyung,
>
> I plan on getting these ready for the 3.15 queue. There was a bit too
> much in 3.14 to add these on top of at the last minute.

I'm fine with it.

>
> Currently, I'm working on some bugs at work as well as some things I
> found in mainline. I'll be reviewing these when I get a chance.

Great, thanks!
Namhyung

Subject: Re: [PATCHSET 0/5] tracing/uprobes: Support multi buffer and event trigger

(2014/01/17 17:08), Namhyung Kim wrote:
> Hello,
> (Resending with LKML CC'ed)
>
> This patchset tries to add support for recent multi buffer and event
> trigger changes to uprobes. The multi buffer support patch is an
> updated version of Zovi's previous patch v6 [1].
>
> Zovi, please tell me if you have any update and/or issues with this.
>
> Masami and Oleg, I kept your Reviewed-by's in the patch since I think
> it's just an rebase. Please take a look again to see whether I added
> some mistakes.

OK, this series looks good to me. Thank you for enhancing it!
Feel free to add my reviewed-by.

Reviewed-by: Masami Hiramatsu <[email protected]>

Thanks,

>
>
> You can also get it from 'uprobe/trigger-v1' branch in my tree
>
> git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
>
> Any comments are welcome, thanks
> Namhyung
>
>
> [1] https://lkml.org/lkml/2013/7/4/165
>
> Cc: Masami Hiramatsu <[email protected]>
> Cc: Oleg Nesterov <[email protected]>
> Cc: Srikar Dronamraju <[email protected]>
> Cc: zhangwei(Jovi) <[email protected]>
> Cc: Tom Zanussi <[email protected]>
>
>
> Namhyung Kim (4):
> tracing/uprobes: Rename uprobe_{trace,perf}_print() functions
> tracing/uprobes: Move argument fetching to uprobe_dispatcher()
> tracing/uprobes: Support event triggering
> tracing/uprobes: Support mix of ftrace and perf
>
> zhangwei(Jovi) (1):
> tracing/uprobes: Support ftrace_event_file base multibuffer
>
> kernel/trace/trace_kprobe.c | 17 ----
> kernel/trace/trace_probe.h | 17 ++++
> kernel/trace/trace_uprobe.c | 191 +++++++++++++++++++++++++++++++-------------
> 3 files changed, 151 insertions(+), 74 deletions(-)
>


--
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]

Subject: Re: [PATCH 2/5] tracing/uprobes: Move argument fetching to uprobe_dispatcher()

(2014/01/17 17:08), Namhyung Kim wrote:
> A single uprobe event might serve different users like ftrace and
> perf. And this is especially important for upcoming multi buffer
> support. But in this case it'll fetch (same) data from userspace
> multiple times. So move it to the beginning of the dispatcher
> function and reuse it for each users.

Looks good to me. :)

BTW, it seems that there is a similar issue on kprobes too.
I'm sure that only one kprobe can run on a CPU, thus I think
I can do the same implementation on kprobes tracer too.

Thank you,

>
> Cc: Masami Hiramatsu <[email protected]>
> Cc: Oleg Nesterov <[email protected]>
> Cc: Srikar Dronamraju <[email protected]>
> Cc: zhangwei(Jovi) <[email protected]>
> Signed-off-by: Namhyung Kim <[email protected]>
> ---
> kernel/trace/trace_uprobe.c | 93 +++++++++++++++++++++++++++------------------
> 1 file changed, 56 insertions(+), 37 deletions(-)
>
> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
> index c5d2612bf233..d83155e0da78 100644
> --- a/kernel/trace/trace_uprobe.c
> +++ b/kernel/trace/trace_uprobe.c
> @@ -759,30 +759,25 @@ static void uprobe_buffer_put(struct uprobe_cpu_buffer *ucb)
> }
>
> static void __uprobe_trace_func(struct trace_uprobe *tu,
> - unsigned long func, struct pt_regs *regs)
> + unsigned long func, struct pt_regs *regs,
> + struct uprobe_cpu_buffer *ucb, int dsize)
> {
> struct uprobe_trace_entry_head *entry;
> struct ring_buffer_event *event;
> struct ring_buffer *buffer;
> - struct uprobe_cpu_buffer *ucb;
> void *data;
> - int size, dsize, esize;
> + int size, esize;
> struct ftrace_event_call *call = &tu->tp.call;
>
> - 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))
> + if (WARN_ON_ONCE(tu->tp.size + dsize > PAGE_SIZE))
> return;
>
> - ucb = uprobe_buffer_get();
> - store_trace_args(esize, &tu->tp, regs, ucb->buf, dsize);
> -
> + esize = SIZEOF_TRACE_ENTRY(is_ret_probe(tu));
> size = esize + tu->tp.size + dsize;
> event = trace_current_buffer_lock_reserve(&buffer, call->event.type,
> size, 0, 0);
> if (!event)
> - goto out;
> + return;
>
> entry = ring_buffer_event_data(event);
> if (is_ret_probe(tu)) {
> @@ -798,23 +793,22 @@ static void __uprobe_trace_func(struct trace_uprobe *tu,
>
> if (!call_filter_check_discard(call, entry, buffer, event))
> trace_buffer_unlock_commit(buffer, event, 0, 0);
> -
> -out:
> - uprobe_buffer_put(ucb);
> }
>
> /* uprobe handler */
> -static int uprobe_trace_func(struct trace_uprobe *tu, struct pt_regs *regs)
> +static int uprobe_trace_func(struct trace_uprobe *tu, struct pt_regs *regs,
> + struct uprobe_cpu_buffer *ucb, int dsize)
> {
> if (!is_ret_probe(tu))
> - __uprobe_trace_func(tu, 0, regs);
> + __uprobe_trace_func(tu, 0, regs, ucb, dsize);
> return 0;
> }
>
> static void uretprobe_trace_func(struct trace_uprobe *tu, unsigned long func,
> - struct pt_regs *regs)
> + struct pt_regs *regs,
> + struct uprobe_cpu_buffer *ucb, int dsize)
> {
> - __uprobe_trace_func(tu, func, regs);
> + __uprobe_trace_func(tu, func, regs, ucb, dsize);
> }
>
> /* Event entry printers */
> @@ -1015,30 +1009,23 @@ static bool uprobe_perf_filter(struct uprobe_consumer *uc,
> }
>
> static void __uprobe_perf_func(struct trace_uprobe *tu,
> - unsigned long func, struct pt_regs *regs)
> + unsigned long func, struct pt_regs *regs,
> + struct uprobe_cpu_buffer *ucb, int dsize)
> {
> 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, dsize, esize;
> + int size, esize;
> int rctx;
>
> - dsize = __get_data_size(&tu->tp, regs);
> esize = SIZEOF_TRACE_ENTRY(is_ret_probe(tu));
>
> - 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);
> if (hlist_empty(head))
> @@ -1068,24 +1055,25 @@ static void __uprobe_perf_func(struct trace_uprobe *tu,
> perf_trace_buf_submit(entry, size, rctx, 0, 1, regs, head, NULL);
> out:
> preempt_enable();
> - uprobe_buffer_put(ucb);
> }
>
> /* uprobe profile handler */
> -static int uprobe_perf_func(struct trace_uprobe *tu, struct pt_regs *regs)
> +static int uprobe_perf_func(struct trace_uprobe *tu, struct pt_regs *regs,
> + struct uprobe_cpu_buffer *ucb, int dsize)
> {
> if (!uprobe_perf_filter(&tu->consumer, 0, current->mm))
> return UPROBE_HANDLER_REMOVE;
>
> if (!is_ret_probe(tu))
> - __uprobe_perf_func(tu, 0, regs);
> + __uprobe_perf_func(tu, 0, regs, ucb, dsize);
> return 0;
> }
>
> static void uretprobe_perf_func(struct trace_uprobe *tu, unsigned long func,
> - struct pt_regs *regs)
> + struct pt_regs *regs,
> + struct uprobe_cpu_buffer *ucb, int dsize)
> {
> - __uprobe_perf_func(tu, func, regs);
> + __uprobe_perf_func(tu, func, regs, ucb, dsize);
> }
> #endif /* CONFIG_PERF_EVENTS */
>
> @@ -1127,8 +1115,11 @@ static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs *regs)
> {
> struct trace_uprobe *tu;
> struct uprobe_dispatch_data udd;
> + struct uprobe_cpu_buffer *ucb;
> + int dsize, esize;
> int ret = 0;
>
> +
> tu = container_of(con, struct trace_uprobe, consumer);
> tu->nhit++;
>
> @@ -1137,13 +1128,29 @@ static int uprobe_dispatcher(struct uprobe_consumer *con, struct pt_regs *regs)
>
> current->utask->vaddr = (unsigned long) &udd;
>
> +#ifdef CONFIG_PERF_EVENTS
> + if ((tu->tp.flags & TP_FLAG_TRACE) == 0 &&
> + !uprobe_perf_filter(&tu->consumer, 0, current->mm))
> + return UPROBE_HANDLER_REMOVE;
> +#endif
> +
> + if (WARN_ON_ONCE(!uprobe_cpu_buffer))
> + return 0;
> +
> + dsize = __get_data_size(&tu->tp, regs);
> + esize = SIZEOF_TRACE_ENTRY(is_ret_probe(tu));
> +
> + ucb = uprobe_buffer_get();
> + store_trace_args(esize, &tu->tp, regs, ucb->buf, dsize);
> +
> if (tu->tp.flags & TP_FLAG_TRACE)
> - ret |= uprobe_trace_func(tu, regs);
> + ret |= uprobe_trace_func(tu, regs, ucb, dsize);
>
> #ifdef CONFIG_PERF_EVENTS
> if (tu->tp.flags & TP_FLAG_PROFILE)
> - ret |= uprobe_perf_func(tu, regs);
> + ret |= uprobe_perf_func(tu, regs, ucb, dsize);
> #endif
> + uprobe_buffer_put(ucb);
> return ret;
> }
>
> @@ -1152,6 +1159,8 @@ static int uretprobe_dispatcher(struct uprobe_consumer *con,
> {
> struct trace_uprobe *tu;
> struct uprobe_dispatch_data udd;
> + struct uprobe_cpu_buffer *ucb;
> + int dsize, esize;
>
> tu = container_of(con, struct trace_uprobe, consumer);
>
> @@ -1160,13 +1169,23 @@ static int uretprobe_dispatcher(struct uprobe_consumer *con,
>
> current->utask->vaddr = (unsigned long) &udd;
>
> + if (WARN_ON_ONCE(!uprobe_cpu_buffer))
> + return 0;
> +
> + dsize = __get_data_size(&tu->tp, regs);
> + esize = SIZEOF_TRACE_ENTRY(is_ret_probe(tu));
> +
> + ucb = uprobe_buffer_get();
> + store_trace_args(esize, &tu->tp, regs, ucb->buf, dsize);
> +
> if (tu->tp.flags & TP_FLAG_TRACE)
> - uretprobe_trace_func(tu, func, regs);
> + uretprobe_trace_func(tu, func, regs, ucb, dsize);
>
> #ifdef CONFIG_PERF_EVENTS
> if (tu->tp.flags & TP_FLAG_PROFILE)
> - uretprobe_perf_func(tu, func, regs);
> + uretprobe_perf_func(tu, func, regs, ucb, dsize);
> #endif
> + uprobe_buffer_put(ucb);
> return 0;
> }
>
>


--
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]

2014-02-06 15:19:41

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCHSET 0/5] tracing/uprobes: Support multi buffer and event trigger

On Wed, 05 Feb 2014 08:31:17 +0900
Namhyung Kim <[email protected]> wrote:

> >
> > Currently, I'm working on some bugs at work as well as some things I
> > found in mainline. I'll be reviewing these when I get a chance.
>
> Great, thanks!

Just an update. I looked at them and ran them though some minor tests
(nothing major yet) and they seem fine. I've added them to the end of
my 3.15 queue, but I wont be posting those patches to linux-next until
after 3.14-rc2, where I'll rebase that work on.

Hopefully 3.14-rc2 will not introduce more bugs and have all the fixes
of 3.14-rc1 requires. Makes testing my code easier, instead of having
to keep adding "fixup" patches that fix other people's code in order to
get my tests working.

-- Steve