* Re: perf_event_detach_bpf_prog() broken? [not found] ` <ZxewvPQX7bq40PK3@krava> @ 2024-10-22 17:33 ` Andrii Nakryiko 2024-10-23 8:19 ` Jiri Olsa 0 siblings, 1 reply; 2+ messages in thread From: Andrii Nakryiko @ 2024-10-22 17:33 UTC (permalink / raw) To: Jiri Olsa; +Cc: Peter Zijlstra, andrii, yhs, linux-kernel, daniel, sean, bpf + bpf ML On Tue, Oct 22, 2024 at 7:03 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > On Tue, Oct 22, 2024 at 01:16:38PM +0200, Peter Zijlstra wrote: > > Hi guys, > > > > Per commit 170a7e3ea070 ("bpf: bpf_prog_array_copy() should return > > -ENOENT if exclude_prog not found") perf_event_detach_bpf_prog() can now > > return without doing bpf_prog_put() and leaving event->prog set. > > > > This is very 'unexpected' behaviour. > > > > I'm not sure what's sane from the BPF side of things here, but leaving > > event->prog set is really rather unexpected. > > > > Help? > > IIUC the ENOENT should never happen in perf event context, so not yep, if it does return an error it's a bug, right? So we can add WARN_ONCE() or just drop the check, probably. > sure why we have that check.. also does not seem to be used from > lirc code, Sean? > > perf_event_detach_bpf_prog is called when the event is being freed > so I think we should always put and clear the event->prog > > jirka ^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: perf_event_detach_bpf_prog() broken? 2024-10-22 17:33 ` perf_event_detach_bpf_prog() broken? Andrii Nakryiko @ 2024-10-23 8:19 ` Jiri Olsa 0 siblings, 0 replies; 2+ messages in thread From: Jiri Olsa @ 2024-10-23 8:19 UTC (permalink / raw) To: Andrii Nakryiko Cc: Jiri Olsa, Peter Zijlstra, andrii, yhs, linux-kernel, daniel, sean, bpf On Tue, Oct 22, 2024 at 10:33:37AM -0700, Andrii Nakryiko wrote: > + bpf ML > > On Tue, Oct 22, 2024 at 7:03 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > > > On Tue, Oct 22, 2024 at 01:16:38PM +0200, Peter Zijlstra wrote: > > > Hi guys, > > > > > > Per commit 170a7e3ea070 ("bpf: bpf_prog_array_copy() should return > > > -ENOENT if exclude_prog not found") perf_event_detach_bpf_prog() can now > > > return without doing bpf_prog_put() and leaving event->prog set. > > > > > > This is very 'unexpected' behaviour. > > > > > > I'm not sure what's sane from the BPF side of things here, but leaving > > > event->prog set is really rather unexpected. > > > > > > Help? > > > > IIUC the ENOENT should never happen in perf event context, so not > > yep, if it does return an error it's a bug, right? So we can add > WARN_ONCE() or just drop the check, probably. I'm now more inclined to have the WARN there, because it's possible return value of bpf_prog_array_copy .. I'll send the patch and let's discuss over the change jirka > > > sure why we have that check.. also does not seem to be used from > > lirc code, Sean? > > > > perf_event_detach_bpf_prog is called when the event is being freed > > so I think we should always put and clear the event->prog > > > > jirka ^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-10-23 8:19 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20241022111638.GC16066@noisy.programming.kicks-ass.net>
[not found] ` <ZxewvPQX7bq40PK3@krava>
2024-10-22 17:33 ` perf_event_detach_bpf_prog() broken? Andrii Nakryiko
2024-10-23 8:19 ` Jiri Olsa
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox