All of lore.kernel.org
 help / color / mirror / Atom feed
From: nico@fluxnic.net (Nicolas Pitre)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] nomadik: prevent sched_clock() wraparound
Date: Wed, 17 Nov 2010 10:15:01 -0500 (EST)	[thread overview]
Message-ID: <alpine.LFD.2.00.1011171004170.6448@xanadu.home> (raw)
In-Reply-To: <20101117083842.GA32476@n2100.arm.linux.org.uk>

On Wed, 17 Nov 2010, Russell King - ARM Linux wrote:

> On Wed, Nov 17, 2010 at 09:34:56AM +0100, Linus Walleij wrote:
> > And the Tegra code in mach-tegra/timer.c:
> > 
> > static cycle_t tegra_clocksource_read(struct clocksource *cs)
> > {
> >         return cnt32_to_63(timer_readl(TIMERUS_CNTR_1US));
> > }
> > 
> > This just won't fly for long either, the mult for the clocksource is
> > usually something in the order of 20.
> 
> I thought John/Thomas said that using cnt32_to_63 in there wasn't a good
> idea - sounds like it needs to be killed.  Colin?

Exact.  The cnt32_to_63() should not be used for clock source as this is 
lying to the clock source core where there is already all the code 
needed to deal with limited hardware timer ranges with all the required 
accuracy.

Furthermore, the above usage of cnt32_to_63() is buggy.  It is 
explicitly mentioned in the cnt32_to_63() documentation that the top bit 
(the 64th one) is undefined and must be cleared by the caller as it 
_does_ contain a random value to the caller.  This is not done by 
cnt32_to_63() directly because its returned value often has to be 
multiplied by a scaling factor and therefore the 64th bit masking can be 
done implicitly by that multiplication, saving on a runtime instruction 
(remember this was designed for sched_clock() usage and the goal there 
is to be fast).


Nicolas

  reply	other threads:[~2010-11-17 15:15 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
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 [this message]
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=alpine.LFD.2.00.1011171004170.6448@xanadu.home \
    --to=nico@fluxnic.net \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.