linux-kernel - Re: [RFC][PATCH 00/13] perf pmu interface changes -v3

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:	Fri, 09 Jul 2010 16:11:42 +0100
From:	Will Deacon <will.deacon@....com>
To:	Peter Zijlstra <a.p.zijlstra@...llo.nl>
Cc:	paulus <paulus@...ba.org>,
	stephane eranian <eranian@...glemail.com>,
	Robert Richter <robert.richter@....com>,
	Paul Mundt <lethal@...ux-sh.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Cyrill Gorcunov <gorcunov@...il.com>,
	Lin Ming <ming.m.lin@...el.com>,
	Yanmin <yanmin_zhang@...ux.intel.com>,
	Deng-Cheng Zhu <dengcheng.zhu@...il.com>,
	David Miller <davem@...emloft.net>,
	Ingo Molnar <mingo@...e.hu>, linux-kernel@...r.kernel.org
Subject: Re: [RFC][PATCH 00/13] perf pmu interface changes -v3

Hi Peter,

On Fri, 2010-07-09 at 09:21 +0100, Peter Zijlstra wrote:
> The patches are boot tested on x86_64 and compile tested on: powerpc,
> ppc-fsl,
> sparc and arm and SH (by courtesy of Matt Fleming).
> 
> X86 -- appears to be fully functional.
> FSL -- should, after the initial few fixes, at least compile again.
> ARM -- is known to have grown some issues, Will was looking into that.

I've been looking at this and after fixing atomic64 operations and a
sign-extension bug, I'm getting closer to finding the *real* cause of
the problem! It appears that the PMU isn't being enabled for pinned
events. The following patch fixes that, but I'm not sure that it's the
correct thing to do:

diff --git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c
index c440ae1..a946a77 100644
--- a/arch/arm/kernel/perf_event.c
+++ b/arch/arm/kernel/perf_event.c
@@ -304,6 +304,8 @@ armpmu_add(struct perf_event *event, int flags)
        int idx;
        int err = 0;
 
+       perf_pmu_disable(event->pmu);
+
        /* If we don't have a space for the counter then finish early.
*/
        idx = armpmu->get_event_idx(cpuc, hwc);
        if (idx < 0) {
@@ -328,6 +330,7 @@ armpmu_add(struct perf_event *event, int flags)
        perf_event_update_userpage(event);
 
 out:
+       perf_pmu_enable(event->pmu);
        return err;
 }

Because only pinned events seem to be broken, I'm worried that this is
just hiding the problem... or is pmu->add(...) supposed to enable the
PMU as well as installing the event?

The function names aren't especially helpful here either:
armpmu_{start,stop} call armpmu->{enable,disable} and
armpmu_{enable,disable} call armpmu->{start,stop}!

Any thoughts?

Will

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