perf: x86 RDPMC and RDTSC support

Put the logic to compute the event index into a per pmu method. This is required because the x86 rules are weird and wonderful and don't match the capabilities of the current scheme.

AFAIK only powerpc actually has a usable userspace read of the PMCs but I'm not at all sure anybody actually used that.

ARM looks like it cared, but I really wouldn't know, Will?

For the rest we preserve the status quo with a default function.

We probably want to provide an explicit method for sw counters and the like so they don't try and dereference event->hw.index which is a random field in the middle of their hrtimer structure.

Cc Michael Cree <[email protected]>

Cc Will Deacon <[email protected]>

Cc Deng-Cheng Zhu <[email protected]>

Cc Anton Blanchard <[email protected]>

Cc Eric B Munson <[email protected]>

Cc Heiko Carstens <[email protected]>

Cc Paul Mundt <[email protected]>

Cc David S. Miller <[email protected]>

Cc Richard Kuo <[email protected]>

Cc Stephane Eranian <[email protected]>

Cc Arun Sharma <[email protected]>

Signed-off-by Peter Zijlstra <[email protected]>

---
 arch/arm/include/asm/perf_event.h            |    4 ----
 arch/arm/kernel/perf_event.c                 |   10 ++++++++++
 arch/frv/include/asm/perf_event.h            |    2 --
 arch/hexagon/include/asm/perf_event.h        |    2 --
 arch/powerpc/include/asm/perf_event_server.h |    2 --
 arch/powerpc/kernel/perf_event.c             |    6 ++++++
 arch/s390/include/asm/perf_event.h           |    1 -
 arch/x86/include/asm/perf_event.h            |    2 --
 include/linux/perf_event.h                   |    6 ++++++
 kernel/events/core.c                         |   14 +++++++++-----
 10 files changed, 31 insertions(+), 18 deletions(-)

Index: linux-2.6/include/linux/perf_event.h
===================================================================

include/linux/perf_event.h +6 -0

@@ -679,6 +679,12 @@ struct pmu {
 	 * for each successful ->add() during the transaction.
 	 */
 	void (*cancel_txn)		(struct pmu *pmu); /* optional */
+
+	/*
+	 * Will return the value for perf_event_mmap_page::index for this event,
+	 * if no implementation is provided it will default to: event->hw.idx + 1.
+	 */
+	int (*event_idx)		(struct perf_event *event); /*optional */
 };
 
 /**

Index: linux-2.6/kernel/events/core.c
===================================================================

kernel/events/core.c +9 -5

@@ -3191,10 +3191,6 @@ int perf_event_task_disable(void)
 	return 0;
 }
 
-#ifndef PERF_EVENT_INDEX_OFFSET
-# define PERF_EVENT_INDEX_OFFSET 0
-#endif
-
 static int perf_event_index(struct perf_event *event)
 {
 	if (event->hw.state & PERF_HES_STOPPED)

@@ -3203,7 +3199,7 @@ static int perf_event_index(struct perf_
 	if (event->state != PERF_EVENT_STATE_ACTIVE)
 		return 0;
 
-	return event->hw.idx + 1 - PERF_EVENT_INDEX_OFFSET;
+	return event->pmu->event_idx(event);
 }
 
 static void calc_timer_values(struct perf_event *event,

@@ -5334,6 +5330,11 @@ static void perf_pmu_cancel_txn(struct p
 	perf_pmu_enable(pmu);
 }
 
+static int perf_event_idx_default(struct perf_event *event)
+{
+	return event->hw.idx + 1;
+}
+
 /*
  * Ensures all contexts with the same task_ctx_nr have the same
  * pmu_cpu_context too.

@@ -5523,6 +5524,9 @@ int perf_pmu_register(struct pmu *pmu, c
 		pmu->pmu_disable = perf_pmu_nop_void;
 	}
 
+	if (!pmu->event_idx)
+		pmu->event_idx = perf_event_idx_default;
+
 	list_add_rcu(&pmu->entry, &pmus);
 	ret = 0;
 unlock:

Index: linux-2.6/arch/arm/include/asm/perf_event.h
===================================================================

arch/arm/include/asm/perf_event.h +0 -4

@@ -12,10 +12,6 @@
 #ifndef __ARM_PERF_EVENT_H__
 #define __ARM_PERF_EVENT_H__
 
-/* ARM performance counters start from 1 (in the cp15 accesses) so use the
- * same indexes here for consistency. */
-#define PERF_EVENT_INDEX_OFFSET 1
-
 /* ARM perf PMU IDs for use by internal perf clients. */
 enum arm_perf_pmu_ids {
 	ARM_PERF_PMU_ID_XSCALE1	= 0,

Index: linux-2.6/arch/arm/kernel/perf_event.c
===================================================================

arch/arm/kernel/perf_event.c +10 -0

@@ -572,6 +572,15 @@ static void armpmu_disable(struct pmu *p
 	armpmu->stop();
 }
 
+/*
+ * ARM performance counters start from 1 (in the cp15 accesses) so use the
+ * same indexes here for consistency.
+ */
+static int armpmu_event_idx(struct perf_event *event)
+{
+	return event->hw.idx;
+}
+
 static void __init armpmu_init(struct arm_pmu *armpmu)
 {
 	atomic_set(&armpmu->active_events, 0);

@@ -586,6 +595,7 @@ static void __init armpmu_init(struct ar
 		.start		= armpmu_start,
 		.stop		= armpmu_stop,
 		.read		= armpmu_read,
+		.event_idx	= armpmu_event_idx,
 	};
 }
 

Index: linux-2.6/arch/frv/include/asm/perf_event.h
===================================================================

arch/frv/include/asm/perf_event.h +0 -2

@@ -12,6 +12,4 @@
 #ifndef _ASM_PERF_EVENT_H
 #define _ASM_PERF_EVENT_H
 
-#define PERF_EVENT_INDEX_OFFSET	0
-
 #endif /* _ASM_PERF_EVENT_H */

Index: linux-2.6/arch/hexagon/include/asm/perf_event.h
===================================================================

arch/hexagon/include/asm/perf_event.h +0 -2

@@ -19,6 +19,4 @@
 #ifndef _ASM_PERF_EVENT_H
 #define _ASM_PERF_EVENT_H
 
-#define PERF_EVENT_INDEX_OFFSET	0
-
 #endif /* _ASM_PERF_EVENT_H */

Index: linux-2.6/arch/powerpc/include/asm/perf_event_server.h
===================================================================

arch/powerpc/include/asm/perf_event_server.h +0 -2

@@ -61,8 +61,6 @@ struct pt_regs;
 extern unsigned long perf_misc_flags(struct pt_regs *regs);
 extern unsigned long perf_instruction_pointer(struct pt_regs *regs);
 
-#define PERF_EVENT_INDEX_OFFSET	1
-
 /*
  * Only override the default definitions in include/linux/perf_event.h
  * if we have hardware PMU support.

Index: linux-2.6/arch/powerpc/kernel/perf_event.c
===================================================================

arch/powerpc/kernel/perf_event.c +6 -0

@@ -1187,6 +1187,11 @@ static int power_pmu_event_init(struct p
 	return err;
 }
 
+static int power_pmu_event_idx(struct perf_event *event)
+{
+	return event->hw.idx;
+}
+
 struct pmu power_pmu = {
 	.pmu_enable	= power_pmu_enable,
 	.pmu_disable	= power_pmu_disable,

@@ -1199,6 +1204,7 @@ struct pmu power_pmu = {
 	.start_txn	= power_pmu_start_txn,
 	.cancel_txn	= power_pmu_cancel_txn,
 	.commit_txn	= power_pmu_commit_txn,
+	.event_idx	= power_pmu_event_idx,
 };
 
 /*

Index: linux-2.6/arch/s390/include/asm/perf_event.h
===================================================================

arch/s390/include/asm/perf_event.h +0 -1

@@ -6,4 +6,3 @@
 
 /* Empty, just to avoid compiling error */
 
-#define PERF_EVENT_INDEX_OFFSET 0

Index: linux-2.6/arch/x86/include/asm/perf_event.h
===================================================================

arch/x86/include/asm/perf_event.h +0 -3

@@ -187,8 +187,6 @@ extern u32 get_ibs_caps(void);
 #ifdef CONFIG_PERF_EVENTS
 extern void perf_events_lapic_init(void);
 
-#define PERF_EVENT_INDEX_OFFSET			0
-
 /*
  * Abuse bit 3 of the cpu eflags register to indicate proper PEBS IP fixups.
  * This flag is otherwise unused and ABI specified to be 0, so nobody should
 
 
--

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Put the logic to compute the event index into a per pmu method. This is required because the x86 rules are weird and wonderful and don't match the capabilities of the current scheme.

AFAIK only powerpc actually has a usable userspace read of the PMCs but I'm not at all sure anybody actually used that.

ARM looks like it cared, but I really wouldn't know, Will?

Hi Peter,

Quoted 7 lines · Peter Zijlstra

Put the logic to compute the event index into a per pmu method. This is required because the x86 rules are weird and wonderful and don't match the capabilities of the current scheme.

AFAIK only powerpc actually has a usable userspace read of the PMCs but I'm not at all sure anybody actually used that.

Quoted 1 line

ARM looks like it cared, but I really wouldn't know, Will?

It used to care, but it doesn't anymore. Feel free to make the offset 0 and use the (now) generic codepath.

With that:

Acked-by Will Deacon <[email protected]>

But does ARM have a read PMU counter from userspace instruction/capability?

Lacking that its all moot of course. If it does, it would be nice to have an ARM version of patch 6.

Quoted 2 lines · Peter Zijlstra

But does ARM have a read PMU counter from userspace instruction/capability?

In ARMv7, you can enable user access to the performance counters but the access is R/W so I don't think it's something we want to do (could interfere with another task doing system-wide profiling).

Quoted 2 lines

Lacking that its all moot of course. If it does, it would be nice to have an ARM version of patch 6.

Well we'd need a way to get around the all-or-nothing user access to the PMU and also the fact that we don't always have a user-readable clocksource.

Either way, the event counters are indexed from 0 on ARMv7 so we should use perf_event_idx_default.

Quoted 7 lines · Will Deacon

Quoted 2 lines · Peter Zijlstra

But does ARM have a read PMU counter from userspace instruction/capability?

In ARMv7, you can enable user access to the performance counters but the access is R/W so I don't think it's something we want to do (could interfere with another task doing system-wide profiling).

Yeah, write access is pushing it a bit..

Quoted 8 lines

Quoted 2 lines

Lacking that its all moot of course. If it does, it would be nice to have an ARM version of patch 6.

Well we'd need a way to get around the all-or-nothing user access to the PMU and also the fact that we don't always have a user-readable clocksource.

Either way, the event counters are indexed from 0 on ARMv7 so we should use perf_event_idx_default.

Ok, done. Thanks!

Quoted 9 lines · Peter Zijlstra

Quoted 7 lines · Will Deacon

Quoted 2 lines · Peter Zijlstra

But does ARM have a read PMU counter from userspace instruction/capability?

In ARMv7, you can enable user access to the performance counters but the access is R/W so I don't think it's something we want to do (could interfere with another task doing system-wide profiling).

Quoted 1 line

Yeah, write access is pushing it a bit..

Perhaps we could disable it while per-cpu events are running, although I think this will probably just lead to SIGILL central for anybody trying to use the counters in userspace.

Quoted 3 lines · Will Deacon

Perhaps we could disable it while per-cpu events are running, although I think this will probably just lead to SIGILL central for anybody trying to use the counters in userspace.

One possibility would be to do as I did in patch 4, except ARM has it disabled by default and the folks who think they know WTF they're doing can enable it or so.

Ostrikov mentioned on #kernelnewbies he wanted to have this enabled because apparently games use it. He mentioned toggling the user access on/off depending on if the kernel was using perf or not, but that would create very unreliable service.

Best would be to use perf to program the thing and use the userspace read and simply agree not to write to these registers (and pray people don't)..

Also, for those ARMs that do have a user readable clock, you could support the new time_{mult,shift,offset} from patch 5.

The user-readable clock will first appear in Cortex-A15, so the code for that still needs to hit mainline before I can look at doing this in perf.

Quoted 3 lines · Will Deacon

Perhaps we could disable it while per-cpu events are running, although I think this will probably just lead to SIGILL central for anybody trying to use the counters in userspace.

One possibility would be to do as I did in patch 4, except ARM has it disabled by default and the folks who think they know WTF they're doing can enable it or so.

Ostrikov mentioned on #kernelnewbies he wanted to have this enabled because apparently games use it. He mentioned toggling the user access on/off depending on if the kernel was using perf or not, but that would create very unreliable service.

Best would be to use perf to program the thing and use the userspace read and simply agree not to write to these registers (and pray people don't)..

Also, for those ARMs that do have a user readable clock, you could support the new time_{mult,shift,offset} from patch 5.

Quoted 1 line

The user-readable clock will first appear in Cortex-A15, so the code for that still needs to hit mainline before I can look at doing this in perf.

This could be very useful. I think that the cycles counter covers the 90% of all the in-app profiler needs.

Quoted 1 line

The user-readable clock will first appear in Cortex-A15, so the code for that still needs to hit mainline before I can look at doing this in perf.

This could be very useful. I think that the cycles counter covers the 90% of all the in-app profiler needs.

Quoted 1 line

The user-readable clock will first appear in Cortex-A15, so the code for that still needs to hit mainline before I can look at doing this in perf.

This could be very useful. I think that the cycles counter covers the 90% of all the in-app profiler needs.

Also, please do not forget that enabling the userspace access to the counters may not only create some problems to the profilers like Perf/OProfile but could also trash the kernel with the interrupts.

Actually, the registers for controlling interrupts aren't visible to userspace with ARMv7. On ARMv6, this is a different story, which is why I didn't mention doing this > for older platforms earlier :)

Also, please do not forget that enabling the userspace access to the
counters may not only create some problems to the profilers like Perf/OProfile but could also trash the kernel with the interrupts.

Quoted 5 lines · Peter Zijlstra

Quoted 3 lines · Will Deacon

Perhaps we could disable it while per-cpu events are running, although I think this will probably just lead to SIGILL central for anybody trying to use the counters in userspace.

Quoted 3 lines

One possibility would be to do as I did in patch 4, except ARM has it disabled by default and the folks who think they know WTF they're doing can enable it or so.

The problem is that everybody thinks they know WTF they're doing!

But you know that the first thing people will do is zero the registers.

Quoted 2 lines

Also, for those ARMs that do have a user readable clock, you could support the new time_{mult,shift,offset} from patch 5.

The user-readable clock will first appear in Cortex-A15, so the code for that still needs to hit mainline before I can look at doing this in perf.

On Tue, 2011-11-22 at 11:47 +0000, Will Deacon wrote:

OK, I had interpreted your "we don't always have" to be slightly more common than just A15.

Quoted 9 lines · Peter Zijlstra

Quoted 7 lines · Will Deacon

Quoted 2 lines · Peter Zijlstra

But does ARM have a read PMU counter from userspace instruction/capability?

In ARMv7, you can enable user access to the performance counters but the access is R/W so I don't think it's something we want to do (could interfere with another task doing system-wide profiling).

Quoted 1 line

Yeah, write access is pushing it a bit..

Perhaps we could disable it while per-cpu events are running, although I think this will probably just lead to SIGILL central for anybody trying to use the counters in userspace.

Extend the mmap control page with fields so that userspace can compute time deltas relative to the provided time fields.

Currently only implemented for x86 with constant and nonstop TSC.

Cc Stephane Eranian <[email protected]>

Cc Arun Sharma <[email protected]>

Signed-off-by Peter Zijlstra <[email protected]>

---
 arch/x86/kernel/cpu/perf_event.c |   14 ++++++++++++++ include/linux/perf_event.h       |    4 +++- kernel/events/core.c             |   21 ++++++++++++++-------

 3 files changed, 31 insertions(+), 8 deletions(-)

Index: linux-2.6/arch/x86/kernel/cpu/perf_event.c
===================================================================

arch/x86/kernel/cpu/perf_event.c +14 -0

@@ -32,6 +32,7 @@
  #include <asm/compat.h>
  #include <asm/smp.h>
  #include <asm/alternative.h>
+#include <asm/timer.h>
 
  #include "perf_event.h"
 

@@ -1621,6 +1622,19 @@ static struct pmu pmu = {
        .event_idx      = x86_pmu_event_idx,
  };
 
+void perf_update_user_clock(struct perf_event_mmap_page *userpg, u64 now)
+{
+       if (!boot_cpu_has(X86_FEATURE_CONSTANT_TSC))
+               return;
+
+       if (!boot_cpu_has(X86_FEATURE_NONSTOP_TSC))
+               return;
+
+       userpg->time_mult = this_cpu_read(cyc2ns);
+       userpg->time_shift = CYC2NS_SCALE_FACTOR;
+       userpg->time_offset = this_cpu_read(cyc2ns_offset) - now;
+}
+
  /*
  * callchain support
  */

Index: linux-2.6/include/linux/perf_event.h
===================================================================

include/linux/perf_event.h +0 -0

@@ -290,12 +290,14 @@ struct perf_event_mmap_page {

       __s64   offset;                 /* add to hardware event value */ __u64   time_enabled;           /* time event active */ __u64   time_running;           /* time event on cpu */
+       __u32   time_mult, time_shift;
+       __u64   time_offset;

               /*
                * Hole for extension of the self monitor capabilities */

-       __u64   __reserved[123];        /* align to 1k */
+       __u64   __reserved[121];        /* align to 1k */

       /*
        * Control data for the mmap() data buffer.
Index: linux-2.6/kernel/events/core.c
===================================================================

kernel/events/core.c +10 -5

@@ -3203,17 +3203,22 @@ static int perf_event_index(struct perf_
  }
 
  static void calc_timer_values(struct perf_event *event,
+                               u64 *now,
                                u64 *enabled,
                                u64 *running)
  {
-       u64 now, ctx_time;
+       u64 ctx_time;
 
-       now = perf_clock();
-       ctx_time = event->shadow_ctx_time + now;
+       *now = perf_clock();
+       ctx_time = event->shadow_ctx_time + *now;
        *enabled = ctx_time - event->tstamp_enabled;
        *running = ctx_time - event->tstamp_running;
  }
 
+void __weak perf_update_user_clock(struct perf_event_mmap_page *userpg, u64 now)
+{
+}
+
  /*
  * Callers need to ensure there can be no nesting of this function, otherwise
  * the seqlock logic goes bad. We can not serialize this because the arch

@@ -3223,7 +3228,7 @@ void perf_event_update_userpage(struct p
  {
        struct perf_event_mmap_page *userpg;
        struct ring_buffer *rb;
-       u64 enabled, running;
+       u64 enabled, running, now;
 
        rcu_read_lock();
        /*

@@ -3235,7 +3240,7 @@ void perf_event_update_userpage(struct p
         * because of locking issue as we can be called in
         * NMI context
         */
-       calc_timer_values(event, &enabled, &running);
+       calc_timer_values(event, &now, &enabled, &running);
        rb = rcu_dereference(event->rb);
        if (!rb)
                goto unlock;

@@ -3260,6 +3265,8 @@ void perf_event_update_userpage(struct p

       userpg->time_running = running + atomic64_read(&event->child_total_time_running);

+       perf_update_user_clock(userpg, now);
+
       barrier();
       ++userpg->lock;
       preempt_enable();

kernel/events/core.c +2 -2

@@ -3692,7 +3699,7 @@ static void perf_output_read_group(struc
  static void perf_output_read(struct perf_output_handle *handle,
                             struct perf_event *event)
  {
-       u64 enabled = 0, running = 0;
+       u64 enabled = 0, running = 0, now;
        u64 read_format = event->attr.read_format;
 
        /*

@@ -3705,7 +3712,7 @@ static void perf_output_read(struct perf
         * NMI context
         */
        if (read_format & PERF_FORMAT_TOTAL_TIMES)
-               calc_timer_values(event, &enabled, &running);
+               calc_timer_values(event, &now, &enabled, &running);
 
        if (event->attr.read_format & PERF_FORMAT_GROUP)
                perf_output_read_group(handle, event, enabled, running);

Implement a simple test for the self-monitoring data from the mmap-control page.

 6: x86 rdpmc test: Ok
0: 6042 2481
1: 60042 23669
2: 605410 245588
3: 6060818 2396231

XXX: come up with logic to automagically turn these numbers into OK/FAIL.

Cc Stephane Eranian <[email protected]>

Cc Arun Sharma <[email protected]>

Signed-off-by Peter Zijlstra <[email protected]>

---
 tools/perf/builtin-test.c |  129 ++++++++++++++++++++++++++++++++++++++++++++++

 1 file changed, 129 insertions(+)

Index: linux-2.6/tools/perf/builtin-test.c
===================================================================

tools/perf/builtin-test.c +123 -0

@@ -14,6 +14,8 @@
  #include "util/thread_map.h"
  #include "../../include/linux/hw_breakpoint.h"
 
+#include <sys/mman.h>
+
  static long page_size;
 
  static int vmlinux_matches_kallsyms_filter(struct map *map __used, struct symbol *sym)

@@ -841,6 +843,127 @@ static int test__parse_events(void)
 
        return ret;
  }
+
+#if defined(__x86_64__) || defined(__i386__)
+
+#define barrier() asm volatile("" ::: "memory")
+
+static u64 rdpmc(unsigned int counter)
+{
+       unsigned int low, high;
+
+       asm volatile("rdpmc" : "=a" (low), "=d" (high) : "c" (counter));
+
+       return low | ((u64)high) << 32;
+}
+
+static u64 rdtsc(void)
+{
+       unsigned int low, high;
+
+       asm volatile("rdtsc" : "=a" (low), "=d" (high));
+
+       return low | ((u64)high) << 32;
+}
+
+static u64 mmap_read_self(void *addr, u64 *enabled, u64 *running)
+{
+       struct perf_event_mmap_page *pc = addr;
+       u32 seq;
+       u64 count, delta;
+
+       do {
+again:
+               seq = pc->lock;
+               barrier();
+               if (seq & 1)
+                       goto again;
+
+               if ((enabled || running) && pc->time_mult) {
+                       u64 cyc = rdtsc();
+                       u64 rem, quot;
+
+                       quot = (cyc >> pc->time_shift);
+                       rem = cyc & ((1 << pc->time_shift) - 1);
+                       delta = pc->time_offset +
+                               quot * pc->time_mult +
+                               ((rem * pc->time_mult) >> pc->time_shift);
+               }
+
+               if (enabled)
+                       *enabled = pc->time_enabled + delta;
+
+               if (running)
+                       *running = pc->time_running + delta;
+
+               if (pc->index) {
+                       count = rdpmc(pc->index - 1);
+                       count += pc->offset;
+               } else
+                       goto fail;
+
+               barrier();
+       } while (pc->lock != seq);
+
+       return count;
+
+fail:
+       /*
+        * XXX do read() here
+        */
+       printf("FAIL FAIL FAIL\n");
+       return 0;
+}
+
+static int test__rdpmc(void)
+{
+       volatile int tmp = 0;
+       int loops = 1000;
+       int n, i;
+       int fd;
+       void *addr;
+       struct perf_event_attr attr = {
+               .type = PERF_TYPE_HARDWARE,
+               .config = PERF_COUNT_HW_INSTRUCTIONS,
+       };
+
+
+       fd = sys_perf_event_open(&attr, 0, -1, -1, 0);
+       if (fd < 0) {
+               die("Error: sys_perf_event_open() syscall returned "
+                   "with %d (%s)\n", fd, strerror(errno));
+       }
+
+       addr = mmap(NULL, page_size, PROT_READ, MAP_SHARED, fd, 0);
+       if (addr == (void *)(-1)) {
+               die("Error: mmap() syscall returned "
+                   "with (%s)\n", strerror(errno));
+       }
+
+       for (n = 0; n < 4; n++) {
+
+               u64 stamp, now;
+               u64 stamp2, now2;
+
+               stamp = mmap_read_self(addr, NULL, &stamp2);
+
+               for (i = 0; i < loops; i++)
+                       tmp++;
+
+               now = mmap_read_self(addr, NULL, &now2);
+               loops *= 10;
+
+               printf("%d: %Lu %Lu\n", n, (unsigned long long)(now - stamp),
+                                          (unsigned long long)(now2 - stamp2));
+       }
+
+       munmap(addr, page_size);
+       close(fd);
+
+       return 0;
+}
+#endif
+
  static struct test {
        const char *desc;
        int (*func)(void);

@@ -865,6 +988,12 @@ static struct test {
                .desc = "parse events tests",
                .func = test__parse_events,

Peter,

I don't see how this test and infrastructure handles the case where the event is multiplexed. I know there is time_enabled and time_running. But those are not sync'd to the moment of the rdpmc(). I think there needs to be some other timestamp in the mmap struct so the user can compute a delta to then add to time_enabled and time_running.

Unless, we assume the two time metrics are there ONLY to compute a scaling ratio. In which case, I think, we don't need the delta because if we can do rdpmc()
it means the event is currently scheduled and thus time_enabled and time_running are both ticking which means the scaling ratio does not change since the moment the event was scheduled in.

Quoted 7 lines · Stephane Eranian

Peter,

I don't see how this test and infrastructure handles the case where the event is multiplexed. I know there is time_enabled and time_running. But those are not sync'd to the moment of the rdpmc(). I think there needs to be some other timestamp in the mmap struct so the user can compute a delta to then add to time_enabled and time_running.

When the counter isn't actually on the PMU, ->index will be 0 and rdpmc should not be attempted.

Quoted 6 lines

Unless, we assume the two time metrics are there ONLY to compute a scaling ratio. In which case, I think, we don't need the delta because if we can do rdpmc()
it means the event is currently scheduled and thus time_enabled and time_running are both ticking which means the scaling ratio does not change since the moment the event was scheduled in.

Right, you don't need delta to compute the scale, but its useful for user-space time based measurements, Arun wanted to do something like that.

Quoted 19 lines · Peter Zijlstra

Quoted 7 lines · Stephane Eranian

Peter,

I don't see how this test and infrastructure handles the case where the event is multiplexed. I know there is time_enabled and time_running. But those are not sync'd to the moment of the rdpmc(). I think there needs to be some other timestamp in the mmap struct so the user can compute a delta to then add to time_enabled and time_running.

When the counter isn't actually on the PMU, ->index will be 0 and rdpmc should not be attempted.

Quoted 6 lines

Unless, we assume the two time metrics are there ONLY to compute a scaling ratio. In which case, I think, we don't need the delta because if we can do rdpmc()
it means the event is currently scheduled and thus time_enabled and time_running are both ticking which means the scaling ratio does not change since the moment the event was scheduled in.

Quoted 3 lines

Right, you don't need delta to compute the scale, but its useful for user-space time based measurements, Arun wanted to do something like that.

I'm full of crap, of course that makes a difference :-)

Even when both running and enabled are incremented, the scaling does still change: 3/2 != 4/3 etc..

Using that we can actually deal with the whole multiplexing thing without ever having to fall back to read(), something like:

static u64 mmap_read_self(void *addr)
{
       struct perf_event_mmap_page *pc = addr;
       u32 seq, idx, time_mult, time_shift;
       u64 count, cyc, time_offset, enabled, running, delta;

       do {
               seq = pc->lock;
               barrier();

               enabled = pc->time_enabled;
               running = pc->time_running;

               if (enabled != running) {
                       cyc = rdtsc(); time_mult = pc->time_mult; time_shift = pc->time_shift; time_offset = pc->time_offset; }

               idx = pc->index;
               count = pc->offset;
               if (idx)
                       count += rdpmc(idx - 1);

               barrier();
       } while (pc->lock != seq);

       if (enabled != running) {
               u64 quot, rem;

               quot = (cyc >> time_shift);
               rem = cyc & ((1 << time_shift) - 1); delta = time_offset + quot * time_mult + ((rem * time_mult) >> time_shift);

               enabled += delta;
               if (idx)
                       running += delta;

               quot = count / running;
               rem = count % running;
               count = quot * enabled + (rem * enabled) / running; }

       return count;
}

Now all I need to do is make sure pc->offset actually makes sense, because currently it looks like we're off by a factor event->hw.prev_count when idx is set.

These few patches implement x86 RDPMC support and add an extention to the self monitoring data to also allow additional time updates using userspace TSC reads.

There's a few loose ends, but it mostly seems to work.

Quoted 4 lines · Peter Zijlstra

These few patches implement x86 RDPMC support and add an extention to the self monitoring data to also allow additional time updates using userspace TSC reads.

There's a few loose ends, but it mostly seems to work.

I'll have to test these out.

I have some low-level benchmarks I've been working on, you can see some preliminary results here:
http://web.eecs.utk.edu/~vweaver1/projects/perf-events/benchmarks/

perf_events was lagging behind perfmon2 and perfctr in self-monitoring overhead, but maybe these changes will help that.

perf_event overhead got a lot worse between 2.6.32 and 3.0, I'm mid-bisect on that trying to isolate the cause.

Vince

Quoted 4 lines · Peter Zijlstra

These few patches implement x86 RDPMC support and add an extention to the self monitoring data to also allow additional time updates using userspace TSC reads.

There's a few loose ends, but it mostly seems to work.

I'll have to test these out.

I have some low-level benchmarks I've been working on, you can see some preliminary results here:
http://web.eecs.utk.edu/~vweaver1/projects/perf-events/benchmarks/

perf_events was lagging behind perfmon2 and perfctr in self-monitoring overhead, but maybe these changes will help that.

perf_event overhead got a lot worse between 2.6.32 and 3.0, I'm mid-bisect on that trying to isolate the cause.

Vince

These few patches implement x86 RDPMC support and add an extention to the self monitoring data to also allow additional time updates using userspace TSC reads.

There's a few loose ends, but it mostly seems to work.

Quoted 7 lines · Peter Zijlstra

These few patches implement x86 RDPMC support and add an extention to the self
monitoring data to also allow additional time updates using userspace TSC reads.

There's a few loose ends, but it mostly seems to work.

I haven't had a chance to test this out yet. But low overhead, always on perf counters is something we're very interested in. Thanks for implementing it.

However, I suspect the major cost of leaving the perf counters always on seems to be in the hit on context switches, rather than the cost of reading the perf counters themselves. For eg:

 Baseline:

  (for i in seq 1 10; do numactl --cpunodebind 1 ./lat_ctx -P1 -s32k 4; done) 2>&1 | tee lmbench1.log

 1 event:

 (for i in seq 1 10; do numactl --cpunodebind 1 perf stat -e instructions ./lat_ctx -P1 -s32k 4; done) 2>&1 | tee lmbench2.log

 2 events:

 (for i in seq 1 10; do numactl --cpunodebind 1 perf stat -e cycles,instructions ./lat_ctx -P1 -s32k 4; done) 2>&1 | tee lmbench3.log

 Baseline: 2.2us
 One event: 6.8us
 Two events: 7.2us

The cost seems to be at roughly 5us (I measured 2.6.38 and 3.2-rc2). I'll dig a bit more on what may be going on here.

 -Arun

Have you tried with inheritance disabled?
I don't remember if perf stat has it enabled buy default.

Quoted 2 lines · Stephane Eranian

Have you tried with inheritance disabled?
I don't remember if perf stat has it enabled buy default.

Disabling inheritance using the -i switch as in:

(for i in seq 1 10; do numactl --cpunodebind 1 perf stat -i -e instructions ./lat_ctx -P1 -s32k 4; done)

shows numbers closer to baseline, since the threads used for benchmarking are not counting events.

Quoted 7 lines · Arun Sharma

Quoted 3 lines · Stephane Eranian

Have you tried with inheritance disabled?
I don't remember if perf stat has it enabled buy default.

Quoted 7 lines

Disabling inheritance using the -i switch as in:

(for i in seq 1 10; do numactl --cpunodebind 1 perf stat -i -e instructions ./lat_ctx -P1 -s32k 4; done)

shows numbers closer to baseline, since the threads used for benchmarking are not counting events.

I spent some more time looking into this. Running:

perf stat -e instructions  ./lat_ctx -P1 -s32k 4 -N 1000000 & perf record -ag -- sleep 3

didn't show high cycles counts on any perf_events related functions in the context switch path. The only PMU related stuff that showed up had to do with x86_pmu_enable called from an IPI.

So just as an experiment I disabled perf_rotate_context() via:

kernel/sched.c +0 -2

@@ -4252,8 +4252,6 @@ void scheduler_tick(void)
        curr->sched_class->task_tick(rq, curr, 0);
        raw_spin_unlock(&rq->lock);
 
-       perf_event_task_tick();
-
 

That seems to bring the lat_ctx numbers very close to baseline.

I suspect that some optimizations are possible in perf_rotate_context that don't involve enabling/disabling the PMU via an IPI for simple cases that involve one or two hardware events (eg: fixed counters).

 -Arun

Sample stack trace:

lat_ctx 29874 [012] 350729.824173: cycles:
       ffffffff8101149c intel_pmu_enable_all ([kernel.kallsyms]) ffffffff810115f7 intel_pmu_nhm_enable_all ([kernel.kallsyms]) ffffffff8100e877 x86_pmu_enable ([kernel.kallsyms]) ffffffff810a99de perf_pmu_enable ([kernel.kallsyms]) ffffffff8100d682 x86_pmu_commit_txn ([kernel.kallsyms]) ffffffff810aa4f9 group_sched_in ([kernel.kallsyms]) ffffffff810ab06d __perf_event_enable ([kernel.kallsyms]) ffffffff810a8c93 remote_function ([kernel.kallsyms]) ffffffff81065eec generic_smp_call_function_single_interrupt ([kernel.kallsyms])
       ffffffff81018064 smp_call_function_single_interrupt ([kernel.kallsyms])
       ffffffff81461fcb call_function_single_interrupt ([kernel.kallsyms]) 401ec4 bread (/root/lat_ctx)

I suspect that some optimizations are possible in perf_rotate_context that don't involve enabling/disabling the PMU via an IPI for simple cases that involve one or two hardware events (eg: fixed counters).

Currently pmu is disabled and re-enabled on each timer interrupt even when no rotation or frequency adjustment is needed. On Intel CPU this results in two writes into PERF_GLOBAL_CTRL MSR per tick. On bare metal it does not cause significant slowdown, but when running perf in a virtual machine it leads to 20% slowdown on my machine.

Quoted 4 lines · Arun Sharma

I suspect that some optimizations are possible in perf_rotate_context that don't involve enabling/disabling the PMU via an IPI for simple cases that involve one or two hardware events (eg: fixed counters).

I've got a patch queued that does exactly that.

These few patches implement x86 RDPMC support and add an extention to the self monitoring data to also allow additional time updates using userspace TSC reads.

There's a few loose ends, but it mostly seems to work.

Is it possible to read multiple counters at once with this interface, i.e. using FORMAT_GROUP?

Also, am I correct that the counters are set to always counting, so you always have to do a rdpmc() before and a rdpmc() after?

I used the mmap_read_self() routine from your example as the "read" performance that I measured.

If start/stop are truly unnecessary when run with your patch then the "total" results shift in your favor. Otherwise perfmon2 and perfctr still win the self-monitoring overhead race by a lot.

Quoted 2 lines

I used the mmap_read_self() routine from your example as the "read" performance that I measured.

Yeah that's about it, if you want to discard the overload scenario, eg you use pinned counters or so, you can optimize it further by stripping out the tsc and scaling muck.

Here's "pinned events" variant i've measured:

static u64 mmap_read_self(void *addr)

{
        struct perf_event_mmap_page *pc = addr;
        u32 seq, idx;
        u64 count;

        do {
                seq = pc->lock;
                barrier();

                idx = pc->index;
                count = pc->offset;

                if (idx)
                        count += rdpmc(idx - 1);

                barrier();
        } while (pc->lock != seq);

        return count;
}

Quoted 2 lines · Vince Weaver

Is it possible to read multiple counters at once with this interface, i.e. using FORMAT_GROUP?

No, that's not something that I had considered, I'll see if I can make that happen by making an array inside the control page instead of a single version.

Quoted 2 lines

Also, am I correct that the counters are set to always counting, so you always have to do a rdpmc() before and a rdpmc() after?

Depending on how you use it, but yes that's how I'd use it, avoids having to do ioctl()s

Quoted 3 lines

If start/stop are truly unnecessary when run with your patch then the "total" results shift in your favor. Otherwise perfmon2 and perfctr still win the self-monitoring overhead race by a lot.

You can of course also do the start/stop/reset in userspace by keeping an offset to the counter, avoiding the ioctl()s and simply keeping the counter running.

currently when I tried doing a start ioctl then an immediate read, the counter values returned were a very high value. So unless I did two rdpmcs and subtracted, the results made no sense.

* One, is it possible to detect at runtime that rdpmc() is supported? Maybe a field in the mmap'd buffer that can be read?

* Second, if I try a start / run 100M instructions / read and measure retired instructions I get:

      281474977711543 instructions

This is 0x10000000f45b7 which is 1<<48 + 100M. I guess an artifact of how the counters are set up? Is it possible to have this start at 0?

Quoted 2 lines · Vince Weaver

* One, is it possible to detect at runtime that rdpmc() is supported? Maybe a field in the mmap'd buffer that can be read?

Ingo suggested a similar thing, will implement (although Christmas might interfere with delivery timelines etc..).

Quoted 6 lines

* Second, if I try a start / run 100M instructions / read and measure retired instructions I get:

      281474977711543 instructions

This is 0x10000000f45b7 which is 1<<48 + 100M. I guess an artifact of how the counters are set up? Is it possible to have this start at 0?

Its supposed to be 0 based, clearly I messed up something and need to go figure out what. Thanks for reporting.

Speaking of bit 48, does your implementation do anything useful about the 32/40/48 bit limitation that some processors have with rdpmc? Or is the answer that if you might get overflow, then you have to use slow syscall-based read?