[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Bug with clock nanoseconds



Yes, I agree :)

> -----Original Message-----
> From: Joel Sherrill [mailto:joel.sherrill at oarcorp.com]
> Sent: Tuesday, March 31, 2009 11:40 AM
> To: Aitor Viana
> Cc: Manuel Coutinho; rtems-users at rtems.com
> Subject: Re: Bug with clock nanoseconds
> 
> Aitor Viana wrote:
> > Yep,
> >
> > crystal clear. BTW, isClockInterruptPending() is not present in the
> > RTEMS distribution, it should be implemented for all BSPs I guess.
> >
> I repeat.  It is the responsibility of the clock tick device driver
> to reliably and correctly report the number of nanoseconds since
> the last time rtems_clock_tick was called.  If that was slightly
> greater than 1 tick ago, then it needs to return a number of nanoseconds
> slightly greater than 1 tick.
> 
> Any code modifications outside the device driver are not correct and
> will not address all cases of using this extension.
> 
> This might be a common issue with get nanoseconds handlers but it will
> NOT be addressed in this way.  Check the is pending bit or that the
> counter overflowed in the get nanoseconds handler and return the correct
> number.
> 
> --joel
> > Best regards,
> >
> > Aitor
> >
> >
> > On Tue, Mar 31, 2009 at 10:30 AM, Manuel Coutinho
> > <manuel.coutinho at edisoft.pt <mailto:manuel.coutinho at edisoft.pt>> wrote:
> >
> >     Hi. Thanks for testing in leon2.was just about to do that :)!
> >
> >
> >
> >     Basically, the test aims to see that the clock is always
> >     increasing. But the result is that, sometimes, it decreases.
> >
> >     This happens when a clock tick occurs.
> >
> >
> >
> >     The RTEMS algorithm to read the time consists of:
> >
> >
> >
> >     1. Disabling interrupts
> >
> >     2. Read how many clock interrupts occurred
> >
> >     3. Read nanoseconds elapsed
> >
> >     4. Enable interrupts
> >
> >
> >
> >     The problem occurs if an interrupt occurs between the 1 and 3
> >     steps: -> the number of clock ticks is NOT updated (because
> >     interrupts are disabled) and the nanoseconds field (which is read
> >     directly from the CPU registers) is restarted.
> >
> >
> >
> >     For example, the clock is 2 clock ticks and 99999 us (a clock tick
> >     is just about to occur). Suppose you disable interrupts, read 2
> >     clock ticks, then the CPU clock counter overflows and the
> >     nanoseconds is reset to 0 and you read 0 nanoseconds.
> >
> >     You end up reading 2 clock ticks and 0 nanoseconds when in fact
> >     you should read 3 clock ticks and 0 nanoseconds - you cannot stop
> >     the clock counter (nor do you wish to!).
> >
> >
> >
> >     Hope this explains the problem a little bit better :)
> >
> >
> >
> >     By the way, by glancing at the code of the remaining BSPs,
> >     specifically at the Clock_driver_nanoseconds_since_last_tick
> >     macro, I don't believe this is a problem just with SPARC. I don't
> >     see this problem address in any board. But, of course, I might be
> >     wrong since it was just a very brief look.
> >
> >
> >
> >     Kind regards
> >
> >     Manuel Coutinho
> >
> >
> >
> >     --------------------------------------------------------------------
> ----
> >
> >     *From:* aitorvs at googlemail.com <mailto:aitorvs at googlemail.com>
> >     [mailto:aitorvs at googlemail.com <mailto:aitorvs at googlemail.com>]
> >     *On Behalf Of *Aitor Viana
> >     *Sent:* Tuesday, March 31, 2009 8:31 AM
> >     *To:* Joel Sherrill
> >     *Cc:* Manuel Coutinho; rtems-users at rtems.com
> >     <mailto:rtems-users at rtems.com>
> >     *Subject:* Re: Bug with clock nanoseconds
> >
> >
> >
> >     Just tried in tsim-leon and happens the same.
> >
> >
> >
> >     I didn't completely understand what's the problem Manuel could you
> >     explain to me again? is early in the morning :D
> >
> >
> >
> >     Best regards,
> >
> >
> >
> >     Aitor
> >
> >
> >
> >     On Mon, Mar 30, 2009 at 6:49 PM, Joel Sherrill
> >     <joel.sherrill at oarcorp.com <mailto:joel.sherrill at oarcorp.com>>
> wrote:
> >
> >     Manuel Coutinho wrote:
> >     >
> >     > Hi
> >     >
> >     >
> >     >
> >     > We have discovered a bug of RTEMS relating to the clock. Even
> though
> >     > we only tested it for the ERC32 board (for now), we believe it is
> >     > common throughout all the targets.
> >     >
> >
> >     No you have discovered a bug in the ERC32's get nanoseconds since
> last
> >     tick handler.
> >     Apparently it doesn't recognize that is has been longer than a tick
> >     since the last clock
> >     tick.  It should determine that is has overflowed and return 1 clock
> >     tick + partial clock
> >     tick.
> >
> >     It is returning the wrong answer.
> >
> >
> >     >
> >     >
> >     > The test consists in checking if the time given by rtems_clock_get
> is
> >     > always increasing. The code is as follows:
> >     >
> >     >
> >     >
> >
> >     > .....
> >
> >
> >     ....
> >
> >     >
> >     > As you can see, the problem is raised when a clock tick occurs:
> the
> >     > time read in "new" is older than "old". This cannot happen since
> >     "new"
> >     > is updated AFTER "old"! This problem occurs if a clock interrupt
> is
> >     > triggered after the number of clock ticks have been read but
> BEFORE
> >     > the nanosecond field has been read. The resulting number of clock
> >     > ticks is "small" (should be plus one).
> >     >
> >     >
> >     >
> >     > We can use the interrupt pending CPU registers info to determine
> if,
> >     > while we are reading the number of clock ticks that occurred and
> the
> >     > nanoseconds, a clock interrupt could be pending.
> >     >
> >     >
> >     >
> >     > We do not have enough knowledge to say that this solution can be
> used
> >     > for all boards (determining if an interrupt is pending). In the
> SPARC
> >     > architecture, at least, it is possible. If it is possible in all
> >     > architectures, then a solution to RTEMS would be to change the
> >     code of
> >     > the _TOD_Get_uptime function to:
> >     >
> >     >
> >     >
> >     >
> >     >
> >     > void _TOD_Get_uptime(struct timespec *uptime) {
> >     >
> >     >     ISR_Level level;
> >     >
> >     >     struct timespec offset;
> >     >
> >     >     volatile uint32_t pending;
> >     >
> >     >
> >     >
> >     >     /* assume uptime checked by caller */
> >     >
> >     >
> >     >
> >     >     offset.tv_sec = 0;
> >     >
> >     >     offset.tv_nsec = 0;
> >     >
> >     >
> >     >
> >     >     _ISR_Disable(level);
> >     >
> >     >     *uptime = _TOD_Uptime;
> >     >
> >     >     if (_Watchdog_Nanoseconds_since_tick_handler)
> >     >
> >     >         offset.tv_nsec =
> >     (*_Watchdog_Nanoseconds_since_tick_handler)();
> >     >
> >     >
> >     >
> >     >     /* code added: */
> >     >
> >     >     pending = isClockInterruptPending();
> >     >
> >     >
> >     >
> >     >     _ISR_Enable(level);
> >     >
> >     >
> >     >
> >     >     /* code added */
> >     >
> >     >     /* if an interrupt occurred while interrupts were disabled
> >     and the
> >     > nanoseconds is too little */
> >     >
> >     >     /* means that a clock interrupt occurred BEFORE the
> nanoseconds
> >     > were read */
> >     >
> >     >     if (pending && offset.tv_nsec <
> >     > rtems_configuration_get_microseconds_per_tick() / 2) {
> >     >
> >     >         struct timespec clockTick = {0,
> >     > rtems_configuration_get_microseconds_per_tick()*1000};
> >     >
> >     >         _Timespec_Add_to(&offset, &clockTick); /* so add one clock
> >     > tick to the offset */
> >     >
> >     >     }
> >     >
> >     >
> >     >
> >     >     /* else, the clock tick occurred AFTER the nanoseconds were
> read,
> >     > so no problem */
> >     >
> >     >
> >     >
> >     >     _Timespec_Add_to(uptime, &offset);
> >     >
> >     > }
> >     >
> >     >
> >     >
> >     >
> >     >
> >     > At least, with these modifications, the test passes :)!
> >     >
> >     >
> >     >
> >     > Kind regards
> >     >
> >     > Manuel Coutinho
> >     >
> >
> >     --
> >     Joel Sherrill, Ph.D.             Director of Research & Development
> >     joel.sherrill at OARcorp.com        On-Line Applications Research
> >     Ask me about RTEMS: a free RTOS  Huntsville AL 35805
> >       Support Available             (256) 722-9985
> >
> >
> >     _______________________________________________
> >     rtems-users mailing list
> >     rtems-users at rtems.com <mailto:rtems-users at rtems.com>
> >     http://rtems.rtems.org/mailman/listinfo/rtems-users
> >
> >
> >
> >