linux-mips
[Top] [All Lists]

Re: Systematic jiffie slip in arch/mips/kernel/timer.c:timer_interrupt

To: David Wuertele <dave-gnus@bfnet.com>
Subject: Re: Systematic jiffie slip in arch/mips/kernel/timer.c:timer_interrupt
From: Jun Sun <jsun@mvista.com>
Date: Wed, 28 Jan 2004 12:58:15 -0800
Cc: linux-mips@linux-mips.org, jsun@mvista.com
In-reply-to: <m3ad47zv0y.fsf@bfnet.com>; from dave-gnus@bfnet.com on Wed, Jan 28, 2004 at 12:18:53PM -0800
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <m3ad47zv0y.fsf@bfnet.com>
Sender: linux-mips-bounce@linux-mips.org
User-agent: Mutt/1.2.5i
On Wed, Jan 28, 2004 at 12:18:53PM -0800, David Wuertele wrote:
> arch/mips/kernel/timer.c:timer_interrupt() says the following:
> 
>   void timer_interrupt(int irq, void *dev_id, struct pt_regs *regs)
>   {
>           if (mips_cpu.options & MIPS_CPU_COUNTER) {
>                   unsigned int count;
> 
>                   /*
>                    * The cycle counter is only 32 bit which is good for about
>                    * a minute at current count rates of upto 150MHz or so.
>                    */
>                   count = read_32bit_cp0_register(CP0_COUNT);
>                   timerhi += (count < timerlo);   /* Wrap around */
>                   timerlo = count;
> 
>                   /*
>                    * set up for next timer interrupt - no harm if the machine
>                    * is using another timer interrupt source.
>                    * Note that writing to COMPARE register clears the 
> interrupt
>                    */
>                   write_32bit_cp0_register (CP0_COMPARE,
>                                             count + cycles_per_jiffy);
>           }
>   <snip>...
> 
> This can't be right: it sets the cycle counter to go off
> cycles_per_jiffy cycles after the timer_interrupt finally gains the
> CPU.  Let's say that the latency between the interrupt being raised
> and timer_interrupt() getting called is LATENCY.  Then the period with
> which this interrupt is raised will actually be cycles_per_jiffy +
> LATENCY, which is a longer period than one jiffy!  So the jiffy
> counter will always run slow.  On my MIPS system I measured the
> average latency of this routine being called, and it averaged around
> 500 cycles!  (Note: we have a very active PIO device that runs with
> interrupts turned off part of the time)
> 
> It seems to me that the jiffy counter could be made to keep better
> average time in one (or both) of two ways:
> 
> 1) Accumulate latencies as "fractional jiffies", and increment the
>    jiffie counter accordingly, like so:
> 
>   void timer_interrupt(int irq, void *dev_id, struct pt_regs *regs) {
>      static unsigned int expect_interrupt_at, frac_jiffy_accumulator;
>      if (mips_cpu.options & MIPS_CPU_COUNTER) {
>       unsigned int count, lateby;
>         count = read_32bit_cp0_register(CP0_COUNT);
>       timerhi += (count < timerlo);   /* Wrap around */
>       timerlo = count;
> 
>         /* Find out how much we're LATE BY */
>       if (count < expect_interrupt_at)
>         lateby = (unsigned int) ((u64) 0x100000000 - (u64) 
> expect_interrupt_at + (u64) count);
>       else
>         lateby = count - expect_interrupt_at;
>       frac_jiffy_accumulator += lateby;
>       while (frac_jiffy_accumulator > cycles_per_jiffy) {
>           /* We've accumulated at least a jiffie of lateness.  Catch up. */
>         (*(unsigned long *)&jiffies)++;
>         frac_jiffy_accumulator -= cycles_per_jiffy;
>       }
> 
>       expect_interrupt_at = count + cycles_per_jiffy;
>       write_32bit_cp0_register (CP0_COMPARE, expect_interrupt_at);
>      }
> 
> 2) Set the clock to actually go off sooner, in order to better
>    approximate a cycle timer:
> 
>   void timer_interrupt(int irq, void *dev_id, struct pt_regs *regs) {
>      static unsigned int expect_interrupt_at;
>      if (mips_cpu.options & MIPS_CPU_COUNTER) {
>       unsigned int count;
>         count = read_32bit_cp0_register(CP0_COUNT);
>       timerhi += (count < timerlo);   /* Wrap around */
>       timerlo = count;
> 
>       expect_interrupt_at += cycles_per_jiffy;
>         /* Add some logic here for dealing with latencies that are
>            greater than a whole jiffy */
>       write_32bit_cp0_register (CP0_COMPARE, expect_interrupt_at);
>      }
> 
> I think option (1) above is better, because it doesn't actually change
> the interrupt frequency compared to the status quo, whereas option (2)
> actualy makes the timer interrupt more regular at the expense of more
> interrupts and more overhead.
> 
> Comments anyone?
> 

Use recent kernels.  The bug is fixed almost a year ago.

Jun

<Prev in Thread] Current Thread [Next in Thread>