From: Peter Zijlstra <peterz@infradead.org>
To: Alexei Starovoitov <ast@fb.com>
Cc: "David S . Miller" <davem@davemloft.net>,
Brendan Gregg <bgregg@netflix.com>,
Daniel Borkmann <daniel@iogearbox.net>, Teng Qin <qinteng@fb.com>,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
kernel-team@fb.com
Subject: Re: [PATCH net-next 1/2] perf, bpf: add support for HW_CACHE and RAW events
Date: Tue, 23 May 2017 18:31:34 +0200 [thread overview]
Message-ID: <20170523163134.saalqlizp5opc5tz@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <b25f4d02-70c1-43ce-8bf1-9b89e573add6@fb.com>
On Tue, May 23, 2017 at 07:38:08AM -0700, Alexei Starovoitov wrote:
> On 5/23/17 12:42 AM, Peter Zijlstra wrote:
> > On Mon, May 22, 2017 at 03:48:39PM -0700, Alexei Starovoitov wrote:
> > > From: Teng Qin <qinteng@fb.com>
> > >
> > > This commit adds support for attach BPF program to RAW and HW_CACHE type
> > > events, and support for read HW_CACHE type event counters in BPF
> > > program. Existing code logic already supports them, so this commit is
> > > just update Enum value checks.
> >
> > So what I'm missing is why they were not supported previously, and what
> > changed to allow it now.
>
> that code path simply wasn't tested previously. Nothing changed on
> bpf side and on perf side.
> Why it wasn't added on day one? There was no demand. Now people
> use bpf more and more and few folks got confused that these types
> of perf events were not supported, hence we're adding it.
OK. Is there anything stopping people from wanting to use the dynamic
types, as found in:
/sys/bus/event_source/devices/*/type
?
In which case, do we want something like this instead?
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 971f7259108f..4aa5f3011cf8 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -8063,12 +8063,8 @@ static int perf_event_set_bpf_prog(struct perf_event *event, u32 prog_fd)
bool is_kprobe, is_tracepoint;
struct bpf_prog *prog;
- if (event->attr.type == PERF_TYPE_HARDWARE ||
- event->attr.type == PERF_TYPE_SOFTWARE)
- return perf_event_set_bpf_handler(event, prog_fd);
-
if (event->attr.type != PERF_TYPE_TRACEPOINT)
- return -EINVAL;
+ return perf_event_set_bpf_handler(event, prog_fd);
if (event->tp_event->prog)
return -EEXIST;
next prev parent reply other threads:[~2017-05-23 16:31 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-22 22:48 [PATCH net-next 0/2] perf, bpf: add support for HW_CACHE and RAW events Alexei Starovoitov
2017-05-22 22:48 ` [PATCH net-next 1/2] " Alexei Starovoitov
2017-05-23 7:42 ` Peter Zijlstra
2017-05-23 14:38 ` Alexei Starovoitov
2017-05-23 16:31 ` Peter Zijlstra [this message]
2017-05-23 17:05 ` Alexei Starovoitov
2017-05-22 22:48 ` [PATCH net-next 2/2] samples/bpf: add samples for HW_CACHE / " Alexei Starovoitov
2017-05-22 23:26 ` David Miller
2017-05-22 23:35 ` Alexei Starovoitov
2017-05-22 23:40 ` David Miller
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=20170523163134.saalqlizp5opc5tz@hirez.programming.kicks-ass.net \
--to=peterz@infradead.org \
--cc=ast@fb.com \
--cc=bgregg@netflix.com \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=kernel-team@fb.com \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=qinteng@fb.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).