mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [REGRESSION] perf: Rewrite core context handling breaks multiplexed cgroup events
@ 2026-10-15  9:03 Antoine HURET
  2026-10-15 11:13 ` Peter Zijlstra
  0 siblings, 1 reply; 4+ messages in thread
From: Antoine HURET @ 2026-10-15  9:03 UTC (permalink / raw)
  To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim
  Cc: Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers,
	Adrian Hunter, James Clark, Ravi Bangoria, linux-perf-users,
	linux-kernel, regressions

Hi,

Since v6.2, cgroup events (perf_event_open() with PERF_FLAG_PID_CGROUP)
report a wrong time_running as soon as they are multiplexed. The raw
counts are fine, but the time_enabled / time_running ratio used to
scale them is not, so the scaled values are wrong: from a few percent
to several times the real value.

It is a regression from commit bd2756811766 ("perf: Rewrite core
context handling"), see the analysis below. The culprit was found by
code inspection, then confirmed by testing it and its parent
(v6.1-rc2). The code is unchanged in v7.3-rc7, tip perf/core and
linux-next.

All three conditions are needed:
  - cgroup events,
  - multiplexed (more events than hardware counters),
  - no cgroup switch on the CPU between two reads.

The last one is the common case of a busy job alone on its CPUs (use
case: HPC jobs).


Reproducer
==========

Needs root (or kernel.perf_event_paranoid <= 0), docker and an idle
CPU (CPU 2 below).

  # A busy loop pinned to CPU 2, in its own cgroup.
  docker run -d --rm --name burn --cpuset-cpus 2 busybox \
         sh -c 'while :; do :; done'
  # Process PID
  PID=$(docker inspect -f '{{.State.Pid}}' burn)

  # Cgroup relative path
  CG=$(cut -d: -f3 /proc/$PID/cgroup); CG=${CG#/}

  # 2 events to reproduce case without multiplexing
  EV2=cycles,instructions

  # 19 architectural events to reproduce case with multiplexing: the
  # repeated ones need more general-purpose counters than the PMU has.
  A=branches,branch-misses,cache-references,cache-misses
  EVMUX=cycles,instructions,ref-cycles,$A,$A,$A,$A

  Note: on a hybrid CPU, prefix every event with the core PMU
  (e.g. cpu_core/cycles/) and use a P-core.

  From the host, run the following commands:

  # 1. Per task, not multiplexed.
  perf stat -e $EV2 -p $PID -- sleep 1

  # 2. cgroup, not multiplexed.
  perf stat -a -C 2 -e $EV2 --for-each-cgroup $CG -- sleep 1

  # 3. Per task, multiplexed.
  perf stat -e $EVMUX -p $PID -- sleep 1

  # 4. cgroup, multiplexed.
  perf stat -a -C 2 -e $EVMUX --for-each-cgroup $CG -- sleep 1

  Focusing on the "cycles" counter is a good way to detect aberrant
  values as it shouldn't exceed the max frequency of the CPU, since the
  CPU is fully busy during the whole second of sleep.

  Run each command several times. 1, 2 and 3 always give the same
  kind of value, never above the max frequency. 4 changes from one
  run to the next, from half to several times the max frequency, that is
  impossible.


Results
=======

Intel Core Ultra 5 135U (Meteor Lake, P-cores up to 4.4 GHz), Ubuntu
24.04 userspace, vanilla kernels, not tainted. Cycles in 1 s, min - max
over 20 runs:

                         v7.3-rc7               v7.3-rc7 + fix below
  1. task, EV2           4.390 - 4.401 G (OK)   4.390 - 4.398 G (OK)
  2. cgroup, EV2         4.392 - 4.404 G (OK)   4.393 - 4.404 G (OK)
  3. task, EVMUX         4.372 - 4.382 G (OK)   4.367 - 4.382 G (OK)
  4. cgroup, EVMUX       2.397 - 8.797 G (KO)   4.371 - 4.386 G (OK)

The 20 runs of 4. on v7.3-rc7, in G:

  8.797  3.986  7.248  4.288  5.851  4.487  5.794  3.180  5.828  4.083
  2.397  3.186  5.187  3.872  6.058  4.300  2.970  3.644  3.933  6.896

Culprit check, case 4, same commands:

  v6.1-rc2 (parent)      4.374 - 4.399 G  (OK)
  bd2756811766           2.094 - 40.427 G (KO)


Analysis
========

A cgroup event takes its times from a per-cpu cgroup clock.
That clock only moves forward when update_cgrp_time_from_cpuctx() or
update_cgrp_time_from_event() is called: on a cgroup switch, on a read
of an active event, and in ctx_sched_out().

Up to v6.1, perf_rotate_context() went through ctx_sched_out(), which
updated the cgroup clock. Since bd2756811766, it only updates ctx->time
(update_context_time()) before __pmu_ctx_sched_out().

So between two cgroup switches, the rotations see a frozen cgroup
clock. The events rotated out lose the running time they accumulated,
and the next update (usually the read) credits the whole interval to
the events that happen to be on the PMU at that moment.

Supporting this: a task from another cgroup waking up every 0.5 ms on
the same CPU (each wake-up goes through perf_cgroup_switch()) makes
the values correct again.


Proposed fix
============

Update the cgroup clock in the rotation, as before v6.2:

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ static bool perf_rotate_context(struct perf_cpu_pmu_context *cpc)
 	if (cpu_event) {
 		update_context_time(&cpuctx->ctx);
+		update_cgrp_time_from_cpuctx(cpuctx, false);
 		__pmu_ctx_sched_out(cpu_epc, EVENT_FLEXIBLE);
 		rotate_ctx(&cpuctx->ctx, cpu_event);
 		__pmu_ctx_sched_in(cpu_epc, EVENT_FLEXIBLE);

Tested on v7.3-rc7 (results above). Also tested on the Ubuntu 6.8
kernel, where the values are correct for a parent cgroup, for two busy
cgroups sharing the CPU, and for idle or mostly sleeping cgroups.

I am not a perf core expert. The root cause analysis and the fix were
done with the help of an AI assistant. I reproduced the bug, then
built and tested the fix on my machines. Corrections or a better fix
are welcome, and I am happy to test them. If this is the right fix, I
can send it as a proper patch.

Assisted-by: Claude:claude-opus-5-5

#regzbot introduced: bd2756811766

Thanks,
Antoine Huret

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [REGRESSION] perf: Rewrite core context handling breaks multiplexed cgroup events
  2026-10-15  9:03 [REGRESSION] perf: Rewrite core context handling breaks multiplexed cgroup events Antoine HURET
@ 2026-10-15 11:13 ` Peter Zijlstra
  2026-10-15 12:30   ` Antoine HURET
  0 siblings, 1 reply; 4+ messages in thread
From: Peter Zijlstra @ 2026-10-15 11:13 UTC (permalink / raw)
  To: Antoine HURET
  Cc: Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers,
	Adrian Hunter, James Clark, Ravi Bangoria, linux-perf-users,
	linux-kernel, regressions

On Thu, Oct 15, 2026 at 09:03:42AM +0000, Antoine HURET wrote:

/me mutters something about having to go install and learn docker and
what's wrong with simple shells scripts that poke about in /cgroup
filesystems :-)

> Analysis
> ========
> 
> A cgroup event takes its times from a per-cpu cgroup clock.
> That clock only moves forward when update_cgrp_time_from_cpuctx() or
> update_cgrp_time_from_event() is called: on a cgroup switch, on a read
> of an active event, and in ctx_sched_out().
> 
> Up to v6.1, perf_rotate_context() went through ctx_sched_out(), which
> updated the cgroup clock. Since bd2756811766, it only updates ctx->time
> (update_context_time()) before __pmu_ctx_sched_out().
> 
> So between two cgroup switches, the rotations see a frozen cgroup
> clock. The events rotated out lose the running time they accumulated,
> and the next update (usually the read) credits the whole interval to
> the events that happen to be on the PMU at that moment.
> 
> Supporting this: a task from another cgroup waking up every 0.5 ms on
> the same CPU (each wake-up goes through perf_cgroup_switch()) makes
> the values correct again.
> 
> 
> Proposed fix
> ============
> 
> Update the cgroup clock in the rotation, as before v6.2:
> 
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ static bool perf_rotate_context(struct perf_cpu_pmu_context *cpc)
>  	if (cpu_event) {
>  		update_context_time(&cpuctx->ctx);
> +		update_cgrp_time_from_cpuctx(cpuctx, false);
>  		__pmu_ctx_sched_out(cpu_epc, EVENT_FLEXIBLE);
>  		rotate_ctx(&cpuctx->ctx, cpu_event);
>  		__pmu_ctx_sched_in(cpu_epc, EVENT_FLEXIBLE);
> 
> Tested on v7.3-rc7 (results above). Also tested on the Ubuntu 6.8
> kernel, where the values are correct for a parent cgroup, for two busy
> cgroups sharing the CPU, and for idle or mostly sleeping cgroups.
> 
> I am not a perf core expert. The root cause analysis and the fix were
> done with the help of an AI assistant. I reproduced the bug, then
> built and tested the fix on my machines. Corrections or a better fix
> are welcome, and I am happy to test them. If this is the right fix, I
> can send it as a proper patch.

Yes, I think this is right.

That said; while looking at this, I wonder if we should write this like
so instead. Although perhaps we should do this on top, your patch is
trivial, correct and more convenient for backport.



---
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 7846d70be57f..029c11e7358d 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -2535,8 +2535,10 @@ __ctx_time_update(struct perf_cpu_context *cpuctx, struct perf_event_context *ct
 			return;
 
 		update_context_time(ctx);
-		/* vPMU should not stop time */
-		update_cgrp_time_from_cpuctx(cpuctx, !(event_type & EVENT_GUEST) && final);
+		if (ctx == &cpuctx->ctx) {
+			/* vPMU should not stop time */
+			update_cgrp_time_from_cpuctx(cpuctx, !(event_type & EVENT_GUEST) && final);
+		}
 	}
 }
 
@@ -4633,12 +4635,12 @@ static bool perf_rotate_context(struct perf_cpu_pmu_context *cpc)
 	 * and then, if needed CPU flexible.
 	 */
 	if (task_event || (task_epc && cpu_event)) {
-		update_context_time(task_epc->ctx);
+		ctx_time_update(cpuctx, task_epc->ctx);
 		__pmu_ctx_sched_out(task_epc, EVENT_FLEXIBLE);
 	}
 
 	if (cpu_event) {
-		update_context_time(&cpuctx->ctx);
+		ctx_time_update(cpuctx, &cpuctx->ctx);
 		__pmu_ctx_sched_out(cpu_epc, EVENT_FLEXIBLE);
 		rotate_ctx(&cpuctx->ctx, cpu_event);
 		__pmu_ctx_sched_in(cpu_epc, EVENT_FLEXIBLE);

^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [REGRESSION] perf: Rewrite core context handling breaks multiplexed cgroup events
  2026-10-15 11:13 ` Peter Zijlstra
@ 2026-10-15 12:30   ` Antoine HURET
  2026-10-15 13:21     ` Peter Zijlstra
  0 siblings, 1 reply; 4+ messages in thread
From: Antoine HURET @ 2026-10-15 12:30 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers,
	Adrian Hunter, James Clark, Ravi Bangoria, linux-perf-users,
	linux-kernel, regressions

On Thu, Oct 15, 2026 at 11:13 UTC, Peter Zijlstra wrote:
> /me mutters something about having to go install and learn docker and
> what's wrong with simple shells scripts that poke about in /cgroup
> filesystems :-)

Fair enough, sorry about that! Next time I'll do plain cgroupfs,
promised :-)

> Yes, I think this is right.
>
> That said; while looking at this, I wonder if we should write this like
> so instead. Although perhaps we should do this on top, your patch is
> trivial, correct and more convenient for backport.

Thanks for the quick review ! I'll send my patch.

FYI, I also tested your variant on top of v7.3-rc7 with the same
reproducer and it fixes the issue too (case 4: 4.367 - 4.388 G over
20 runs).

Thanks,
Antoine

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [REGRESSION] perf: Rewrite core context handling breaks multiplexed cgroup events
  2026-10-15 12:30   ` Antoine HURET
@ 2026-10-15 13:21     ` Peter Zijlstra
  0 siblings, 0 replies; 4+ messages in thread
From: Peter Zijlstra @ 2026-10-15 13:21 UTC (permalink / raw)
  To: Antoine HURET
  Cc: Ingo Molnar, Arnaldo Carvalho de Melo, Namhyung Kim,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Ian Rogers,
	Adrian Hunter, James Clark, Ravi Bangoria, linux-perf-users,
	linux-kernel, regressions

On Thu, Oct 15, 2026 at 12:30:55PM +0000, Antoine HURET wrote:
> On Thu, Oct 15, 2026 at 11:13 UTC, Peter Zijlstra wrote:
> > /me mutters something about having to go install and learn docker and
> > what's wrong with simple shells scripts that poke about in /cgroup
> > filesystems :-)
> 
> Fair enough, sorry about that! Next time I'll do plain cgroupfs,
> promised :-)

FWIW, it might be useful to extend perf-test with a bunch of cgroup
tests - this one included. AFAICT we have nothing there atm.


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-10-15 13:21 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-15  9:03 [REGRESSION] perf: Rewrite core context handling breaks multiplexed cgroup events Antoine HURET
2026-10-15 11:13 ` Peter Zijlstra
2026-10-15 12:30   ` Antoine HURET
2026-10-15 13:21     ` Peter Zijlstra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®