| Message ID | 20181114141425.1892-1-ravi.bangoria@linux.ibm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Powerpc/perf: Wire up PMI throttling | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | next/apply_patch Successfully applied |
| snowpatch_ozlabs/build-ppc64le | success | build succeded & removed 0 sparse warning(s) |
| snowpatch_ozlabs/build-ppc64be | success | build succeded & removed 0 sparse warning(s) |
| snowpatch_ozlabs/build-ppc64e | success | build succeded & removed 0 sparse warning(s) |
| snowpatch_ozlabs/build-pmac32 | success | build succeded & removed 0 sparse warning(s) |
| snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 19 lines checked |
Ravi Bangoria <ravi.bangoria@linux.ibm.com> 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? > 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(). cheers
On 11/15/18 6:13 PM, Michael Ellerman wrote: > Ravi Bangoria <ravi.bangoria@linux.ibm.com> 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? Right. This throttles based on time taken to process the interrupts. > >> 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
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
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 <ravi.bangoria@linux.ibm.com> --- arch/powerpc/kernel/traps.c | 4 ++++ 1 file changed, 4 insertions(+)