* [PATCH v1] perf/core: Restore header fields in sideband output callbacks
@ 2026-09-29 1:42 Ian Rogers
2026-09-29 12:04 ` Peter Zijlstra
2026-09-29 23:37 ` [PATCH v1] perf/core: Restore header fields in sideband output callbacks bot+bpf-ci
0 siblings, 2 replies; 5+ messages in thread
From: Ian Rogers @ 2026-09-29 1:42 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim
Cc: Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers,
Adrian Hunter, James Clark, Song Liu, Alexei Starovoitov,
linux-perf-users, linux-kernel, bpf, stable
perf_iterate_sb() invokes its callback for each matching perf_event on
the CPU and task context, passing a shared caller-allocated event
structure.
perf_event_header__init_id() increments header->size by
event->id_header_size. Unlike perf_event_task_output(),
perf_event_comm_output(), perf_event_namespaces_output(),
perf_event_cgroup_output(), perf_event_mmap_output(), and
perf_callchain_deferred_output(), three sideband callbacks failed to
save and restore header.size around perf_event_header__init_id():
- perf_event_ksymbol_output()
- perf_event_bpf_output()
- perf_event_text_poke_output()
When multiple events with attr.ksymbol, attr.bpf_event, or
attr.text_poke and sample_id_all are active on the same CPU, each
subsequent event receives a record whose header.size is inflated by all
preceding events' id_header_size values while only a single id_sample is
written, leaving uninitialized ring-buffer bytes at the end of the
record and causing userspace perf to fail with -EFAULT ("Bad address")
when parsing the sample_id trailer.
Similarly, perf_event_mmap_output() sets PERF_RECORD_MISC_MMAP_BUILD_ID
in mmap_event->event_id.header.misc when event->attr.build_id is
enabled, but only saved and restored header.size and header.type. If an
event with attr.build_id is followed by an event with attr.mmap2 and
!attr.build_id, the second event receives PERF_RECORD_MISC_MMAP_BUILD_ID
in header.misc while its payload contains maj/min/ino/ino_generation
instead of a build ID.
Save and restore header.size in the ksymbol, bpf, and text_poke output
callbacks, and save and restore header.misc in perf_event_mmap_output().
Fixes: 76193a94522f ("perf, bpf: Introduce PERF_RECORD_KSYMBOL")
Fixes: 6ee52e2a3fe4 ("perf, bpf: Introduce PERF_RECORD_BPF_EVENT")
Fixes: e17d43b93e54 ("perf: Add perf text poke event")
Fixes: 88a16a130933 ("perf: Add build id data in mmap2 event")
Assisted-by: Antigravity:gemini-3.1-pro
Signed-off-by: Ian Rogers <irogers@google.com>
---
kernel/events/core.c | 22 +++++++++++++++++++---
1 file changed, 19 insertions(+), 3 deletions(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 33210aff3ee6..ea3697295bd4 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -9029,6 +9029,11 @@ static void perf_iterate_sb_cpu(perf_iterate_f output, void *data)
*
* For new callers; ensure that account_pmu_sb_event() includes
* your event, otherwise it might not get delivered.
+ *
+ * Note: @data is shared across all @output calls, so any fields modified
+ * incrementally or conditionally (e.g. header.size via
+ * perf_event_header__init_id()) must be saved and restored by @output,
+ * or unconditionally re-initialized on each call.
*/
static void
perf_iterate_sb(perf_iterate_f output, void *data,
@@ -9723,6 +9728,7 @@ static void perf_event_mmap_output(struct perf_event *event,
struct perf_sample_data sample;
int size = mmap_event->event_id.header.size;
u32 type = mmap_event->event_id.header.type;
+ u16 misc = mmap_event->event_id.header.misc;
bool use_build_id;
int ret;
@@ -9780,6 +9786,7 @@ static void perf_event_mmap_output(struct perf_event *event,
out:
mmap_event->event_id.header.size = size;
mmap_event->event_id.header.type = type;
+ mmap_event->event_id.header.misc = misc;
}
static void perf_event_mmap_event(struct perf_mmap_event *mmap_event)
@@ -10244,6 +10251,7 @@ static void perf_event_ksymbol_output(struct perf_event *event, void *data)
struct perf_ksymbol_event *ksymbol_event = data;
struct perf_output_handle handle;
struct perf_sample_data sample;
+ u16 header_size = ksymbol_event->event_id.header.size;
int ret;
if (!perf_event_ksymbol_match(event))
@@ -10254,13 +10262,15 @@ static void perf_event_ksymbol_output(struct perf_event *event, void *data)
ret = perf_output_begin(&handle, &sample, event,
ksymbol_event->event_id.header.size);
if (ret)
- return;
+ goto out;
perf_output_put(&handle, ksymbol_event->event_id);
__output_copy(&handle, ksymbol_event->name, ksymbol_event->name_len);
perf_event__output_id_sample(event, &handle, &sample);
perf_output_end(&handle);
+out:
+ ksymbol_event->event_id.header.size = header_size;
}
void perf_event_ksymbol(u16 ksym_type, u64 addr, u32 len, bool unregister,
@@ -10334,6 +10344,7 @@ static void perf_event_bpf_output(struct perf_event *event, void *data)
struct perf_bpf_event *bpf_event = data;
struct perf_output_handle handle;
struct perf_sample_data sample;
+ u16 header_size = bpf_event->event_id.header.size;
int ret;
if (!perf_event_bpf_match(event))
@@ -10344,12 +10355,14 @@ static void perf_event_bpf_output(struct perf_event *event, void *data)
ret = perf_output_begin(&handle, &sample, event,
bpf_event->event_id.header.size);
if (ret)
- return;
+ goto out;
perf_output_put(&handle, bpf_event->event_id);
perf_event__output_id_sample(event, &handle, &sample);
perf_output_end(&handle);
+out:
+ bpf_event->event_id.header.size = header_size;
}
static void perf_event_bpf_emit_ksymbols(struct bpf_prog *prog,
@@ -10496,6 +10509,7 @@ static void perf_event_text_poke_output(struct perf_event *event, void *data)
struct perf_text_poke_event *text_poke_event = data;
struct perf_output_handle handle;
struct perf_sample_data sample;
+ u16 header_size = text_poke_event->event_id.header.size;
u64 padding = 0;
int ret;
@@ -10507,7 +10521,7 @@ static void perf_event_text_poke_output(struct perf_event *event, void *data)
ret = perf_output_begin(&handle, &sample, event,
text_poke_event->event_id.header.size);
if (ret)
- return;
+ goto out;
perf_output_put(&handle, text_poke_event->event_id);
perf_output_put(&handle, text_poke_event->old_len);
@@ -10522,6 +10536,8 @@ static void perf_event_text_poke_output(struct perf_event *event, void *data)
perf_event__output_id_sample(event, &handle, &sample);
perf_output_end(&handle);
+out:
+ text_poke_event->event_id.header.size = header_size;
}
void perf_event_text_poke(const void *addr, const void *old_bytes,
--
2.56.0.rc1.315.gc6ed9934b7-goog
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v1] perf/core: Restore header fields in sideband output callbacks 2026-09-29 1:42 [PATCH v1] perf/core: Restore header fields in sideband output callbacks Ian Rogers @ 2026-09-29 12:04 ` Peter Zijlstra 2026-09-29 18:21 ` Ian Rogers 2026-09-29 23:37 ` [PATCH v1] perf/core: Restore header fields in sideband output callbacks bot+bpf-ci 1 sibling, 1 reply; 5+ messages in thread From: Peter Zijlstra @ 2026-09-29 12:04 UTC (permalink / raw) To: Ian Rogers Cc: Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter, James Clark, Song Liu, Alexei Starovoitov, linux-perf-users, linux-kernel, bpf, stable On Mon, Sep 28, 2026 at 06:42:06PM -0700, Ian Rogers wrote: > perf_iterate_sb() invokes its callback for each matching perf_event on > the CPU and task context, passing a shared caller-allocated event > structure. > > perf_event_header__init_id() increments header->size by > event->id_header_size. Unlike perf_event_task_output(), > perf_event_comm_output(), perf_event_namespaces_output(), > perf_event_cgroup_output(), perf_event_mmap_output(), and > perf_callchain_deferred_output(), three sideband callbacks failed to > save and restore header.size around perf_event_header__init_id(): > - perf_event_ksymbol_output() > - perf_event_bpf_output() > - perf_event_text_poke_output() I also found perf_event_switch_output(). Does something like so also work? --- kernel/events/core.c | 131 ++++++++++++++++++++++++--------------------------- 1 file changed, 61 insertions(+), 70 deletions(-) diff --git a/kernel/events/core.c b/kernel/events/core.c index de05df65ab3d..28770ca689c8 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -9108,12 +9108,22 @@ perf_event_read_event(struct perf_event *event, perf_output_end(&handle); } -typedef void (perf_iterate_f)(struct perf_event *event, void *data); +typedef void (perf_iterate_f)(struct perf_event *event, struct perf_event_header *header); + +static __always_inline +void __perf_iterate_output(perf_iterate_f output, + struct perf_event *event, + struct perf_event_header *header) +{ + struct perf_event_header old = *header; + output(event, header); + *header = old; +} static void perf_iterate_ctx(struct perf_event_context *ctx, perf_iterate_f output, - void *data, bool all) + struct perf_event_header *header, bool all) { struct perf_event *event; @@ -9125,11 +9135,11 @@ perf_iterate_ctx(struct perf_event_context *ctx, continue; } - output(event, data); + __perf_iterate_output(output, event, header); } } -static void perf_iterate_sb_cpu(perf_iterate_f output, void *data) +static void perf_iterate_sb_cpu(perf_iterate_f output, struct perf_event_header *header) { struct pmu_event_list *pel = this_cpu_ptr(&pmu_sb_events); struct perf_event *event; @@ -9147,7 +9157,8 @@ static void perf_iterate_sb_cpu(perf_iterate_f output, void *data) continue; if (!event_filter_match(event)) continue; - output(event, data); + + __perf_iterate_output(output, event, header); } } @@ -9158,7 +9169,7 @@ static void perf_iterate_sb_cpu(perf_iterate_f output, void *data) * your event, otherwise it might not get delivered. */ static void -perf_iterate_sb(perf_iterate_f output, void *data, +perf_iterate_sb(perf_iterate_f output, struct perf_event_header *header, struct perf_event_context *task_ctx) { struct perf_event_context *ctx; @@ -9172,15 +9183,15 @@ perf_iterate_sb(perf_iterate_f output, void *data, * context. */ if (task_ctx) { - perf_iterate_ctx(task_ctx, output, data, false); + perf_iterate_ctx(task_ctx, output, header, false); goto done; } - perf_iterate_sb_cpu(output, data); + perf_iterate_sb_cpu(output, header); ctx = rcu_dereference(current->perf_event_ctxp); if (ctx) - perf_iterate_ctx(ctx, output, data, false); + perf_iterate_ctx(ctx, output, header, false); done: preempt_enable(); rcu_read_unlock(); @@ -9347,13 +9358,13 @@ static int perf_event_task_match(struct perf_event *event) } static void perf_event_task_output(struct perf_event *event, - void *data) + struct perf_event_header *header) { - struct perf_task_event *task_event = data; + auto task_event = container_of(header, struct perf_task_event, event_id.header); + struct task_struct *task = task_event->task; struct perf_output_handle handle; struct perf_sample_data sample; - struct task_struct *task = task_event->task; - int ret, size = task_event->event_id.header.size; + int ret; if (!perf_event_task_match(event)) return; @@ -9363,7 +9374,7 @@ static void perf_event_task_output(struct perf_event *event, ret = perf_output_begin(&handle, &sample, event, task_event->event_id.header.size); if (ret) - goto out; + return; task_event->event_id.pid = perf_event_pid(event, task); task_event->event_id.tid = perf_event_tid(event, task); @@ -9385,8 +9396,6 @@ static void perf_event_task_output(struct perf_event *event, perf_event__output_id_sample(event, &handle, &sample); perf_output_end(&handle); -out: - task_event->event_id.header.size = size; } static void perf_event_task(struct task_struct *task, @@ -9418,7 +9427,7 @@ static void perf_event_task(struct task_struct *task, }; perf_iterate_sb(perf_event_task_output, - &task_event, + &task_event.event_id.header, task_ctx); } @@ -9499,12 +9508,11 @@ static int perf_event_comm_match(struct perf_event *event) } static void perf_event_comm_output(struct perf_event *event, - void *data) + struct perf_event_header *header) { - struct perf_comm_event *comm_event = data; + auto comm_event = container_of(header, struct perf_comm_event, event_id.header); struct perf_output_handle handle; struct perf_sample_data sample; - int size = comm_event->event_id.header.size; int ret; if (!perf_event_comm_match(event)) @@ -9513,9 +9521,8 @@ static void perf_event_comm_output(struct perf_event *event, perf_event_header__init_id(&comm_event->event_id.header, &sample, event); ret = perf_output_begin(&handle, &sample, event, comm_event->event_id.header.size); - if (ret) - goto out; + return; comm_event->event_id.pid = perf_event_pid(event, comm_event->task); comm_event->event_id.tid = perf_event_tid(event, comm_event->task); @@ -9527,8 +9534,6 @@ static void perf_event_comm_output(struct perf_event *event, perf_event__output_id_sample(event, &handle, &sample); perf_output_end(&handle); -out: - comm_event->event_id.header.size = size; } static void perf_event_comm_event(struct perf_comm_event *comm_event) @@ -9546,7 +9551,7 @@ static void perf_event_comm_event(struct perf_comm_event *comm_event) comm_event->event_id.header.size = sizeof(comm_event->event_id) + size; perf_iterate_sb(perf_event_comm_output, - comm_event, + &comm_event->event_id.header, NULL); } @@ -9598,12 +9603,11 @@ static int perf_event_namespaces_match(struct perf_event *event) } static void perf_event_namespaces_output(struct perf_event *event, - void *data) + struct perf_event_header *header) { - struct perf_namespaces_event *namespaces_event = data; + auto namespaces_event = container_of(header, struct perf_namespaces_event, event_id.header); struct perf_output_handle handle; struct perf_sample_data sample; - u16 header_size = namespaces_event->event_id.header.size; int ret; if (!perf_event_namespaces_match(event)) @@ -9614,7 +9618,7 @@ static void perf_event_namespaces_output(struct perf_event *event, ret = perf_output_begin(&handle, &sample, event, namespaces_event->event_id.header.size); if (ret) - goto out; + return; namespaces_event->event_id.pid = perf_event_pid(event, namespaces_event->task); @@ -9626,8 +9630,6 @@ static void perf_event_namespaces_output(struct perf_event *event, perf_event__output_id_sample(event, &handle, &sample); perf_output_end(&handle); -out: - namespaces_event->event_id.header.size = header_size; } static void perf_fill_ns_link_info(struct perf_ns_link_info *ns_link_info, @@ -9701,7 +9703,7 @@ void perf_event_namespaces(struct task_struct *task) #endif perf_iterate_sb(perf_event_namespaces_output, - &namespaces_event, + &namespaces_event.event_id.header, NULL); } @@ -9725,12 +9727,11 @@ static int perf_event_cgroup_match(struct perf_event *event) return event->attr.cgroup; } -static void perf_event_cgroup_output(struct perf_event *event, void *data) +static void perf_event_cgroup_output(struct perf_event *event, struct perf_event_header *header) { - struct perf_cgroup_event *cgroup_event = data; + auto cgroup_event = container_of(header, struct perf_cgroup_event, event_id.header); struct perf_output_handle handle; struct perf_sample_data sample; - u16 header_size = cgroup_event->event_id.header.size; int ret; if (!perf_event_cgroup_match(event)) @@ -9741,7 +9742,7 @@ static void perf_event_cgroup_output(struct perf_event *event, void *data) ret = perf_output_begin(&handle, &sample, event, cgroup_event->event_id.header.size); if (ret) - goto out; + return; perf_output_put(&handle, cgroup_event->event_id); __output_copy(&handle, cgroup_event->path, cgroup_event->path_size); @@ -9749,8 +9750,6 @@ static void perf_event_cgroup_output(struct perf_event *event, void *data) perf_event__output_id_sample(event, &handle, &sample); perf_output_end(&handle); -out: - cgroup_event->event_id.header.size = header_size; } static void perf_event_cgroup(struct cgroup *cgrp) @@ -9796,7 +9795,7 @@ static void perf_event_cgroup(struct cgroup *cgrp) cgroup_event.path_size = size; perf_iterate_sb(perf_event_cgroup_output, - &cgroup_event, + &cgroup_event.event_id.header, NULL); kfree(pathname); @@ -9832,9 +9831,8 @@ struct perf_mmap_event { }; static int perf_event_mmap_match(struct perf_event *event, - void *data) + struct perf_mmap_event *mmap_event) { - struct perf_mmap_event *mmap_event = data; struct vm_area_struct *vma = mmap_event->vma; int executable = vma->vm_flags & VM_EXEC; @@ -9843,17 +9841,15 @@ static int perf_event_mmap_match(struct perf_event *event, } static void perf_event_mmap_output(struct perf_event *event, - void *data) + struct perf_event_header *header) { - struct perf_mmap_event *mmap_event = data; + auto mmap_event = container_of(header, struct perf_mmap_event, event_id.header); struct perf_output_handle handle; struct perf_sample_data sample; - int size = mmap_event->event_id.header.size; - u32 type = mmap_event->event_id.header.type; bool use_build_id; int ret; - if (!perf_event_mmap_match(event, data)) + if (!perf_event_mmap_match(event, mmap_event)) return; if (event->attr.mmap2) { @@ -9870,7 +9866,7 @@ static void perf_event_mmap_output(struct perf_event *event, ret = perf_output_begin(&handle, &sample, event, mmap_event->event_id.header.size); if (ret) - goto out; + return; mmap_event->event_id.pid = perf_event_pid(event, current); mmap_event->event_id.tid = perf_event_tid(event, current); @@ -9904,9 +9900,6 @@ static void perf_event_mmap_output(struct perf_event *event, perf_event__output_id_sample(event, &handle, &sample); perf_output_end(&handle); -out: - mmap_event->event_id.header.size = size; - mmap_event->event_id.header.type = type; } static void perf_event_mmap_event(struct perf_mmap_event *mmap_event) @@ -10011,7 +10004,7 @@ static void perf_event_mmap_event(struct perf_mmap_event *mmap_event) build_id_parse_nofault(vma, mmap_event->build_id, &mmap_event->build_id_size); perf_iterate_sb(perf_event_mmap_output, - mmap_event, + &mmap_event->event_id.header, NULL); kfree(buf); @@ -10236,9 +10229,9 @@ static int perf_event_switch_match(struct perf_event *event) return event->attr.context_switch; } -static void perf_event_switch_output(struct perf_event *event, void *data) +static void perf_event_switch_output(struct perf_event *event, struct perf_event_header *header) { - struct perf_switch_event *se = data; + auto se = container_of(header, struct perf_switch_event, event_id.header); struct perf_output_handle handle; struct perf_sample_data sample; int ret; @@ -10301,7 +10294,7 @@ static void perf_event_switch(struct task_struct *task, PERF_RECORD_MISC_SWITCH_OUT_PREEMPT; } - perf_iterate_sb(perf_event_switch_output, &switch_event, NULL); + perf_iterate_sb(perf_event_switch_output, &switch_event.event_id.header, NULL); } /* @@ -10366,9 +10359,9 @@ static int perf_event_ksymbol_match(struct perf_event *event) return event->attr.ksymbol; } -static void perf_event_ksymbol_output(struct perf_event *event, void *data) +static void perf_event_ksymbol_output(struct perf_event *event, struct perf_event_header *header) { - struct perf_ksymbol_event *ksymbol_event = data; + auto ksymbol_event = container_of(header, struct perf_ksymbol_event, event_id.header); struct perf_output_handle handle; struct perf_sample_data sample; int ret; @@ -10430,7 +10423,7 @@ void perf_event_ksymbol(u16 ksym_type, u64 addr, u32 len, bool unregister, }, }; - perf_iterate_sb(perf_event_ksymbol_output, &ksymbol_event, NULL); + perf_iterate_sb(perf_event_ksymbol_output, &ksymbol_event.event_id.header, NULL); return; err: WARN_ONCE(1, "%s: Invalid KSYMBOL type 0x%x\n", __func__, ksym_type); @@ -10456,9 +10449,9 @@ static int perf_event_bpf_match(struct perf_event *event) return event->attr.bpf_event; } -static void perf_event_bpf_output(struct perf_event *event, void *data) +static void perf_event_bpf_output(struct perf_event *event, struct perf_event_header *header) { - struct perf_bpf_event *bpf_event = data; + auto bpf_event = container_of(header, struct perf_bpf_event, event_id.header); struct perf_output_handle handle; struct perf_sample_data sample; int ret; @@ -10536,7 +10529,7 @@ void perf_event_bpf_event(struct bpf_prog *prog, BUILD_BUG_ON(BPF_TAG_SIZE % sizeof(u64)); memcpy(bpf_event.event_id.tag, prog->tag, BPF_TAG_SIZE); - perf_iterate_sb(perf_event_bpf_output, &bpf_event, NULL); + perf_iterate_sb(perf_event_bpf_output, &bpf_event.event_id.header, NULL); } struct perf_callchain_deferred_event { @@ -10549,12 +10542,12 @@ struct perf_callchain_deferred_event { } event; }; -static void perf_callchain_deferred_output(struct perf_event *event, void *data) +static void perf_callchain_deferred_output(struct perf_event *event, struct perf_event_header *header) { - struct perf_callchain_deferred_event *deferred_event = data; + auto deferred_event = container_of(header, struct perf_callchain_deferred_event, event.header); struct perf_output_handle handle; struct perf_sample_data sample; - int ret, size = deferred_event->event.header.size; + int ret; if (!event->attr.defer_output) return; @@ -10565,7 +10558,7 @@ static void perf_callchain_deferred_output(struct perf_event *event, void *data) ret = perf_output_begin(&handle, &sample, event, deferred_event->event.header.size); if (ret) - goto out; + return; perf_output_put(&handle, deferred_event->event); for (int i = 0; i < deferred_event->trace->nr; i++) { @@ -10575,8 +10568,6 @@ static void perf_callchain_deferred_output(struct perf_event *event, void *data) perf_event__output_id_sample(event, &handle, &sample); perf_output_end(&handle); -out: - deferred_event->event.header.size = size; } static void perf_unwind_deferred_callback(struct unwind_work *work, @@ -10596,7 +10587,7 @@ static void perf_unwind_deferred_callback(struct unwind_work *work, }, }; - perf_iterate_sb(perf_callchain_deferred_output, &deferred_event, NULL); + perf_iterate_sb(perf_callchain_deferred_output, &deferred_event.event.header, NULL); } struct perf_text_poke_event { @@ -10618,9 +10609,9 @@ static int perf_event_text_poke_match(struct perf_event *event) return event->attr.text_poke; } -static void perf_event_text_poke_output(struct perf_event *event, void *data) +static void perf_event_text_poke_output(struct perf_event *event, struct perf_event_header *header) { - struct perf_text_poke_event *text_poke_event = data; + auto text_poke_event = container_of(header, struct perf_text_poke_event, event_id.header); struct perf_output_handle handle; struct perf_sample_data sample; u64 padding = 0; @@ -10680,7 +10671,7 @@ void perf_event_text_poke(const void *addr, const void *old_bytes, }, }; - perf_iterate_sb(perf_event_text_poke_output, &text_poke_event, NULL); + perf_iterate_sb(perf_event_text_poke_output, &text_poke_event.event_id.header, NULL); } void perf_event_itrace_started(struct perf_event *event) ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1] perf/core: Restore header fields in sideband output callbacks 2026-09-29 12:04 ` Peter Zijlstra @ 2026-09-29 18:21 ` Ian Rogers 2026-09-29 22:23 ` [PATCH v1] perf/core: Replace perf_event_header__init_id with full header init Ian Rogers 0 siblings, 1 reply; 5+ messages in thread From: Ian Rogers @ 2026-09-29 18:21 UTC (permalink / raw) To: peterz Cc: acme, adrian.hunter, alexander.shishkin, ast, bpf, irogers, james.clark, jolsa, linux-kernel, linux-perf-users, mark.rutland, mingo, namhyung, song, stable On Tue, Sep 29, 2026 at 5:04 AM Peter Zijlstra <peterz@infradead.org> wrote: > > On Mon, Sep 28, 2026 at 06:42:06PM -0700, Ian Rogers wrote: > > perf_iterate_sb() invokes its callback for each matching perf_event on > > the CPU and task context, passing a shared caller-allocated event > > structure. > > > > perf_event_header__init_id() increments header->size by > > event->id_header_size. Unlike perf_event_task_output(), > > perf_event_comm_output(), perf_event_namespaces_output(), > > perf_event_cgroup_output(), perf_event_mmap_output(), and > > perf_callchain_deferred_output(), three sideband callbacks failed to > > save and restore header.size around perf_event_header__init_id(): > > - perf_event_ksymbol_output() > > - perf_event_bpf_output() > > - perf_event_text_poke_output() > > I also found perf_event_switch_output(). Does something like so also > work? It does. I was considering a general pattern but in the case of perf_event_switch_output the header size is computed per callback event rather than in the caller. I wonder if the problem is really with perf_event_header__init_id mutating and not just assigning the header. It tangles the header size computation between the caller and the callback. If we're assigning the header in every callback then there is no need to save and restore it. It is inefficient to reinitialize values in the header every time, but saving and restoring the header is also inefficient. Below is the refactoring I mean and I think it simplifies the code overall: diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c index f401181d8669..ada2ebfe1fcf 100644 --- a/arch/powerpc/perf/imc-pmu.c +++ b/arch/powerpc/perf/imc-pmu.c @@ -1275,6 +1275,8 @@ static int trace_imc_prepare_sample(struct trace_imc_data *mem, struct perf_event_header *header, struct perf_event *event) { + __u16 misc = 0; + /* Sanity checks for a valid record */ if (be64_to_cpu(READ_ONCE(mem->tb1)) > *prev_tb) *prev_tb = be64_to_cpu(READ_ONCE(mem->tb1)); @@ -1289,23 +1291,19 @@ static int trace_imc_prepare_sample(struct trace_imc_data *mem, data->ip = be64_to_cpu(READ_ONCE(mem->ip)); data->period = event->hw.last_period; - header->type = PERF_RECORD_SAMPLE; - header->size = sizeof(*header) + event->header_size; - header->misc = 0; - if (cpu_has_feature(CPU_FTR_ARCH_31)) { switch (IMC_TRACE_RECORD_VAL_HVPR(be64_to_cpu(READ_ONCE(mem->val)))) { case 0:/* when MSR HV and PR not set in the trace-record */ - header->misc |= PERF_RECORD_MISC_GUEST_KERNEL; + misc |= PERF_RECORD_MISC_GUEST_KERNEL; break; case 1: /* MSR HV is 0 and PR is 1 */ - header->misc |= PERF_RECORD_MISC_GUEST_USER; + misc |= PERF_RECORD_MISC_GUEST_USER; break; case 2: /* MSR HV is 1 and PR is 0 */ - header->misc |= PERF_RECORD_MISC_KERNEL; + misc |= PERF_RECORD_MISC_KERNEL; break; case 3: /* MSR HV is 1 and PR is 1 */ - header->misc |= PERF_RECORD_MISC_USER; + misc |= PERF_RECORD_MISC_USER; break; default: pr_info("IMC: Unable to set the flag based on MSR bits\n"); @@ -1313,11 +1311,14 @@ static int trace_imc_prepare_sample(struct trace_imc_data *mem, } } else { if (is_kernel_addr(data->ip)) - header->misc |= PERF_RECORD_MISC_KERNEL; + misc |= PERF_RECORD_MISC_KERNEL; else - header->misc |= PERF_RECORD_MISC_USER; + misc |= PERF_RECORD_MISC_USER; } - perf_event_header__init_id(header, data, event); + perf_event_header__init_header_and_id(header, data, + PERF_RECORD_SAMPLE, misc, + sizeof(*header) + event->header_size, + event); return 0; } diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h index 5842552294c1..8912298e5d4d 100644 --- a/include/linux/perf_event.h +++ b/include/linux/perf_event.h @@ -1523,9 +1523,10 @@ is_default_overflow_handler(struct perf_event *event) } extern void -perf_event_header__init_id(struct perf_event_header *header, - struct perf_sample_data *data, - struct perf_event *event); +perf_event_header__init_header_and_id(struct perf_event_header *header, + struct perf_sample_data *data, + u32 type, u16 misc, u16 size, + struct perf_event *event); extern void perf_event__output_id_sample(struct perf_event *event, struct perf_output_handle *handle, diff --git a/kernel/events/core.c b/kernel/events/core.c index 33210aff3ee6..5a1bfffd46e6 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -8119,13 +8119,18 @@ static void __perf_event_header__init_id(struct perf_sample_data *data, } } -void perf_event_header__init_id(struct perf_event_header *header, - struct perf_sample_data *data, - struct perf_event *event) +void perf_event_header__init_header_and_id(struct perf_event_header *header, + struct perf_sample_data *data, + u32 type, u16 misc, u16 size, + struct perf_event *event) { + header->type = type; + header->misc = misc; if (event->attr.sample_id_all) { - header->size += event->id_header_size; + header->size = size + event->id_header_size; __perf_event_header__init_id(data, event, event->attr.sample_type); + } else { + header->size = size; } } @@ -8959,17 +8964,16 @@ perf_event_read_event(struct perf_event *event, struct perf_output_handle handle; struct perf_sample_data sample; struct perf_read_event read_event = { - .header = { - .type = PERF_RECORD_READ, - .misc = 0, - .size = sizeof(read_event) + event->read_size, - }, .pid = perf_event_pid(event, task), .tid = perf_event_tid(event, task), }; int ret; - perf_event_header__init_id(&read_event.header, &sample, event); + perf_event_header__init_header_and_id(&read_event.header, &sample, + PERF_RECORD_READ, + /*misc=*/0, + sizeof(read_event) + event->read_size, + event); ret = perf_output_begin(&handle, &sample, event, read_event.header.size); if (ret) return; @@ -9210,6 +9214,7 @@ struct perf_task_event { u32 ptid; u64 time; } event_id; + int new; }; static int perf_event_task_match(struct perf_event *event) @@ -9226,17 +9231,21 @@ static void perf_event_task_output(struct perf_event *event, struct perf_output_handle handle; struct perf_sample_data sample; struct task_struct *task = task_event->task; - int ret, size = task_event->event_id.header.size; + int ret; if (!perf_event_task_match(event)) return; - perf_event_header__init_id(&task_event->event_id.header, &sample, event); + perf_event_header__init_header_and_id(&task_event->event_id.header, &sample, + task_event->new ? PERF_RECORD_FORK : PERF_RECORD_EXIT, + /*misc=*/0, + sizeof(task_event->event_id), + event); ret = perf_output_begin(&handle, &sample, event, task_event->event_id.header.size); if (ret) - goto out; + return; task_event->event_id.pid = perf_event_pid(event, task); task_event->event_id.tid = perf_event_tid(event, task); @@ -9258,8 +9267,6 @@ static void perf_event_task_output(struct perf_event *event, perf_event__output_id_sample(event, &handle, &sample); perf_output_end(&handle); -out: - task_event->event_id.header.size = size; } static void perf_event_task(struct task_struct *task, @@ -9276,18 +9283,7 @@ static void perf_event_task(struct task_struct *task, task_event = (struct perf_task_event){ .task = task, .task_ctx = task_ctx, - .event_id = { - .header = { - .type = new ? PERF_RECORD_FORK : PERF_RECORD_EXIT, - .misc = 0, - .size = sizeof(task_event.event_id), - }, - /* .pid */ - /* .ppid */ - /* .tid */ - /* .ptid */ - /* .time */ - }, + .new = new, }; perf_iterate_sb(perf_event_task_output, @@ -9364,6 +9360,7 @@ struct perf_comm_event { u32 pid; u32 tid; } event_id; + bool exec; }; static int perf_event_comm_match(struct perf_event *event) @@ -9377,18 +9374,21 @@ static void perf_event_comm_output(struct perf_event *event, struct perf_comm_event *comm_event = data; struct perf_output_handle handle; struct perf_sample_data sample; - int size = comm_event->event_id.header.size; int ret; if (!perf_event_comm_match(event)) return; - perf_event_header__init_id(&comm_event->event_id.header, &sample, event); + perf_event_header__init_header_and_id(&comm_event->event_id.header, &sample, + PERF_RECORD_COMM, + comm_event->exec ? PERF_RECORD_MISC_COMM_EXEC : 0, + sizeof(comm_event->event_id) + comm_event->comm_size, + event); ret = perf_output_begin(&handle, &sample, event, comm_event->event_id.header.size); if (ret) - goto out; + return; comm_event->event_id.pid = perf_event_pid(event, comm_event->task); comm_event->event_id.tid = perf_event_tid(event, comm_event->task); @@ -9400,8 +9400,6 @@ static void perf_event_comm_output(struct perf_event *event, perf_event__output_id_sample(event, &handle, &sample); perf_output_end(&handle); -out: - comm_event->event_id.header.size = size; } static void perf_event_comm_event(struct perf_comm_event *comm_event) @@ -9416,8 +9414,6 @@ static void perf_event_comm_event(struct perf_comm_event *comm_event) comm_event->comm = comm; comm_event->comm_size = size; - comm_event->event_id.header.size = sizeof(comm_event->event_id) + size; - perf_iterate_sb(perf_event_comm_output, comm_event, NULL); @@ -9434,15 +9430,8 @@ void perf_event_comm(struct task_struct *task, bool exec) .task = task, /* .comm */ /* .comm_size */ - .event_id = { - .header = { - .type = PERF_RECORD_COMM, - .misc = exec ? PERF_RECORD_MISC_COMM_EXEC : 0, - /* .size */ - }, - /* .pid */ - /* .tid */ - }, + /* .event_id */ + .exec = exec, }; perf_event_comm_event(&comm_event); @@ -9476,18 +9465,20 @@ static void perf_event_namespaces_output(struct perf_event *event, struct perf_namespaces_event *namespaces_event = data; struct perf_output_handle handle; struct perf_sample_data sample; - u16 header_size = namespaces_event->event_id.header.size; int ret; if (!perf_event_namespaces_match(event)) return; - perf_event_header__init_id(&namespaces_event->event_id.header, - &sample, event); + perf_event_header__init_header_and_id(&namespaces_event->event_id.header, &sample, + PERF_RECORD_NAMESPACES, + /*misc=*/0, + sizeof(namespaces_event->event_id), + event); ret = perf_output_begin(&handle, &sample, event, namespaces_event->event_id.header.size); if (ret) - goto out; + return; namespaces_event->event_id.pid = perf_event_pid(event, namespaces_event->task); @@ -9499,8 +9490,6 @@ static void perf_event_namespaces_output(struct perf_event *event, perf_event__output_id_sample(event, &handle, &sample); perf_output_end(&handle); -out: - namespaces_event->event_id.header.size = header_size; } static void perf_fill_ns_link_info(struct perf_ns_link_info *ns_link_info, @@ -9531,11 +9520,7 @@ void perf_event_namespaces(struct task_struct *task) namespaces_event = (struct perf_namespaces_event){ .task = task, .event_id = { - .header = { - .type = PERF_RECORD_NAMESPACES, - .misc = 0, - .size = sizeof(namespaces_event.event_id), - }, + /* .header */ /* .pid */ /* .tid */ .nr_namespaces = NR_NAMESPACES, @@ -9603,18 +9588,19 @@ static void perf_event_cgroup_output(struct perf_event *event, void *data) struct perf_cgroup_event *cgroup_event = data; struct perf_output_handle handle; struct perf_sample_data sample; - u16 header_size = cgroup_event->event_id.header.size; int ret; + __u16 size = sizeof(cgroup_event->event_id) + cgroup_event->path_size; if (!perf_event_cgroup_match(event)) return; - perf_event_header__init_id(&cgroup_event->event_id.header, - &sample, event); + perf_event_header__init_header_and_id(&cgroup_event->event_id.header, &sample, + PERF_RECORD_CGROUP, /*misc=*/0, size, + event); ret = perf_output_begin(&handle, &sample, event, cgroup_event->event_id.header.size); if (ret) - goto out; + return; perf_output_put(&handle, cgroup_event->event_id); __output_copy(&handle, cgroup_event->path, cgroup_event->path_size); @@ -9622,8 +9608,6 @@ static void perf_event_cgroup_output(struct perf_event *event, void *data) perf_event__output_id_sample(event, &handle, &sample); perf_output_end(&handle); -out: - cgroup_event->event_id.header.size = header_size; } static void perf_event_cgroup(struct cgroup *cgrp) @@ -9638,11 +9622,6 @@ static void perf_event_cgroup(struct cgroup *cgrp) cgroup_event = (struct perf_cgroup_event){ .event_id = { - .header = { - .type = PERF_RECORD_CGROUP, - .misc = 0, - .size = sizeof(cgroup_event.event_id), - }, .id = cgroup_id(cgrp), }, }; @@ -9665,7 +9644,6 @@ static void perf_event_cgroup(struct cgroup *cgrp) while (!IS_ALIGNED(size, sizeof(u64))) cgroup_event.path[size++] = '\0'; - cgroup_event.event_id.header.size += size; cgroup_event.path_size = size; perf_iterate_sb(perf_event_cgroup_output, @@ -9721,8 +9699,9 @@ static void perf_event_mmap_output(struct perf_event *event, struct perf_mmap_event *mmap_event = data; struct perf_output_handle handle; struct perf_sample_data sample; - int size = mmap_event->event_id.header.size; - u32 type = mmap_event->event_id.header.type; + int size = sizeof(mmap_event->event_id) + mmap_event->file_size; + u32 type = PERF_RECORD_MMAP; + u16 misc = PERF_RECORD_MISC_USER; bool use_build_id; int ret; @@ -9730,20 +9709,23 @@ static void perf_event_mmap_output(struct perf_event *event, return; if (event->attr.mmap2) { - mmap_event->event_id.header.type = PERF_RECORD_MMAP2; - mmap_event->event_id.header.size += sizeof(mmap_event->maj); - mmap_event->event_id.header.size += sizeof(mmap_event->min); - mmap_event->event_id.header.size += sizeof(mmap_event->ino); - mmap_event->event_id.header.size += sizeof(mmap_event->ino_generation); - mmap_event->event_id.header.size += sizeof(mmap_event->prot); - mmap_event->event_id.header.size += sizeof(mmap_event->flags); - } - - perf_event_header__init_id(&mmap_event->event_id.header, &sample, event); + type = PERF_RECORD_MMAP2; + size += sizeof(mmap_event->maj); + size += sizeof(mmap_event->min); + size += sizeof(mmap_event->ino); + size += sizeof(mmap_event->ino_generation); + size += sizeof(mmap_event->prot); + size += sizeof(mmap_event->flags); + } + if (!(mmap_event->vma->vm_flags & VM_EXEC)) + misc |= PERF_RECORD_MISC_MMAP_DATA; + + perf_event_header__init_header_and_id(&mmap_event->event_id.header, &sample, + type, misc, size, event); ret = perf_output_begin(&handle, &sample, event, mmap_event->event_id.header.size); if (ret) - goto out; + return; mmap_event->event_id.pid = perf_event_pid(event, current); mmap_event->event_id.tid = perf_event_tid(event, current); @@ -9777,9 +9759,6 @@ static void perf_event_mmap_output(struct perf_event *event, perf_event__output_id_sample(event, &handle, &sample); perf_output_end(&handle); -out: - mmap_event->event_id.header.size = size; - mmap_event->event_id.header.type = type; } static void perf_event_mmap_event(struct perf_mmap_event *mmap_event) @@ -9875,11 +9854,6 @@ static void perf_event_mmap_event(struct perf_mmap_event *mmap_event) mmap_event->prot = prot; mmap_event->flags = flags; - if (!(vma->vm_flags & VM_EXEC)) - mmap_event->event_id.header.misc |= PERF_RECORD_MISC_MMAP_DATA; - - mmap_event->event_id.header.size = sizeof(mmap_event->event_id) + size; - if (atomic_read(&nr_build_id_events)) build_id_parse_nofault(vma, mmap_event->build_id, &mmap_event->build_id_size); @@ -9999,11 +9973,7 @@ void perf_event_mmap(struct vm_area_struct *vma) /* .file_name */ /* .file_size */ .event_id = { - .header = { - .type = PERF_RECORD_MMAP, - .misc = PERF_RECORD_MISC_USER, - /* .size */ - }, + /* .header */ /* .pid */ /* .tid */ .start = vma->vm_start, @@ -10033,18 +10003,15 @@ void perf_event_aux_event(struct perf_event *event, unsigned long head, u64 size; u64 flags; } rec = { - .header = { - .type = PERF_RECORD_AUX, - .misc = 0, - .size = sizeof(rec), - }, .offset = head, .size = size, .flags = flags, }; int ret; - perf_event_header__init_id(&rec.header, &sample, event); + perf_event_header__init_header_and_id(&rec.header, &sample, + PERF_RECORD_AUX, /*misc=*/0, sizeof(rec), + event); ret = perf_output_begin(&handle, &sample, event, rec.header.size); if (ret) @@ -10069,15 +10036,14 @@ void perf_log_lost_samples(struct perf_event *event, u64 lost) struct perf_event_header header; u64 lost; } lost_samples_event = { - .header = { - .type = PERF_RECORD_LOST_SAMPLES, - .misc = 0, - .size = sizeof(lost_samples_event), - }, .lost = lost, }; - perf_event_header__init_id(&lost_samples_event.header, &sample, event); + perf_event_header__init_header_and_id(&lost_samples_event.header, &sample, + PERF_RECORD_LOST_SAMPLES, + /*misc=*/0, + sizeof(lost_samples_event), + event); ret = perf_output_begin(&handle, &sample, event, lost_samples_event.header.size); @@ -10102,6 +10068,8 @@ struct perf_switch_event { u32 next_prev_pid; u32 next_prev_tid; } event_id; + bool sched_in; + bool preempt; }; static int perf_event_switch_match(struct perf_event *event) @@ -10114,6 +10082,9 @@ static void perf_event_switch_output(struct perf_event *event, void *data) struct perf_switch_event *se = data; struct perf_output_handle handle; struct perf_sample_data sample; + __u32 type; + __u16 misc; + __u16 size; int ret; if (!perf_event_switch_match(event)) @@ -10121,18 +10092,22 @@ static void perf_event_switch_output(struct perf_event *event, void *data) /* Only CPU-wide events are allowed to see next/prev pid/tid */ if (event->ctx->task) { - se->event_id.header.type = PERF_RECORD_SWITCH; - se->event_id.header.size = sizeof(se->event_id.header); + type = PERF_RECORD_SWITCH; + size = sizeof(se->event_id.header); } else { - se->event_id.header.type = PERF_RECORD_SWITCH_CPU_WIDE; - se->event_id.header.size = sizeof(se->event_id); + type = PERF_RECORD_SWITCH_CPU_WIDE; + size = sizeof(se->event_id); se->event_id.next_prev_pid = perf_event_pid(event, se->next_prev); se->event_id.next_prev_tid = perf_event_tid(event, se->next_prev); } + misc = se->sched_in ? 0 : PERF_RECORD_MISC_SWITCH_OUT; + if (se->preempt) + misc |= PERF_RECORD_MISC_SWITCH_OUT_PREEMPT; - perf_event_header__init_id(&se->event_id.header, &sample, event); + perf_event_header__init_header_and_id(&se->event_id.header, &sample, + type, misc, size, event); ret = perf_output_begin(&handle, &sample, event, se->event_id.header.size); if (ret) @@ -10158,15 +10133,9 @@ static void perf_event_switch(struct task_struct *task, switch_event = (struct perf_switch_event){ .task = task, .next_prev = next_prev, - .event_id = { - .header = { - /* .type */ - .misc = sched_in ? 0 : PERF_RECORD_MISC_SWITCH_OUT, - /* .size */ - }, - /* .next_prev_pid */ - /* .next_prev_tid */ - }, + /* .event_id */ + .sched_in = sched_in, + .preempt = !sched_in && task_is_runnable(task), }; if (!sched_in && task_is_runnable(task)) { @@ -10193,11 +10162,6 @@ static void perf_log_throttle(struct perf_event *event, int enable) u64 id; u64 stream_id; } throttle_event = { - .header = { - .type = PERF_RECORD_THROTTLE, - .misc = 0, - .size = sizeof(throttle_event), - }, .time = perf_event_clock(event), .id = primary_event_id(event), .stream_id = event->id, @@ -10206,7 +10170,11 @@ static void perf_log_throttle(struct perf_event *event, int enable) if (enable) throttle_event.header.type = PERF_RECORD_UNTHROTTLE; - perf_event_header__init_id(&throttle_event.header, &sample, event); + perf_event_header__init_header_and_id(&throttle_event.header, &sample, + PERF_RECORD_THROTTLE, + /*misc=*/0, + sizeof(throttle_event), + event); ret = perf_output_begin(&handle, &sample, event, throttle_event.header.size); @@ -10245,12 +10213,14 @@ static void perf_event_ksymbol_output(struct perf_event *event, void *data) struct perf_output_handle handle; struct perf_sample_data sample; int ret; + __u16 size = sizeof(ksymbol_event->event_id) + ksymbol_event->name_len; if (!perf_event_ksymbol_match(event)) return; - perf_event_header__init_id(&ksymbol_event->event_id.header, - &sample, event); + perf_event_header__init_header_and_id(&ksymbol_event->event_id.header, &sample, + PERF_RECORD_KSYMBOL, /*misc=*/0, size, + event); ret = perf_output_begin(&handle, &sample, event, ksymbol_event->event_id.header.size); if (ret) @@ -10291,11 +10261,6 @@ void perf_event_ksymbol(u16 ksym_type, u64 addr, u32 len, bool unregister, .name = name, .name_len = name_len, .event_id = { - .header = { - .type = PERF_RECORD_KSYMBOL, - .size = sizeof(ksymbol_event.event_id) + - name_len, - }, .addr = addr, .len = len, .ksym_type = ksym_type, @@ -10339,8 +10304,11 @@ static void perf_event_bpf_output(struct perf_event *event, void *data) if (!perf_event_bpf_match(event)) return; - perf_event_header__init_id(&bpf_event->event_id.header, - &sample, event); + perf_event_header__init_header_and_id(&bpf_event->event_id.header, &sample, + PERF_RECORD_BPF_EVENT, + /*misc=*/0, + sizeof(bpf_event->event_id), + event); ret = perf_output_begin(&handle, &sample, event, bpf_event->event_id.header.size); if (ret) @@ -10396,10 +10364,6 @@ void perf_event_bpf_event(struct bpf_prog *prog, bpf_event = (struct perf_bpf_event){ .prog = prog, .event_id = { - .header = { - .type = PERF_RECORD_BPF_EVENT, - .size = sizeof(bpf_event.event_id), - }, .type = type, .flags = flags, .id = prog->aux->id, @@ -10427,18 +10391,23 @@ static void perf_callchain_deferred_output(struct perf_event *event, void *data) struct perf_callchain_deferred_event *deferred_event = data; struct perf_output_handle handle; struct perf_sample_data sample; - int ret, size = deferred_event->event.header.size; + int ret; + __u16 size = sizeof(deferred_event->event) + (deferred_event->trace->nr * sizeof(u64)); if (!event->attr.defer_output) return; /* XXX do we really need sample_id_all for this ??? */ - perf_event_header__init_id(&deferred_event->event.header, &sample, event); + perf_event_header__init_header_and_id(&deferred_event->event.header, &sample, + PERF_RECORD_CALLCHAIN_DEFERRED, + PERF_RECORD_MISC_USER, + size, + event); ret = perf_output_begin(&handle, &sample, event, deferred_event->event.header.size); if (ret) - goto out; + return; perf_output_put(&handle, deferred_event->event); for (int i = 0; i < deferred_event->trace->nr; i++) { @@ -10448,8 +10417,6 @@ static void perf_callchain_deferred_output(struct perf_event *event, void *data) perf_event__output_id_sample(event, &handle, &sample); perf_output_end(&handle); -out: - deferred_event->event.header.size = size; } static void perf_unwind_deferred_callback(struct unwind_work *work, @@ -10458,12 +10425,6 @@ static void perf_unwind_deferred_callback(struct unwind_work *work, struct perf_callchain_deferred_event deferred_event = { .trace = trace, .event = { - .header = { - .type = PERF_RECORD_CALLCHAIN_DEFERRED, - .misc = PERF_RECORD_MISC_USER, - .size = sizeof(deferred_event.event) + - (trace->nr * sizeof(u64)), - }, .cookie = cookie, .nr = trace->nr, }, @@ -10475,7 +10436,8 @@ static void perf_unwind_deferred_callback(struct unwind_work *work, struct perf_text_poke_event { const void *old_bytes; const void *new_bytes; - size_t pad; + u16 tot; + u16 pad; u16 old_len; u16 new_len; @@ -10496,13 +10458,18 @@ static void perf_event_text_poke_output(struct perf_event *event, void *data) struct perf_text_poke_event *text_poke_event = data; struct perf_output_handle handle; struct perf_sample_data sample; + __u16 size = sizeof(text_poke_event->event_id) + ALIGN(text_poke_event->tot, sizeof(u64)); u64 padding = 0; int ret; if (!perf_event_text_poke_match(event)) return; - perf_event_header__init_id(&text_poke_event->event_id.header, &sample, event); + perf_event_header__init_header_and_id(&text_poke_event->event_id.header, &sample, + PERF_RECORD_TEXT_POKE, + PERF_RECORD_MISC_KERNEL, + size, + event); ret = perf_output_begin(&handle, &sample, event, text_poke_event->event_id.header.size); @@ -10535,20 +10502,15 @@ void perf_event_text_poke(const void *addr, const void *old_bytes, tot = sizeof(text_poke_event.old_len) + old_len; tot += sizeof(text_poke_event.new_len) + new_len; - pad = ALIGN(tot, sizeof(u64)) - tot; text_poke_event = (struct perf_text_poke_event){ .old_bytes = old_bytes, .new_bytes = new_bytes, + .tot = tot, .pad = pad, .old_len = old_len, .new_len = new_len, .event_id = { - .header = { - .type = PERF_RECORD_TEXT_POKE, - .misc = PERF_RECORD_MISC_KERNEL, - .size = sizeof(text_poke_event.event_id) + tot + pad, - }, .addr = (unsigned long)addr, }, }; @@ -10579,13 +10541,12 @@ static void perf_log_itrace_start(struct perf_event *event) event->attach_state & PERF_ATTACH_ITRACE) return; - rec.header.type = PERF_RECORD_ITRACE_START; - rec.header.misc = 0; - rec.header.size = sizeof(rec); rec.pid = perf_event_pid(event, current); rec.tid = perf_event_tid(event, current); - perf_event_header__init_id(&rec.header, &sample, event); + perf_event_header__init_header_and_id(&rec.header, &sample, + PERF_RECORD_ITRACE_START, /*misc=*/0, sizeof(rec), + event); ret = perf_output_begin(&handle, &sample, event, rec.header.size); if (ret) @@ -10610,12 +10571,10 @@ void perf_report_aux_output_id(struct perf_event *event, u64 hw_id) if (event->parent) event = event->parent; - rec.header.type = PERF_RECORD_AUX_OUTPUT_HW_ID; - rec.header.misc = 0; - rec.header.size = sizeof(rec); - rec.hw_id = hw_id; - - perf_event_header__init_id(&rec.header, &sample, event); + rec.hw_id = hw_id; + perf_event_header__init_header_and_id(&rec.header, &sample, + PERF_RECORD_AUX_OUTPUT_HW_ID, /*misc=*/0, sizeof(rec), + event); ret = perf_output_begin(&handle, &sample, event, rec.header.size); if (ret) ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v1] perf/core: Replace perf_event_header__init_id with full header init 2026-09-29 18:21 ` Ian Rogers @ 2026-09-29 22:23 ` Ian Rogers 0 siblings, 0 replies; 5+ messages in thread From: Ian Rogers @ 2026-09-29 22:23 UTC (permalink / raw) To: irogers Cc: acme, adrian.hunter, alexander.shishkin, ast, bpf, james.clark, jolsa, linux-kernel, linux-perf-users, mark.rutland, mingo, namhyung, peterz, song, stable perf_iterate_sb() invokes its callback for each matching perf_event on the CPU and task context, passing a shared caller-allocated event structure. perf_event_header__init_id() mutated header->size in place by adding event->id_header_size, requiring sideband output callbacks to save and restore header fields across iterations. Three sideband callbacks failed to save and restore header.size around perf_event_header__init_id(): - perf_event_ksymbol_output() - perf_event_bpf_output() - perf_event_text_poke_output() When multiple events with attr.ksymbol, attr.bpf_event, or attr.text_poke and sample_id_all are active on the same CPU, each subsequent event receives a record whose header.size is inflated by all preceding events' id_header_size values while only a single id_sample is written, leaving uninitialized ring-buffer bytes at the end of the record and causing userspace perf to fail with -EFAULT ("Bad address") when parsing the sample_id trailer. Similarly, perf_event_mmap_output() set PERF_RECORD_MISC_MMAP_BUILD_ID in mmap_event->event_id.header.misc when event->attr.build_id was enabled, but only saved and restored header.size and header.type. If an event with attr.build_id was followed by an event with attr.mmap2 and !attr.build_id, the second event received PERF_RECORD_MISC_MMAP_BUILD_ID in header.misc while its payload contained maj/min/ino/ino_generation instead of a build ID. Rather than splitting header initialization between callers and output callbacks and saving/restoring mutated header fields, replace perf_event_header__init_id() with perf_event_header__init_header_and_id(), which initializes header->type, header->misc, and header->size alongside the sample_id fields on each invocation. Fixes: 76193a94522f ("perf, bpf: Introduce PERF_RECORD_KSYMBOL") Fixes: 6ee52e2a3fe4 ("perf, bpf: Introduce PERF_RECORD_BPF_EVENT") Fixes: e17d43b93e54 ("perf: Add perf text poke event") Fixes: 88a16a130933 ("perf: Add build id data in mmap2 event") Assisted-by: Antigravity:gemini-3.1-pro Signed-off-by: Ian Rogers <irogers@google.com> --- arch/powerpc/perf/imc-pmu.c | 23 +-- include/linux/perf_event.h | 7 +- kernel/events/core.c | 309 +++++++++++++++--------------------- kernel/events/ring_buffer.c | 7 +- 4 files changed, 149 insertions(+), 197 deletions(-) diff --git a/arch/powerpc/perf/imc-pmu.c b/arch/powerpc/perf/imc-pmu.c index f401181d8669..cd427f2a6d26 100644 --- a/arch/powerpc/perf/imc-pmu.c +++ b/arch/powerpc/perf/imc-pmu.c @@ -1275,6 +1275,8 @@ static int trace_imc_prepare_sample(struct trace_imc_data *mem, struct perf_event_header *header, struct perf_event *event) { + u16 misc = 0; + /* Sanity checks for a valid record */ if (be64_to_cpu(READ_ONCE(mem->tb1)) > *prev_tb) *prev_tb = be64_to_cpu(READ_ONCE(mem->tb1)); @@ -1289,23 +1291,19 @@ static int trace_imc_prepare_sample(struct trace_imc_data *mem, data->ip = be64_to_cpu(READ_ONCE(mem->ip)); data->period = event->hw.last_period; - header->type = PERF_RECORD_SAMPLE; - header->size = sizeof(*header) + event->header_size; - header->misc = 0; - if (cpu_has_feature(CPU_FTR_ARCH_31)) { switch (IMC_TRACE_RECORD_VAL_HVPR(be64_to_cpu(READ_ONCE(mem->val)))) { case 0:/* when MSR HV and PR not set in the trace-record */ - header->misc |= PERF_RECORD_MISC_GUEST_KERNEL; + misc |= PERF_RECORD_MISC_GUEST_KERNEL; break; case 1: /* MSR HV is 0 and PR is 1 */ - header->misc |= PERF_RECORD_MISC_GUEST_USER; + misc |= PERF_RECORD_MISC_GUEST_USER; break; case 2: /* MSR HV is 1 and PR is 0 */ - header->misc |= PERF_RECORD_MISC_KERNEL; + misc |= PERF_RECORD_MISC_KERNEL; break; case 3: /* MSR HV is 1 and PR is 1 */ - header->misc |= PERF_RECORD_MISC_USER; + misc |= PERF_RECORD_MISC_USER; break; default: pr_info("IMC: Unable to set the flag based on MSR bits\n"); @@ -1313,11 +1311,14 @@ static int trace_imc_prepare_sample(struct trace_imc_data *mem, } } else { if (is_kernel_addr(data->ip)) - header->misc |= PERF_RECORD_MISC_KERNEL; + misc |= PERF_RECORD_MISC_KERNEL; else - header->misc |= PERF_RECORD_MISC_USER; + misc |= PERF_RECORD_MISC_USER; } - perf_event_header__init_id(header, data, event); + perf_event_header__init_header_and_id(header, data, + PERF_RECORD_SAMPLE, misc, + sizeof(*header) + event->header_size, + event); return 0; } diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h index 5842552294c1..8912298e5d4d 100644 --- a/include/linux/perf_event.h +++ b/include/linux/perf_event.h @@ -1523,9 +1523,10 @@ is_default_overflow_handler(struct perf_event *event) } extern void -perf_event_header__init_id(struct perf_event_header *header, - struct perf_sample_data *data, - struct perf_event *event); +perf_event_header__init_header_and_id(struct perf_event_header *header, + struct perf_sample_data *data, + u32 type, u16 misc, u16 size, + struct perf_event *event); extern void perf_event__output_id_sample(struct perf_event *event, struct perf_output_handle *handle, diff --git a/kernel/events/core.c b/kernel/events/core.c index 33210aff3ee6..c1b2d0c9b2af 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -8119,13 +8119,18 @@ static void __perf_event_header__init_id(struct perf_sample_data *data, } } -void perf_event_header__init_id(struct perf_event_header *header, - struct perf_sample_data *data, - struct perf_event *event) +void perf_event_header__init_header_and_id(struct perf_event_header *header, + struct perf_sample_data *data, + u32 type, u16 misc, u16 size, + struct perf_event *event) { + header->type = type; + header->misc = misc; if (event->attr.sample_id_all) { - header->size += event->id_header_size; + header->size = size + event->id_header_size; __perf_event_header__init_id(data, event, event->attr.sample_type); + } else { + header->size = size; } } @@ -8959,17 +8964,16 @@ perf_event_read_event(struct perf_event *event, struct perf_output_handle handle; struct perf_sample_data sample; struct perf_read_event read_event = { - .header = { - .type = PERF_RECORD_READ, - .misc = 0, - .size = sizeof(read_event) + event->read_size, - }, .pid = perf_event_pid(event, task), .tid = perf_event_tid(event, task), }; int ret; - perf_event_header__init_id(&read_event.header, &sample, event); + perf_event_header__init_header_and_id(&read_event.header, &sample, + PERF_RECORD_READ, + /*misc=*/0, + sizeof(read_event) + event->read_size, + event); ret = perf_output_begin(&handle, &sample, event, read_event.header.size); if (ret) return; @@ -9210,6 +9214,7 @@ struct perf_task_event { u32 ptid; u64 time; } event_id; + int new; }; static int perf_event_task_match(struct perf_event *event) @@ -9226,17 +9231,21 @@ static void perf_event_task_output(struct perf_event *event, struct perf_output_handle handle; struct perf_sample_data sample; struct task_struct *task = task_event->task; - int ret, size = task_event->event_id.header.size; + int ret; if (!perf_event_task_match(event)) return; - perf_event_header__init_id(&task_event->event_id.header, &sample, event); + perf_event_header__init_header_and_id(&task_event->event_id.header, &sample, + task_event->new ? PERF_RECORD_FORK : PERF_RECORD_EXIT, + /*misc=*/0, + sizeof(task_event->event_id), + event); ret = perf_output_begin(&handle, &sample, event, task_event->event_id.header.size); if (ret) - goto out; + return; task_event->event_id.pid = perf_event_pid(event, task); task_event->event_id.tid = perf_event_tid(event, task); @@ -9258,8 +9267,6 @@ static void perf_event_task_output(struct perf_event *event, perf_event__output_id_sample(event, &handle, &sample); perf_output_end(&handle); -out: - task_event->event_id.header.size = size; } static void perf_event_task(struct task_struct *task, @@ -9276,18 +9283,7 @@ static void perf_event_task(struct task_struct *task, task_event = (struct perf_task_event){ .task = task, .task_ctx = task_ctx, - .event_id = { - .header = { - .type = new ? PERF_RECORD_FORK : PERF_RECORD_EXIT, - .misc = 0, - .size = sizeof(task_event.event_id), - }, - /* .pid */ - /* .ppid */ - /* .tid */ - /* .ptid */ - /* .time */ - }, + .new = new, }; perf_iterate_sb(perf_event_task_output, @@ -9364,6 +9360,7 @@ struct perf_comm_event { u32 pid; u32 tid; } event_id; + bool exec; }; static int perf_event_comm_match(struct perf_event *event) @@ -9377,18 +9374,21 @@ static void perf_event_comm_output(struct perf_event *event, struct perf_comm_event *comm_event = data; struct perf_output_handle handle; struct perf_sample_data sample; - int size = comm_event->event_id.header.size; int ret; if (!perf_event_comm_match(event)) return; - perf_event_header__init_id(&comm_event->event_id.header, &sample, event); + perf_event_header__init_header_and_id(&comm_event->event_id.header, &sample, + PERF_RECORD_COMM, + comm_event->exec ? PERF_RECORD_MISC_COMM_EXEC : 0, + sizeof(comm_event->event_id) + comm_event->comm_size, + event); ret = perf_output_begin(&handle, &sample, event, comm_event->event_id.header.size); if (ret) - goto out; + return; comm_event->event_id.pid = perf_event_pid(event, comm_event->task); comm_event->event_id.tid = perf_event_tid(event, comm_event->task); @@ -9400,8 +9400,6 @@ static void perf_event_comm_output(struct perf_event *event, perf_event__output_id_sample(event, &handle, &sample); perf_output_end(&handle); -out: - comm_event->event_id.header.size = size; } static void perf_event_comm_event(struct perf_comm_event *comm_event) @@ -9416,8 +9414,6 @@ static void perf_event_comm_event(struct perf_comm_event *comm_event) comm_event->comm = comm; comm_event->comm_size = size; - comm_event->event_id.header.size = sizeof(comm_event->event_id) + size; - perf_iterate_sb(perf_event_comm_output, comm_event, NULL); @@ -9434,15 +9430,8 @@ void perf_event_comm(struct task_struct *task, bool exec) .task = task, /* .comm */ /* .comm_size */ - .event_id = { - .header = { - .type = PERF_RECORD_COMM, - .misc = exec ? PERF_RECORD_MISC_COMM_EXEC : 0, - /* .size */ - }, - /* .pid */ - /* .tid */ - }, + /* .event_id */ + .exec = exec, }; perf_event_comm_event(&comm_event); @@ -9476,18 +9465,20 @@ static void perf_event_namespaces_output(struct perf_event *event, struct perf_namespaces_event *namespaces_event = data; struct perf_output_handle handle; struct perf_sample_data sample; - u16 header_size = namespaces_event->event_id.header.size; int ret; if (!perf_event_namespaces_match(event)) return; - perf_event_header__init_id(&namespaces_event->event_id.header, - &sample, event); + perf_event_header__init_header_and_id(&namespaces_event->event_id.header, &sample, + PERF_RECORD_NAMESPACES, + /*misc=*/0, + sizeof(namespaces_event->event_id), + event); ret = perf_output_begin(&handle, &sample, event, namespaces_event->event_id.header.size); if (ret) - goto out; + return; namespaces_event->event_id.pid = perf_event_pid(event, namespaces_event->task); @@ -9499,8 +9490,6 @@ static void perf_event_namespaces_output(struct perf_event *event, perf_event__output_id_sample(event, &handle, &sample); perf_output_end(&handle); -out: - namespaces_event->event_id.header.size = header_size; } static void perf_fill_ns_link_info(struct perf_ns_link_info *ns_link_info, @@ -9531,11 +9520,7 @@ void perf_event_namespaces(struct task_struct *task) namespaces_event = (struct perf_namespaces_event){ .task = task, .event_id = { - .header = { - .type = PERF_RECORD_NAMESPACES, - .misc = 0, - .size = sizeof(namespaces_event.event_id), - }, + /* .header */ /* .pid */ /* .tid */ .nr_namespaces = NR_NAMESPACES, @@ -9603,18 +9588,19 @@ static void perf_event_cgroup_output(struct perf_event *event, void *data) struct perf_cgroup_event *cgroup_event = data; struct perf_output_handle handle; struct perf_sample_data sample; - u16 header_size = cgroup_event->event_id.header.size; int ret; + u16 size = sizeof(cgroup_event->event_id) + cgroup_event->path_size; if (!perf_event_cgroup_match(event)) return; - perf_event_header__init_id(&cgroup_event->event_id.header, - &sample, event); + perf_event_header__init_header_and_id(&cgroup_event->event_id.header, &sample, + PERF_RECORD_CGROUP, /*misc=*/0, size, + event); ret = perf_output_begin(&handle, &sample, event, cgroup_event->event_id.header.size); if (ret) - goto out; + return; perf_output_put(&handle, cgroup_event->event_id); __output_copy(&handle, cgroup_event->path, cgroup_event->path_size); @@ -9622,8 +9608,6 @@ static void perf_event_cgroup_output(struct perf_event *event, void *data) perf_event__output_id_sample(event, &handle, &sample); perf_output_end(&handle); -out: - cgroup_event->event_id.header.size = header_size; } static void perf_event_cgroup(struct cgroup *cgrp) @@ -9638,11 +9622,6 @@ static void perf_event_cgroup(struct cgroup *cgrp) cgroup_event = (struct perf_cgroup_event){ .event_id = { - .header = { - .type = PERF_RECORD_CGROUP, - .misc = 0, - .size = sizeof(cgroup_event.event_id), - }, .id = cgroup_id(cgrp), }, }; @@ -9665,7 +9644,6 @@ static void perf_event_cgroup(struct cgroup *cgrp) while (!IS_ALIGNED(size, sizeof(u64))) cgroup_event.path[size++] = '\0'; - cgroup_event.event_id.header.size += size; cgroup_event.path_size = size; perf_iterate_sb(perf_event_cgroup_output, @@ -9721,38 +9699,40 @@ static void perf_event_mmap_output(struct perf_event *event, struct perf_mmap_event *mmap_event = data; struct perf_output_handle handle; struct perf_sample_data sample; - int size = mmap_event->event_id.header.size; - u32 type = mmap_event->event_id.header.type; - bool use_build_id; + int size = sizeof(mmap_event->event_id) + mmap_event->file_size; + u32 type = PERF_RECORD_MMAP; + u16 misc = PERF_RECORD_MISC_USER; + bool use_build_id = false; int ret; if (!perf_event_mmap_match(event, data)) return; if (event->attr.mmap2) { - mmap_event->event_id.header.type = PERF_RECORD_MMAP2; - mmap_event->event_id.header.size += sizeof(mmap_event->maj); - mmap_event->event_id.header.size += sizeof(mmap_event->min); - mmap_event->event_id.header.size += sizeof(mmap_event->ino); - mmap_event->event_id.header.size += sizeof(mmap_event->ino_generation); - mmap_event->event_id.header.size += sizeof(mmap_event->prot); - mmap_event->event_id.header.size += sizeof(mmap_event->flags); - } - - perf_event_header__init_id(&mmap_event->event_id.header, &sample, event); + type = PERF_RECORD_MMAP2; + size += sizeof(mmap_event->maj); + size += sizeof(mmap_event->min); + size += sizeof(mmap_event->ino); + size += sizeof(mmap_event->ino_generation); + size += sizeof(mmap_event->prot); + size += sizeof(mmap_event->flags); + use_build_id = event->attr.build_id && mmap_event->build_id_size; + if (use_build_id) + misc |= PERF_RECORD_MISC_MMAP_BUILD_ID; + } + if (!(mmap_event->vma->vm_flags & VM_EXEC)) + misc |= PERF_RECORD_MISC_MMAP_DATA; + + perf_event_header__init_header_and_id(&mmap_event->event_id.header, &sample, + type, misc, size, event); ret = perf_output_begin(&handle, &sample, event, mmap_event->event_id.header.size); if (ret) - goto out; + return; mmap_event->event_id.pid = perf_event_pid(event, current); mmap_event->event_id.tid = perf_event_tid(event, current); - use_build_id = event->attr.build_id && mmap_event->build_id_size; - - if (event->attr.mmap2 && use_build_id) - mmap_event->event_id.header.misc |= PERF_RECORD_MISC_MMAP_BUILD_ID; - perf_output_put(&handle, mmap_event->event_id); if (event->attr.mmap2) { @@ -9777,9 +9757,6 @@ static void perf_event_mmap_output(struct perf_event *event, perf_event__output_id_sample(event, &handle, &sample); perf_output_end(&handle); -out: - mmap_event->event_id.header.size = size; - mmap_event->event_id.header.type = type; } static void perf_event_mmap_event(struct perf_mmap_event *mmap_event) @@ -9875,11 +9852,6 @@ static void perf_event_mmap_event(struct perf_mmap_event *mmap_event) mmap_event->prot = prot; mmap_event->flags = flags; - if (!(vma->vm_flags & VM_EXEC)) - mmap_event->event_id.header.misc |= PERF_RECORD_MISC_MMAP_DATA; - - mmap_event->event_id.header.size = sizeof(mmap_event->event_id) + size; - if (atomic_read(&nr_build_id_events)) build_id_parse_nofault(vma, mmap_event->build_id, &mmap_event->build_id_size); @@ -9999,11 +9971,7 @@ void perf_event_mmap(struct vm_area_struct *vma) /* .file_name */ /* .file_size */ .event_id = { - .header = { - .type = PERF_RECORD_MMAP, - .misc = PERF_RECORD_MISC_USER, - /* .size */ - }, + /* .header */ /* .pid */ /* .tid */ .start = vma->vm_start, @@ -10033,18 +10001,15 @@ void perf_event_aux_event(struct perf_event *event, unsigned long head, u64 size; u64 flags; } rec = { - .header = { - .type = PERF_RECORD_AUX, - .misc = 0, - .size = sizeof(rec), - }, .offset = head, .size = size, .flags = flags, }; int ret; - perf_event_header__init_id(&rec.header, &sample, event); + perf_event_header__init_header_and_id(&rec.header, &sample, + PERF_RECORD_AUX, /*misc=*/0, sizeof(rec), + event); ret = perf_output_begin(&handle, &sample, event, rec.header.size); if (ret) @@ -10069,15 +10034,14 @@ void perf_log_lost_samples(struct perf_event *event, u64 lost) struct perf_event_header header; u64 lost; } lost_samples_event = { - .header = { - .type = PERF_RECORD_LOST_SAMPLES, - .misc = 0, - .size = sizeof(lost_samples_event), - }, .lost = lost, }; - perf_event_header__init_id(&lost_samples_event.header, &sample, event); + perf_event_header__init_header_and_id(&lost_samples_event.header, &sample, + PERF_RECORD_LOST_SAMPLES, + /*misc=*/0, + sizeof(lost_samples_event), + event); ret = perf_output_begin(&handle, &sample, event, lost_samples_event.header.size); @@ -10102,6 +10066,8 @@ struct perf_switch_event { u32 next_prev_pid; u32 next_prev_tid; } event_id; + bool sched_in; + bool preempt; }; static int perf_event_switch_match(struct perf_event *event) @@ -10114,6 +10080,9 @@ static void perf_event_switch_output(struct perf_event *event, void *data) struct perf_switch_event *se = data; struct perf_output_handle handle; struct perf_sample_data sample; + u32 type; + u16 misc; + u16 size; int ret; if (!perf_event_switch_match(event)) @@ -10121,18 +10090,22 @@ static void perf_event_switch_output(struct perf_event *event, void *data) /* Only CPU-wide events are allowed to see next/prev pid/tid */ if (event->ctx->task) { - se->event_id.header.type = PERF_RECORD_SWITCH; - se->event_id.header.size = sizeof(se->event_id.header); + type = PERF_RECORD_SWITCH; + size = sizeof(se->event_id.header); } else { - se->event_id.header.type = PERF_RECORD_SWITCH_CPU_WIDE; - se->event_id.header.size = sizeof(se->event_id); + type = PERF_RECORD_SWITCH_CPU_WIDE; + size = sizeof(se->event_id); se->event_id.next_prev_pid = perf_event_pid(event, se->next_prev); se->event_id.next_prev_tid = perf_event_tid(event, se->next_prev); } + misc = se->sched_in ? 0 : PERF_RECORD_MISC_SWITCH_OUT; + if (se->preempt) + misc |= PERF_RECORD_MISC_SWITCH_OUT_PREEMPT; - perf_event_header__init_id(&se->event_id.header, &sample, event); + perf_event_header__init_header_and_id(&se->event_id.header, &sample, + type, misc, size, event); ret = perf_output_begin(&handle, &sample, event, se->event_id.header.size); if (ret) @@ -10158,22 +10131,11 @@ static void perf_event_switch(struct task_struct *task, switch_event = (struct perf_switch_event){ .task = task, .next_prev = next_prev, - .event_id = { - .header = { - /* .type */ - .misc = sched_in ? 0 : PERF_RECORD_MISC_SWITCH_OUT, - /* .size */ - }, - /* .next_prev_pid */ - /* .next_prev_tid */ - }, + /* .event_id */ + .sched_in = sched_in, + .preempt = !sched_in && task_is_runnable(task), }; - if (!sched_in && task_is_runnable(task)) { - switch_event.event_id.header.misc |= - PERF_RECORD_MISC_SWITCH_OUT_PREEMPT; - } - perf_iterate_sb(perf_event_switch_output, &switch_event, NULL); } @@ -10193,20 +10155,17 @@ static void perf_log_throttle(struct perf_event *event, int enable) u64 id; u64 stream_id; } throttle_event = { - .header = { - .type = PERF_RECORD_THROTTLE, - .misc = 0, - .size = sizeof(throttle_event), - }, .time = perf_event_clock(event), .id = primary_event_id(event), .stream_id = event->id, }; - if (enable) - throttle_event.header.type = PERF_RECORD_UNTHROTTLE; - - perf_event_header__init_id(&throttle_event.header, &sample, event); + perf_event_header__init_header_and_id(&throttle_event.header, &sample, + enable ? PERF_RECORD_UNTHROTTLE + : PERF_RECORD_THROTTLE, + /*misc=*/0, + sizeof(throttle_event), + event); ret = perf_output_begin(&handle, &sample, event, throttle_event.header.size); @@ -10245,12 +10204,14 @@ static void perf_event_ksymbol_output(struct perf_event *event, void *data) struct perf_output_handle handle; struct perf_sample_data sample; int ret; + u16 size = sizeof(ksymbol_event->event_id) + ksymbol_event->name_len; if (!perf_event_ksymbol_match(event)) return; - perf_event_header__init_id(&ksymbol_event->event_id.header, - &sample, event); + perf_event_header__init_header_and_id(&ksymbol_event->event_id.header, &sample, + PERF_RECORD_KSYMBOL, /*misc=*/0, size, + event); ret = perf_output_begin(&handle, &sample, event, ksymbol_event->event_id.header.size); if (ret) @@ -10291,11 +10252,6 @@ void perf_event_ksymbol(u16 ksym_type, u64 addr, u32 len, bool unregister, .name = name, .name_len = name_len, .event_id = { - .header = { - .type = PERF_RECORD_KSYMBOL, - .size = sizeof(ksymbol_event.event_id) + - name_len, - }, .addr = addr, .len = len, .ksym_type = ksym_type, @@ -10339,8 +10295,11 @@ static void perf_event_bpf_output(struct perf_event *event, void *data) if (!perf_event_bpf_match(event)) return; - perf_event_header__init_id(&bpf_event->event_id.header, - &sample, event); + perf_event_header__init_header_and_id(&bpf_event->event_id.header, &sample, + PERF_RECORD_BPF_EVENT, + /*misc=*/0, + sizeof(bpf_event->event_id), + event); ret = perf_output_begin(&handle, &sample, event, bpf_event->event_id.header.size); if (ret) @@ -10396,10 +10355,6 @@ void perf_event_bpf_event(struct bpf_prog *prog, bpf_event = (struct perf_bpf_event){ .prog = prog, .event_id = { - .header = { - .type = PERF_RECORD_BPF_EVENT, - .size = sizeof(bpf_event.event_id), - }, .type = type, .flags = flags, .id = prog->aux->id, @@ -10427,18 +10382,23 @@ static void perf_callchain_deferred_output(struct perf_event *event, void *data) struct perf_callchain_deferred_event *deferred_event = data; struct perf_output_handle handle; struct perf_sample_data sample; - int ret, size = deferred_event->event.header.size; + int ret; + u16 size = sizeof(deferred_event->event) + (deferred_event->trace->nr * sizeof(u64)); if (!event->attr.defer_output) return; /* XXX do we really need sample_id_all for this ??? */ - perf_event_header__init_id(&deferred_event->event.header, &sample, event); + perf_event_header__init_header_and_id(&deferred_event->event.header, &sample, + PERF_RECORD_CALLCHAIN_DEFERRED, + PERF_RECORD_MISC_USER, + size, + event); ret = perf_output_begin(&handle, &sample, event, deferred_event->event.header.size); if (ret) - goto out; + return; perf_output_put(&handle, deferred_event->event); for (int i = 0; i < deferred_event->trace->nr; i++) { @@ -10448,8 +10408,6 @@ static void perf_callchain_deferred_output(struct perf_event *event, void *data) perf_event__output_id_sample(event, &handle, &sample); perf_output_end(&handle); -out: - deferred_event->event.header.size = size; } static void perf_unwind_deferred_callback(struct unwind_work *work, @@ -10458,12 +10416,6 @@ static void perf_unwind_deferred_callback(struct unwind_work *work, struct perf_callchain_deferred_event deferred_event = { .trace = trace, .event = { - .header = { - .type = PERF_RECORD_CALLCHAIN_DEFERRED, - .misc = PERF_RECORD_MISC_USER, - .size = sizeof(deferred_event.event) + - (trace->nr * sizeof(u64)), - }, .cookie = cookie, .nr = trace->nr, }, @@ -10475,7 +10427,8 @@ static void perf_unwind_deferred_callback(struct unwind_work *work, struct perf_text_poke_event { const void *old_bytes; const void *new_bytes; - size_t pad; + u16 tot; + u16 pad; u16 old_len; u16 new_len; @@ -10496,13 +10449,18 @@ static void perf_event_text_poke_output(struct perf_event *event, void *data) struct perf_text_poke_event *text_poke_event = data; struct perf_output_handle handle; struct perf_sample_data sample; + u16 size = sizeof(text_poke_event->event_id) + text_poke_event->tot + text_poke_event->pad; u64 padding = 0; int ret; if (!perf_event_text_poke_match(event)) return; - perf_event_header__init_id(&text_poke_event->event_id.header, &sample, event); + perf_event_header__init_header_and_id(&text_poke_event->event_id.header, &sample, + PERF_RECORD_TEXT_POKE, + PERF_RECORD_MISC_KERNEL, + size, + event); ret = perf_output_begin(&handle, &sample, event, text_poke_event->event_id.header.size); @@ -10540,15 +10498,11 @@ void perf_event_text_poke(const void *addr, const void *old_bytes, text_poke_event = (struct perf_text_poke_event){ .old_bytes = old_bytes, .new_bytes = new_bytes, + .tot = tot, .pad = pad, .old_len = old_len, .new_len = new_len, .event_id = { - .header = { - .type = PERF_RECORD_TEXT_POKE, - .misc = PERF_RECORD_MISC_KERNEL, - .size = sizeof(text_poke_event.event_id) + tot + pad, - }, .addr = (unsigned long)addr, }, }; @@ -10579,13 +10533,12 @@ static void perf_log_itrace_start(struct perf_event *event) event->attach_state & PERF_ATTACH_ITRACE) return; - rec.header.type = PERF_RECORD_ITRACE_START; - rec.header.misc = 0; - rec.header.size = sizeof(rec); rec.pid = perf_event_pid(event, current); rec.tid = perf_event_tid(event, current); - perf_event_header__init_id(&rec.header, &sample, event); + perf_event_header__init_header_and_id(&rec.header, &sample, + PERF_RECORD_ITRACE_START, /*misc=*/0, sizeof(rec), + event); ret = perf_output_begin(&handle, &sample, event, rec.header.size); if (ret) @@ -10610,12 +10563,10 @@ void perf_report_aux_output_id(struct perf_event *event, u64 hw_id) if (event->parent) event = event->parent; - rec.header.type = PERF_RECORD_AUX_OUTPUT_HW_ID; - rec.header.misc = 0; - rec.header.size = sizeof(rec); - rec.hw_id = hw_id; - - perf_event_header__init_id(&rec.header, &sample, event); + rec.hw_id = hw_id; + perf_event_header__init_header_and_id(&rec.header, &sample, + PERF_RECORD_AUX_OUTPUT_HW_ID, /*misc=*/0, sizeof(rec), + event); ret = perf_output_begin(&handle, &sample, event, rec.header.size); if (ret) diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c index 1b1ffe0533e5..90fae9c41b9b 100644 --- a/kernel/events/ring_buffer.c +++ b/kernel/events/ring_buffer.c @@ -246,14 +246,13 @@ __perf_output_begin(struct perf_output_handle *handle, handle->size = (1UL << page_shift) - offset; if (unlikely(have_lost)) { - lost_event.header.size = sizeof(lost_event); - lost_event.header.type = PERF_RECORD_LOST; - lost_event.header.misc = 0; lost_event.id = event->id; lost_event.lost = local_xchg(&rb->lost, 0); /* XXX mostly redundant; @data is already fully initializes */ - perf_event_header__init_id(&lost_event.header, data, event); + perf_event_header__init_header_and_id(&lost_event.header, data, + PERF_RECORD_LOST, /*misc=*/0, + sizeof(lost_event), event); perf_output_put(handle, lost_event); perf_event__output_id_sample(event, handle, data); } -- 2.56.0.rc1.315.gc6ed9934b7-goog ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1] perf/core: Restore header fields in sideband output callbacks 2026-09-29 1:42 [PATCH v1] perf/core: Restore header fields in sideband output callbacks Ian Rogers 2026-09-29 12:04 ` Peter Zijlstra @ 2026-09-29 23:37 ` bot+bpf-ci 1 sibling, 0 replies; 5+ messages in thread From: bot+bpf-ci @ 2026-09-29 23:37 UTC (permalink / raw) To: irogers, peterz, mingo, acme, namhyung Cc: mark.rutland, alexander.shishkin, jolsa, irogers, adrian.hunter, james.clark, song, ast, linux-perf-users, linux-kernel, bpf, stable, ast, andrii, daniel, martin.lau, eddyz87, yonghong.song, mason, ihor.solodrai [-- Attachment #1: Type: text/plain, Size: 5160 bytes --] > diff --git a/kernel/events/core.c b/kernel/events/core.c > index db7b76d6b68a..2be915a293ce 100644 > --- a/kernel/events/core.c > +++ b/kernel/events/core.c > @@ -9032,6 +9032,11 @@ static void perf_iterate_sb_cpu(perf_iterate_f output, void *data) > * > * For new callers; ensure that account_pmu_sb_event() includes > * your event, otherwise it might not get delivered. > + * > + * Note: @data is shared across all @output calls, so any fields modified > + * incrementally or conditionally (e.g. header.size via > + * perf_event_header__init_id()) must be saved and restored by @output, > + * or unconditionally re-initialized on each call. > */ > static void > perf_iterate_sb(perf_iterate_f output, void *data, Peter Zijlstra identified that perf_event_switch_output() also modifies header.type and header.size before calling perf_event_header__init_id() and suggested it might need similar treatment. Looking at perf_event_switch_output(): if (event->ctx->task) { se->event_id.header.type = PERF_RECORD_SWITCH; se->event_id.header.size = sizeof(se->event_id.header); } else { se->event_id.header.type = PERF_RECORD_SWITCH_CPU_WIDE; se->event_id.header.size = sizeof(se->event_id); ... } perf_event_header__init_id(&se->event_id.header, &sample, event); The new comment mentions that fields must be "saved and restored by @output, or unconditionally re-initialized on each call." perf_event_switch_output() does unconditionally reinitialize both header.type and header.size at the start of each callback invocation, which should prevent the accumulation bug. Is this analysis correct, or does perf_event_switch_output() need the save/restore pattern as well? > @@ -9726,6 +9731,7 @@ static void perf_event_mmap_output(struct perf_event *event, > struct perf_sample_data sample; > int size = mmap_event->event_id.header.size; > u32 type = mmap_event->event_id.header.type; > + u16 misc = mmap_event->event_id.header.misc; > bool use_build_id; > int ret; > [ ... ] > @@ -9783,6 +9789,7 @@ static void perf_event_mmap_output(struct perf_event *event, > out: > mmap_event->event_id.header.size = size; > mmap_event->event_id.header.type = type; > + mmap_event->event_id.header.misc = misc; > } [ ... ] > @@ -10247,6 +10254,7 @@ static void perf_event_ksymbol_output(struct perf_event *event, void *data) > struct perf_ksymbol_event *ksymbol_event = data; > struct perf_output_handle handle; > struct perf_sample_data sample; > + u16 header_size = ksymbol_event->event_id.header.size; > int ret; > > if (!perf_event_ksymbol_match(event)) > @@ -10257,13 +10265,15 @@ static void perf_event_ksymbol_output(struct perf_event *event, void *data) > ret = perf_output_begin(&handle, &sample, event, > ksymbol_event->event_id.header.size); > if (ret) > - return; > + goto out; > > perf_output_put(&handle, ksymbol_event->event_id); > __output_copy(&handle, ksymbol_event->name, ksymbol_event->name_len); > perf_event__output_id_sample(event, &handle, &sample); > > perf_output_end(&handle); > +out: > + ksymbol_event->event_id.header.size = header_size; > } [ ... ] > @@ -10337,6 +10347,7 @@ static void perf_event_bpf_output(struct perf_event *event, void *data) > struct perf_bpf_event *bpf_event = data; > struct perf_output_handle handle; > struct perf_sample_data sample; > + u16 header_size = bpf_event->event_id.header.size; > int ret; > > if (!perf_event_bpf_match(event)) > @@ -10347,12 +10358,14 @@ static void perf_event_bpf_output(struct perf_event *event, void *data) > ret = perf_output_begin(&handle, &sample, event, > bpf_event->event_id.header.size); > if (ret) > - return; > + goto out; > > perf_output_put(&handle, bpf_event->event_id); > perf_event__output_id_sample(event, &handle, &sample); > > perf_output_end(&handle); > +out: > + bpf_event->event_id.header.size = header_size; > } [ ... ] > @@ -10499,6 +10512,7 @@ static void perf_event_text_poke_output(struct perf_event *event, void *data) > struct perf_text_poke_event *text_poke_event = data; > struct perf_output_handle handle; > struct perf_sample_data sample; > + u16 header_size = text_poke_event->event_id.header.size; > u64 padding = 0; > int ret; > > @@ -10510,7 +10524,7 @@ static void perf_event_text_poke_output(struct perf_event *event, void *data) > ret = perf_output_begin(&handle, &sample, event, > text_poke_event->event_id.header.size); > if (ret) > - return; > + goto out; > > perf_output_put(&handle, text_poke_event->event_id); > perf_output_put(&handle, text_poke_event->old_len); > @@ -10525,6 +10539,8 @@ static void perf_event_text_poke_output(struct perf_event *event, void *data) > perf_event__output_id_sample(event, &handle, &sample); > > perf_output_end(&handle); > +out: > + text_poke_event->event_id.header.size = header_size; > } --- AI reviewed your patch. Please fix the bug or email reply why it's not a bug. See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md CI run summary: https://github.com/kernel-patches/bpf/actions/runs/36643934404 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-09-29 23:37 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-09-29 1:42 [PATCH v1] perf/core: Restore header fields in sideband output callbacks Ian Rogers 2026-09-29 12:04 ` Peter Zijlstra 2026-09-29 18:21 ` Ian Rogers 2026-09-29 22:23 ` [PATCH v1] perf/core: Replace perf_event_header__init_id with full header init Ian Rogers 2026-09-29 23:37 ` [PATCH v1] perf/core: Restore header fields in sideband output callbacks bot+bpf-ci
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®