linux-kernel - Re: [PATCH] Powerpc/perf: Wire up PMI throttling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 15 Nov 2018 23:43:39 +1100
From:   Michael Ellerman <mpe@...erman.id.au>
To:     Ravi Bangoria <ravi.bangoria@...ux.ibm.com>,
        linuxppc-dev@...ts.ozlabs.org
Cc:     benh@...nel.crashing.org, paulus@...ba.org, ebiederm@...ssion.com,
        muriloo@...ux.ibm.com, christophe.leroy@....fr, npiggin@...il.com,
        leitao@...ian.org, aneesh.kumar@...ux.vnet.ibm.com,
        linux-kernel@...r.kernel.org, naveen.n.rao@...ux.vnet.ibm.com,
        Ravi Bangoria <ravi.bangoria@...ux.ibm.com>
Subject: Re: [PATCH] Powerpc/perf: Wire up PMI throttling

Ravi Bangoria <ravi.bangoria@...ux.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