Re: perf_event_open.2: mmap ring buffer requirement for receiving overflow notifications

[Peter Zijlstra <peterz@infradead.org>]

On Mon, Dec 02, 2024 at 12:53:56PM -0800, Namhyung Kim wrote:
> Hi Peter and Ingo,
> 
> On Tue, Nov 26, 2024 at 05:13:45PM -0500, Vince Weaver wrote:
> > On Sat, 23 Nov 2024, Mikołaj Kołek wrote:
> > 
> > > findings. In the MMAP layout section of the page, you can find this
> > > sentence:
> > > Before Linux 2.6.39, there is a bug that means you must allocate
> > > an mmap ring buffer when sampling even if you do not plan to
> > > access it.
> > > Unless I'm somehow misunderstanding it, this statement does not seem
> > > to be well worded, or alternatively this bug does not seem to be
> > > fixed.
> > 
> > That text was probably written by me.
> > 
> > I tried looking at the 2.6.39 code, my perf_tests, and also PAPI which was 
> > where the problem was probably noticed but I can't find a firm reference 
> > for how the issue was fixed.
> > 
> > If I recall, the problem was if you were trying to create a sampling event 
> > without mmap (say you want to get a signal every 100,000 retired 
> > instructions, but you don't actually want any sample data).  I think 
> > before 2.6.39 if you tried setting that up you'd get some sort of error 
> > (an EINVAL?) when trying to start(?) the event.
> > 
> > It is possible this wasn't fixed.  I tried to be pretty good 
> > about putting relevant git commits as comments in the manpage but there 
> > doesn't seem to be one for that part of the text.  I'm guessing it was 
> > PeterZ doing the work on this so maybe he remembers.
> 
> Do you remember what was the issue exactly on sampling events w/o mmap?

I can barely remember last release :/ But looking at the code, I don't
see a reason that wouldn't work today.

Only the overflow handler should care about there being a buffer, and
that just NOPs out if there isn't -- expensively, if this is a common
thing, this could perhaps be optimized a little.


---
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 4c6f6c286b2d..190b5c3cec10 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -8117,6 +8117,8 @@ __perf_event_output(struct perf_event *event,
 
 	/* protect the callchain buffers */
 	rcu_read_lock();
+	if (unlikely(!event->rb))
+		goto exit;
 
 	perf_prepare_sample(data, event, regs);
 	perf_prepare_header(&header, data, event, regs);