Re: [PATCH 2/4] perf/ftrace: Fix function trace events

[Jiri Olsa]

On Wed, Oct 11, 2017 at 09:45:30AM +0200, Peter Zijlstra wrote:

SNIP

>  void perf_trace_del(struct perf_event *p_event, int flags)
>  {
>       struct trace_event_call *tp_event = p_event->tp_event;
> -     hlist_del_rcu(&p_event->hlist_entry);
> -     tp_event->class->reg(tp_event, TRACE_REG_PERF_DEL, p_event);
> +
> +     /*
> +      * If TRACE_REG_PERF_DEL returns false; no custom action was performed
> +      * and we need to take the default action of dequeueing our event from
> +      * the right per-cpu hlist.
> +      */
> +     if (!tp_event->class->reg(tp_event, TRACE_REG_PERF_DEL, p_event))
> +             hlist_del_rcu(&p_event->hlist_entry);
>  }
>  
>  void *perf_trace_buf_alloc(int size, struct pt_regs **regs, int *rctxp)
> @@ -307,14 +321,24 @@ perf_ftrace_function_call(unsigned long
>                         struct ftrace_ops *ops, struct pt_regs *pt_regs)
>  {
>       struct ftrace_entry *entry;
> -     struct hlist_head *head;
> +     struct perf_event *event;
> +     struct hlist_head head;
>       struct pt_regs regs;
>       int rctx;
>  
> -     head = this_cpu_ptr(event_function.perf_events);
> -     if (hlist_empty(head))
> +     if ((unsigned long)ops->private != smp_processor_id())
>               return;
>  
> +     event = container_of(ops, struct perf_event, ftrace_ops);
> +
> +     /*
> +      * @event->hlist entry is NULL (per INIT_HLIST_NODE), and all
> +      * the perf code does is hlist_for_each_entry_rcu(), so we can
> +      * get away with simply setting the @head.first pointer in order
> +      * to create a singular list.
> +      */
> +     head.first = &event->hlist_entry;
> +
>  #define ENTRY_SIZE (ALIGN(sizeof(struct ftrace_entry) + sizeof(u32), \
>                   sizeof(u64)) - sizeof(u32))
>  
> @@ -330,7 +354,7 @@ perf_ftrace_function_call(unsigned long
>       entry->ip = ip;
>       entry->parent_ip = parent_ip;
>       perf_trace_buf_submit(entry, ENTRY_SIZE, rctx, TRACE_FN,
> -                           1, &regs, head, NULL);
> +                           1, &regs, &head, NULL);

nice idea.. I'll test it

jirka