2018-09-27 16:00:44

by Masami Hiramatsu

[permalink] [raw]
Subject: [RFC PATCH 0/5] tracing: Unifying dynamic event interface

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) <[email protected]>


2018-09-27 16:01:05

by Masami Hiramatsu

[permalink] [raw]
Subject: [RFC PATCH 2/5] tracing: Add a unified dynamic event framework

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 <[email protected]>
---
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 <[email protected]>
+ */
+
+#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


2018-09-27 16:01:08

by Masami Hiramatsu

[permalink] [raw]
Subject: [RFC PATCH 1/5] tracing/uprobes: Add busy check when cleanup all uprobes

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 <[email protected]>
---
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;
}


2018-09-27 16:01:17

by Masami Hiramatsu

[permalink] [raw]
Subject: [RFC PATCH 3/5] tracing/kprobes: Use dyn_event framework for kprobe events

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 <[email protected]>
---
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++;


2018-09-27 16:01:53

by Masami Hiramatsu

[permalink] [raw]
Subject: [RFC PATCH 4/5] tracing/uprobes: Use dyn_event framework for uprobe events

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 <[email protected]>
---
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))


2018-09-27 16:02:37

by Masami Hiramatsu

[permalink] [raw]
Subject: [RFC PATCH 5/5] tracing: Add generic event-name based remove event method

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 <[email protected]>
---
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).
*/


2018-10-01 13:50:37

by Tom Zanussi

[permalink] [raw]
Subject: Re: [RFC PATCH 0/5] tracing: Unifying dynamic event interface

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 <[email protected]>

> 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) <[email protected]>


2018-10-02 07:47:20

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [RFC PATCH 0/5] tracing: Unifying dynamic event interface

Hi Tom,

On Mon, 01 Oct 2018 08:49:24 -0500
Tom Zanussi <[email protected]> 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 <[email protected]>

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 <[email protected]>