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

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

[gregkh]

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 <stable@vger.kernel.org> know about it.


>From 8fdc65391c6ad16461526a685f03262b3b01bfde Mon Sep 17 00:00:00 2001
From: Peter Zijlstra <peterz@infradead.org>
Date: Tue, 29 Mar 2016 09:26:44 +0200
Subject: perf/core: Fix time tracking bug with multiplexing

From: Peter Zijlstra <peterz@infradead.org>

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:

> 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
>
> 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 <eranian@google.com>
Tested-by: Stephane Eranian <eranian@google.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vince Weaver <vincent.weaver@maine.edu>
Cc: kan.liang@intel.com
Cc: namhyung@kernel.org
Fixes: 3cbaa5906967 ("perf: Fix ctx time tracking by introducing EVENT_TIME")
Link: http://lkml.kernel.org/r/20160329072644.GB3408@twins.programming.kicks-ass.net
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 kernel/events/core.c |   14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -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 peterz@infradead.org 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