Re: [PATCH v7 1/3] perf/core: Fix NULL pmu_ctx passed to pmu->sched_task()

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Per-CPU PMUs missing `sched_task` callbacks during context switches if a task context exists.
--

--- Patch [1]: [PATCH v7 1/3] perf/core: Fix NULL pmu_ctx passed to pmu->sched_task() ---
commit 333df3e02c2d6b54e5bd528f91779820677d118e
Author: Puranjay Mohan <[email protected]>

perf/core: Fix NULL pmu_ctx passed to pmu->sched_task()

perf_pmu_sched_task() returns early when cpuctx->task_ctx is set, and
cpc->task_epc is only non-NULL while a task context is scheduled in on
this CPU. This patch passes &cpc->epc instead, which is the CPU-wide
context for this PMU, avoiding a NULL pointer dereference.

> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index ba5bd6a78fe7b..c372213ffc883 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -3907,7 +3907,7 @@ static void __perf_pmu_sched_task(struct perf_cpu_pmu_context *cpc,
>  	perf_ctx_lock(cpuctx, cpuctx->task_ctx);
>  	perf_pmu_disable(pmu);
>  
> -	pmu->sched_task(cpc->task_epc, task, sched_in);
> +	pmu->sched_task(&cpc->epc, task, sched_in);

[Severity: High]
This isn't a bug introduced by this patch, but does the early return in
perf_pmu_sched_task() skip callbacks for CPU-wide PMUs?

If a system-wide PMU event is active (sched_cb_usage > 0), and a context
switch occurs to or from a task with an active perf event (meaning
cpuctx->task_ctx is set), perf_pmu_sched_task() returns early:

kernel/events/core.c:perf_pmu_sched_task() {
	...
	/* cpuctx->task_ctx will be handled in perf_event_context_sched_in/out */
	if (prev == next || cpuctx->task_ctx)
		return;
	...
}

It seems to rely on perf_event_context_sched_out/in() to handle the
callbacks. However, perf_ctx_sched_task_cb() only iterates over the task's
pmu_ctx_list:

kernel/events/core.c:perf_ctx_sched_task_cb() {
	...
	list_for_each_entry(pmu_ctx, &ctx->pmu_ctx_list, pmu_ctx_entry) {
		cpc = this_cpc(pmu_ctx->pmu);

		if (cpc->sched_cb_usage && pmu_ctx->pmu->sched_task)
			pmu_ctx->pmu->sched_task(pmu_ctx, task, sched_in);
	}
	...
}

If the task's perf event is for a different PMU (like a software event), the
CPU-wide PMU won't be in ctx->pmu_ctx_list. Does this mean the CPU-wide
PMU's sched_task() callback is completely skipped during the context switch,
potentially corrupting hardware tracing or leaking branch records across tasks?

>  
>  	perf_pmu_enable(pmu);
>  	perf_ctx_unlock(cpuctx, cpuctx->task_ctx);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1