[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <200707020942.10744.david-b@pacbell.net>
Date: Mon, 2 Jul 2007 09:42:10 -0700
From: David Brownell <david-b@...bell.net>
To: Marc Pignat <marc.pignat@...s.ch>
Cc: tglx@...utronix.de, linux-kernel@...r.kernel.org,
"Remy Bohmer" <l.pinguin@...il.com>
Subject: Re: [BUG] clockevents : clockevent_delta2ns usage
On Monday 02 July 2007, Marc Pignat wrote:
> Hello all!
>
> We're currently working with David Brownell and Remy Bohmer on implementation
> of clocksource/events on the at91rm9200 processor (arm arch).
>
> While debugging our implementation, we came about a rounding problem, in the
> setting of min_delta_ns field, and I think it can be in almost all clock_event_device.
>
> Almost all clock_event_device initialize the field min_delta_ns this way:
> clkevt32k.mult = div_sc(CLOCK_TICK_RATE, NSEC_PER_SEC, clkevt32k.shift);
> clkevt32k.max_delta_ns = clockevent_delta2ns(0x000fffff, &clkevt32k);
> clkevt32k.min_delta_ns = clockevent_delta2ns(1, &clkevt32k);
>
> The clockevent_delta2ns function rounds the result down, so the rounded min_delta_ns
> can be smaller than the real min_delta_ns.
For concrete numbers: CLOCK_TICK_RATE = 32768, making 30517.58 nsec
per tick But min_delta_ns becomes 30517. (And the shift is 32 bits;
in-kernel docs could stand to include guidance about how to choose
a good shift value.)
> When clockevents_program_event is called, this rounding problem shows up:
> ...
> if (delta < dev->min_delta_ns)
> delta = dev->min_delta_ns;
>
> clc = delta * dev->mult;
> clc >>= dev->shift;
>
> return dev->set_next_event((unsigned long) clc, dev)
>
> When delta is to small, min_delta_ns will be used, and with a wrong rounding,
> will call set_next_event with a value smaller than the expected, and
> set_next_event will fail.
Will be passed a value of zero, that is -- rather troublesome.
> This will result in an infinite loop when clockevents_program_event is called
> from tick_program_event with a too short delay.
Unless zero ticks gets special cased to return -ETIME and then the
backup strategies kick in, eventually asking for more ticks...
> In our driver, we fixed this problem using that:
> - clkevt32k.min_delta_ns = clockevent_delta2ns(1, &clkevt32k);
> + clkevt32k.min_delta_ns = clockevent_delta2ns(1, &clkevt32k)+1;
>
> I think other clock_event_device can have the same problem.
>
> Regards
>
> Marc
>
-
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/
Powered by blists - more mailing lists