Re: [PATCH v6 2/3] perf/core: Run sched_task() for PMUs with only CPU-wide events

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: [email protected]
> Acked-by: Usama Arif <[email protected]>
> Signed-off-by: Puranjay Mohan <[email protected]>
> ---
>  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
>