On Wed, Aug 6, 2025 at 3:53 PM Namhyung Kim <namhyung@xxxxxxxxxx> wrote:
>
> Hello,
>
> On Wed, Aug 06, 2025 at 01:40:35PM +0200, Ilya Leoshkevich wrote:
> > On s390, and, in general, on all platforms where the respective event
> > supports auxiliary data gathering, the command:
> >
> > # ./perf record -u 0 -aB --synth=no -- ./perf test -w thloop
> > [ perf record: Woken up 1 times to write data ]
> > [ perf record: Captured and wrote 0.011 MB perf.data ]
> > # ./perf report --stats | grep SAMPLE
> > #
> >
> > does not generate samples in the perf.data file. On x86 the command:
> >
> > # sudo perf record -e intel_pt// -u 0 ls
> >
> > is broken too.
> >
> > Looking at the sequence of calls in 'perf record' reveals this
> > behavior:
> >
> > 1. The event 'cycles' is created and enabled:
> >
> > record__open()
> > +-> evlist__apply_filters()
> > +-> perf_bpf_filter__prepare()
> > +-> bpf_program.attach_perf_event()
> > +-> bpf_program.attach_perf_event_opts()
> > +-> __GI___ioctl(..., PERF_EVENT_IOC_ENABLE, ...)
> >
> > The event 'cycles' is enabled and active now. However the event's
> > ring-buffer to store the samples generated by hardware is not
> > allocated yet.
> >
> > 2. The event's fd is mmap()ed to create the ring buffer:
> >
> > record__open()
> > +-> record__mmap()
> > +-> record__mmap_evlist()
> > +-> evlist__mmap_ex()
> > +-> perf_evlist__mmap_ops()
> > +-> mmap_per_cpu()
> > +-> mmap_per_evsel()
> > +-> mmap__mmap()
> > +-> perf_mmap__mmap()
> > +-> mmap()
> >
> > This allocates the ring buffer for the event 'cycles'. With mmap()
> > the kernel creates the ring buffer:
> >
> > perf_mmap(): kernel function to create the event's ring
> > | buffer to save the sampled data.
> > |
> > +-> ring_buffer_attach(): Allocates memory for ring buffer.
> > | The PMU has auxiliary data setup function. The
> > | has_aux(event) condition is true and the PMU's
> > | stop() is called to stop sampling. It is not
> > | restarted:
> > |
> > | if (has_aux(event))
> > | perf_event_stop(event, 0);
> > |
> > +-> cpumsf_pmu_stop():
> >
> > Hardware sampling is stopped. No samples are generated and saved
> > anymore.
> >
> > 3. After the event 'cycles' has been mapped, the event is enabled a
> > second time in:
> >
> > __cmd_record()
> > +-> evlist__enable()
> > +-> __evlist__enable()
> > +-> evsel__enable_cpu()
> > +-> perf_evsel__enable_cpu()
> > +-> perf_evsel__run_ioctl()
> > +-> perf_evsel__ioctl()
> > +-> __GI___ioctl(., PERF_EVENT_IOC_ENABLE, .)
> >
> > The second
> >
> > ioctl(fd, PERF_EVENT_IOC_ENABLE, 0);
> >
> > is just a NOP in this case. The first invocation in (1.) sets the
> > event::state to PERF_EVENT_STATE_ACTIVE. The kernel functions
> >
> > perf_ioctl()
> > +-> _perf_ioctl()
> > +-> _perf_event_enable()
> > +-> __perf_event_enable()
> >
> > return immediately because event::state is already set to
> > PERF_EVENT_STATE_ACTIVE.
> >
> > This happens on s390, because the event 'cycles' offers the possibility
> > to save auxilary data. The PMU callbacks setup_aux() and free_aux() are
> > defined. Without both callback functions, cpumsf_pmu_stop() is not
> > invoked and sampling continues.
> >
> > To remedy this, remove the first invocation of
> >
> > ioctl(..., PERF_EVENT_IOC_ENABLE, ...).
> >
> > in step (1.) Create the event in step (1.) and enable it in step (3.)
> > after the ring buffer has been mapped.
> >
> > Output after:
> >
> > # ./perf record -aB --synth=no -u 0 -- ./perf test -w thloop 2
> > [ perf record: Woken up 3 times to write data ]
> > [ perf record: Captured and wrote 0.876 MB perf.data ]
> > # ./perf report --stats | grep SAMPLE
> > SAMPLE events: 16200 (99.5%)
> > SAMPLE events: 16200
> > #
> >
> > The software event succeeded both before and after the patch:
> >
> > # ./perf record -e cpu-clock -aB --synth=no -u 0 -- \
> > ./perf test -w thloop 2
> > [ perf record: Woken up 7 times to write data ]
> > [ perf record: Captured and wrote 2.870 MB perf.data ]
> > # ./perf report --stats | grep SAMPLE
> > SAMPLE events: 53506 (99.8%)
> > SAMPLE events: 53506
> > #
> >
> > Fixes: b4c658d4d63d61 ("perf target: Remove uid from target")
> > Suggested-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> > Tested-by: Thomas Richter <tmricht@xxxxxxxxxxxxx>
> > Co-developed-by: Thomas Richter <tmricht@xxxxxxxxxxxxx>
> > Signed-off-by: Thomas Richter <tmricht@xxxxxxxxxxxxx>
> > Signed-off-by: Ilya Leoshkevich <iii@xxxxxxxxxxxxx>
>
> Acked-by: Namhyung Kim <namhyung@xxxxxxxxxx>
Do you mind if I take the whole set through the bpf tree ?
I'm planning to send bpf PR in a couple days, so by -rc1
all trees will see the fix.
