[PATCH] Powerpc/perf: Wire up PMI throttling

Commit 14c63f17b1fde ("perf: Drop sample rate when sampling is too
slow") introduced a way to throttle PMU interrupts if we're spending
too much time just processing those. Wire up powerpc PMI handler to
use this infrastructure.

Signed-off-by: Ravi Bangoria <redacted>
---
 arch/powerpc/kernel/traps.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 9a86572db1ef..44f85fa22356 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c

@@ -18,6 +18,7 @@
 #include <linux/errno.h>
 #include <linux/sched.h>
 #include <linux/sched/debug.h>
+#include <linux/sched/clock.h>
 #include <linux/kernel.h>
 #include <linux/mm.h>
 #include <linux/pkeys.h>

@@ -1803,9 +1804,12 @@ void vsx_unavailable_tm(struct pt_regs *regs)
 
 void performance_monitor_exception(struct pt_regs *regs)
 {
+	u64 start_clock;
 	__this_cpu_inc(irq_stat.pmu_irqs);
 
+	start_clock = sched_clock();
 	perf_irq(regs);
+	perf_sample_event_took(sched_clock() - start_clock);
 }
 
 #ifdef CONFIG_PPC_ADV_DEBUG_REGS

-- 
2.19.1

Ravi Bangoria [off-list ref] writes:

Commit 14c63f17b1fde ("perf: Drop sample rate when sampling is too
slow") introduced a way to throttle PMU interrupts if we're spending
too much time just processing those. Wire up powerpc PMI handler to
use this infrastructure.

To be clear we have throttling of the *rate* of interrupts, but this
adds throttling based on the *time taken* to process the interrupts. Or
at least that's my understanding?

Despite the name, perf_irq() may not actually be the perf IRQ handler :)

It's a function pointer which might call perf or might call oprofile, or
a dummy handler.

I don't think we should be calling perf_sample_event_took() if we're not
actually using perf, that is wasteful at best.

So the timing logic should go in the perf specific handler I think.
ie. perf_event_interrupt().

cheers

On 11/15/18 6:13 PM, Michael Ellerman wrote:

Ravi Bangoria [off-list ref] writes:

quoted

Commit 14c63f17b1fde ("perf: Drop sample rate when sampling is too
slow") introduced a way to throttle PMU interrupts if we're spending
too much time just processing those. Wire up powerpc PMI handler to
use this infrastructure.

To be clear we have throttling of the *rate* of interrupts, but this
adds throttling based on the *time taken* to process the interrupts. Or
at least that's my understanding?

Right. This throttles based on time taken to process the interrupts.

quoted

diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 9a86572db1ef..44f85fa22356 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c

@@ -18,6 +18,7 @@
 #include <linux/errno.h>
 #include <linux/sched.h>
 #include <linux/sched/debug.h>
+#include <linux/sched/clock.h>
 #include <linux/kernel.h>
 #include <linux/mm.h>
 #include <linux/pkeys.h>

@@ -1803,9 +1804,12 @@ void vsx_unavailable_tm(struct pt_regs *regs)
 
 void performance_monitor_exception(struct pt_regs *regs)
 {
+	u64 start_clock;
 	__this_cpu_inc(irq_stat.pmu_irqs);
 
+	start_clock = sched_clock();
 	perf_irq(regs);
+	perf_sample_event_took(sched_clock() - start_clock);
 }

Despite the name, perf_irq() may not actually be the perf IRQ handler :)

It's a function pointer which might call perf or might call oprofile, or
a dummy handler.

I don't think we should be calling perf_sample_event_took() if we're not
actually using perf, that is wasteful at best.

So the timing logic should go in the perf specific handler I think.
ie. perf_event_interrupt().

Makes sense. I'll re-send with that change.

Thanks,
Ravi