(2014/01/17 17:08), Namhyung Kim wrote:
> A single uprobe event might serve different users like ftrace and
> perf.  And this is especially important for upcoming multi buffer
> support.  But in this case it'll fetch (same) data from userspace
> multiple times.  So move it to the beginning of the dispatcher
> function and reuse it for each users.

Looks good to me. :)

BTW, it seems that there is a similar issue on kprobes too.
I'm sure that only one kprobe can run on a CPU, thus I think
I can do the same implementation on kprobes tracer too.

Thank you,

> 
> Cc: Masami Hiramatsu <masami.hiramatsu...@hitachi.com>
> Cc: Oleg Nesterov <o...@redhat.com>
> Cc: Srikar Dronamraju <sri...@linux.vnet.ibm.com>
> Cc: zhangwei(Jovi) <jovi.zhang...@huawei.com>
> Signed-off-by: Namhyung Kim <namhy...@kernel.org>
> ---
>  kernel/trace/trace_uprobe.c | 93 
> +++++++++++++++++++++++++++------------------
>  1 file changed, 56 insertions(+), 37 deletions(-)
> 
> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
> index c5d2612bf233..d83155e0da78 100644
> --- a/kernel/trace/trace_uprobe.c
> +++ b/kernel/trace/trace_uprobe.c
> @@ -759,30 +759,25 @@ static void uprobe_buffer_put(struct uprobe_cpu_buffer 
> *ucb)
>  }
>  
>  static void __uprobe_trace_func(struct trace_uprobe *tu,
> -                             unsigned long func, struct pt_regs *regs)
> +                             unsigned long func, struct pt_regs *regs,
> +                             struct uprobe_cpu_buffer *ucb, int dsize)
>  {
>       struct uprobe_trace_entry_head *entry;
>       struct ring_buffer_event *event;
>       struct ring_buffer *buffer;
> -     struct uprobe_cpu_buffer *ucb;
>       void *data;
> -     int size, dsize, esize;
> +     int size, esize;
>       struct ftrace_event_call *call = &tu->tp.call;
>  
> -     dsize = __get_data_size(&tu->tp, regs);
> -     esize = SIZEOF_TRACE_ENTRY(is_ret_probe(tu));
> -
> -     if (WARN_ON_ONCE(!uprobe_cpu_buffer || tu->tp.size + dsize > PAGE_SIZE))
> +     if (WARN_ON_ONCE(tu->tp.size + dsize > PAGE_SIZE))
>               return;
>  
> -     ucb = uprobe_buffer_get();
> -     store_trace_args(esize, &tu->tp, regs, ucb->buf, dsize);
> -
> +     esize = SIZEOF_TRACE_ENTRY(is_ret_probe(tu));
>       size = esize + tu->tp.size + dsize;
>       event = trace_current_buffer_lock_reserve(&buffer, call->event.type,
>                                                 size, 0, 0);
>       if (!event)
> -             goto out;
> +             return;
>  
>       entry = ring_buffer_event_data(event);
>       if (is_ret_probe(tu)) {
> @@ -798,23 +793,22 @@ static void __uprobe_trace_func(struct trace_uprobe *tu,
>  
>       if (!call_filter_check_discard(call, entry, buffer, event))
>               trace_buffer_unlock_commit(buffer, event, 0, 0);
> -
> -out:
> -     uprobe_buffer_put(ucb);
>  }
>  
>  /* uprobe handler */
> -static int uprobe_trace_func(struct trace_uprobe *tu, struct pt_regs *regs)
> +static int uprobe_trace_func(struct trace_uprobe *tu, struct pt_regs *regs,
> +                          struct uprobe_cpu_buffer *ucb, int dsize)
>  {
>       if (!is_ret_probe(tu))
> -             __uprobe_trace_func(tu, 0, regs);
> +             __uprobe_trace_func(tu, 0, regs, ucb, dsize);
>       return 0;
>  }
>  
>  static void uretprobe_trace_func(struct trace_uprobe *tu, unsigned long func,
> -                             struct pt_regs *regs)
> +                              struct pt_regs *regs,
> +                              struct uprobe_cpu_buffer *ucb, int dsize)
>  {
> -     __uprobe_trace_func(tu, func, regs);
> +     __uprobe_trace_func(tu, func, regs, ucb, dsize);
>  }
>  
>  /* Event entry printers */
> @@ -1015,30 +1009,23 @@ static bool uprobe_perf_filter(struct uprobe_consumer 
> *uc,
>  }
>  
>  static void __uprobe_perf_func(struct trace_uprobe *tu,
> -                             unsigned long func, struct pt_regs *regs)
> +                            unsigned long func, struct pt_regs *regs,
> +                            struct uprobe_cpu_buffer *ucb, int dsize)
>  {
>       struct ftrace_event_call *call = &tu->tp.call;
>       struct uprobe_trace_entry_head *entry;
>       struct hlist_head *head;
> -     struct uprobe_cpu_buffer *ucb;
>       void *data;
> -     int size, dsize, esize;
> +     int size, esize;
>       int rctx;
>  
> -     dsize = __get_data_size(&tu->tp, regs);
>       esize = SIZEOF_TRACE_ENTRY(is_ret_probe(tu));
>  
> -     if (WARN_ON_ONCE(!uprobe_cpu_buffer))
> -             return;
> -
>       size = esize + tu->tp.size + dsize;
>       size = ALIGN(size + sizeof(u32), sizeof(u64)) - sizeof(u32);
>       if (WARN_ONCE(size > PERF_MAX_TRACE_SIZE, "profile buffer not large 
> enough"))
>               return;
>  
> -     ucb = uprobe_buffer_get();
> -     store_trace_args(esize, &tu->tp, regs, ucb->buf, dsize);
> -
>       preempt_disable();
>       head = this_cpu_ptr(call->perf_events);
>       if (hlist_empty(head))
> @@ -1068,24 +1055,25 @@ static void __uprobe_perf_func(struct trace_uprobe 
> *tu,
>       perf_trace_buf_submit(entry, size, rctx, 0, 1, regs, head, NULL);
>   out:
>       preempt_enable();
> -     uprobe_buffer_put(ucb);
>  }
>  
>  /* uprobe profile handler */
> -static int uprobe_perf_func(struct trace_uprobe *tu, struct pt_regs *regs)
> +static int uprobe_perf_func(struct trace_uprobe *tu, struct pt_regs *regs,
> +                         struct uprobe_cpu_buffer *ucb, int dsize)
>  {
>       if (!uprobe_perf_filter(&tu->consumer, 0, current->mm))
>               return UPROBE_HANDLER_REMOVE;
>  
>       if (!is_ret_probe(tu))
> -             __uprobe_perf_func(tu, 0, regs);
> +             __uprobe_perf_func(tu, 0, regs, ucb, dsize);
>       return 0;
>  }
>  
>  static void uretprobe_perf_func(struct trace_uprobe *tu, unsigned long func,
> -                             struct pt_regs *regs)
> +                             struct pt_regs *regs,
> +                             struct uprobe_cpu_buffer *ucb, int dsize)
>  {
> -     __uprobe_perf_func(tu, func, regs);
> +     __uprobe_perf_func(tu, func, regs, ucb, dsize);
>  }
>  #endif       /* CONFIG_PERF_EVENTS */
>  
> @@ -1127,8 +1115,11 @@ static int uprobe_dispatcher(struct uprobe_consumer 
> *con, struct pt_regs *regs)
>  {
>       struct trace_uprobe *tu;
>       struct uprobe_dispatch_data udd;
> +     struct uprobe_cpu_buffer *ucb;
> +     int dsize, esize;
>       int ret = 0;
>  
> +
>       tu = container_of(con, struct trace_uprobe, consumer);
>       tu->nhit++;
>  
> @@ -1137,13 +1128,29 @@ static int uprobe_dispatcher(struct uprobe_consumer 
> *con, struct pt_regs *regs)
>  
>       current->utask->vaddr = (unsigned long) &udd;
>  
> +#ifdef CONFIG_PERF_EVENTS
> +     if ((tu->tp.flags & TP_FLAG_TRACE) == 0 &&
> +         !uprobe_perf_filter(&tu->consumer, 0, current->mm))
> +             return UPROBE_HANDLER_REMOVE;
> +#endif
> +
> +     if (WARN_ON_ONCE(!uprobe_cpu_buffer))
> +             return 0;
> +
> +     dsize = __get_data_size(&tu->tp, regs);
> +     esize = SIZEOF_TRACE_ENTRY(is_ret_probe(tu));
> +
> +     ucb = uprobe_buffer_get();
> +     store_trace_args(esize, &tu->tp, regs, ucb->buf, dsize);
> +
>       if (tu->tp.flags & TP_FLAG_TRACE)
> -             ret |= uprobe_trace_func(tu, regs);
> +             ret |= uprobe_trace_func(tu, regs, ucb, dsize);
>  
>  #ifdef CONFIG_PERF_EVENTS
>       if (tu->tp.flags & TP_FLAG_PROFILE)
> -             ret |= uprobe_perf_func(tu, regs);
> +             ret |= uprobe_perf_func(tu, regs, ucb, dsize);
>  #endif
> +     uprobe_buffer_put(ucb);
>       return ret;
>  }
>  
> @@ -1152,6 +1159,8 @@ static int uretprobe_dispatcher(struct uprobe_consumer 
> *con,
>  {
>       struct trace_uprobe *tu;
>       struct uprobe_dispatch_data udd;
> +     struct uprobe_cpu_buffer *ucb;
> +     int dsize, esize;
>  
>       tu = container_of(con, struct trace_uprobe, consumer);
>  
> @@ -1160,13 +1169,23 @@ static int uretprobe_dispatcher(struct 
> uprobe_consumer *con,
>  
>       current->utask->vaddr = (unsigned long) &udd;
>  
> +     if (WARN_ON_ONCE(!uprobe_cpu_buffer))
> +             return 0;
> +
> +     dsize = __get_data_size(&tu->tp, regs);
> +     esize = SIZEOF_TRACE_ENTRY(is_ret_probe(tu));
> +
> +     ucb = uprobe_buffer_get();
> +     store_trace_args(esize, &tu->tp, regs, ucb->buf, dsize);
> +
>       if (tu->tp.flags & TP_FLAG_TRACE)
> -             uretprobe_trace_func(tu, func, regs);
> +             uretprobe_trace_func(tu, func, regs, ucb, dsize);
>  
>  #ifdef CONFIG_PERF_EVENTS
>       if (tu->tp.flags & TP_FLAG_PROFILE)
> -             uretprobe_perf_func(tu, func, regs);
> +             uretprobe_perf_func(tu, func, regs, ucb, dsize);
>  #endif
> +     uprobe_buffer_put(ucb);
>       return 0;
>  }
>  
> 


-- 
Masami HIRAMATSU
IT Management Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu...@hitachi.com


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to