Skip to contentSkip to conversation

Patch "perf/core: Fix time tracking bug with multiplexing" has been added to the 4.5-stable tree

PATCH [email protected] 1 participant 1 message May 2, 2016

Subsystems (2)
Stable branch perf events
lore
Share & tools
lore

[email protected]

author maintainer Applied
#0 lore raw

This is a note to let you know that I've just added the patch titled

perf/core: Fix time tracking bug with multiplexing

to the 4.5-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
perf-core-fix-time-tracking-bug-with-multiplexing.patch
and it can be found in the queue-4.5 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From 8fdc65391c6ad16461526a685f03262b3b01bfde Mon Sep 17 00:00:00 2001

From: Peter Zijlstra <[email protected]>
Date: Tue, 29 Mar 2016 09:26:44 +0200
Subject: perf/core: Fix time tracking bug with multiplexing

From: Peter Zijlstra <[email protected]>

commit 8fdc65391c6ad16461526a685f03262b3b01bfde upstream.

Stephane reported that commit:

3cbaa5906967 ("perf: Fix ctx time tracking by introducing EVENT_TIME")

introduced a regression wrt. time tracking, as easily observed by:

Quoted 34 lines

This patch introduce a bug in the time tracking of events when multiplexing is used.

The issue is easily reproducible with the following perf run:

 $ perf stat -a -C 0 -e branches,branches,branches,branches,branches,branches -I 1000
     1.000730239            652,394      branches   (66.41%)
     1.000730239            597,809      branches   (66.41%)
     1.000730239            593,870      branches   (66.63%)
     1.000730239            651,440      branches   (67.03%)
     1.000730239            656,725      branches   (66.96%)
     1.000730239      <not counted>      branches

One branches event is shown as not having run. Yet, with multiplexing, all events should run especially with a 1s (-I 1000) interval. The delta for time_running comes out to 0. Yet, the event has run because the kernel is actually multiplexing the events. The problem is that the time tracking is the kernel and especially in ctx_sched_out() is wrong now.

The problem is that in case that the kernel enters ctx_sched_out() with the following state:

   ctx->is_active=0x7 event_type=0x1
   Call Trace:
    [<ffffffff813ddd41>] dump_stack+0x63/0x82
    [<ffffffff81182bdc>] ctx_sched_out+0x2bc/0x2d0
    [<ffffffff81183896>] perf_mux_hrtimer_handler+0xf6/0x2c0
    [<ffffffff811837a0>] ? __perf_install_in_context+0x130/0x130
    [<ffffffff810f5818>] __hrtimer_run_queues+0xf8/0x2f0
    [<ffffffff810f6097>] hrtimer_interrupt+0xb7/0x1d0
    [<ffffffff810509a8>] local_apic_timer_interrupt+0x38/0x60
    [<ffffffff8175ca9d>] smp_apic_timer_interrupt+0x3d/0x50
    [<ffffffff8175ac7c>] apic_timer_interrupt+0x8c/0xa0
Quoted 5 lines

In that case, the test:
if (is_active & EVENT_TIME)

will be false and the time will not be updated. Time must always be updated on sched out.

Fix this by always updating time if EVENT_TIME was set, as opposed to only updating time when EVENT_TIME changed.

Reported-by Stephane Eranian <[email protected]>
Tested-by Stephane Eranian <[email protected]>
Signed-off-by Peter Zijlstra (Intel) <[email protected]>
Cc Alexander Shishkin <[email protected]>
Cc Arnaldo Carvalho de Melo <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Linus Torvalds <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Vince Weaver <[email protected]>
Cc [email protected]
Cc [email protected]
Fixes 3cbaa5906967 ("perf: Fix ctx time tracking by introducing EVENT_TIME")
Link http://lkml.kernel.org/r/[email protected] open here
Signed-off-by Ingo Molnar <[email protected]>
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 kernel/events/core.c |   14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)
kernel/events/core.c +12 -2
@@ -2402,14 +2402,24 @@ static void ctx_sched_out(struct perf_ev
 			cpuctx->task_ctx = NULL;
 	}
 
-	is_active ^= ctx->is_active; /* changed bits */
-
+	/*
+	 * Always update time if it was set; not only when it changes.
+	 * Otherwise we can 'forget' to update time for any but the last
+	 * context we sched out. For example:
+	 *
+	 *   ctx_sched_out(.event_type = EVENT_FLEXIBLE)
+	 *   ctx_sched_out(.event_type = EVENT_PINNED)
+	 *
+	 * would only update time for the pinned events.
+	 */
 	if (is_active & EVENT_TIME) {
 		/* update (and stop) ctx time */
 		update_context_time(ctx);
 		update_cgrp_time_from_cpuctx(cpuctx);
 	}
 
+	is_active ^= ctx->is_active; /* changed bits */
+
 	if (!ctx->nr_active || !(is_active & EVENT_ALL))
 		return;

Patches currently in stable-queue which might be from [email protected] are

queue-4.5/x86-edac-sb_edac.c-repair-damage-introduced-when-fixing-channel-address.patch queue-4.5/efi-expose-non-blocking-set_variable-wrapper-to-efivars.patch queue-4.5/sched-cgroup-fix-cleanup-cgroup-teardown-init.patch queue-4.5/asm-generic-futex-re-enable-preemption-in-futex_atomic_cmpxchg_inatomic.patch queue-4.5/x86-mm-kmmio-fix-mmiotrace-for-hugepages.patch queue-4.5/perf-hists-browser-fix-dump-to-show-correct-callchain-style.patch queue-4.5/perf-hists-browser-only-offer-symbol-scripting-when-a-symbol-is-under-the-cursor.patch queue-4.5/perf-tools-fix-perf-script-python-database-export-crash.patch queue-4.5/perf-core-fix-time-tracking-bug-with-multiplexing.patch queue-4.5/x86-mce-avoid-using-object-after-free-in-genpool.patch queue-4.5/x86-edac-sb_edac.c-take-account-of-channel-hashing-when-needed.patch queue-4.5/futex-handle-unlock_pi-race-gracefully.patch queue-4.5/perf-stat-document-detailed-option.patch queue-4.5/perf-core-don-t-leak-event-in-the-syscall-error-path.patch queue-4.5/locking-mcs-fix-mcs_spin_lock-ordering.patch queue-4.5/futex-acknowledge-a-new-waiter-in-counter-before-plist.patch queue-4.5/x86-mm-xen-suppress-hugetlbfs-in-pv-guests.patch