tracing/uprobes: Support ftrace_event_file base multibuffer

PATCH In review zhangwei(Jovi) 3 participants 5 messages Jul 4, 2013 Tracing

lore

Share & tools

lore

zhangwei(Jovi)

author

2013-07-04 06:54 UTC #0 lore raw

PATCH v5 1/2 tracing/uprobes: Support ftrace_event_file base multibuffer

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 dont't have this limitation now)

Signed-off-by zhangwei(Jovi) <[email protected]>

Cc Masami Hiramatsu <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc Oleg Nesterov <[email protected]>

Cc Srikar Dronamraju <[email protected]>

---
 kernel/trace/trace_uprobe.c |  119 ++++++++++++++++++++++++++++++++++++-------
 1 file changed, 100 insertions(+), 19 deletions(-)

kernel/trace/trace_uprobe.c +100 ~~-19~~

index 32494fb0..2fc9931 100644
@@ -53,6 +53,7 @@ struct trace_uprobe {
 	struct list_head		list;
 	struct ftrace_event_class	class;
 	struct ftrace_event_call	call;
+	struct list_head		files;
 	struct trace_uprobe_filter	filter;
 	struct uprobe_consumer		consumer;
 	struct inode			*inode;

@@ -65,6 +66,11 @@ struct trace_uprobe {
 	struct probe_arg		args[];
 };
 
+struct event_file_link {
+	struct ftrace_event_file	*file;
+	struct list_head		list;
+};
+
 #define SIZEOF_TRACE_UPROBE(n)			\
 	(offsetof(struct trace_uprobe, args) +	\
 	(sizeof(struct probe_arg) * (n)))

@@ -124,6 +130,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->files);
 	tu->consumer.handler = uprobe_dispatcher;
 	if (is_ret)
 		tu->consumer.ret_handler = uretprobe_dispatcher;

@@ -511,7 +518,8 @@ static const struct file_operations uprobe_profile_ops = {
 };
 
 static void uprobe_trace_print(struct trace_uprobe *tu,
-				unsigned long func, struct pt_regs *regs)
+				unsigned long func, struct pt_regs *regs,
+				struct ftrace_event_file *ftrace_file)
 {
 	struct uprobe_trace_entry_head *entry;
 	struct ring_buffer_event *event;

@@ -520,9 +528,12 @@ static void uprobe_trace_print(struct trace_uprobe *tu,
 	int size, i;
 	struct ftrace_event_call *call = &tu->call;
 
+	WARN_ON(call != ftrace_file->event_call);
+
 	size = SIZEOF_TRACE_ENTRY(is_ret_probe(tu));
-	event = trace_current_buffer_lock_reserve(&buffer, call->event.type,
-						  size + tu->size, 0, 0);
+	event = trace_event_buffer_lock_reserve(&buffer, ftrace_file,
+						call->event.type,
+						size + tu->size, 0, 0);
 	if (!event)
 		return;

@@ -546,15 +557,28 @@ static void uprobe_trace_print(struct trace_uprobe *tu,
 /* uprobe handler */
 static int uprobe_trace_func(struct trace_uprobe *tu, struct pt_regs *regs)
 {
-	if (!is_ret_probe(tu))
-		uprobe_trace_print(tu, 0, regs);
+	struct event_file_link *link;
+
+	if (is_ret_probe(tu))
+		return 0;
+
+	rcu_read_lock();
+	list_for_each_entry(link, &tu->files, list)
+		uprobe_trace_print(tu, 0, regs, link->file);
+	rcu_read_unlock();
+
 	return 0;
 }
 
 static void uretprobe_trace_func(struct trace_uprobe *tu, unsigned long func,
 				struct pt_regs *regs)
 {
-	uprobe_trace_print(tu, func, regs);
+	struct event_file_link *link;
+
+	rcu_read_lock();
+	list_for_each_entry(link, &tu->files, list)
+		uprobe_trace_print(tu, func, regs, link->file);
+	rcu_read_unlock();
 }
 
 /* Event entry printers */

@@ -605,33 +629,89 @@ 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 = is_trace_uprobe_enabled(tu);
+	struct event_file_link *link;
+	int ret;
+
+	if (file) {
+		if (tu->flags & TP_FLAG_PROFILE)
+			return -EINTR;
 
-	if (is_trace_uprobe_enabled(tu))
-		return -EINTR;
+		link = kmalloc(sizeof(*link), GFP_KERNEL);
+		if (!link)
+			return -ENOMEM;
+
+		link->file = file;
+		list_add_tail_rcu(&link->list, &tu->files);
+
+		tu->flags |= TP_FLAG_TRACE;
+	} else {
+		if (tu->flags & TP_FLAG_TRACE)
+			return -EINTR;
+
+		tu->flags |= TP_FLAG_PROFILE;
+	}
 
 	WARN_ON(!uprobe_filter_is_empty(&tu->filter));
 
-	tu->flags |= flag;
+	if (enabled)
+		return;
+
 	tu->consumer.filter = filter;
 	ret = uprobe_register(tu->inode, tu->offset, &tu->consumer);
-	if (ret)
-		tu->flags &= ~flag;
+	if (ret) {
+		if (file) {
+			list_del(&link->list);
+			kfree(link);
+			tu->flags &= ~TP_FLAG_TRACE;
+		} else
+			tu->flags &= ~TP_FLAG_PROFILE;
+	}
 
 	return ret;
 }
 
-static void probe_event_disable(struct trace_uprobe *tu, int flag)
+static struct event_file_link *
+find_event_file_link(struct trace_uprobe *tu, struct ftrace_event_file *file)
+{
+	struct event_file_link *link;
+
+	list_for_each_entry(link, &tu->files, list)
+		if (link->file == file)
+			return link;
+
+	return NULL;
+}
+
+static void
+probe_event_disable(struct trace_uprobe *tu, struct ftrace_event_file *file)
 {
 	if (!is_trace_uprobe_enabled(tu))
 		return;
 
+	if (file) {
+		struct event_file_link *link;
+
+		link = find_event_file_link(tu, file);
+		if (!link)
+			return;
+
+		list_del_rcu(&link->list);
+		/* synchronize with uprobe_trace_func/uretprobe_trace_func */
+		synchronize_sched();
+		kfree(link);
+
+		if (!list_empty(&tu->files))
+			return;
+	}
+
 	WARN_ON(!uprobe_filter_is_empty(&tu->filter));
 
 	uprobe_unregister(tu->inode, tu->offset, &tu->consumer);
-	tu->flags &= ~flag;
+	tu->flags &= file ? ~TP_FLAG_TRACE : ~TP_FLAG_PROFILE;
 }
 
 static int uprobe_event_define_fields(struct ftrace_event_call *event_call)

@@ -867,21 +947,22 @@ 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:

Signature · 1 line

1.7.9.7

zhangwei(Jovi)

author

2013-07-04 07:03 UTC #1 lore raw

PATCH v5 1/2 tracing/uprobes: Support ftrace_event_file base multibuffer

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.

Signed-off-by zhangwei(Jovi) <[email protected]>

Cc Masami Hiramatsu <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc Oleg Nesterov <[email protected]>

Cc Srikar Dronamraju <[email protected]>

---
 kernel/trace/trace_uprobe.c |  119 ++++++++++++++++++++++++++++++++++++-------
 1 file changed, 100 insertions(+), 19 deletions(-)

kernel/trace/trace_uprobe.c +100 ~~-19~~

index 32494fb0..2fc9931 100644
@@ -53,6 +53,7 @@ struct trace_uprobe {
 	struct list_head		list;
 	struct ftrace_event_class	class;
 	struct ftrace_event_call	call;
+	struct list_head		files;
 	struct trace_uprobe_filter	filter;
 	struct uprobe_consumer		consumer;
 	struct inode			*inode;

@@ -65,6 +66,11 @@ struct trace_uprobe {
 	struct probe_arg		args[];
 };
 
+struct event_file_link {
+	struct ftrace_event_file	*file;
+	struct list_head		list;
+};
+
 #define SIZEOF_TRACE_UPROBE(n)			\
 	(offsetof(struct trace_uprobe, args) +	\
 	(sizeof(struct probe_arg) * (n)))

@@ -124,6 +130,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->files);
 	tu->consumer.handler = uprobe_dispatcher;
 	if (is_ret)
 		tu->consumer.ret_handler = uretprobe_dispatcher;

@@ -511,7 +518,8 @@ static const struct file_operations uprobe_profile_ops = {
 };
 
 static void uprobe_trace_print(struct trace_uprobe *tu,
-				unsigned long func, struct pt_regs *regs)
+				unsigned long func, struct pt_regs *regs,
+				struct ftrace_event_file *ftrace_file)
 {
 	struct uprobe_trace_entry_head *entry;
 	struct ring_buffer_event *event;

@@ -520,9 +528,12 @@ static void uprobe_trace_print(struct trace_uprobe *tu,
 	int size, i;
 	struct ftrace_event_call *call = &tu->call;
 
+	WARN_ON(call != ftrace_file->event_call);
+
 	size = SIZEOF_TRACE_ENTRY(is_ret_probe(tu));
-	event = trace_current_buffer_lock_reserve(&buffer, call->event.type,
-						  size + tu->size, 0, 0);
+	event = trace_event_buffer_lock_reserve(&buffer, ftrace_file,
+						call->event.type,
+						size + tu->size, 0, 0);
 	if (!event)
 		return;

@@ -546,15 +557,28 @@ static void uprobe_trace_print(struct trace_uprobe *tu,
 /* uprobe handler */
 static int uprobe_trace_func(struct trace_uprobe *tu, struct pt_regs *regs)
 {
-	if (!is_ret_probe(tu))
-		uprobe_trace_print(tu, 0, regs);
+	struct event_file_link *link;
+
+	if (is_ret_probe(tu))
+		return 0;
+
+	rcu_read_lock();
+	list_for_each_entry(link, &tu->files, list)
+		uprobe_trace_print(tu, 0, regs, link->file);
+	rcu_read_unlock();
+
 	return 0;
 }
 
 static void uretprobe_trace_func(struct trace_uprobe *tu, unsigned long func,
 				struct pt_regs *regs)
 {
-	uprobe_trace_print(tu, func, regs);
+	struct event_file_link *link;
+
+	rcu_read_lock();
+	list_for_each_entry(link, &tu->files, list)
+		uprobe_trace_print(tu, func, regs, link->file);
+	rcu_read_unlock();
 }
 
 /* Event entry printers */

@@ -605,33 +629,89 @@ 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 = is_trace_uprobe_enabled(tu);
+	struct event_file_link *link;
+	int ret;
+
+	if (file) {
+		if (tu->flags & TP_FLAG_PROFILE)
+			return -EINTR;
 
-	if (is_trace_uprobe_enabled(tu))
-		return -EINTR;
+		link = kmalloc(sizeof(*link), GFP_KERNEL);
+		if (!link)
+			return -ENOMEM;
+
+		link->file = file;
+		list_add_tail_rcu(&link->list, &tu->files);
+
+		tu->flags |= TP_FLAG_TRACE;
+	} else {
+		if (tu->flags & TP_FLAG_TRACE)
+			return -EINTR;
+
+		tu->flags |= TP_FLAG_PROFILE;
+	}
 
 	WARN_ON(!uprobe_filter_is_empty(&tu->filter));
 
-	tu->flags |= flag;
+	if (enabled)
+		return 0;
+
 	tu->consumer.filter = filter;
 	ret = uprobe_register(tu->inode, tu->offset, &tu->consumer);
-	if (ret)
-		tu->flags &= ~flag;
+	if (ret) {
+		if (file) {
+			list_del(&link->list);
+			kfree(link);
+			tu->flags &= ~TP_FLAG_TRACE;
+		} else
+			tu->flags &= ~TP_FLAG_PROFILE;
+	}
 
 	return ret;
 }
 
-static void probe_event_disable(struct trace_uprobe *tu, int flag)
+static struct event_file_link *
+find_event_file_link(struct trace_uprobe *tu, struct ftrace_event_file *file)
+{
+	struct event_file_link *link;
+
+	list_for_each_entry(link, &tu->files, list)
+		if (link->file == file)
+			return link;
+
+	return NULL;
+}
+
+static void
+probe_event_disable(struct trace_uprobe *tu, struct ftrace_event_file *file)
 {
 	if (!is_trace_uprobe_enabled(tu))
 		return;
 
+	if (file) {
+		struct event_file_link *link;
+
+		link = find_event_file_link(tu, file);
+		if (!link)
+			return;
+
+		list_del_rcu(&link->list);
+		/* synchronize with uprobe_trace_func/uretprobe_trace_func */
+		synchronize_sched();
+		kfree(link);
+
+		if (!list_empty(&tu->files))
+			return;
+	}
+
 	WARN_ON(!uprobe_filter_is_empty(&tu->filter));
 
 	uprobe_unregister(tu->inode, tu->offset, &tu->consumer);
-	tu->flags &= ~flag;
+	tu->flags &= file ? ~TP_FLAG_TRACE : ~TP_FLAG_PROFILE;
 }
 
 static int uprobe_event_define_fields(struct ftrace_event_call *event_call)

@@ -867,21 +947,22 @@ 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.9.7

Namhyung Kim

2013-07-04 07:41 UTC #2 lore raw

Hi Jovi,

Just a few of dummy questions..

Quoted 14 lines · zhangwei

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.

Quoted 7 lines

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 dont't have this limitation now)

So why does this limitation exist? Didn't we support this kind of thing in the original code?

Quoted 7 lines

Signed-off-by zhangwei(Jovi) <[email protected]>
Cc Masami Hiramatsu <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Oleg Nesterov <[email protected]>
Cc Srikar Dronamraju <[email protected]>
---

[SNIP]

Review

Quoted 2 lines

(quoted diff) +2 -0

+	rcu_read_lock();
+	list_for_each_entry(link, &tu->files, list)

list_for_each_entry_rcu() ?

Quoted 4 lines

kernel/trace/trace_uprobe.c +3 -0

+		uprobe_trace_print(tu, 0, regs, link->file);
+	rcu_read_unlock();
+
 	return 0;

kernel/trace/trace_uprobe.c static void uprobe_trace_print(struct trace_uprobe *tu, · line 579

Quoted 10 lines

kernel/trace/trace_uprobe.c +4 -1

 }
 
 static void uretprobe_trace_func(struct trace_uprobe *tu, unsigned long func,
 				struct pt_regs *regs)
 {
-	uprobe_trace_print(tu, func, regs);
+	struct event_file_link *link;
+
+	rcu_read_lock();
+	list_for_each_entry(link, &tu->files, list)

Ditto.

Quoted 3 lines

kernel/trace/trace_uprobe.c +2 -0

+		uprobe_trace_print(tu, func, regs, link->file);
+	rcu_read_unlock();
 }

[SNIP]

kernel/trace/trace_uprobe.c typedef bool (*filter_func_t)(struct uprobe_consumer *self, · line 682

Quoted 7 lines

kernel/trace/trace_uprobe.c +6 -1

-static void probe_event_disable(struct trace_uprobe *tu, int flag)
+static struct event_file_link *
+find_event_file_link(struct trace_uprobe *tu, struct ftrace_event_file *file)
+{
+	struct event_file_link *link;
+
+	list_for_each_entry(link, &tu->files, list)

Not sure of this case. ;)

Thanks,
Namhyung

Quoted 64 lines

kernel/trace/trace_uprobe.c +30 -6

+		if (link->file == file)
+			return link;
+
+	return NULL;
+}
+
+static void
+probe_event_disable(struct trace_uprobe *tu, struct ftrace_event_file *file)
 {
 	if (!is_trace_uprobe_enabled(tu))
 		return;
 
+	if (file) {
+		struct event_file_link *link;
+
+		link = find_event_file_link(tu, file);
+		if (!link)
+			return;
+
+		list_del_rcu(&link->list);
+		/* synchronize with uprobe_trace_func/uretprobe_trace_func */
+		synchronize_sched();
+		kfree(link);
+
+		if (!list_empty(&tu->files))
+			return;
+	}
+
 	WARN_ON(!uprobe_filter_is_empty(&tu->filter));
 
 	uprobe_unregister(tu->inode, tu->offset, &tu->consumer);
-	tu->flags &= ~flag;
+	tu->flags &= file ? ~TP_FLAG_TRACE : ~TP_FLAG_PROFILE;
 }
 
 static int uprobe_event_define_fields(struct ftrace_event_call *event_call)

@@ -867,21 +947,22 @@ 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.9.7

zhangwei(Jovi)

author

2013-07-04 08:03 UTC #3 lore raw

Quoted 28 lines · Namhyung Kim

Hi Jovi,
Just a few of dummy questions..
Quoted 14 lines · zhangwei
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.
Quoted 7 lines
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 dont't have this limitation now)

Quoted 3 lines

So why does this limitation exist? Didn't we support this kind of thing in the original code?

Yes, it existed(maybe not exist before uprobe pre-filter work), because uprobe filter is associated with trace_uprobe tightly at present, so we cannot assign TP_FLAG_PROFILE/TP_FLAG_TRACE for same trace_uprobe with different filter.

Perhaps we need to remove the limitation in future.

Quoted 7 lines

Quoted 7 lines
Signed-off-by zhangwei(Jovi) <[email protected]>
Cc Masami Hiramatsu <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Oleg Nesterov <[email protected]>
Cc Srikar Dronamraju <[email protected]>
---

Quoted 6 lines

[SNIP]
Review
Quoted 2 lines
(quoted diff) +2 -0
+	rcu_read_lock();
+	list_for_each_entry(link, &tu->files, list)
list_for_each_entry_rcu() ?

I haven't noticed this, thanks, I will update it.

Quoted 23 lines

Quoted 5 lines
kernel/trace/trace_uprobe.c +3 -0
+		uprobe_trace_print(tu, 0, regs, link->file);
+	rcu_read_unlock();
+
 	return 0;
kernel/trace/trace_uprobe.c static void uprobe_trace_print(struct trace_uprobe *tu, · line 579
Quoted 10 lines
kernel/trace/trace_uprobe.c +4 -1
 }
 
 static void uretprobe_trace_func(struct trace_uprobe *tu, unsigned long func,
 				struct pt_regs *regs)
 {
-	uprobe_trace_print(tu, func, regs);
+	struct event_file_link *link;
+
+	rcu_read_lock();
+	list_for_each_entry(link, &tu->files, list)
Ditto.
Quoted 3 lines
kernel/trace/trace_uprobe.c +2 -0
+		uprobe_trace_print(tu, func, regs, link->file);
+	rcu_read_unlock();
 }
[SNIP]

Quoted 10 lines

kernel/trace/trace_uprobe.c typedef bool (*filter_func_t)(struct uprobe_consumer *self, · line 682
Quoted 7 lines
kernel/trace/trace_uprobe.c +6 -1
-static void probe_event_disable(struct trace_uprobe *tu, int flag)
+static struct event_file_link *
+find_event_file_link(struct trace_uprobe *tu, struct ftrace_event_file *file)
+{
+	struct event_file_link *link;
+
+	list_for_each_entry(link, &tu->files, list)
Not sure of this case. ;)

Yes, _rcu is not needed in here, it's only called in event disable serialized case.

Quoted 70 lines

Thanks,
Namhyung

Quoted 54 lines

kernel/trace/trace_uprobe.c +28 -3

+		if (link->file == file)
+			return link;
+
+	return NULL;
+}
+
+static void
+probe_event_disable(struct trace_uprobe *tu, struct ftrace_event_file *file)
 {
 	if (!is_trace_uprobe_enabled(tu))
 		return;
 
+	if (file) {
+		struct event_file_link *link;
+
+		link = find_event_file_link(tu, file);
+		if (!link)
+			return;
+
+		list_del_rcu(&link->list);
+		/* synchronize with uprobe_trace_func/uretprobe_trace_func */
+		synchronize_sched();
+		kfree(link);
+
+		if (!list_empty(&tu->files))
+			return;
+	}
+
 	WARN_ON(!uprobe_filter_is_empty(&tu->filter));
 
 	uprobe_unregister(tu->inode, tu->offset, &tu->consumer);
-	tu->flags &= ~flag;
+	tu->flags &= file ? ~TP_FLAG_TRACE : ~TP_FLAG_PROFILE;
 }
 
 static int uprobe_event_define_fields(struct ftrace_event_call *event_call)

@@ -867,21 +947,22 @@ 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:

kernel/trace/trace_uprobe.c static

Quoted 10 lines

kernel/trace/trace_uprobe.c +2 -3

-		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.9.7

Oleg Nesterov

2013-07-04 18:01 UTC #4 lore raw

Quoted 11 lines · zhangwei(Jovi)

Quoted 9 lines · Namhyung Kim
Quoted 8 lines
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 dont't have this limitation now)

Quoted 2 lines

Quoted 2 lines
So why does this limitation exist? Didn't we support this kind of thing in the original code?

For no reason.

Quoted 1 line

Yes, it existed(maybe not exist before uprobe pre-filter work),

No, it was always here and I never understood it.

Quoted 3 lines

because uprobe filter
is associated with trace_uprobe tightly at present, so we cannot assign TP_FLAG_PROFILE/TP_FLAG_TRACE for same trace_uprobe with different filter.

More precisely, TRACE_REG_REGISTER should participate (inc/dec nr_systemwide) if CONFIG_PERF_EVENTS.

Quoted 1 line

Perhaps we need to remove the limitation in future.

Yes, I'll remove it.

Oleg.