From: Ming Lei <ming.lei@canonical.com>
To: Will Deacon <will.deacon@arm.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>,
"eranian@gmail.com" <eranian@gmail.com>,
"Shilimkar, Santosh" <santosh.shilimkar@ti.com>,
David Long <dave.long@linaro.org>,
"b-cousson@ti.com" <b-cousson@ti.com>,
"mans@mansr.com" <mans@mansr.com>,
linux-arm <linux-arm-kernel@lists.infradead.org>,
Ingo Molnar <mingo@elte.hu>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: oprofile and ARM A9 hardware counter
Date: Fri, 17 Feb 2012 13:24:02 +0800 [thread overview]
Message-ID: <CACVXFVO4eexVK-S5TdRk2T6pyKRzpW1HxBQxo0Yfk_ziaO_U4Q@mail.gmail.com> (raw)
In-Reply-To: <20120216180841.GC31977@mudshark.cambridge.arm.com>
On Fri, Feb 17, 2012 at 2:08 AM, Will Deacon <will.deacon@arm.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").
>
> Thanks,
>
> Will
>
> git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c
> index 5bb91bf..ef597a3 100644
> --- a/arch/arm/kernel/perf_event.c
> +++ b/arch/arm/kernel/perf_event.c
> @@ -193,13 +193,7 @@ 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 = (new_raw_count - prev_raw_count) & armpmu->max_period;
>
> local64_add(delta, &event->count);
> local64_sub(delta, &hwc->period_left);
> @@ -518,7 +512,7 @@ __hw_perf_event_init(struct perf_event *event)
> hwc->config_base |= (unsigned long)mapping;
>
> 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.
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.
So we may use the overflow information to make perf more robust, IMO.
thanks
--
Ming Lei
next prev parent reply other threads:[~2012-02-17 5:24 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <1328578047.1724.17.camel@dave-Dell-System-XPS-L502X>
[not found] ` <CAMQu2gwfo5JXAqQaLUNs7C7J3TUhEO2zOcyD0Rk-D_O61Yrfag@mail.gmail.com>
[not found] ` <CAMsRxfLNBbQO5XF+EHJqNsnW+s=ay3mjSV5dh=sxdwzktUu03g@mail.gmail.com>
[not found] ` <CAMQu2gzfNAwtf1c6jrTZpfGMSqBgBrQKmFTeCFzbvMh9ESBDUg@mail.gmail.com>
[not found] ` <CAMsRxfKR1ODH56BtcUT5Dv6qOVEYGVheEcW9ugXsZmLKok==bg@mail.gmail.com>
[not found] ` <CAMQu2gwWt6oQPjxc1YHgOoxVkHdck_q78Xf==7ncwRj0_uS-JQ@mail.gmail.com>
[not found] ` <CAMQu2gyCPtbNfBa5V8ve2JORN+sDeAY+hvR0g0_9U354JgcuiA@mail.gmail.com>
[not found] ` <CAMsRxfJVEgVJoHcaPQbMg5mwz=tZUce4oibDUxtnevSF8bGY9g@mail.gmail.com>
[not found] ` <CAMQu2gxVUqDF2HkPD=QeX0N_tUd=9FTd+T+1LHCR0tsCgQxC8Q@mail.gmail.com>
[not found] ` <CAMsRxfJH+9MS=Svmh0BvDSb8xqsPe2HPobkeKcn8JO35npjhwQ@mail.gmail.com>
2012-02-08 2:24 ` oprofile and ARM A9 hardware counter Ming Lei
2012-02-15 16:38 ` Peter Zijlstra
2012-02-16 10:25 ` Ming Lei
2012-02-16 15:00 ` Will Deacon
2012-02-16 16:12 ` Ming Lei
2012-02-16 16:19 ` Peter Zijlstra
2012-02-16 16:37 ` Ming Lei
2012-02-16 18:08 ` Will Deacon
2012-02-17 5:24 ` Ming Lei [this message]
2012-02-17 10:26 ` Will Deacon
2012-02-20 3:19 ` Ming Lei
2012-02-20 9:44 ` Will Deacon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CACVXFVO4eexVK-S5TdRk2T6pyKRzpW1HxBQxo0Yfk_ziaO_U4Q@mail.gmail.com \
--to=ming.lei@canonical.com \
--cc=a.p.zijlstra@chello.nl \
--cc=b-cousson@ti.com \
--cc=dave.long@linaro.org \
--cc=eranian@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mans@mansr.com \
--cc=mingo@elte.hu \
--cc=santosh.shilimkar@ti.com \
--cc=will.deacon@arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).