Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1381872 > unrolled thread

Re: [PATCH net-next 0/8] allow bpf attach to tracepoints

Started bySteven Rostedt <rostedt@goodmis.org>
First post2016-04-18 18:20 +0200
Last post2016-04-18 23:30 +0200
Articles 4 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH net-next 0/8] allow bpf attach to tracepoints Steven Rostedt <rostedt@goodmis.org> - 2016-04-18 18:20 +0200
    Re: [PATCH net-next 0/8] allow bpf attach to tracepoints Alexei Starovoitov <ast@fb.com> - 2016-04-18 22:00 +0200
      Re: [PATCH net-next 0/8] allow bpf attach to tracepoints Steven Rostedt <rostedt@goodmis.org> - 2016-04-18 22:50 +0200
        Re: [PATCH net-next 0/8] allow bpf attach to tracepoints Alexei Starovoitov <ast@fb.com> - 2016-04-18 23:30 +0200

#1381872 — Re: [PATCH net-next 0/8] allow bpf attach to tracepoints

FromSteven Rostedt <rostedt@goodmis.org>
Date2016-04-18 18:20 +0200
SubjectRe: [PATCH net-next 0/8] allow bpf attach to tracepoints
Message-ID<rpk43-5g-39@gated-at.bofh.it>
On Mon, 4 Apr 2016 21:52:46 -0700
Alexei Starovoitov <ast@fb.com> wrote:

> Hi Steven, Peter,
> 
> last time we discussed bpf+tracepoints it was a year ago [1] and the reason
> we didn't proceed with that approach was that bpf would make arguments
> arg1, arg2 to trace_xx(arg1, arg2) call to be exposed to bpf program
> and that was considered unnecessary extension of abi. Back then I wanted
> to avoid the cost of buffer alloc and field assign part in all
> of the tracepoints, but looks like when optimized the cost is acceptable.
> So this new apporach doesn't expose any new abi to bpf program.
> The program is looking at tracepoint fields after they were copied
> by perf_trace_xx() and described in /sys/kernel/debug/tracing/events/xxx/format

Does this mean that ftrace could use this ability as well? As all the
current filtering of ftrace was done after it was copied to the buffer,
and that was what you wanted to avoid.

-- Steve

[toc] | [next] | [standalone]


#1382018

FromAlexei Starovoitov <ast@fb.com>
Date2016-04-18 22:00 +0200
Message-ID<rpnuX-2F6-51@gated-at.bofh.it>
In reply to#1381872
On 4/18/16 9:13 AM, Steven Rostedt wrote:
> On Mon, 4 Apr 2016 21:52:46 -0700
> Alexei Starovoitov <ast@fb.com> wrote:
>
>> Hi Steven, Peter,
>>
>> last time we discussed bpf+tracepoints it was a year ago [1] and the reason
>> we didn't proceed with that approach was that bpf would make arguments
>> arg1, arg2 to trace_xx(arg1, arg2) call to be exposed to bpf program
>> and that was considered unnecessary extension of abi. Back then I wanted
>> to avoid the cost of buffer alloc and field assign part in all
>> of the tracepoints, but looks like when optimized the cost is acceptable.
>> So this new apporach doesn't expose any new abi to bpf program.
>> The program is looking at tracepoint fields after they were copied
>> by perf_trace_xx() and described in /sys/kernel/debug/tracing/events/xxx/format
>
> Does this mean that ftrace could use this ability as well? As all the
> current filtering of ftrace was done after it was copied to the buffer,
> and that was what you wanted to avoid.

yeah, it could be added to ftrace as well, but it won't be as effective
as perf_trace, since the cost of trace_event_buffer_reserve() in
trace_event_raw_event_() handler is significantly higher than 
perf_trace_buf_alloc() in perf_trace_().
Then from the program point of view it wouldn't care how that memory
is allocated, so the user tools will just use perf_trace_() style.
The only use case I see for bpf with ftrace's tracepoint handler
is to work as an actual filter, but we already have filters there...
so not clear to me of the actual value of adding bpf to ftrace's
tracepoint handler.
At the same time there is whole ftrace as function tracer. That is
very lucrative field for bpf to plug into ;)

As far as 2nd part of your question about copying. Yeah, it adds to
the cost, so kprobe sometimes is faster than perf_trace tracepoint
that is copying a lot of args which are not going to be used.

[toc] | [prev] | [next] | [standalone]


#1382045

FromSteven Rostedt <rostedt@goodmis.org>
Date2016-04-18 22:50 +0200
Message-ID<rpohk-3i4-21@gated-at.bofh.it>
In reply to#1382018
On Mon, 18 Apr 2016 12:51:43 -0700
Alexei Starovoitov <ast@fb.com> wrote:


> yeah, it could be added to ftrace as well, but it won't be as effective
> as perf_trace, since the cost of trace_event_buffer_reserve() in
> trace_event_raw_event_() handler is significantly higher than 
> perf_trace_buf_alloc() in perf_trace_().

Right, because ftrace optimizes the case where all traces make it into
the ring buffer (zero copy). Of course, if a filter is a attached, it
would be trivial to add a case to copy first into a per cpu context
buffer, and only copy into the ring buffer if the filter succeeds. Hmm,
that may actually be something I want to do regardless with the current
filter system.

> Then from the program point of view it wouldn't care how that memory
> is allocated, so the user tools will just use perf_trace_() style.
> The only use case I see for bpf with ftrace's tracepoint handler
> is to work as an actual filter, but we already have filters there...

But wasn't it shown that eBPF could speed up the current filters? If we
hook into eBPF then we could replace what we have with a faster filter.

> so not clear to me of the actual value of adding bpf to ftrace's
> tracepoint handler.

Well, you wouldn't because you don't use ftrace ;-) But I'm sure others
might.

> At the same time there is whole ftrace as function tracer. That is
> very lucrative field for bpf to plug into ;)

Which could get this as a side-effect of this change.

-- Steve

[toc] | [prev] | [next] | [standalone]


#1382068

FromAlexei Starovoitov <ast@fb.com>
Date2016-04-18 23:30 +0200
Message-ID<rpoU2-422-13@gated-at.bofh.it>
In reply to#1382045
On 4/18/16 1:47 PM, Steven Rostedt wrote:
> On Mon, 18 Apr 2016 12:51:43 -0700
> Alexei Starovoitov <ast@fb.com> wrote:
>
>
>> yeah, it could be added to ftrace as well, but it won't be as effective
>> as perf_trace, since the cost of trace_event_buffer_reserve() in
>> trace_event_raw_event_() handler is significantly higher than
>> perf_trace_buf_alloc() in perf_trace_().
>
> Right, because ftrace optimizes the case where all traces make it into
> the ring buffer (zero copy). Of course, if a filter is a attached, it
> would be trivial to add a case to copy first into a per cpu context
> buffer, and only copy into the ring buffer if the filter succeeds. Hmm,
> that may actually be something I want to do regardless with the current
> filter system.
>
>> Then from the program point of view it wouldn't care how that memory
>> is allocated, so the user tools will just use perf_trace_() style.
>> The only use case I see for bpf with ftrace's tracepoint handler
>> is to work as an actual filter, but we already have filters there...
>
> But wasn't it shown that eBPF could speed up the current filters? If we
> hook into eBPF then we could replace what we have with a faster filter.

yes. Long ago I had a patch to accelerate filter_match_preds(),
but then abandoned it, since, as you said, ftrace is primarily targeting
streaming these events to user space and faster filtering probably
won't be much noticeable to the end user.
So far all the tools around bpf have been capitalizing on
the ability to aggregate data in the kernel.

>> At the same time there is whole ftrace as function tracer. That is
>> very lucrative field for bpf to plug into ;)
>
> Which could get this as a side-effect of this change.

not really as side-effect. That would be the new thing to design.
I only have few rough ideas on how to approach that.

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web