tracing/probe: Add trace_event_file access APIs for trace_probe

Add trace_event_file access APIs for trace_probe data structure.
This simplifies enabling/disabling operations in uprobe and kprobe
events so that those don't touch deep inside the trace_probe.

This also removing a redundant synchronization when the
kprobe event is used from perf, since the perf itself uses
tracepoint_synchronize_unregister() after disabling (ftrace-
defined) event, thus we don't have to synchronize in that
path. Also we don't need to identify local trace_kprobe too
anymore.

Link: http://lkml.kernel.org/r/155931584587.28323.372301976283354629.stgit@devnote2
Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 7f802ee27266b79f7d1cc6fdae9b455a520db59f..87a52094378c7700ae149bac1537c0bcdf881a87 100644 (file)
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -290,34 +290,27 @@ static inline int __enable_trace_kprobe(struct trace_kprobe *tk)
 static int
 enable_trace_kprobe(struct trace_kprobe *tk, struct trace_event_file *file)
 {
-       struct event_file_link *link;
+       bool enabled = trace_probe_is_enabled(&tk->tp);
        int ret = 0;
 
        if (file) {
-               link = kmalloc(sizeof(*link), GFP_KERNEL);
-               if (!link) {
-                       ret = -ENOMEM;
-                       goto out;
-               }
-
-               link->file = file;
-               list_add_tail_rcu(&link->list, &tk->tp.files);
+               ret = trace_probe_add_file(&tk->tp, file);
+               if (ret)
+                       return ret;
+       } else
+               tk->tp.flags |= TP_FLAG_PROFILE;
 
-               tk->tp.flags |= TP_FLAG_TRACE;
-               ret = __enable_trace_kprobe(tk);
-               if (ret) {
-                       list_del_rcu(&link->list);
-                       kfree(link);
-                       tk->tp.flags &= ~TP_FLAG_TRACE;
-               }
+       if (enabled)
+               return 0;
 
-       } else {
-               tk->tp.flags |= TP_FLAG_PROFILE;
-               ret = __enable_trace_kprobe(tk);
-               if (ret)
+       ret = __enable_trace_kprobe(tk);
+       if (ret) {
+               if (file)
+                       trace_probe_remove_file(&tk->tp, file);
+               else
                        tk->tp.flags &= ~TP_FLAG_PROFILE;
        }
- out:
+
        return ret;
 }
 
@@ -328,54 +321,34 @@ enable_trace_kprobe(struct trace_kprobe *tk, struct trace_event_file *file)
 static int
 disable_trace_kprobe(struct trace_kprobe *tk, struct trace_event_file *file)
 {
-       struct event_file_link *link = NULL;
-       int wait = 0;
+       struct trace_probe *tp = &tk->tp;
        int ret = 0;
 
        if (file) {
-               link = find_event_file_link(&tk->tp, file);
-               if (!link) {
-                       ret = -EINVAL;
-                       goto out;
-               }
-
-               list_del_rcu(&link->list);
-               wait = 1;
-               if (!list_empty(&tk->tp.files))
+               if (!trace_probe_get_file_link(tp, file))
+                       return -ENOENT;
+               if (!trace_probe_has_single_file(tp))
                        goto out;
-
-               tk->tp.flags &= ~TP_FLAG_TRACE;
+               tp->flags &= ~TP_FLAG_TRACE;
        } else
-               tk->tp.flags &= ~TP_FLAG_PROFILE;
+               tp->flags &= ~TP_FLAG_PROFILE;
 
-       if (!trace_probe_is_enabled(&tk->tp) && trace_probe_is_registered(&tk->tp)) {
+       if (!trace_probe_is_enabled(tp) && trace_probe_is_registered(tp)) {
                if (trace_kprobe_is_return(tk))
                        disable_kretprobe(&tk->rp);
                else
                        disable_kprobe(&tk->rp.kp);
-               wait = 1;
        }
 
-       /*
-        * if tk is not added to any list, it must be a local trace_kprobe
-        * created with perf_event_open. We don't need to wait for these
-        * trace_kprobes
-        */
-       if (list_empty(&tk->devent.list))
-               wait = 0;
  out:
-       if (wait) {
+       if (file)
                /*
-                * Synchronize with kprobe_trace_func/kretprobe_trace_func
-                * to ensure disabled (all running handlers are finished).
-                * This is not only for kfree(), but also the caller,
-                * trace_remove_event_call() supposes it for releasing
-                * event_call related objects, which will be accessed in
-                * the kprobe_trace_func/kretprobe_trace_func.
+                * Synchronization is done in below function. For perf event,
+                * file == NULL and perf_trace_event_unreg() calls
+                * tracepoint_synchronize_unregister() to ensure synchronize
+                * event. We don't need to care about it.
                 */
-               synchronize_rcu();
-               kfree(link);    /* Ignored if link == NULL */
-       }
+               trace_probe_remove_file(tp, file);
 
        return ret;
 }
@@ -1044,7 +1017,7 @@ kprobe_trace_func(struct trace_kprobe *tk, struct pt_regs *regs)
 {
        struct event_file_link *link;
 
-       list_for_each_entry_rcu(link, &tk->tp.files, list)
+       trace_probe_for_each_link_rcu(link, &tk->tp)
                __kprobe_trace_func(tk, regs, link->file);
 }
 NOKPROBE_SYMBOL(kprobe_trace_func);
@@ -1094,7 +1067,7 @@ kretprobe_trace_func(struct trace_kprobe *tk, struct kretprobe_instance *ri,
 {
        struct event_file_link *link;
 
-       list_for_each_entry_rcu(link, &tk->tp.files, list)
+       trace_probe_for_each_link_rcu(link, &tk->tp)
                __kretprobe_trace_func(tk, ri, regs, link->file);
 }
 NOKPROBE_SYMBOL(kretprobe_trace_func);

diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 509a26024b4f5031e8aff6681074b569bbe0f646..abb05608a09d5d37c1e15775f9b277514fd36a5a 100644 (file)
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -936,3 +936,50 @@ int trace_probe_register_event_call(struct trace_probe *tp)
 
        return ret;
 }
+
+int trace_probe_add_file(struct trace_probe *tp, struct trace_event_file *file)
+{
+       struct event_file_link *link;
+
+       link = kmalloc(sizeof(*link), GFP_KERNEL);
+       if (!link)
+               return -ENOMEM;
+
+       link->file = file;
+       INIT_LIST_HEAD(&link->list);
+       list_add_tail_rcu(&link->list, &tp->files);
+       tp->flags |= TP_FLAG_TRACE;
+       return 0;
+}
+
+struct event_file_link *trace_probe_get_file_link(struct trace_probe *tp,
+                                                 struct trace_event_file *file)
+{
+       struct event_file_link *link;
+
+       trace_probe_for_each_link(link, tp) {
+               if (link->file == file)
+                       return link;
+       }
+
+       return NULL;
+}
+
+int trace_probe_remove_file(struct trace_probe *tp,
+                           struct trace_event_file *file)
+{
+       struct event_file_link *link;
+
+       link = trace_probe_get_file_link(tp, file);
+       if (!link)
+               return -ENOENT;
+
+       list_del_rcu(&link->list);
+       synchronize_rcu();
+       kfree(link);
+
+       if (list_empty(&tp->files))
+               tp->flags &= ~TP_FLAG_TRACE;
+
+       return 0;
+}

diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 01d7b222e0043b10f68e833a5bbe025b12498e28..ab02007e131d1256fcb0351c5e87767e8d8ec637 100644 (file)
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -248,16 +248,32 @@ static inline bool trace_probe_is_registered(struct trace_probe *tp)
        return !!(tp->flags & TP_FLAG_REGISTERED);
 }
 
-int trace_probe_init(struct trace_probe *tp, const char *event,
-                    const char *group);
-void trace_probe_cleanup(struct trace_probe *tp);
-int trace_probe_register_event_call(struct trace_probe *tp);
 static inline int trace_probe_unregister_event_call(struct trace_probe *tp)
 {
        /* tp->event is unregistered in trace_remove_event_call() */
        return trace_remove_event_call(&tp->call);
 }
 
+static inline bool trace_probe_has_single_file(struct trace_probe *tp)
+{
+       return !!list_is_singular(&tp->files);
+}
+
+int trace_probe_init(struct trace_probe *tp, const char *event,
+                    const char *group);
+void trace_probe_cleanup(struct trace_probe *tp);
+int trace_probe_register_event_call(struct trace_probe *tp);
+int trace_probe_add_file(struct trace_probe *tp, struct trace_event_file *file);
+int trace_probe_remove_file(struct trace_probe *tp,
+                           struct trace_event_file *file);
+struct event_file_link *trace_probe_get_file_link(struct trace_probe *tp,
+                                               struct trace_event_file *file);
+
+#define trace_probe_for_each_link(pos, tp)     \
+       list_for_each_entry(pos, &(tp)->files, list)
+#define trace_probe_for_each_link_rcu(pos, tp) \
+       list_for_each_entry_rcu(pos, &(tp)->files, list)
+
 /* Check the name is good for event/group/fields */
 static inline bool is_good_name(const char *name)
 {
@@ -270,18 +286,6 @@ static inline bool is_good_name(const char *name)
        return true;
 }
 
-static inline struct event_file_link *
-find_event_file_link(struct trace_probe *tp, struct trace_event_file *file)
-{
-       struct event_file_link *link;
-
-       list_for_each_entry(link, &tp->files, list)
-               if (link->file == file)
-                       return link;
-
-       return NULL;
-}
-
 #define TPARG_FL_RETURN BIT(0)
 #define TPARG_FL_KERNEL BIT(1)
 #define TPARG_FL_FENTRY BIT(2)

diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index c262494fa793f6e414bcc2e3975c16a5dfc4019c..a9f8045b66954fe2265125fc6782ddf3d4d80389 100644 (file)
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -863,7 +863,7 @@ static int uprobe_trace_func(struct trace_uprobe *tu, struct pt_regs *regs,
                return 0;
 
        rcu_read_lock();
-       list_for_each_entry_rcu(link, &tu->tp.files, list)
+       trace_probe_for_each_link_rcu(link, &tu->tp)
                __uprobe_trace_func(tu, 0, regs, ucb, dsize, link->file);
        rcu_read_unlock();
 
@@ -877,7 +877,7 @@ static void uretprobe_trace_func(struct trace_uprobe *tu, unsigned long func,
        struct event_file_link *link;
 
        rcu_read_lock();
-       list_for_each_entry_rcu(link, &tu->tp.files, list)
+       trace_probe_for_each_link_rcu(link, &tu->tp)
                __uprobe_trace_func(tu, func, regs, ucb, dsize, link->file);
        rcu_read_unlock();
 }
@@ -924,21 +924,15 @@ probe_event_enable(struct trace_uprobe *tu, struct trace_event_file *file,
                   filter_func_t filter)
 {
        bool enabled = trace_probe_is_enabled(&tu->tp);
-       struct event_file_link *link = NULL;
        int ret;
 
        if (file) {
                if (tu->tp.flags & TP_FLAG_PROFILE)
                        return -EINTR;
 
-               link = kmalloc(sizeof(*link), GFP_KERNEL);
-               if (!link)
-                       return -ENOMEM;
-
-               link->file = file;
-               list_add_tail_rcu(&link->list, &tu->tp.files);
-
-               tu->tp.flags |= TP_FLAG_TRACE;
+               ret = trace_probe_add_file(&tu->tp, file);
+               if (ret < 0)
+                       return ret;
        } else {
                if (tu->tp.flags & TP_FLAG_TRACE)
                        return -EINTR;
@@ -973,13 +967,11 @@ probe_event_enable(struct trace_uprobe *tu, struct trace_event_file *file,
        uprobe_buffer_disable();
 
  err_flags:
-       if (file) {
-               list_del(&link->list);
-               kfree(link);
-               tu->tp.flags &= ~TP_FLAG_TRACE;
-       } else {
+       if (file)
+               trace_probe_remove_file(&tu->tp, file);
+       else
                tu->tp.flags &= ~TP_FLAG_PROFILE;
-       }
+
        return ret;
 }
 
@@ -990,26 +982,18 @@ probe_event_disable(struct trace_uprobe *tu, struct trace_event_file *file)
                return;
 
        if (file) {
-               struct event_file_link *link;
-
-               link = find_event_file_link(&tu->tp, file);
-               if (!link)
+               if (trace_probe_remove_file(&tu->tp, file) < 0)
                        return;
 
-               list_del_rcu(&link->list);
-               /* synchronize with u{,ret}probe_trace_func */
-               synchronize_rcu();
-               kfree(link);
-
-               if (!list_empty(&tu->tp.files))
+               if (trace_probe_is_enabled(&tu->tp))
                        return;
-       }
+       } else
+               tu->tp.flags &= ~TP_FLAG_PROFILE;
 
        WARN_ON(!uprobe_filter_is_empty(&tu->filter));
 
        uprobe_unregister(tu->inode, tu->offset, &tu->consumer);
        tu->inode = NULL;
-       tu->tp.flags &= file ? ~TP_FLAG_TRACE : ~TP_FLAG_PROFILE;
 
        uprobe_buffer_disable();
 }