[PATCH v6 2/3] perf/core: Run sched_task() for PMUs with only CPU-wide events
Peter Zijlstra
peterz at infradead.org
Fri Aug 7 03:08:28 PDT 2026
On Thu, Aug 06, 2026 at 06:52:22AM -0700, Puranjay Mohan wrote:
> perf_pmu_sched_task() returns early when cpuctx->task_ctx is set and
> leaves the work to perf_ctx_sched_task_cb(). That one only walks
> ctx->pmu_ctx_list, so a PMU whose events are all CPU-wide is never
> visited and its sched_task() callback does not run. With
>
> perf record -b -e cycles -a -- ls
>
> armv8pmu_sched_task() is skipped on every switch to a task that has a
> perf event of its own, and BRBE records leak across the task boundary.
> intel_pmu_lbr_add() calls perf_sched_cb_inc() unconditionally as well,
> so LBR records leak the same way on x86.
>
> Drop the early return and instead skip the individual CPCs that
> perf_ctx_sched_task_cb() already handles.
>
> That requires the two to agree on which CPC belongs to which path, and
> they do not. perf_ctx_sched_task_cb() gates on cpc->sched_cb_usage,
> which perf_sched_cb_inc() sets per CPU for every branch stack user,
> while the new gate uses cpc->task_epc, which __link_epc() sets only on
> the CPU the task context is scheduled in on. A task with an event for
> that PMU pinned to another CPU has an epc on ctx->pmu_ctx_list while
> cpc->task_epc is NULL, so both paths would run and sched_task() would be
> called twice per context switch. On x86 the second
> __intel_pmu_lbr_restore() finds lbr_stack_state == LBR_NONE and calls
> intel_pmu_lbr_reset(), throwing away the callstack the first one
> restored. So gate perf_ctx_sched_task_cb() on cpc->task_epc too.
>
> For the CPCs that perf_pmu_sched_task() now handles, the callback no
> longer runs inside the perf_ctx_disable() and perf_ctx_enable() pair in
> perf_event_context_sched_in(). __perf_pmu_sched_task() disables the PMU
> around the call itself, so the callback still runs with it disabled.
>
> Fixes: bd2756811766 ("perf: Rewrite core context handling")
> Cc: stable at vger.kernel.org
> Acked-by: Usama Arif <usama.arif at linux.dev>
> Signed-off-by: Puranjay Mohan <puranjay at kernel.org>
> ---
> kernel/events/core.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 9815894b67e77..675dd05935f35 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -3757,6 +3757,9 @@ static void perf_ctx_sched_task_cb(struct perf_event_context *ctx,
> list_for_each_entry(pmu_ctx, &ctx->pmu_ctx_list, pmu_ctx_entry) {
> cpc = this_cpc(pmu_ctx->pmu);
>
> + if (cpc->task_epc != pmu_ctx)
> + continue;
Why isn't this the inverse condition of the below? That is, we should
call either this or the other, right?
> +
> if (cpc->sched_cb_usage && pmu_ctx->pmu->sched_task)
> pmu_ctx->pmu->sched_task(pmu_ctx, task, sched_in);
> }
> @@ -3921,12 +3924,15 @@ static void perf_pmu_sched_task(struct task_struct *prev,
> struct perf_cpu_context *cpuctx = this_cpu_ptr(&perf_cpu_context);
> struct perf_cpu_pmu_context *cpc;
>
> - /* cpuctx->task_ctx will be handled in perf_event_context_sched_in/out */
> - if (prev == next || cpuctx->task_ctx)
> + if (prev == next)
> return;
>
> - list_for_each_entry(cpc, this_cpu_ptr(&sched_cb_list), sched_cb_entry)
> + list_for_each_entry(cpc, this_cpu_ptr(&sched_cb_list), sched_cb_entry) {
> + if (cpuctx->task_ctx && cpc->task_epc)
I'm not sure I see the need for the first part of that clause; why
isn't: 'cpuc->task_epc' sufficient?
> + continue;
> +
> __perf_pmu_sched_task(cpc, sched_in ? next : prev, sched_in);
> + }
> }
>
> static void perf_event_switch(struct task_struct *task,
> --
> 2.53.0-Meta
>
More information about the linux-arm-kernel
mailing list