linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/5] tracing: Unifying dynamic event interface
@ 2018-09-27 15:58 Masami Hiramatsu
  2018-09-27 15:59 ` [RFC PATCH 1/5] tracing/uprobes: Add busy check when cleanup all uprobes Masami Hiramatsu
                   ` (5 more replies)
  0 siblings, 6 replies; 8+ messages in thread
From: Masami Hiramatsu @ 2018-09-27 15:58 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: linux-kernel, Ingo Molnar, Tom Zanussi, Ravi Bangoria

Hi,

This is an RFC series of unifying dynamic event interface on ftrace.
Currently ftrace has 3 dynamic event interfaces, kprobes, uprobes
and synthetic. This series unifies kprobes and uprobes event
interface on "dynamic_events". This enables us to add new dynamic
events easily on same interface, e.g. function events.
The older interfaces are left on the tracefs for backward
compatibility at this moment.

dynamic_events syntax has no different from kprobe_events and
uprobe_events. You can use same syntax for dynamic_events interface.

I think we can integrate synthetic events to this dynamic_events
interface but it will requires new syntax. e.g.

echo "s:<event-name> <args>" >> dynamic_events

If it is OK, I'll add it in next version.

BTW, since this dynamic_events interface derived from *probe_events,
it inherits "all clear when truncate file open" behavior. But if you
think this is too aggressive, I can drop it. (even in that case,
kprobe_events/uprobe_events behavior is not changed)

I also introduced a widely used way to erase entries in other
interfaces of ftrace, that is '!'. So you can now use '!event-name'
or '!group/event' to erase an entry in dynamic_events.
(Wait... it has to be '!p:event' as others do??)

Any thought?

Thank you,

---

Masami Hiramatsu (5):
      tracing/uprobes: Add busy check when cleanup all uprobes
      tracing: Add a unified dynamic event framework
      tracing/kprobes: Use dyn_event framework for kprobe events
      tracing/uprobes: Use dyn_event framework for uprobe events
      tracing: Add generic event-name based remove event method


 Documentation/trace/kprobetrace.rst  |    3 +
 Documentation/trace/uprobetracer.rst |    4 +
 kernel/trace/Kconfig                 |    4 +
 kernel/trace/Makefile                |    1 
 kernel/trace/trace.c                 |    4 +
 kernel/trace/trace_dynevent.c        |  183 +++++++++++++++++++++++++++++++++
 kernel/trace/trace_dynevent.h        |   89 ++++++++++++++++
 kernel/trace/trace_kprobe.c          |  191 +++++++++++++++++++++++-----------
 kernel/trace/trace_uprobe.c          |  175 +++++++++++++++++++++++--------
 9 files changed, 547 insertions(+), 107 deletions(-)
 create mode 100644 kernel/trace/trace_dynevent.c
 create mode 100644 kernel/trace/trace_dynevent.h

--
Masami Hiramatsu (Linaro) <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [RFC PATCH 1/5] tracing/uprobes: Add busy check when cleanup all uprobes
  2018-09-27 15:58 [RFC PATCH 0/5] tracing: Unifying dynamic event interface Masami Hiramatsu
@ 2018-09-27 15:59 ` Masami Hiramatsu
  2018-09-27 15:59 ` [RFC PATCH 2/5] tracing: Add a unified dynamic event framework Masami Hiramatsu
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Masami Hiramatsu @ 2018-09-27 15:59 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: linux-kernel, Ingo Molnar, Tom Zanussi, Ravi Bangoria

Add a busy check loop in cleanup_all_probes() before
trying to remove all events in uprobe_events as same as
kprobe_events does.

Without this change, writing null to uprobe_events will
try to remove events but if one of them is enabled, it
stopped there but some of events are already cleared.

With this change, writing null to uprobe_events make
sure all events are not enabled before removing events.
So, it clears all events, or return an error (-EBUSY)
with keeping all events.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 kernel/trace/trace_uprobe.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 3a7c73c40007..a49583ece5fd 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -609,12 +609,19 @@ static int cleanup_all_probes(void)
 	int ret = 0;
 
 	mutex_lock(&uprobe_lock);
+	/* Ensure no probe is in use. */
+	list_for_each_entry(tu, &uprobe_list, list)
+		if (trace_probe_is_enabled(&tu->tp)) {
+			ret = -EBUSY;
+			goto end;
+		}
 	while (!list_empty(&uprobe_list)) {
 		tu = list_entry(uprobe_list.next, struct trace_uprobe, list);
 		ret = unregister_trace_uprobe(tu);
 		if (ret)
 			break;
 	}
+end:
 	mutex_unlock(&uprobe_lock);
 	return ret;
 }


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [RFC PATCH 2/5] tracing: Add a unified dynamic event framework
  2018-09-27 15:58 [RFC PATCH 0/5] tracing: Unifying dynamic event interface Masami Hiramatsu
  2018-09-27 15:59 ` [RFC PATCH 1/5] tracing/uprobes: Add busy check when cleanup all uprobes Masami Hiramatsu
@ 2018-09-27 15:59 ` Masami Hiramatsu
  2018-09-27 15:59 ` [RFC PATCH 3/5] tracing/kprobes: Use dyn_event framework for kprobe events Masami Hiramatsu
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Masami Hiramatsu @ 2018-09-27 15:59 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: linux-kernel, Ingo Molnar, Tom Zanussi, Ravi Bangoria

Add an unified dynamic event framework for kprobes
and uprobes. Those dynamic events can be co-exist on
same file because those syntax doesn't over-wrapped.

This introduces a framework part which provides a
unified tracefs interface and operations.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 kernel/trace/Kconfig          |    3 +
 kernel/trace/Makefile         |    1 
 kernel/trace/trace.c          |    4 +
 kernel/trace/trace_dynevent.c |  149 +++++++++++++++++++++++++++++++++++++++++
 kernel/trace/trace_dynevent.h |   87 ++++++++++++++++++++++++
 5 files changed, 244 insertions(+)
 create mode 100644 kernel/trace/trace_dynevent.c
 create mode 100644 kernel/trace/trace_dynevent.h

diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 5e3de28c7677..bf2e8a5a91f1 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -518,6 +518,9 @@ config BPF_EVENTS
 	help
 	  This allows the user to attach BPF programs to kprobe events.
 
+config DYNAMIC_EVENTS
+	def_bool n
+
 config PROBE_EVENTS
 	def_bool n
 
diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile
index f81dadbc7c4a..9ff3c4fa91b6 100644
--- a/kernel/trace/Makefile
+++ b/kernel/trace/Makefile
@@ -78,6 +78,7 @@ endif
 ifeq ($(CONFIG_TRACING),y)
 obj-$(CONFIG_KGDB_KDB) += trace_kdb.o
 endif
+obj-$(CONFIG_DYNAMIC_EVENTS) += trace_dynevent.o
 obj-$(CONFIG_PROBE_EVENTS) += trace_probe.o
 obj-$(CONFIG_UPROBE_EVENTS) += trace_uprobe.o
 
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 147be8523560..8368dea25762 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -4603,6 +4603,10 @@ static const char readme_msg[] =
 	"\t\t\t  traces\n"
 #endif
 #endif /* CONFIG_STACK_TRACER */
+#ifdef CONFIG_DYNAMIC_EVENTS
+	"  dynamic_events\t\t- Add/remove/show the generic dynamic events\n"
+	"\t\t\t  Write into this file to define/undefine new trace events.\n"
+#endif
 #ifdef CONFIG_KPROBE_EVENTS
 	"  kprobe_events\t\t- Add/remove/show the kernel dynamic events\n"
 	"\t\t\t  Write into this file to define/undefine new trace events.\n"
diff --git a/kernel/trace/trace_dynevent.c b/kernel/trace/trace_dynevent.c
new file mode 100644
index 000000000000..c829742cfe5d
--- /dev/null
+++ b/kernel/trace/trace_dynevent.c
@@ -0,0 +1,149 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Generic dynamic event control interface
+ *
+ * Copyright (C) 2018 Masami Hiramatsu <mhiramat@kernel.org>
+ */
+
+#include <linux/debugfs.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/tracefs.h>
+
+#include "trace.h"
+#include "trace_dynevent.h"
+
+static DEFINE_MUTEX(dyn_event_ops_mutex);
+static LIST_HEAD(dyn_event_ops_list);
+
+int dyn_event_register(struct dyn_event_operations *ops)
+{
+	if (!ops || !ops->create || !ops->show || !ops->is_busy || !ops->free)
+		return -EINVAL;
+
+	INIT_LIST_HEAD(&ops->list);
+	mutex_lock(&dyn_event_ops_mutex);
+	list_add_tail(&ops->list, &dyn_event_ops_list);
+	mutex_unlock(&dyn_event_ops_mutex);
+	return 0;
+}
+
+static int create_dyn_event(int argc, char **argv)
+{
+	struct dyn_event_operations *ops;
+	int ret;
+
+	mutex_lock(&dyn_event_ops_mutex);
+	list_for_each_entry(ops, &dyn_event_ops_list, list) {
+		ret = ops->create(argc, argv);
+		if (!ret)
+			break;
+	}
+	mutex_unlock(&dyn_event_ops_mutex);
+	return ret;
+}
+
+DEFINE_MUTEX(dyn_event_mutex);
+LIST_HEAD(dyn_event_list);
+
+void *dyn_event_seq_start(struct seq_file *m, loff_t *pos)
+{
+	mutex_lock(&dyn_event_mutex);
+	return seq_list_start(&dyn_event_list, *pos);
+}
+
+void *dyn_event_seq_next(struct seq_file *m, void *v, loff_t *pos)
+{
+	return seq_list_next(v, &dyn_event_list, pos);
+}
+
+void dyn_event_seq_stop(struct seq_file *m, void *v)
+{
+	mutex_unlock(&dyn_event_mutex);
+}
+
+static int dyn_event_seq_show(struct seq_file *m, void *v)
+{
+	struct dyn_event *ev = v;
+
+	if (ev && ev->ops)
+		return ev->ops->show(m, ev);
+
+	return 0;
+}
+
+static const struct seq_operations dyn_event_seq_op = {
+	.start	= dyn_event_seq_start,
+	.next	= dyn_event_seq_next,
+	.stop	= dyn_event_seq_stop,
+	.show	= dyn_event_seq_show
+};
+
+static int release_all_dyn_events(void)
+{
+	struct dyn_event *ev, *tmp;
+	int ret = 0;
+
+	for_each_dyn_event(ev) {
+		if (ev->ops->is_busy(ev))
+			return -EBUSY;
+	}
+	for_each_dyn_event_safe(ev, tmp) {
+		ret = ev->ops->free(ev);
+		if (ret)
+			break;
+	}
+
+	return ret;
+}
+
+static int dyn_event_open(struct inode *inode, struct file *file)
+{
+	int ret;
+
+	if ((file->f_mode & FMODE_WRITE) && (file->f_flags & O_TRUNC)) {
+		ret = release_all_dyn_events();
+		if (ret < 0)
+			return ret;
+	}
+
+	return seq_open(file, &dyn_event_seq_op);
+}
+
+static ssize_t dyn_event_write(struct file *file, const char __user *buffer,
+				size_t count, loff_t *ppos)
+{
+	return trace_parse_run_command(file, buffer, count, ppos,
+				       create_dyn_event);
+}
+
+static const struct file_operations dynamic_events_ops = {
+	.owner          = THIS_MODULE,
+	.open           = dyn_event_open,
+	.read           = seq_read,
+	.llseek         = seq_lseek,
+	.release        = seq_release,
+	.write		= dyn_event_write,
+};
+
+/* Make a tracefs interface for controlling dynamic events */
+static __init int init_dynamic_event(void)
+{
+	struct dentry *d_tracer;
+	struct dentry *entry;
+
+	d_tracer = tracing_init_dentry();
+	if (IS_ERR(d_tracer))
+		return 0;
+
+	entry = tracefs_create_file("dynamic_events", 0644, d_tracer,
+				    NULL, &dynamic_events_ops);
+
+	/* Event list interface */
+	if (!entry)
+		pr_warn("Could not create tracefs 'dynamic_events' entry\n");
+
+	return 0;
+}
+fs_initcall(init_dynamic_event);
diff --git a/kernel/trace/trace_dynevent.h b/kernel/trace/trace_dynevent.h
new file mode 100644
index 000000000000..96cf9ca7adb9
--- /dev/null
+++ b/kernel/trace/trace_dynevent.h
@@ -0,0 +1,87 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Common header file for generic dynamic events.
+ */
+
+#ifndef _TRACE_DYNEVENT_H
+#define _TRACE_DYNEVENT_H
+
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/mutex.h>
+#include <linux/seq_file.h>
+
+struct dyn_event;
+
+/* These ops must be set. No default operations */
+struct dyn_event_operations {
+	struct list_head	list;
+	int (*create)(int argc, char **argv);
+	int (*show)(struct seq_file *m, struct dyn_event *ev);
+	bool (*is_busy)(struct dyn_event *ev);
+	int (*free)(struct dyn_event *ev);
+};
+
+/* Register new dyn_event type -- must be called at first */
+int dyn_event_register(struct dyn_event_operations *ops);
+
+/* Dynamic event list header -- include this in each event structure */
+struct dyn_event {
+	struct list_head		list;
+	struct dyn_event_operations	*ops;
+};
+
+extern struct mutex dyn_event_mutex;
+extern struct list_head dyn_event_list;
+
+static inline
+int dyn_event_init(struct dyn_event *ev, struct dyn_event_operations *ops)
+{
+	if (!ev || !ops)
+		return -EINVAL;
+
+	INIT_LIST_HEAD(&ev->list);
+	ev->ops = ops;
+	return 0;
+}
+
+static inline int dyn_event_add(struct dyn_event *ev)
+{
+	lockdep_assert_held(&dyn_event_mutex);
+
+	if (!ev || !ev->ops)
+		return -EINVAL;
+
+	list_add_tail(&ev->list, &dyn_event_list);
+	return 0;
+}
+
+static inline void dyn_event_remove(struct dyn_event *ev)
+{
+	lockdep_assert_held(&dyn_event_mutex);
+	list_del_init(&ev->list);
+}
+
+void *dyn_event_seq_start(struct seq_file *m, loff_t *pos);
+void *dyn_event_seq_next(struct seq_file *m, void *v, loff_t *pos);
+void dyn_event_seq_stop(struct seq_file *m, void *v);
+
+/*
+ * for_each_dyn_event	-	iterate over the dyn_event list
+ * @pos:	the struct dyn_event * to use as a loop cursor
+ *
+ * This is just a basement of for_each macro. Wrap this for
+ * each actual event structure with ops filtering.
+ */
+#define for_each_dyn_event(pos)	\
+	list_for_each_entry(pos, &dyn_event_list, list)
+
+/*
+ * for_each_dyn_event	-	iterate over the dyn_event list safely
+ * @pos:	the struct dyn_event * to use as a loop cursor
+ * @n:		the struct dyn_event * to use as temporary storage
+ */
+#define for_each_dyn_event_safe(pos, n)	\
+	list_for_each_entry_safe(pos, n, &dyn_event_list, list)
+
+#endif


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [RFC PATCH 3/5] tracing/kprobes: Use dyn_event framework for kprobe events
  2018-09-27 15:58 [RFC PATCH 0/5] tracing: Unifying dynamic event interface Masami Hiramatsu
  2018-09-27 15:59 ` [RFC PATCH 1/5] tracing/uprobes: Add busy check when cleanup all uprobes Masami Hiramatsu
  2018-09-27 15:59 ` [RFC PATCH 2/5] tracing: Add a unified dynamic event framework Masami Hiramatsu
@ 2018-09-27 15:59 ` Masami Hiramatsu
  2018-09-27 16:00 ` [RFC PATCH 4/5] tracing/uprobes: Use dyn_event framework for uprobe events Masami Hiramatsu
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Masami Hiramatsu @ 2018-09-27 15:59 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: linux-kernel, Ingo Molnar, Tom Zanussi, Ravi Bangoria

Use dyn_event framework for kprobe events. This shows
kprobe events on "tracing/dynamic_events" file.

User can also define new events via tracing/dynamic_events.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 Documentation/trace/kprobetrace.rst |    3 +
 kernel/trace/Kconfig                |    1 
 kernel/trace/trace_kprobe.c         |  179 +++++++++++++++++++++++------------
 3 files changed, 124 insertions(+), 59 deletions(-)

diff --git a/Documentation/trace/kprobetrace.rst b/Documentation/trace/kprobetrace.rst
index 8bfc75c90806..96cc071b7103 100644
--- a/Documentation/trace/kprobetrace.rst
+++ b/Documentation/trace/kprobetrace.rst
@@ -20,6 +20,9 @@ current_tracer. Instead of that, add probe points via
 /sys/kernel/debug/tracing/kprobe_events, and enable it via
 /sys/kernel/debug/tracing/events/kprobes/<EVENT>/enable.
 
+You can also use /sys/kernel/debug/tracing/dynamic_events instead of
+kprobe_events. That interface will provide unified access to other
+dynamic events too.
 
 Synopsis of kprobe_events
 -------------------------
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index bf2e8a5a91f1..c0f6b0105609 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -461,6 +461,7 @@ config KPROBE_EVENTS
 	bool "Enable kprobes-based dynamic events"
 	select TRACING
 	select PROBE_EVENTS
+	select DYNAMIC_EVENTS
 	default y
 	help
 	  This allows the user to add tracing events (similar to tracepoints)
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 508396edc56a..421b9d71fedf 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -12,23 +12,65 @@
 #include <linux/rculist.h>
 #include <linux/error-injection.h>
 
+#include "trace_dynevent.h"
 #include "trace_kprobe_selftest.h"
 #include "trace_probe.h"
 
 #define KPROBE_EVENT_SYSTEM "kprobes"
 #define KRETPROBE_MAXACTIVE_MAX 4096
 
+static int trace_kprobe_create(int argc, char **argv);
+static int trace_kprobe_show(struct seq_file *m, struct dyn_event *ev);
+static int trace_kprobe_release(struct dyn_event *ev);
+static bool trace_kprobe_is_busy(struct dyn_event *ev);
+
+static struct dyn_event_operations trace_kprobe_ops = {
+	.create = trace_kprobe_create,
+	.show = trace_kprobe_show,
+	.is_busy = trace_kprobe_is_busy,
+	.free = trace_kprobe_release,
+};
+
 /**
  * Kprobe event core functions
  */
 struct trace_kprobe {
-	struct list_head	list;
+	struct dyn_event	devent;
 	struct kretprobe	rp;	/* Use rp.kp for kprobe use */
 	unsigned long __percpu *nhit;
 	const char		*symbol;	/* symbol name */
 	struct trace_probe	tp;
 };
 
+static bool is_trace_kprobe(struct dyn_event *ev)
+{
+	return ev->ops == &trace_kprobe_ops;
+}
+
+static struct trace_kprobe *to_trace_kprobe(struct dyn_event *ev)
+{
+	return container_of(ev, struct trace_kprobe, devent);
+}
+
+/**
+ * for_each_trace_kprobe - iterate over the trace_kprobe list
+ * @pos:	the struct trace_kprobe * for each entry
+ * @dpos:	the struct dyn_event * to use as a loop cursor
+ */
+#define for_each_trace_kprobe(pos, dpos)	\
+	for_each_dyn_event(dpos)		\
+		if (is_trace_kprobe(dpos) && (pos = to_trace_kprobe(dpos)))
+
+/**
+ * for_each_trace_kprobe_safe - iterate over the trace_kprobe list safely
+ * @pos:	the struct trace_kprobe * for each entry
+ * @dpos:	the struct dyn_event * to use as a loop cursor
+ * @n:		another struct dyn_event * to use as  temporary storage
+ */
+#define for_each_trace_kprobe_safe(pos, dpos, n)	\
+	for_each_dyn_event_safe(dpos, n)		\
+		if (is_trace_kprobe(dpos) && (pos = to_trace_kprobe(dpos)))
+
 #define SIZEOF_TRACE_KPROBE(n)				\
 	(offsetof(struct trace_kprobe, tp.args) +	\
 	(sizeof(struct probe_arg) * (n)))
@@ -66,6 +108,13 @@ static nokprobe_inline bool trace_kprobe_is_on_module(struct trace_kprobe *tk)
 	return !!strchr(trace_kprobe_symbol(tk), ':');
 }
 
+static bool trace_kprobe_is_busy(struct dyn_event *ev)
+{
+	struct trace_kprobe *tk = to_trace_kprobe(ev);
+
+	return trace_probe_is_enabled(&tk->tp);
+}
+
 static nokprobe_inline unsigned long trace_kprobe_nhit(struct trace_kprobe *tk)
 {
 	unsigned long nhit = 0;
@@ -113,9 +162,6 @@ bool trace_kprobe_error_injectable(struct trace_event_call *call)
 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);
-
 static int kprobe_dispatcher(struct kprobe *kp, struct pt_regs *regs);
 static int kretprobe_dispatcher(struct kretprobe_instance *ri,
 				struct pt_regs *regs);
@@ -355,7 +401,7 @@ static struct trace_kprobe *alloc_trace_kprobe(const char *group,
 	if (!tk->tp.class.system)
 		goto error;
 
-	INIT_LIST_HEAD(&tk->list);
+	dyn_event_init(&tk->devent, &trace_kprobe_ops);
 	INIT_LIST_HEAD(&tk->tp.files);
 	return tk;
 error:
@@ -383,9 +429,10 @@ static void free_trace_kprobe(struct trace_kprobe *tk)
 static struct trace_kprobe *find_trace_kprobe(const char *event,
 					      const char *group)
 {
+	struct dyn_event *pos;
 	struct trace_kprobe *tk;
 
-	list_for_each_entry(tk, &probe_list, list)
+	for_each_trace_kprobe(tk, pos)
 		if (strcmp(trace_event_name(&tk->tp.call), event) == 0 &&
 		    strcmp(tk->tp.call.class->system, group) == 0)
 			return tk;
@@ -484,7 +531,7 @@ disable_trace_kprobe(struct trace_kprobe *tk, struct trace_event_file *file)
 	 * created with perf_event_open. We don't need to wait for these
 	 * trace_kprobes
 	 */
-	if (list_empty(&tk->list))
+	if (list_empty(&tk->devent.list))
 		wait = 0;
  out:
 	if (wait) {
@@ -585,7 +632,7 @@ static void __unregister_trace_kprobe(struct trace_kprobe *tk)
 	}
 }
 
-/* Unregister a trace_probe and probe_event: call with locking probe_lock */
+/* Unregister a trace_probe and probe_event */
 static int unregister_trace_kprobe(struct trace_kprobe *tk)
 {
 	/* Enabled event can not be unregistered */
@@ -597,7 +644,7 @@ static int unregister_trace_kprobe(struct trace_kprobe *tk)
 		return -EBUSY;
 
 	__unregister_trace_kprobe(tk);
-	list_del(&tk->list);
+	dyn_event_remove(&tk->devent);
 
 	return 0;
 }
@@ -608,7 +655,7 @@ static int register_trace_kprobe(struct trace_kprobe *tk)
 	struct trace_kprobe *old_tk;
 	int ret;
 
-	mutex_lock(&probe_lock);
+	mutex_lock(&dyn_event_mutex);
 
 	/* Delete old (same name) event if exist */
 	old_tk = find_trace_kprobe(trace_event_name(&tk->tp.call),
@@ -632,10 +679,10 @@ static int register_trace_kprobe(struct trace_kprobe *tk)
 	if (ret < 0)
 		unregister_kprobe_event(tk);
 	else
-		list_add_tail(&tk->list, &probe_list);
+		dyn_event_add(&tk->devent);
 
 end:
-	mutex_unlock(&probe_lock);
+	mutex_unlock(&dyn_event_mutex);
 	return ret;
 }
 
@@ -644,6 +691,7 @@ static int trace_kprobe_module_callback(struct notifier_block *nb,
 				       unsigned long val, void *data)
 {
 	struct module *mod = data;
+	struct dyn_event *pos;
 	struct trace_kprobe *tk;
 	int ret;
 
@@ -651,8 +699,8 @@ static int trace_kprobe_module_callback(struct notifier_block *nb,
 		return NOTIFY_DONE;
 
 	/* Update probes on coming module */
-	mutex_lock(&probe_lock);
-	list_for_each_entry(tk, &probe_list, list) {
+	mutex_lock(&dyn_event_mutex);
+	for_each_trace_kprobe(tk, pos) {
 		if (trace_kprobe_within_module(tk, mod)) {
 			/* Don't need to check busy - this should have gone. */
 			__unregister_trace_kprobe(tk);
@@ -663,7 +711,7 @@ static int trace_kprobe_module_callback(struct notifier_block *nb,
 					mod->name, ret);
 		}
 	}
-	mutex_unlock(&probe_lock);
+	mutex_unlock(&dyn_event_mutex);
 
 	return NOTIFY_DONE;
 }
@@ -681,7 +729,7 @@ static inline void sanitize_event_name(char *name)
 			*name = '_';
 }
 
-static int create_trace_kprobe(int argc, char **argv)
+static int trace_kprobe_create(int argc, char **argv)
 {
 	/*
 	 * Argument syntax:
@@ -774,10 +822,10 @@ static int create_trace_kprobe(int argc, char **argv)
 			pr_info("Delete command needs an event name.\n");
 			return -EINVAL;
 		}
-		mutex_lock(&probe_lock);
+		mutex_lock(&dyn_event_mutex);
 		tk = find_trace_kprobe(event, group);
 		if (!tk) {
-			mutex_unlock(&probe_lock);
+			mutex_unlock(&dyn_event_mutex);
 			pr_info("Event %s/%s doesn't exist.\n", group, event);
 			return -ENOENT;
 		}
@@ -785,7 +833,7 @@ static int create_trace_kprobe(int argc, char **argv)
 		ret = unregister_trace_kprobe(tk);
 		if (ret == 0)
 			free_trace_kprobe(tk);
-		mutex_unlock(&probe_lock);
+		mutex_unlock(&dyn_event_mutex);
 		return ret;
 	}
 
@@ -894,53 +942,46 @@ static int create_trace_kprobe(int argc, char **argv)
 	return ret;
 }
 
+static int trace_kprobe_release(struct dyn_event *ev)
+{
+	struct trace_kprobe *tk = to_trace_kprobe(ev);
+	int ret = unregister_trace_kprobe(tk);
+
+	if (!ret)
+		free_trace_kprobe(tk);
+	return ret;
+}
+
 static int release_all_trace_kprobes(void)
 {
+	struct dyn_event *pos, *n;
 	struct trace_kprobe *tk;
 	int ret = 0;
 
-	mutex_lock(&probe_lock);
+	mutex_lock(&dyn_event_mutex);
 	/* Ensure no probe is in use. */
-	list_for_each_entry(tk, &probe_list, list)
+	for_each_trace_kprobe(tk, pos) {
 		if (trace_probe_is_enabled(&tk->tp)) {
 			ret = -EBUSY;
 			goto end;
 		}
+	}
 	/* TODO: Use batch unregistration */
-	while (!list_empty(&probe_list)) {
-		tk = list_entry(probe_list.next, struct trace_kprobe, list);
-		ret = unregister_trace_kprobe(tk);
+	for_each_trace_kprobe_safe(tk, pos, n) {
+		ret = trace_kprobe_release(&tk->devent);
 		if (ret)
-			goto end;
-		free_trace_kprobe(tk);
+			break;
 	}
 
 end:
-	mutex_unlock(&probe_lock);
+	mutex_unlock(&dyn_event_mutex);
 
 	return ret;
 }
 
-/* Probes listing interfaces */
-static void *probes_seq_start(struct seq_file *m, loff_t *pos)
-{
-	mutex_lock(&probe_lock);
-	return seq_list_start(&probe_list, *pos);
-}
-
-static void *probes_seq_next(struct seq_file *m, void *v, loff_t *pos)
-{
-	return seq_list_next(v, &probe_list, pos);
-}
-
-static void probes_seq_stop(struct seq_file *m, void *v)
+static int trace_kprobe_show(struct seq_file *m, struct dyn_event *ev)
 {
-	mutex_unlock(&probe_lock);
-}
-
-static int probes_seq_show(struct seq_file *m, void *v)
-{
-	struct trace_kprobe *tk = v;
+	struct trace_kprobe *tk = to_trace_kprobe(ev);
 	int i;
 
 	seq_putc(m, trace_kprobe_is_return(tk) ? 'r' : 'p');
@@ -962,10 +1003,20 @@ static int probes_seq_show(struct seq_file *m, void *v)
 	return 0;
 }
 
+static int probes_seq_show(struct seq_file *m, void *v)
+{
+	struct dyn_event *ev = v;
+
+	if (!is_trace_kprobe(ev))
+		return 0;
+
+	return trace_kprobe_show(m, ev);
+}
+
 static const struct seq_operations probes_seq_op = {
-	.start  = probes_seq_start,
-	.next   = probes_seq_next,
-	.stop   = probes_seq_stop,
+	.start  = dyn_event_seq_start,
+	.next   = dyn_event_seq_next,
+	.stop   = dyn_event_seq_stop,
 	.show   = probes_seq_show
 };
 
@@ -986,7 +1037,7 @@ static ssize_t probes_write(struct file *file, const char __user *buffer,
 			    size_t count, loff_t *ppos)
 {
 	return trace_parse_run_command(file, buffer, count, ppos,
-				       create_trace_kprobe);
+				       trace_kprobe_create);
 }
 
 static const struct file_operations kprobe_events_ops = {
@@ -1001,8 +1052,13 @@ 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_kprobe *tk = v;
+	struct dyn_event *ev = v;
+	struct trace_kprobe *tk;
 
+	if (!is_trace_kprobe(ev))
+		return 0;
+
+	tk = to_trace_kprobe(ev);
 	seq_printf(m, "  %-44s %15lu %15lu\n",
 		   trace_event_name(&tk->tp.call),
 		   trace_kprobe_nhit(tk),
@@ -1012,9 +1068,9 @@ static int probes_profile_seq_show(struct seq_file *m, void *v)
 }
 
 static const struct seq_operations profile_seq_op = {
-	.start  = probes_seq_start,
-	.next   = probes_seq_next,
-	.stop   = probes_seq_stop,
+	.start  = dyn_event_seq_start,
+	.next   = dyn_event_seq_next,
+	.stop   = dyn_event_seq_stop,
 	.show   = probes_profile_seq_show
 };
 
@@ -1499,7 +1555,7 @@ create_local_trace_kprobe(char *func, void *addr, unsigned long offs,
 	char *event;
 
 	/*
-	 * local trace_kprobes are not added to probe_list, so they are never
+	 * local trace_kprobes are not added to dyn_event, so they are never
 	 * searched in find_trace_kprobe(). Therefore, there is no concern of
 	 * duplicated name here.
 	 */
@@ -1557,6 +1613,11 @@ static __init int init_kprobe_trace(void)
 {
 	struct dentry *d_tracer;
 	struct dentry *entry;
+	int ret;
+
+	ret = dyn_event_register(&trace_kprobe_ops);
+	if (ret)
+		return ret;
 
 	if (register_module_notifier(&trace_kprobe_module_nb))
 		return -EINVAL;
@@ -1616,7 +1677,7 @@ static __init int kprobe_trace_self_tests_init(void)
 
 	ret = trace_run_command("p:testprobe kprobe_trace_selftest_target "
 				"$stack $stack0 +0($stack)",
-				create_trace_kprobe);
+				trace_kprobe_create);
 	if (WARN_ON_ONCE(ret)) {
 		pr_warn("error on probing function entry.\n");
 		warn++;
@@ -1637,7 +1698,7 @@ static __init int kprobe_trace_self_tests_init(void)
 	}
 
 	ret = trace_run_command("r:testprobe2 kprobe_trace_selftest_target "
-				"$retval", create_trace_kprobe);
+				"$retval", trace_kprobe_create);
 	if (WARN_ON_ONCE(ret)) {
 		pr_warn("error on probing function return.\n");
 		warn++;
@@ -1707,13 +1768,13 @@ static __init int kprobe_trace_self_tests_init(void)
 			disable_trace_kprobe(tk, file);
 	}
 
-	ret = trace_run_command("-:testprobe", create_trace_kprobe);
+	ret = trace_run_command("-:testprobe", trace_kprobe_create);
 	if (WARN_ON_ONCE(ret)) {
 		pr_warn("error on deleting a probe.\n");
 		warn++;
 	}
 
-	ret = trace_run_command("-:testprobe2", create_trace_kprobe);
+	ret = trace_run_command("-:testprobe2", trace_kprobe_create);
 	if (WARN_ON_ONCE(ret)) {
 		pr_warn("error on deleting a probe.\n");
 		warn++;


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [RFC PATCH 4/5] tracing/uprobes: Use dyn_event framework for uprobe events
  2018-09-27 15:58 [RFC PATCH 0/5] tracing: Unifying dynamic event interface Masami Hiramatsu
                   ` (2 preceding siblings ...)
  2018-09-27 15:59 ` [RFC PATCH 3/5] tracing/kprobes: Use dyn_event framework for kprobe events Masami Hiramatsu
@ 2018-09-27 16:00 ` Masami Hiramatsu
  2018-09-27 16:00 ` [RFC PATCH 5/5] tracing: Add generic event-name based remove event method Masami Hiramatsu
  2018-10-01 13:49 ` [RFC PATCH 0/5] tracing: Unifying dynamic event interface Tom Zanussi
  5 siblings, 0 replies; 8+ messages in thread
From: Masami Hiramatsu @ 2018-09-27 16:00 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: linux-kernel, Ingo Molnar, Tom Zanussi, Ravi Bangoria

Use dyn_event framework for uprobe events. This shows
uprobe events on "dynamic_events" file.
User can also define new uprobe events via dynamic_events.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 Documentation/trace/uprobetracer.rst |    4 +
 kernel/trace/trace_uprobe.c          |  158 +++++++++++++++++++++++-----------
 2 files changed, 113 insertions(+), 49 deletions(-)

diff --git a/Documentation/trace/uprobetracer.rst b/Documentation/trace/uprobetracer.rst
index d0822811527a..4c3bfde2ba47 100644
--- a/Documentation/trace/uprobetracer.rst
+++ b/Documentation/trace/uprobetracer.rst
@@ -18,6 +18,10 @@ current_tracer. Instead of that, add probe points via
 However unlike kprobe-event tracer, the uprobe event interface expects the
 user to calculate the offset of the probepoint in the object.
 
+You can also use /sys/kernel/debug/tracing/dynamic_events instead of
+uprobe_events. That interface will provide unified access to other
+dynamic events too.
+
 Synopsis of uprobe_tracer
 -------------------------
 ::
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index a49583ece5fd..90d10ef02f6b 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -14,6 +14,7 @@
 #include <linux/string.h>
 #include <linux/rculist.h>
 
+#include "trace_dynevent.h"
 #include "trace_probe.h"
 
 #define UPROBE_EVENT_SYSTEM	"uprobes"
@@ -36,11 +37,23 @@ struct trace_uprobe_filter {
 	struct list_head	perf_events;
 };
 
+static int trace_uprobe_create(int argc, char **argv);
+static int trace_uprobe_show(struct seq_file *m, struct dyn_event *ev);
+static int trace_uprobe_release(struct dyn_event *ev);
+static bool trace_uprobe_is_busy(struct dyn_event *ev);
+
+static struct dyn_event_operations trace_uprobe_ops = {
+	.create = trace_uprobe_create,
+	.show = trace_uprobe_show,
+	.is_busy = trace_uprobe_is_busy,
+	.free = trace_uprobe_release,
+};
+
 /*
  * uprobe event core functions
  */
 struct trace_uprobe {
-	struct list_head		list;
+	struct dyn_event		devent;
 	struct trace_uprobe_filter	filter;
 	struct uprobe_consumer		consumer;
 	struct path			path;
@@ -52,6 +65,35 @@ struct trace_uprobe {
 	struct trace_probe		tp;
 };
 
+static bool is_trace_uprobe(struct dyn_event *ev)
+{
+	return ev->ops == &trace_uprobe_ops;
+}
+
+static struct trace_uprobe *to_trace_uprobe(struct dyn_event *ev)
+{
+	return container_of(ev, struct trace_uprobe, devent);
+}
+
+/**
+ * for_each_trace_uprobe - iterate over the trace_uprobe list
+ * @pos:	the struct trace_uprobe * for each entry
+ * @dpos:	the struct dyn_event * to use as a loop cursor
+ */
+#define for_each_trace_uprobe(pos, dpos)	\
+	for_each_dyn_event(dpos)		\
+		if (is_trace_uprobe(dpos) && (pos = to_trace_uprobe(dpos)))
+
+/**
+ * for_each_trace_uprobe_safe - iterate over the trace_uprobe list safely
+ * @pos:	the struct trace_uprobe * for each entry
+ * @dpos:	the struct dyn_event * to use as a loop cursor
+ * @n:		another struct dyn_event * to use as  temporary storage
+ */
+#define for_each_trace_uprobe_safe(pos, dpos, n)	\
+	for_each_dyn_event_safe(dpos, n)		\
+		if (is_trace_uprobe(dpos) && (pos = to_trace_uprobe(dpos)))
+
 #define SIZEOF_TRACE_UPROBE(n)				\
 	(offsetof(struct trace_uprobe, tp.args) +	\
 	(sizeof(struct probe_arg) * (n)))
@@ -59,9 +101,6 @@ struct trace_uprobe {
 static int register_uprobe_event(struct trace_uprobe *tu);
 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;
@@ -230,6 +269,13 @@ static inline bool is_ret_probe(struct trace_uprobe *tu)
 	return tu->consumer.ret_handler != NULL;
 }
 
+static bool trace_uprobe_is_busy(struct dyn_event *ev)
+{
+	struct trace_uprobe *tu = to_trace_uprobe(ev);
+
+	return trace_probe_is_enabled(&tu->tp);
+}
+
 /*
  * Allocate new trace_uprobe and initialize it (including uprobes).
  */
@@ -257,7 +303,7 @@ alloc_trace_uprobe(const char *group, const char *event, int nargs, bool is_ret)
 	if (!tu->tp.class.system)
 		goto error;
 
-	INIT_LIST_HEAD(&tu->list);
+	dyn_event_init(&tu->devent, &trace_uprobe_ops);
 	INIT_LIST_HEAD(&tu->tp.files);
 	tu->consumer.handler = uprobe_dispatcher;
 	if (is_ret)
@@ -288,9 +334,10 @@ static void free_trace_uprobe(struct trace_uprobe *tu)
 
 static struct trace_uprobe *find_probe_event(const char *event, const char *group)
 {
+	struct dyn_event *pos;
 	struct trace_uprobe *tu;
 
-	list_for_each_entry(tu, &uprobe_list, list)
+	for_each_trace_uprobe(tu, pos)
 		if (strcmp(trace_event_name(&tu->tp.call), event) == 0 &&
 		    strcmp(tu->tp.call.class->system, group) == 0)
 			return tu;
@@ -298,7 +345,7 @@ static struct trace_uprobe *find_probe_event(const char *event, const char *grou
 	return NULL;
 }
 
-/* Unregister a trace_uprobe and probe_event: call with locking uprobe_lock */
+/* Unregister a trace_uprobe and probe_event */
 static int unregister_trace_uprobe(struct trace_uprobe *tu)
 {
 	int ret;
@@ -307,7 +354,7 @@ static int unregister_trace_uprobe(struct trace_uprobe *tu)
 	if (ret)
 		return ret;
 
-	list_del(&tu->list);
+	dyn_event_remove(&tu->devent);
 	free_trace_uprobe(tu);
 	return 0;
 }
@@ -323,13 +370,14 @@ static int unregister_trace_uprobe(struct trace_uprobe *tu)
  */
 static struct trace_uprobe *find_old_trace_uprobe(struct trace_uprobe *new)
 {
+	struct dyn_event *pos;
 	struct trace_uprobe *tmp, *old = NULL;
 	struct inode *new_inode = d_real_inode(new->path.dentry);
 
 	old = find_probe_event(trace_event_name(&new->tp.call),
 				new->tp.call.class->system);
 
-	list_for_each_entry(tmp, &uprobe_list, list) {
+	for_each_trace_uprobe(tmp, pos) {
 		if ((old ? old != tmp : true) &&
 		    new_inode == d_real_inode(tmp->path.dentry) &&
 		    new->offset == tmp->offset &&
@@ -347,7 +395,7 @@ static int register_trace_uprobe(struct trace_uprobe *tu)
 	struct trace_uprobe *old_tu;
 	int ret;
 
-	mutex_lock(&uprobe_lock);
+	mutex_lock(&dyn_event_mutex);
 
 	/* register as an event */
 	old_tu = find_old_trace_uprobe(tu);
@@ -369,10 +417,10 @@ static int register_trace_uprobe(struct trace_uprobe *tu)
 		goto end;
 	}
 
-	list_add_tail(&tu->list, &uprobe_list);
+	dyn_event_add(&tu->devent);
 
 end:
-	mutex_unlock(&uprobe_lock);
+	mutex_unlock(&dyn_event_mutex);
 
 	return ret;
 }
@@ -383,7 +431,7 @@ static int register_trace_uprobe(struct trace_uprobe *tu)
  *
  *  - Remove uprobe: -:[GRP/]EVENT
  */
-static int create_trace_uprobe(int argc, char **argv)
+static int trace_uprobe_create(int argc, char **argv)
 {
 	struct trace_uprobe *tu;
 	char *arg, *event, *group, *filename, *rctr, *rctr_end;
@@ -439,17 +487,17 @@ static int create_trace_uprobe(int argc, char **argv)
 			pr_info("Delete command needs an event name.\n");
 			return -EINVAL;
 		}
-		mutex_lock(&uprobe_lock);
+		mutex_lock(&dyn_event_mutex);
 		tu = find_probe_event(event, group);
 
 		if (!tu) {
-			mutex_unlock(&uprobe_lock);
+			mutex_unlock(&dyn_event_mutex);
 			pr_info("Event %s/%s doesn't exist.\n", group, event);
 			return -ENOENT;
 		}
 		/* delete an event */
 		ret = unregister_trace_uprobe(tu);
-		mutex_unlock(&uprobe_lock);
+		mutex_unlock(&dyn_event_mutex);
 		return ret;
 	}
 
@@ -603,49 +651,40 @@ static int create_trace_uprobe(int argc, char **argv)
 	return ret;
 }
 
+static int trace_uprobe_release(struct dyn_event *ev)
+{
+	struct trace_uprobe *tu = to_trace_uprobe(ev);
+
+	return unregister_trace_uprobe(tu);
+}
+
 static int cleanup_all_probes(void)
 {
+	struct dyn_event *pos, *n;
 	struct trace_uprobe *tu;
 	int ret = 0;
 
-	mutex_lock(&uprobe_lock);
+	mutex_lock(&dyn_event_mutex);
 	/* Ensure no probe is in use. */
-	list_for_each_entry(tu, &uprobe_list, list)
+	for_each_trace_uprobe(tu, pos)
 		if (trace_probe_is_enabled(&tu->tp)) {
 			ret = -EBUSY;
 			goto end;
 		}
-	while (!list_empty(&uprobe_list)) {
-		tu = list_entry(uprobe_list.next, struct trace_uprobe, list);
+	for_each_trace_uprobe_safe(tu, pos, n) {
 		ret = unregister_trace_uprobe(tu);
 		if (ret)
 			break;
 	}
 end:
-	mutex_unlock(&uprobe_lock);
+	mutex_unlock(&dyn_event_mutex);
 	return ret;
 }
 
 /* Probes listing interfaces */
-static void *probes_seq_start(struct seq_file *m, loff_t *pos)
+static int trace_uprobe_show(struct seq_file *m, struct dyn_event *ev)
 {
-	mutex_lock(&uprobe_lock);
-	return seq_list_start(&uprobe_list, *pos);
-}
-
-static void *probes_seq_next(struct seq_file *m, void *v, loff_t *pos)
-{
-	return seq_list_next(v, &uprobe_list, pos);
-}
-
-static void probes_seq_stop(struct seq_file *m, void *v)
-{
-	mutex_unlock(&uprobe_lock);
-}
-
-static int probes_seq_show(struct seq_file *m, void *v)
-{
-	struct trace_uprobe *tu = v;
+	struct trace_uprobe *tu = to_trace_uprobe(ev);
 	char c = is_ret_probe(tu) ? 'r' : 'p';
 	int i;
 
@@ -663,11 +702,21 @@ static int probes_seq_show(struct seq_file *m, void *v)
 	return 0;
 }
 
+static int probes_seq_show(struct seq_file *m, void *v)
+{
+	struct dyn_event *ev = v;
+
+	if (!is_trace_uprobe(ev))
+		return 0;
+
+	return trace_uprobe_show(m, ev);
+}
+
 static const struct seq_operations probes_seq_op = {
-	.start	= probes_seq_start,
-	.next	= probes_seq_next,
-	.stop	= probes_seq_stop,
-	.show	= probes_seq_show
+	.start  = dyn_event_seq_start,
+	.next   = dyn_event_seq_next,
+	.stop   = dyn_event_seq_stop,
+	.show   = probes_seq_show
 };
 
 static int probes_open(struct inode *inode, struct file *file)
@@ -686,7 +735,8 @@ static int probes_open(struct inode *inode, struct file *file)
 static ssize_t probes_write(struct file *file, const char __user *buffer,
 			    size_t count, loff_t *ppos)
 {
-	return trace_parse_run_command(file, buffer, count, ppos, create_trace_uprobe);
+	return trace_parse_run_command(file, buffer, count, ppos,
+					trace_uprobe_create);
 }
 
 static const struct file_operations uprobe_events_ops = {
@@ -701,17 +751,22 @@ static const struct file_operations uprobe_events_ops = {
 /* Probes profiling interfaces */
 static int probes_profile_seq_show(struct seq_file *m, void *v)
 {
-	struct trace_uprobe *tu = v;
+	struct dyn_event *ev = v;
+	struct trace_uprobe *tu;
 
+	if (!is_trace_uprobe(ev))
+		return 0;
+
+	tu = to_trace_uprobe(ev);
 	seq_printf(m, "  %s %-44s %15lu\n", tu->filename,
 			trace_event_name(&tu->tp.call), tu->nhit);
 	return 0;
 }
 
 static const struct seq_operations profile_seq_op = {
-	.start	= probes_seq_start,
-	.next	= probes_seq_next,
-	.stop	= probes_seq_stop,
+	.start  = dyn_event_seq_start,
+	.next   = dyn_event_seq_next,
+	.stop   = dyn_event_seq_stop,
 	.show	= probes_profile_seq_show
 };
 
@@ -1428,7 +1483,7 @@ create_local_trace_uprobe(char *name, unsigned long offs, bool is_return)
 	}
 
 	/*
-	 * local trace_kprobes are not added to probe_list, so they are never
+	 * local trace_kprobes are not added to dyn_event, so they are never
 	 * searched in find_trace_kprobe(). Therefore, there is no concern of
 	 * duplicated name "DUMMY_EVENT" here.
 	 */
@@ -1475,6 +1530,11 @@ void destroy_local_trace_uprobe(struct trace_event_call *event_call)
 static __init int init_uprobe_trace(void)
 {
 	struct dentry *d_tracer;
+	int ret;
+
+	ret = dyn_event_register(&trace_uprobe_ops);
+	if (ret)
+		return ret;
 
 	d_tracer = tracing_init_dentry();
 	if (IS_ERR(d_tracer))


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [RFC PATCH 5/5] tracing: Add generic event-name based remove event method
  2018-09-27 15:58 [RFC PATCH 0/5] tracing: Unifying dynamic event interface Masami Hiramatsu
                   ` (3 preceding siblings ...)
  2018-09-27 16:00 ` [RFC PATCH 4/5] tracing/uprobes: Use dyn_event framework for uprobe events Masami Hiramatsu
@ 2018-09-27 16:00 ` Masami Hiramatsu
  2018-10-01 13:49 ` [RFC PATCH 0/5] tracing: Unifying dynamic event interface Tom Zanussi
  5 siblings, 0 replies; 8+ messages in thread
From: Masami Hiramatsu @ 2018-09-27 16:00 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: linux-kernel, Ingo Molnar, Tom Zanussi, Ravi Bangoria

Add a generic method to remove event from dynamic event
list. This is same as other system under ftrace. You
just need to pass the event name with '!' prefix, e.g.

  # echo p:new_grp/new_event _do_fork > dynamic_events

  This creates an event, and

  # echo '!new_grp/new_event' > dynamic_events

  Or,

  # echo '!new_event' > dynamic_events

  will remove new_grp/new_event event.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 kernel/trace/trace_dynevent.c |   36 +++++++++++++++++++++++++++++++++++-
 kernel/trace/trace_dynevent.h |    2 ++
 kernel/trace/trace_kprobe.c   |   12 ++++++++++++
 kernel/trace/trace_uprobe.c   |   12 ++++++++++++
 4 files changed, 61 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/trace_dynevent.c b/kernel/trace/trace_dynevent.c
index c829742cfe5d..c33551ad0b15 100644
--- a/kernel/trace/trace_dynevent.c
+++ b/kernel/trace/trace_dynevent.c
@@ -19,7 +19,8 @@ static LIST_HEAD(dyn_event_ops_list);
 
 int dyn_event_register(struct dyn_event_operations *ops)
 {
-	if (!ops || !ops->create || !ops->show || !ops->is_busy || !ops->free)
+	if (!ops || !ops->create || !ops->show || !ops->is_busy ||
+	    !ops->free || !ops->match)
 		return -EINVAL;
 
 	INIT_LIST_HEAD(&ops->list);
@@ -29,11 +30,19 @@ int dyn_event_register(struct dyn_event_operations *ops)
 	return 0;
 }
 
+static int delete_dyn_event(int argc, char **argv);
+
 static int create_dyn_event(int argc, char **argv)
 {
 	struct dyn_event_operations *ops;
 	int ret;
 
+	if (argc == 0)
+		return 0;
+
+	if (argv[0][0] == '!')
+		return delete_dyn_event(argc, argv);
+
 	mutex_lock(&dyn_event_ops_mutex);
 	list_for_each_entry(ops, &dyn_event_ops_list, list) {
 		ret = ops->create(argc, argv);
@@ -80,6 +89,31 @@ static const struct seq_operations dyn_event_seq_op = {
 	.show	= dyn_event_seq_show
 };
 
+static int delete_dyn_event(int argc, char **argv)
+{
+	struct dyn_event *pos, *n;
+	char *system = NULL, *event, *p;
+	int ret = -ENOENT;
+
+	event = &argv[0][1];
+	p = strchr(event, '/');
+	if (p) {
+		system = event;
+		event = p + 1;
+		*p = '\0';
+	}
+	mutex_lock(&dyn_event_mutex);
+	for_each_dyn_event_safe(pos, n) {
+		if (pos->ops->match(system, event, pos)) {
+			ret = pos->ops->free(pos);
+			break;
+		}
+	}
+	mutex_unlock(&dyn_event_mutex);
+
+	return ret;
+}
+
 static int release_all_dyn_events(void)
 {
 	struct dyn_event *ev, *tmp;
diff --git a/kernel/trace/trace_dynevent.h b/kernel/trace/trace_dynevent.h
index 96cf9ca7adb9..705c95b435a3 100644
--- a/kernel/trace/trace_dynevent.h
+++ b/kernel/trace/trace_dynevent.h
@@ -20,6 +20,8 @@ struct dyn_event_operations {
 	int (*show)(struct seq_file *m, struct dyn_event *ev);
 	bool (*is_busy)(struct dyn_event *ev);
 	int (*free)(struct dyn_event *ev);
+	bool (*match)(const char *system, const char *event,
+		      struct dyn_event *ev);
 };
 
 /* Register new dyn_event type -- must be called at first */
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 421b9d71fedf..b1602f3584d5 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -23,12 +23,15 @@ static int trace_kprobe_create(int argc, char **argv);
 static int trace_kprobe_show(struct seq_file *m, struct dyn_event *ev);
 static int trace_kprobe_release(struct dyn_event *ev);
 static bool trace_kprobe_is_busy(struct dyn_event *ev);
+static bool trace_kprobe_match(const char *system, const char *event,
+			       struct dyn_event *ev);
 
 static struct dyn_event_operations trace_kprobe_ops = {
 	.create = trace_kprobe_create,
 	.show = trace_kprobe_show,
 	.is_busy = trace_kprobe_is_busy,
 	.free = trace_kprobe_release,
+	.match = trace_kprobe_match,
 };
 
 /**
@@ -115,6 +118,15 @@ static bool trace_kprobe_is_busy(struct dyn_event *ev)
 	return trace_probe_is_enabled(&tk->tp);
 }
 
+static bool trace_kprobe_match(const char *system, const char *event,
+			       struct dyn_event *ev)
+{
+	struct trace_kprobe *tk = to_trace_kprobe(ev);
+
+	return strcmp(trace_event_name(&tk->tp.call), event) == 0 &&
+	    (!system || strcmp(tk->tp.call.class->system, system) == 0);
+}
+
 static nokprobe_inline unsigned long trace_kprobe_nhit(struct trace_kprobe *tk)
 {
 	unsigned long nhit = 0;
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 90d10ef02f6b..7c15e22098af 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -41,12 +41,15 @@ static int trace_uprobe_create(int argc, char **argv);
 static int trace_uprobe_show(struct seq_file *m, struct dyn_event *ev);
 static int trace_uprobe_release(struct dyn_event *ev);
 static bool trace_uprobe_is_busy(struct dyn_event *ev);
+static bool trace_uprobe_match(const char *system, const char *event,
+			       struct dyn_event *ev);
 
 static struct dyn_event_operations trace_uprobe_ops = {
 	.create = trace_uprobe_create,
 	.show = trace_uprobe_show,
 	.is_busy = trace_uprobe_is_busy,
 	.free = trace_uprobe_release,
+	.match = trace_uprobe_match,
 };
 
 /*
@@ -276,6 +279,15 @@ static bool trace_uprobe_is_busy(struct dyn_event *ev)
 	return trace_probe_is_enabled(&tu->tp);
 }
 
+static bool trace_uprobe_match(const char *system, const char *event,
+			       struct dyn_event *ev)
+{
+	struct trace_uprobe *tu = to_trace_uprobe(ev);
+
+	return strcmp(trace_event_name(&tu->tp.call), event) == 0 &&
+	    (!system || strcmp(tu->tp.call.class->system, system) == 0);
+}
+
 /*
  * Allocate new trace_uprobe and initialize it (including uprobes).
  */


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [RFC PATCH 0/5] tracing: Unifying dynamic event interface
  2018-09-27 15:58 [RFC PATCH 0/5] tracing: Unifying dynamic event interface Masami Hiramatsu
                   ` (4 preceding siblings ...)
  2018-09-27 16:00 ` [RFC PATCH 5/5] tracing: Add generic event-name based remove event method Masami Hiramatsu
@ 2018-10-01 13:49 ` Tom Zanussi
  2018-10-02  7:45   ` Masami Hiramatsu
  5 siblings, 1 reply; 8+ messages in thread
From: Tom Zanussi @ 2018-10-01 13:49 UTC (permalink / raw)
  To: Masami Hiramatsu, Steven Rostedt; +Cc: linux-kernel, Ingo Molnar, Ravi Bangoria

Hi Masami,

On Fri, 2018-09-28 at 00:58 +0900, Masami Hiramatsu wrote:
> Hi,
> 
> This is an RFC series of unifying dynamic event interface on ftrace.
> Currently ftrace has 3 dynamic event interfaces, kprobes, uprobes
> and synthetic. This series unifies kprobes and uprobes event
> interface on "dynamic_events". This enables us to add new dynamic
> events easily on same interface, e.g. function events.

This seems like a nice idea to me and I don't see any problems with the
patches themselves, so consider it

Acked-by: Tom Zanussi <tom.zanussi@linux.intel.com>

> The older interfaces are left on the tracefs for backward
> compatibility at this moment.
> 
> dynamic_events syntax has no different from kprobe_events and
> uprobe_events. You can use same syntax for dynamic_events interface.
> 
> I think we can integrate synthetic events to this dynamic_events
> interface but it will requires new syntax. e.g.
> 
> echo "s:<event-name> <args>" >> dynamic_events
> 

So that's just the existing syntax, prefaced by s: , right?

> If it is OK, I'll add it in next version.
> 

Makes sense to me.

> BTW, since this dynamic_events interface derived from *probe_events,
> it inherits "all clear when truncate file open" behavior. But if you
> think this is too aggressive, I can drop it. (even in that case,
> kprobe_events/uprobe_events behavior is not changed)
> 
> I also introduced a widely used way to erase entries in other
> interfaces of ftrace, that is '!'. So you can now use '!event-name'
> or '!group/event' to erase an entry in dynamic_events.
> (Wait... it has to be '!p:event' as others do??)
> 

I'd think the full form should always be accepted, but would only be
necessary in cases requiring disambiguation.

Thanks,

Tom

> Any thought?
> 
> Thank you,
> 
> ---
> 
> Masami Hiramatsu (5):
>       tracing/uprobes: Add busy check when cleanup all uprobes
>       tracing: Add a unified dynamic event framework
>       tracing/kprobes: Use dyn_event framework for kprobe events
>       tracing/uprobes: Use dyn_event framework for uprobe events
>       tracing: Add generic event-name based remove event method
> 
> 
>  Documentation/trace/kprobetrace.rst  |    3 +
>  Documentation/trace/uprobetracer.rst |    4 +
>  kernel/trace/Kconfig                 |    4 +
>  kernel/trace/Makefile                |    1 
>  kernel/trace/trace.c                 |    4 +
>  kernel/trace/trace_dynevent.c        |  183
> +++++++++++++++++++++++++++++++++
>  kernel/trace/trace_dynevent.h        |   89 ++++++++++++++++
>  kernel/trace/trace_kprobe.c          |  191 +++++++++++++++++++++++-
> ----------
>  kernel/trace/trace_uprobe.c          |  175 +++++++++++++++++++++++-
> -------
>  9 files changed, 547 insertions(+), 107 deletions(-)
>  create mode 100644 kernel/trace/trace_dynevent.c
>  create mode 100644 kernel/trace/trace_dynevent.h
> 
> --
> Masami Hiramatsu (Linaro) <mhiramat@kernel.org>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC PATCH 0/5] tracing: Unifying dynamic event interface
  2018-10-01 13:49 ` [RFC PATCH 0/5] tracing: Unifying dynamic event interface Tom Zanussi
@ 2018-10-02  7:45   ` Masami Hiramatsu
  0 siblings, 0 replies; 8+ messages in thread
From: Masami Hiramatsu @ 2018-10-02  7:45 UTC (permalink / raw)
  To: Tom Zanussi; +Cc: Steven Rostedt, linux-kernel, Ingo Molnar, Ravi Bangoria

Hi Tom,

On Mon, 01 Oct 2018 08:49:24 -0500
Tom Zanussi <tom.zanussi@linux.intel.com> wrote:

> Hi Masami,
> 
> On Fri, 2018-09-28 at 00:58 +0900, Masami Hiramatsu wrote:
> > Hi,
> > 
> > This is an RFC series of unifying dynamic event interface on ftrace.
> > Currently ftrace has 3 dynamic event interfaces, kprobes, uprobes
> > and synthetic. This series unifies kprobes and uprobes event
> > interface on "dynamic_events". This enables us to add new dynamic
> > events easily on same interface, e.g. function events.
> 
> This seems like a nice idea to me and I don't see any problems with the
> patches themselves, so consider it
> 
> Acked-by: Tom Zanussi <tom.zanussi@linux.intel.com>

Thanks!

> 
> > The older interfaces are left on the tracefs for backward
> > compatibility at this moment.
> > 
> > dynamic_events syntax has no different from kprobe_events and
> > uprobe_events. You can use same syntax for dynamic_events interface.
> > 
> > I think we can integrate synthetic events to this dynamic_events
> > interface but it will requires new syntax. e.g.
> > 
> > echo "s:<event-name> <args>" >> dynamic_events
> > 
> 
> So that's just the existing syntax, prefaced by s: , right?

Yes, just for identifying.

> > If it is OK, I'll add it in next version.
> > 
> 
> Makes sense to me.

OK, I'll try :)

> 
> > BTW, since this dynamic_events interface derived from *probe_events,
> > it inherits "all clear when truncate file open" behavior. But if you
> > think this is too aggressive, I can drop it. (even in that case,
> > kprobe_events/uprobe_events behavior is not changed)
> > 
> > I also introduced a widely used way to erase entries in other
> > interfaces of ftrace, that is '!'. So you can now use '!event-name'
> > or '!group/event' to erase an entry in dynamic_events.
> > (Wait... it has to be '!p:event' as others do??)
> > 
> 
> I'd think the full form should always be accepted, but would only be
> necessary in cases requiring disambiguation.

OK, I'll add full form support.

Thank you,


-- 
Masami Hiramatsu <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-10-02  7:45 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-27 15:58 [RFC PATCH 0/5] tracing: Unifying dynamic event interface Masami Hiramatsu
2018-09-27 15:59 ` [RFC PATCH 1/5] tracing/uprobes: Add busy check when cleanup all uprobes Masami Hiramatsu
2018-09-27 15:59 ` [RFC PATCH 2/5] tracing: Add a unified dynamic event framework Masami Hiramatsu
2018-09-27 15:59 ` [RFC PATCH 3/5] tracing/kprobes: Use dyn_event framework for kprobe events Masami Hiramatsu
2018-09-27 16:00 ` [RFC PATCH 4/5] tracing/uprobes: Use dyn_event framework for uprobe events Masami Hiramatsu
2018-09-27 16:00 ` [RFC PATCH 5/5] tracing: Add generic event-name based remove event method Masami Hiramatsu
2018-10-01 13:49 ` [RFC PATCH 0/5] tracing: Unifying dynamic event interface Tom Zanussi
2018-10-02  7:45   ` Masami Hiramatsu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).