Re: [PATCH net-next 4/6] perf, bpf: add perf events core support for BPF_PROG_TYPE_PERF_EVENT programs
From: Peter Zijlstra <peterz@infradead.org>
Date: 2016-08-29 12:17:33
Also in:
lkml
On Fri, Aug 26, 2016 at 07:31:22PM -0700, Alexei Starovoitov wrote:
+static int perf_event_set_bpf_handler(struct perf_event *event, u32 prog_fd)
+{
+ struct bpf_prog *prog;
+
+ if (event->overflow_handler_context)
+ /* hw breakpoint or kernel counter */
+ return -EINVAL;
+
+ if (event->prog)
+ return -EEXIST;
+
+ prog = bpf_prog_get_type(prog_fd, BPF_PROG_TYPE_PERF_EVENT);
+ if (IS_ERR(prog))
+ return PTR_ERR(prog);
+
+ event->prog = prog;
+ event->orig_overflow_handler = READ_ONCE(event->overflow_handler);
+ WRITE_ONCE(event->overflow_handler, bpf_overflow_handler);
+ return 0;
+}
+
+static void perf_event_free_bpf_handler(struct perf_event *event)
+{
+ struct bpf_prog *prog = event->prog;
+
+ if (!prog)
+ return;Does it make sense to do something like: WARN_ON_ONCE(event->overflow_handler != bpf_overflow_handler); ?
+ + WRITE_ONCE(event->overflow_handler, event->orig_overflow_handler); + event->prog = NULL; + bpf_prog_put(prog); +}
quoted hunk ↗ jump to hunk
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;@@ -7647,6 +7711,8 @@ static void perf_event_free_bpf_prog(struct perf_event *event) { struct bpf_prog *prog; + perf_event_free_bpf_handler(event); + if (!event->tp_event) return;
Does it at all make sense to merge the tp_event->prog thing into this new event->prog?
quoted hunk ↗ jump to hunk
#ifdef CONFIG_HAVE_HW_BREAKPOINT@@ -8957,6 +9029,14 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu, if (!overflow_handler && parent_event) { overflow_handler = parent_event->overflow_handler; context = parent_event->overflow_handler_context; + if (overflow_handler == bpf_overflow_handler) { + event->prog = bpf_prog_inc(parent_event->prog); + event->orig_overflow_handler = parent_event->orig_overflow_handler; + if (IS_ERR(event->prog)) { + event->prog = NULL; + overflow_handler = NULL; + } + } }
Should we not fail the entire perf_event_alloc() call in that IS_ERR() case?