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@xxxxxxxxxxxxxxx> know about it. >From 8fdc65391c6ad16461526a685f03262b3b01bfde Mon Sep 17 00:00:00 2001 From: Peter Zijlstra <peterz@xxxxxxxxxxxxx> Date: Tue, 29 Mar 2016 09:26:44 +0200 Subject: perf/core: Fix time tracking bug with multiplexing From: Peter Zijlstra <peterz@xxxxxxxxxxxxx> 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@xxxxxxxxxx> Tested-by: Stephane Eranian <eranian@xxxxxxxxxx> Signed-off-by: Peter Zijlstra (Intel) <peterz@xxxxxxxxxxxxx> Cc: Alexander Shishkin <alexander.shishkin@xxxxxxxxxxxxxxx> Cc: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx> Cc: Jiri Olsa <jolsa@xxxxxxxxxx> Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx> Cc: Vince Weaver <vincent.weaver@xxxxxxxxx> Cc: kan.liang@xxxxxxxxx Cc: namhyung@xxxxxxxxxx Fixes: 3cbaa5906967 ("perf: Fix ctx time tracking by introducing EVENT_TIME") Link: http://lkml.kernel.org/r/20160329072644.GB3408@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx Signed-off-by: Ingo Molnar <mingo@xxxxxxxxxx> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> --- 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@xxxxxxxxxxxxx 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 -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html