Patchwork Use pmc_overflow() to detect rolled back events

login
register
mail settings
Submitter sukadev@linux.vnet.ibm.com
Date Aug. 8, 2012, 1:07 a.m.
Message ID <20120808010719.GA32730@us.ibm.com>
Download mbox | patch
Permalink /patch/175821/
State Accepted
Commit 813312110bede27bffd082c25cd31730bd567beb
Delegated to: Benjamin Herrenschmidt
Headers show

Comments

sukadev@linux.vnet.ibm.com - Aug. 8, 2012, 1:07 a.m.
From 21e9d1775f0c6f37a39e5d682ff74693fa9a4004 Mon Sep 17 00:00:00 2001
From: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Date: Tue, 7 Aug 2012 17:53:24 -0700
Subject: [PATCH] Use pmc_overflow to detect rolled back events.

For certain speculative events on Power7, 'perf stat' reports far higher
event count than 'perf record' for the same event.

As described in following commit, a performance monitor exception is raised
even when the the performance events are rolled back.

        commit 0837e3242c73566fc1c0196b4ec61779c25ffc93
        Author: Anton Blanchard <anton@samba.org>
        Date:   Wed Mar 9 14:38:42 2011 +1100

perf_event_interrupt() records an event only when an overflow occurs. But
this check for overflow is a simple 'if (val < 0)'.

Because the events are rolled back, this check for overflow fails and the
event is not recorded. perf_event_interrupt() later uses pmc_overflow() to
detect the overflow and resets the counters and the events are lost completely.

To properly detect the overflow of rolled back events, use pmc_overflow()
even when recording events.

To reproduce:
        $ cat strcpy.c
        #include <stdio.h>
        #include <string.h>
        main()
        {
                char buf[256];

                alarm(5);
                while(1)
                        strcpy(buf, "string1");
        }

        $ perf record -e r20014 ./strcpy
        $ perf report -n > report.1
        $ perf stat -e r20014 > report.2
        # Compare report.1 and report.2

Reported-by: Maynard Johnson <mpjohn@us.ibm.com>
Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
---
 arch/powerpc/perf/core-book3s.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
sukadev@linux.vnet.ibm.com - Sept. 21, 2012, 12:38 a.m.
Sukadev Bhattiprolu [sukadev@linux.vnet.ibm.com] wrote:
| 
| >From 21e9d1775f0c6f37a39e5d682ff74693fa9a4004 Mon Sep 17 00:00:00 2001
| From: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
| Date: Tue, 7 Aug 2012 17:53:24 -0700
| Subject: [PATCH] Use pmc_overflow to detect rolled back events.
| 
| For certain speculative events on Power7, 'perf stat' reports far higher
| event count than 'perf record' for the same event.
| 
| As described in following commit, a performance monitor exception is raised
| even when the the performance events are rolled back.
| 
|         commit 0837e3242c73566fc1c0196b4ec61779c25ffc93
|         Author: Anton Blanchard <anton@samba.org>
|         Date:   Wed Mar 9 14:38:42 2011 +1100
| 
| perf_event_interrupt() records an event only when an overflow occurs. But
| this check for overflow is a simple 'if (val < 0)'.
| 
| Because the events are rolled back, this check for overflow fails and the
| event is not recorded. perf_event_interrupt() later uses pmc_overflow() to
| detect the overflow and resets the counters and the events are lost completely.
| 
| To properly detect the overflow of rolled back events, use pmc_overflow()
| even when recording events.

Ben,

Sorry for the noise, but please revert this patch (following commit):

	commit 813312110bede27bffd082c25cd31730bd567beb
	Author: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
	Date:   Tue Aug 7 15:07:19 2012 +0000

While it does fix the problem described above and works for the limit-pmc
events, it seems to break on Power7 for other events and when the sample
period is low.

I am still investigating the problem and will follow up with a separate
mail.

Sukadev

Patch

diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 8f84bcb..f74b90d 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -1394,7 +1394,7 @@  static void perf_event_interrupt(struct pt_regs *regs)
 		if (!event->hw.idx || is_limited_pmc(event->hw.idx))
 			continue;
 		val = read_pmc(event->hw.idx);
-		if ((int)val < 0) {
+		if (pmc_overflow(val)) {
 			/* event has overflowed */
 			found = 1;
 			record_and_restart(event, val, regs);