tools/lib/bpf/libbpf.c | 19 +++++++++++++------ tools/lib/bpf/libbpf.h | 3 ++- 2 files changed, 15 insertions(+), 7 deletions(-)
V1 --> V2: Added Jiri Olsa's suggestion and introduced
member bpf_perf_event_opts::no_ioctl_enable.
On linux-next
commit b4c658d4d63d61 ("perf target: Remove uid from target")
introduces a regression on s390. In fact the regression exists
on all platforms when the event supports auxiliary data gathering.
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 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. This happens now after enabling the event:
2. The event's fd is mmap() 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() returns 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 call backs setup_aux() and
free_aux() are defined. Without both call back 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.
Make the change backward compatible and introduce a new structure
member bpf_perf_event_opts::no_ioctl_enable. It defaults to false and only
bpf_program__attach_perf_event() sets it to true. This way only
perf tool invocation do not enable the sampling event.
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 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: 63f2f5ee856ba ("libbpf: add ability to attach/detach BPF program to perf event")
To: Andrii Nakryiko <andriin@fb.com>
To: Ian Rogers <irogers@google.com>
To: Ilya Leoshkevich <iii@linux.ibm.com>
Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
Suggested-by: Jiri Olsa <jolsa@redhat.com>
---
tools/lib/bpf/libbpf.c | 19 +++++++++++++------
tools/lib/bpf/libbpf.h | 3 ++-
2 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index e067cb5776bd..8e0fb4391b54 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -10911,6 +10911,7 @@ struct bpf_link *bpf_program__attach_perf_event_opts(const struct bpf_program *p
struct bpf_link_perf *link;
int prog_fd, link_fd = -1, err;
bool force_ioctl_attach;
+ bool no_ioctl_enable;
if (!OPTS_VALID(opts, bpf_perf_event_opts))
return libbpf_err_ptr(-EINVAL);
@@ -10965,11 +10966,14 @@ struct bpf_link *bpf_program__attach_perf_event_opts(const struct bpf_program *p
}
link->link.fd = pfd;
}
- if (ioctl(pfd, PERF_EVENT_IOC_ENABLE, 0) < 0) {
- err = -errno;
- pr_warn("prog '%s': failed to enable perf_event FD %d: %s\n",
- prog->name, pfd, errstr(err));
- goto err_out;
+ no_ioctl_enable = OPTS_GET(opts, no_ioctl_enable, false);
+ if (!no_ioctl_enable) {
+ if (ioctl(pfd, PERF_EVENT_IOC_ENABLE, 0) < 0) {
+ err = -errno;
+ pr_warn("prog '%s': failed to enable perf_event FD %d: %s\n",
+ prog->name, pfd, errstr(err));
+ goto err_out;
+ }
}
return &link->link;
@@ -10982,7 +10986,10 @@ struct bpf_link *bpf_program__attach_perf_event_opts(const struct bpf_program *p
struct bpf_link *bpf_program__attach_perf_event(const struct bpf_program *prog, int pfd)
{
- return bpf_program__attach_perf_event_opts(prog, pfd, NULL);
+ DECLARE_LIBBPF_OPTS(bpf_perf_event_opts, pe_opts);
+
+ pe_opts.no_ioctl_enable = true;
+ return bpf_program__attach_perf_event_opts(prog, pfd, &pe_opts);
}
/*
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index d1cf813a057b..4be2b7664031 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -499,9 +499,10 @@ struct bpf_perf_event_opts {
__u64 bpf_cookie;
/* don't use BPF link when attach BPF program */
bool force_ioctl_attach;
+ bool no_ioctl_enable;
size_t :0;
};
-#define bpf_perf_event_opts__last_field force_ioctl_attach
+#define bpf_perf_event_opts__last_field no_ioctl_enable
LIBBPF_API struct bpf_link *
bpf_program__attach_perf_event(const struct bpf_program *prog, int pfd);
--
2.50.1
On Mon, 2025-07-28 at 16:43 +0200, Thomas Richter wrote: > V1 --> V2: Added Jiri Olsa's suggestion and introduced > member bpf_perf_event_opts::no_ioctl_enable. > > On linux-next > commit b4c658d4d63d61 ("perf target: Remove uid from target") > introduces a regression on s390. In fact the regression exists > on all platforms when the event supports auxiliary data gathering. > > 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 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. This happens now after enabling the event: > > 2. The event's fd is mmap() 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() returns 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 call backs setup_aux() and > free_aux() are defined. Without both call back 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. > Make the change backward compatible and introduce a new structure > member bpf_perf_event_opts::no_ioctl_enable. It defaults to false and > only > bpf_program__attach_perf_event() sets it to true. This way only > perf tool invocation do not enable the sampling event. > > 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 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: 63f2f5ee856ba ("libbpf: add ability to attach/detach BPF > program to perf event") > To: Andrii Nakryiko <andriin@fb.com> > To: Ian Rogers <irogers@google.com> > To: Ilya Leoshkevich <iii@linux.ibm.com> > Signed-off-by: Thomas Richter <tmricht@linux.ibm.com> > Suggested-by: Jiri Olsa <jolsa@redhat.com> > --- > tools/lib/bpf/libbpf.c | 19 +++++++++++++------ > tools/lib/bpf/libbpf.h | 3 ++- > 2 files changed, 15 insertions(+), 7 deletions(-) What do you think about rather calling the new field ioctl_enable? So that we don't get double negations in the API users and implementation - they are sometimes unnecessarily confusing. I also think enablement should be the default in bpf_program__attach_perf_event(), and perf should now call bpf_program__attach_perf_event_opts() instead. Based on your request in v1, I can offer to take over the patch and send a v3 with the changes I suggested above.
On 7/31/25 10:38, Ilya Leoshkevich wrote: > On Mon, 2025-07-28 at 16:43 +0200, Thomas Richter wrote: >> V1 --> V2: Added Jiri Olsa's suggestion and introduced >> member bpf_perf_event_opts::no_ioctl_enable. >> >> On linux-next >> commit b4c658d4d63d61 ("perf target: Remove uid from target") >> introduces a regression on s390. In fact the regression exists >> on all platforms when the event supports auxiliary data gathering. >> >> 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 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. This happens now after enabling the event: >> >> 2. The event's fd is mmap() 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() returns 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 call backs setup_aux() and >> free_aux() are defined. Without both call back 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. >> Make the change backward compatible and introduce a new structure >> member bpf_perf_event_opts::no_ioctl_enable. It defaults to false and >> only >> bpf_program__attach_perf_event() sets it to true. This way only >> perf tool invocation do not enable the sampling event. >> >> 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 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: 63f2f5ee856ba ("libbpf: add ability to attach/detach BPF >> program to perf event") >> To: Andrii Nakryiko <andriin@fb.com> >> To: Ian Rogers <irogers@google.com> >> To: Ilya Leoshkevich <iii@linux.ibm.com> >> Signed-off-by: Thomas Richter <tmricht@linux.ibm.com> >> Suggested-by: Jiri Olsa <jolsa@redhat.com> >> --- >> tools/lib/bpf/libbpf.c | 19 +++++++++++++------ >> tools/lib/bpf/libbpf.h | 3 ++- >> 2 files changed, 15 insertions(+), 7 deletions(-) > > What do you think about rather calling the new field ioctl_enable? > So that we don't get double negations in the API users and > implementation - they are sometimes unnecessarily confusing. > > I also think enablement should be the default in > bpf_program__attach_perf_event(), and perf should now call > bpf_program__attach_perf_event_opts() instead. > > Based on your request in v1, I can offer to take over the patch and > send a v3 with the changes I suggested above. Yes Ilya, please go ahead. Thanks very much for your support. -- Thomas Richter, Dept 3303, IBM s390 Linux Development, Boeblingen, Germany -- IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Wolfgang Wendt Geschäftsführung: David Faller Sitz der Gesellschaft: Böblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294
© 2016 - 2025 Red Hat, Inc.