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: <CACVXFVPce0A3LfL=mFo3UbN-Om7xLOo_d-LfKXDjdN5dFQdhiA@mail.gmail.com>
Date:	Thu, 16 Feb 2012 18:25:05 +0800
From:	Ming Lei <ming.lei@...onical.com>
To:	Peter Zijlstra <a.p.zijlstra@...llo.nl>
Cc:	eranian@...il.com, "Shilimkar, Santosh" <santosh.shilimkar@...com>,
	David Long <dave.long@...aro.org>, b-cousson@...com,
	mans@...sr.com, will.deacon@....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

Hi,

On Thu, Feb 16, 2012 at 12:38 AM, Peter Zijlstra <a.p.zijlstra@...llo.nl> wrote:
>
> So what this patch seems to do is put that filter on period in
> perf_ctx_adjust_freq(). Not making sense.. nor can I see a HZ
> dependency, perf_ctx_adjust_freq() uses TICK_NSEC as time base.

Yes, you are right, I remembered it was observed it on -rc1, and
Stephane's unthrottling
patch was not merged at that time. Today I investigated the problem
further on -rc3 and found that seems the problem is caused by arm pmu code.

The patch below may fix the problem, now about 40000 sample events
can be generated on the command:

	'perf record -e cycles -F 4000  ./noploop 10&& perf report -D | tail -20'

armpmu_event_update may be called in tick path, so the running counter
will be overflowed and produce a great value of 'delta', then a mistaken
count is stored into event->count and event->hw.freq_count_stamp. Finally
the two variables are not synchronous, then a invalid and large period is
computed and written to pmu, and sample events are decreased much.

Will, this patch simplifies the 'delta' computation and doesn't use the
overflow flag, even though which can be read directly from PMOVSR, could you
comment on the patch?

diff --git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c
index 5bb91bf..789700a 100644
--- a/arch/arm/kernel/perf_event.c
+++ b/arch/arm/kernel/perf_event.c
@@ -193,13 +193,8 @@ again:
 			     new_raw_count) != prev_raw_count)
 		goto again;

-	new_raw_count &= armpmu->max_period;
-	prev_raw_count &= armpmu->max_period;
-
-	if (overflow)
-		delta = armpmu->max_period - prev_raw_count + new_raw_count + 1;
-	else
-		delta = new_raw_count - prev_raw_count;
+	delta = (armpmu->max_period - prev_raw_count + new_raw_count
+				+ 1) & armpmu->max_period;

 	local64_add(delta, &event->count);
 	local64_sub(delta, &hwc->period_left);

thanks,
--
Ming Lei
--
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/