Re: [PATCH 1/1] perf/core: fix dangling cgroup pointer in cpuctx

[Leo Yan <leo.yan@arm.com>]

On Wed, Jun 04, 2025 at 04:16:40PM +0200, Peter Zijlstra wrote:

[...]

> It might be prudent to do something like so:

Thanks for the patch.

> +static void __event_disable(struct perf_event *event,
> +			    struct perf_event_context *ctx,
> +			    enum perf_event_state state)
> +{
> +	if (event == event->group_leader)
> +		group_sched_out(event, ctx);

I am a bit struggle for this code line. It disables all events in a
group, but only clear cgroup pointer for group leader but miss to clear
for sibling events.

If the cgroup pointer is only used for group leader, maybe we only
maintain (set and clear) the cgroup pointer for the leader?

Thanks,
Leo

> +	else
> +		event_sched_out(event, ctx);
> +
> +	perf_event_set_state(event, state);
> +	perf_cgroup_event_disable(event, ctx);
> +}
> +
>  /*
>   * Cross CPU call to disable a performance event
>   */
> @@ -2575,15 +2583,7 @@ static void __perf_event_disable(struct perf_event *event,
>  
>  	perf_pmu_disable(event->pmu_ctx->pmu);
>  	ctx_time_update_event(ctx, event);
> -
> -	if (event == event->group_leader)
> -		group_sched_out(event, ctx);
> -	else
> -		event_sched_out(event, ctx);
> -
> -	perf_event_set_state(event, PERF_EVENT_STATE_OFF);
> -	perf_cgroup_event_disable(event, ctx);
> -
> +	__event_disable(event, ctx, PERF_EVENT_STATE_OFF);
>  	perf_pmu_enable(event->pmu_ctx->pmu);
>  }
>