On 2/08/24 12:25, Peter Zijlstra wrote:
> On Thu, Aug 01, 2024 at 02:13:41PM -0700, Andrii Nakryiko wrote:
>
>> Ok, this bisected to:
>>
>> 675ad74989c2 ("perf/core: Add aux_pause, aux_resume, aux_start_paused")
>
> Adrian, there are at least two obvious bugs there:
>
> - aux_action was key's off of PERF_PMU_CAP_AUX_OUTPUT, which is not
> right, that's the capability where events can output to AUX -- aka.
> PEBS-to-PT. It should be PERF_PMU_CAP_ITRACE, which is the
> PT/CoreSight thing.
>
> - it sets aux_paused unconditionally, which is scribbling in the giant
> union which is overwriting state set by perf_init_event().
>
> But I think there's more problems, we need to do the aux_action
> validation after perf_get_aux_event(), we can't know if having those
> bits set makes sense before that. This means the perf_event_alloc() site
> is wrong in the first place.
>
> I'm going to drop these patches for now. Please rework.
Yes I will do that.
FWIW, I'd expect the reported issue would go away with just:
diff --git a/kernel/events/core.c b/kernel/events/core.c
index e4cb6e5a5f40..2072aaa4d449 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -12151,7 +12151,8 @@ perf_event_alloc(struct perf_event_attr *attr, int cpu,
err = -EOPNOTSUPP;
goto err_pmu;
}
- event->hw.aux_paused = event->attr.aux_start_paused;
+ if (event->attr.aux_start_paused)
+ event->hw.aux_paused = 1;
if (cgroup_fd != -1) {
err = perf_cgroup_connect(cgroup_fd, event, attr, group_leader);
