All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexei Starovoitov <ast@fb.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	"David S . Miller" <davem@davemloft.net>,
	Ingo Molnar <mingo@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Arnaldo Carvalho de Melo <acme@infradead.org>,
	Wang Nan <wangnan0@huawei.com>, Josef Bacik <jbacik@fb.com>,
	Brendan Gregg <brendan.d.gregg@gmail.com>,
	<netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<kernel-team@fb.com>
Subject: Re: [PATCH net-next 2/8] perf, bpf: allow bpf programs attach to tracepoints
Date: Tue, 5 Apr 2016 11:21:49 -0700	[thread overview]
Message-ID: <5704023D.8050601@fb.com> (raw)
In-Reply-To: <20160405181654.GR3408@twins.programming.kicks-ass.net>

On 4/5/16 11:16 AM, Peter Zijlstra wrote:
> On Tue, Apr 05, 2016 at 11:09:30AM -0700, Alexei Starovoitov wrote:
>>>> @@ -67,6 +69,14 @@ perf_trace_##call(void *__data, proto)					\
>>>>   									\
>>>>   	{ assign; }							\
>>>>   									\
>>>> +	if (prog) {							\
>>>> +		*(struct pt_regs **)entry = __regs;			\
>>>> +		if (!trace_call_bpf(prog, entry) || hlist_empty(head)) { \
>>>> +			perf_swevent_put_recursion_context(rctx);	\
>>>> +			return;						\
>>>> +		}							\
>>>> +		memset(&entry->ent, 0, sizeof(entry->ent));		\
>>>
>>> But if not, you destroy it and then feed it to perf?
>>
>> yes. If bpf prog returns 1 the buffer goes into normal ring-buffer
>> with all perf_event attributes and so on.
>> So far there wasn't a single real use case where we went this path.
>> Programs always do aggregation inside and pass stuff to user space
>> either via bpf maps or via bpf_perf_event_output() helper.
>> I wanted to keep perf_trace_xx() calls to be minimal in .text size
>> so memset above is one x86 instruction, but I don't mind
>> replacing this memset with a call to a helper function that will do:
>>     local_save_flags(flags);
>>     tracing_generic_entry_update(entry, flags, preempt_count());
>>     entry->type = type;
>> Then whether bpf attached or not the ring buffer will see the same
>> raw tracepoint entry. You think it's cleaner?
>
> Yeah, otherwise you get very weird and surprising behaviour.

ok. will respin.

> Also, one possible use-case is dynamic filters where the BPF program is
> basically used to filter events, although I suppose we already have a
> hook for that elsewhere.

There is no other bpf hook for tracepoints. This patch is it.
And yes, after respin such use case will be possible.
Thanks!

  reply	other threads:[~2016-04-05 18:24 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-05  4:52 [PATCH net-next 0/8] allow bpf attach to tracepoints Alexei Starovoitov
2016-04-05  4:52 ` [PATCH net-next 1/8] perf: optimize perf_fetch_caller_regs Alexei Starovoitov
2016-04-05 12:06   ` Peter Zijlstra
2016-04-05 17:41     ` Alexei Starovoitov
2016-04-08 22:12     ` Steven Rostedt
2016-04-05  4:52 ` [PATCH net-next 2/8] perf, bpf: allow bpf programs attach to tracepoints Alexei Starovoitov
2016-04-05 14:18   ` Peter Zijlstra
2016-04-05 18:09     ` Alexei Starovoitov
2016-04-05 18:16       ` Peter Zijlstra
2016-04-05 18:21         ` Alexei Starovoitov [this message]
2016-04-18 20:29   ` Steven Rostedt
2016-04-18 21:43     ` Alexei Starovoitov
2016-04-18 22:16       ` Steven Rostedt
2016-04-19  1:15         ` Alexei Starovoitov
2016-04-19  2:58           ` Steven Rostedt
2016-04-05  4:52 ` [PATCH net-next 3/8] bpf: register BPF_PROG_TYPE_TRACEPOINT program type Alexei Starovoitov
2016-04-05  4:52 ` [PATCH net-next 4/8] bpf: support bpf_get_stackid() and bpf_perf_event_output() in tracepoint programs Alexei Starovoitov
2016-04-05  4:52 ` [PATCH net-next 5/8] bpf: sanitize bpf tracepoint access Alexei Starovoitov
2016-04-05  4:52 ` [PATCH net-next 6/8] samples/bpf: add tracepoint support to bpf loader Alexei Starovoitov
2016-04-05  4:52 ` [PATCH net-next 7/8] samples/bpf: tracepoint example Alexei Starovoitov
2016-04-05  4:52 ` [PATCH net-next 8/8] samples/bpf: add tracepoint vs kprobe performance tests Alexei Starovoitov
2016-04-18 16:13 ` [PATCH net-next 0/8] allow bpf attach to tracepoints Steven Rostedt
2016-04-18 19:51   ` Alexei Starovoitov
2016-04-18 20:47     ` Steven Rostedt
2016-04-18 21:25       ` Alexei Starovoitov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5704023D.8050601@fb.com \
    --to=ast@fb.com \
    --cc=acme@infradead.org \
    --cc=brendan.d.gregg@gmail.com \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=jbacik@fb.com \
    --cc=kernel-team@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=wangnan0@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.