All of lore.kernel.org
 help / color / mirror / Atom feed
From: <gregkh@linuxfoundation.org>
To: peterz@infradead.org, acme@redhat.com,
	alexander.shishkin@linux.intel.com, eranian@google.com,
	gregkh@linuxfoundation.org, jolsa@redhat.com, mingo@kernel.org,
	tglx@linutronix.de, torvalds@linux-foundation.org,
	vincent.weaver@maine.edu
Cc: <stable@vger.kernel.org>, <stable-commits@vger.kernel.org>
Subject: Patch "perf/core: Fix time tracking bug with multiplexing" has been added to the 4.5-stable tree
Date: Mon, 02 May 2016 16:31:29 -0700	[thread overview]
Message-ID: <146223188914764@kroah.com> (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 <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

                 reply	other threads:[~2016-05-02 23:31 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=146223188914764@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=acme@redhat.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=eranian@google.com \
    --cc=jolsa@redhat.com \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=stable-commits@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=vincent.weaver@maine.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.