On Wed, Dec 04, 2024 at 01:28:01PM +0100, Peter Zijlstra wrote: > 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; We have a similar logic in __perf_output_begin() and it checks with the parent event due to inheritance. But doing it here would save some cycles for preparing samples to be dropped. Thanks, Namhyung > > perf_prepare_sample(data, event, regs); > perf_prepare_header(&header, data, event, regs);
