lkml.org 
[lkml]   [2019]   [Jun]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[for-next][PATCH 08/12] tracing/probe: Add trace_event_file access APIs for trace_probe
    From: Masami Hiramatsu <mhiramat@kernel.org>

    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>
    ---
    kernel/trace/trace_kprobe.c | 85 +++++++++++++------------------------
    kernel/trace/trace_probe.c | 47 ++++++++++++++++++++
    kernel/trace/trace_probe.h | 36 +++++++++-------
    kernel/trace/trace_uprobe.c | 42 ++++++------------
    4 files changed, 109 insertions(+), 101 deletions(-)

    diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
    index 7f802ee27266..87a52094378c 100644
    --- 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 509a26024b4f..abb05608a09d 100644
    --- 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 01d7b222e004..ab02007e131d 100644
    --- 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 c262494fa793..a9f8045b6695 100644
    --- 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();
    }
    --
    2.20.1

    \
     
     \ /
      Last update: 2019-06-25 21:17    [W:2.787 / U:0.280 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site