From: johnstul@us.ibm.com (john stultz)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] nomadik: prevent sched_clock() wraparound
Date: Tue, 16 Nov 2010 12:53:49 -0800 [thread overview]
Message-ID: <1289940829.3860.22.camel@localhost.localdomain> (raw)
In-Reply-To: <1289898690-29910-1-git-send-email-linus.walleij@stericsson.com>
On Tue, 2010-11-16 at 10:11 +0100, Linus Walleij wrote:
> +unsigned long long sched_clock(void)
> +{
> + unsigned long long v;
> +
> + if (unlikely(!mtu_base))
> + return 0;
> +
> + v = cnt32_to_63(-readl(mtu_base + MTU_VAL(0)));
> + /* The highest bit is not valid */
> + v &= 0x7FFFFFFFFFFFFFFFLLU;
> + return clocksource_cyc2ns(v, nmdk_clksrc.mult, nmdk_clksrc.shift);
I suspect this will still break.
The cycle value being passed as v is likely to be large, and the
clocksource mult and shift are calculated to be as large as possible
without overflowing when given only a few seconds worth of cycles.
So its very likely that after a few seconds of running (or even less,
with the 32_to_63 conversion), the cyc2ns function will overflow, as
v*mult will be greater then 64bits.
So I'd advise *not* using the mult/shift pair calculated for
timekeeping, and possibly not using the clocksource_cyc2ns function (as
I'm likely to make that private to timekeeping at somepoint in the
future). Instead calculate a mult/shift pair using a smaller shift
value, so the overflow point will be much more rare.
For instance, on x86, the TSC based sched_clock uses a shift value of
10, but the TSC clocksource uses a shift value of ~30.
> +}
> +
> +/* Just kick sched_clock every so often */
> +static void cnt32_to_63_keepwarm(unsigned long data)
> {
> - return clocksource_cyc2ns(nmdk_clksrc.read(
> - &nmdk_clksrc),
> - nmdk_clksrc.mult,
> - nmdk_clksrc.shift);
> + mod_timer(&cnt32_to_63_keepwarm_timer, round_jiffies(jiffies + data));
> + (void) sched_clock();
> +}
Also, just FYI: This can be problematic on PREEMPT_RT systems, where
timers can be delayed by high priority processes hogging the cpu.
thanks
-john
next prev parent reply other threads:[~2010-11-16 20:53 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-11-16 9:11 [PATCH] nomadik: prevent sched_clock() wraparound Linus Walleij
2010-11-16 20:53 ` john stultz [this message]
2010-11-16 22:15 ` Linus Walleij
2010-11-16 22:31 ` Russell King - ARM Linux
2010-11-17 8:34 ` Linus Walleij
2010-11-17 8:38 ` Russell King - ARM Linux
2010-11-17 15:15 ` Nicolas Pitre
2010-11-17 9:26 ` Linus Walleij
2010-11-17 15:04 ` Nicolas Pitre
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=1289940829.3860.22.camel@localhost.localdomain \
--to=johnstul@us.ibm.com \
--cc=linux-arm-kernel@lists.infradead.org \
/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