linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Stephane Eranian <eranian@google.com>
To: linux-kernel@vger.kernel.org
Cc: acme@redhat.com, peterz@infradead.org, mingo@elte.hu,
	ak@linux.intel.com, kan.liang@intel.com, jolsa@redhat.com,
	namhyung@kernel.org, vincent.weaver@maine.edu
Subject: [PATCH] perf/core: Fix time tracking bug with multiplexing
Date: Tue, 29 Mar 2016 03:33:23 +0200	[thread overview]
Message-ID: <1459215203-12885-1-git-send-email-eranian@google.com> (raw)

This patch fixes a bug introduced by:

commit 3cbaa59069677920186dcf502632ca1df4329f80
Author: Peter Zijlstra <peterz@infradead.org>
Date:   Wed Feb 24 18:45:47 2016 +0100

    perf: Fix ctx time tracking by introducing EVENT_TIME

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 -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. This patch fixes the problem.

Patch is relative to PeterZ queue.tip perf/core branch.

Signed-off-by: Stephane Eranian <eranian@google.com>
---
 kernel/events/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 339aa46..4ba41f6 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -2447,7 +2447,7 @@ static void ctx_sched_out(struct perf_event_context *ctx,
 
 	is_active ^= ctx->is_active; /* changed bits */
 
-	if (is_active & EVENT_TIME) {
+	if (ctx->is_active & EVENT_TIME) {
 		/* update (and stop) ctx time */
 		update_context_time(ctx);
 		update_cgrp_time_from_cpuctx(cpuctx);
-- 
2.5.0

             reply	other threads:[~2016-03-29  1:33 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-29  1:33 Stephane Eranian [this message]
2016-03-29  7:26 ` [PATCH] perf/core: Fix time tracking bug with multiplexing Peter Zijlstra
2016-03-29 15:43   ` Arnaldo Carvalho de Melo
2016-03-29 21:23   ` Stephane Eranian
2016-03-31  9:18   ` [tip:perf/core] " tip-bot for Peter Zijlstra

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=1459215203-12885-1-git-send-email-eranian@google.com \
    --to=eranian@google.com \
    --cc=acme@redhat.com \
    --cc=ak@linux.intel.com \
    --cc=jolsa@redhat.com \
    --cc=kan.liang@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).