linux-kernel - Re: oprofile and ARM A9 hardware counter

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]

Message-ID: <20120217102643.GA17909@mudshark.cambridge.arm.com>
Date:	Fri, 17 Feb 2012 10:26:43 +0000
From:	Will Deacon <will.deacon@....com>
To:	Ming Lei <ming.lei@...onical.com>
Cc:	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	"eranian@...il.com" <eranian@...il.com>,
	"Shilimkar, Santosh" <santosh.shilimkar@...com>,
	David Long <dave.long@...aro.org>,
	"b-cousson@...com" <b-cousson@...com>,
	"mans@...sr.com" <mans@...sr.com>,
	linux-arm <linux-arm-kernel@...ts.infradead.org>,
	Ingo Molnar <mingo@...e.hu>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: oprofile and ARM A9 hardware counter

On Fri, Feb 17, 2012 at 05:24:02AM +0000, Ming Lei wrote:
> On Fri, Feb 17, 2012 at 2:08 AM, Will Deacon <will.deacon@....com> wrote:
> 
> >
> > The more I think about this, the more I think that the overflow parameter to
> > armpmu_event_update needs to go. It was introduced to prevent massive event
> > loss in non-sampling mode, but I think we can get around that by changing
> > the default sample_period to be half of the max_period, therefore giving
> > ourselves a much better chance of handling the interrupt before new wraps
> > around past prev.
> >
> > Ming Lei - can you try the following please? If it works for you, then I'll
> > do it properly and kill the overflow parameter altogether.
> 
> Of course, it does work for the problem reported by Stephane since
> it changes the delta computation basically as I did, but I am afraid that
> it may be not good enough for the issue fixed in a737823d ("ARM: 6835/1:
> perf: ensure overflows aren't missed due to IRQ latency").

I think it does solve that problem.

> >
> >        if (!hwc->sample_period) {
> > -               hwc->sample_period  = armpmu->max_period;
> > +               hwc->sample_period  = armpmu->max_period >> 1;
> 
> If you assume that the issue addressed by a737823d can only happen in
> non-sample situation, Peter's idea of u32 cast is OK and maybe simpler.

I don't want to assume that the counters are 32-bit in this code as we may
want to plug in some other PMU with 16-bit counters, for example. That's why
we have max_period defined for each PMU. Furthermore, it still doesn't help us
in the stat case where prev will typically be quite small after we've had an
overflow and new may well overtake it.

> But I am afraid that the issue still can be triggered in sample-based situation,
> especially in very high frequency case: suppose the sample freq is 10000,
> 100us IRQ delay may trigger the issue.

Not sure I follow. If the frequency is 10000, then we write 0xffffd8f0 to
the counter. That means we have a 0xffffd8f0 event window to read the
counter after it overflows before new overtakes prev and we get confused.
If this passed in 100us then either your clock speed is 4.3*10^12Hz or you
have a seriously wide issue :)

> So we may use the overflow information to make perf more robust, IMO.

There's a trade off between allowing the counter to wrap back around past
its previous value or being able to handle overflow on a non-interrupt path.
Given that the former problem is only a real issue for non-sampling runs,
halving the period in that case should sort it (and it does stop my simple
test case from exploding).

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/