perf_event: fix broken calc_timer_values()
PATCH 1/1 perf_event: fix broken calc_timer_values()
We detected a serious issue with PERF_SAMPLE_READ and timing information when events were being multiplexing.
Samples would have time_running > time_enabled. That was easy to reproduce with a libpfm4 example (ran 3 times to cause multiplexing on Core 2):
$ syst_smpl -e uops_retired:freq=1 & $ syst_smpl -e uops_retired:freq=1 & $ syst_smpl -e uops_retired:freq=1 & IIP:0x0000000040062d ... PERIOD:2355332948 ENA=40144625315 RUN=60014875184 syst_smpl: WARNING: time_running > time_enabled 63277537998 uops_retired:freq=1 , scaled
The bug was not present in kernel up to 3.0. It turns out the bug was introduced by the following commit:
commit c4794295917ebeda8013b6cb9c8d71ab4f74a1fa
Author: Eric B Munson <[email protected]> Date: Thu Jun 23 16:34:38 2011 -0400
events: Move lockless timer calculation into helper function
The parameters of the function got reversed yet the call sites were not updated to reflect the change. That lead to time_running and time_enabled being swapped. That had no effect when there was no multiplexing because in that case time_running = time_enabled but it would show up in any other scenario.
---
kernel/events/core.c +2 -2
@@ -3354,8 +3354,8 @@ static int perf_event_index(struct perf_event *event) }static void calc_timer_values (struct perf_event *event, -u64 *running, -u64 *enabled) +u64 *enabled, +u64 *running) {u64 now, ctx_time;
Peter Zijlstra
maintainerkernel/events/core.c static int perf_event_index(struct perf_event *event) · line 3361
Quoted 11 lines · Stephane Eranian
kernel/events/core.c +2
-2@@ -3354,8 +3354,8 @@ static int perf_event_index(struct perf_event *event) }static void calc_timer_values (struct perf_event *event, -u64 *running, -u64 *enabled) +u64 *enabled, +u64 *running) {u64 now, ctx_time;
Urgh, nice catch.. I'll queue it for perf/urgent and Cc stable for 3.0.x
Peter Zijlstra
maintainerQuoted 4 lines · Stephane Eranian
The bug was not present in kernel up to 3.0. It turns out the bug was introduced by the following commit:
commit
c4794295917ebeda8013b6cb9c8d71ab4f74a1fa
Ah, up to /and including/ 3.0, since:
git describe --contains c4794295917ebeda8013b6cb9c8d71ab4f74a1fa --match "v*" v3.1-rc1~299^2~32
So only perf/urgent is needed.