Re: [PATCH] perf/core: Do not resurrect cpuctx->cgrp after last cgroup event exit

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] perf/core: perf_cgroup_switch() skips pmu_ctx after last cgroup
  event disable
--

commit 16552da1b5b468a49aa24ce4ee62d6e477839323
Author: Phat Dinh <phatdt1116@gmail.com>

perf/core: Do not resurrect cpuctx->cgrp after last cgroup event exit

This commit avoids setting cpuctx->cgrp in perf_cgroup_switch() if
ctx_sched_out() disabled the last cgroup event on the CPU context.

> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index f36a3bbaa33cf..54b87584c752e 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -1072,8 +1072,13 @@ static void perf_cgroup_switch(struct task_struct *task)
>  	 * must not be done before ctxswout due
>  	 * to update_cgrp_time_from_cpuctx() in
>  	 * ctx_sched_out()
> +	 *
> +	 * ctx_sched_out() can consume a pending disable and drop the last cgroup
> +	 * event, which clear the cpuctx->cgrp. Cgroup tracking follow active cgroup
> +	 * event, so do not resurrect it
>  	 */
> -	cpuctx->cgrp = cgrp;
> +	if (cpuctx->ctx.nr_cgroups)
> +		cpuctx->cgrp = cgrp;
>  	/*
>  	 * set cgrp before ctxsw in to allow
>  	 * perf_cgroup_set_timestamp() in ctx_sched_in()

[Severity: High]
Does perf_cgroup_switch() still leak pmu_disable_count and leave
non-cgroup per-CPU events unscheduled when ctx_sched_out() consumes a
pending_disable on the last cgroup event of a pmu_ctx?

Before ctx_sched_out(), perf_ctx_disable(&cpuctx->ctx, EVENT_CGROUP)
calls perf_pmu_disable(pmu_ctx->pmu) for each pmu_ctx with
pmu_ctx->nr_cgroups > 0, and ctx_sched_out() schedules out all events on
that pmu_ctx.  When event_sched_out() consumes event->pending_disable on
the last cgroup event of that pmu_ctx, perf_cgroup_event_disable()
decrements event->pmu_ctx->nr_cgroups to 0 alongside ctx->nr_cgroups.

The subsequent ctx_sched_in() and perf_ctx_enable() calls pass
EVENT_CGROUP to for_each_epc(), which skips the pmu_ctx:

kernel/events/core.c:perf_skip_pmu_ctx() {
	if ((event_type & EVENT_CGROUP) && !pmu_ctx->nr_cgroups)
		return true;
	...
}

As a result, perf_ctx_enable() never calls perf_pmu_enable(pmu_ctx->pmu),
leaving pmu_disable_count elevated and the PMU permanently disabled on
that CPU.  ctx_sched_in() also skips __pmu_ctx_sched_in() for that
pmu_ctx while setting cpuctx->ctx.is_active back to EVENT_ALL |
EVENT_TIME, leaving any non-cgroup per-CPU events on that PMU stuck in
PERF_EVENT_STATE_INACTIVE.  On x86 hardware PMUs, because x86_pmu_enable()
no longer runs to assign event->hw.idx, scheduling in and then scheduling
out a later hardware event calls x86_pmu_del() with event->hw.idx == -1,
writing out of bounds to cpuc->dirty and cpuc->events[-1].

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106101642.494674-1-phatdt1116@gmail.com?part=1