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

Bug with clock nanoseconds



This solution is actually very robust. Only if the number of microseconds
per tick is very small (~10 us) should you get problems. This is, if two
clock interrupts can occur while interrupts are disabled. But we consider
this to be impossible.

 

Remember that the clock ticks and nanoseconds are read while interrupts are
disabled. So after the disabling of interrupts at most only one clock tick
could occur.

 

The _TOD_Tickle_ticks only states that one more clock tick occurred. The
nanoseconds field is not updated. Haven't tested, but don't believe that
just by commenting those lines the problem will be solved.

 

 

  _____  

From: aitorvs at googlemail.com [mailto:aitorvs at googlemail.com] On Behalf Of
Aitor Viana
Sent: Tuesday, March 31, 2009 10:57 AM
To: Manuel Coutinho
Cc: Joel Sherrill; rtems-users at rtems.com
Subject: Re: Bug with clock nanoseconds

 

The current solucion also gets a problem because really depends on the
CONFIGURE_MICROSECONDS_PER_TICK parameter. If this value is small enough it
is possible to get another overrun of tv_nsec while executing

 

 /* 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 */

    }

 

I see that _TOD_Uptime is updated every clock tick in
cpukit/score/src/coretodtickle.c (see bellow) with microseconds precision.
It is not enough with the next patch?

 

=== modified file 'cpukit/score/src/coretodgetuptime.c'

--- cpukit/score/src/coretodgetuptime.c 2009-03-31 07:56:38 +0000

+++ cpukit/score/src/coretodgetuptime.c 2009-03-31 09:49:25 +0000

@@ -20,6 +20,7 @@

 #include <rtems/score/isr.h>

 #include <rtems/score/timespec.h>

 #include <rtems/score/tod.h>

+#include <rtems/config.h>

 

 /*

  *  _TOD_Get_uptime

@@ -46,8 +47,8 @@

 

   _ISR_Disable( level );

     *uptime = _TOD_Uptime;

-    if ( _Watchdog_Nanoseconds_since_tick_handler )

-      offset.tv_nsec = (*_Watchdog_Nanoseconds_since_tick_handler)();

+//    if ( _Watchdog_Nanoseconds_since_tick_handler )

+//      offset.tv_nsec = (*_Watchdog_Nanoseconds_since_tick_handler)();

 

   _ISR_Enable( level );

 

 

 

On Tue, Mar 31, 2009 at 11:22 AM, Manuel Coutinho
<manuel.coutinho at edisoft.pt> wrote:

I'm not sure that the isClockInterruptPending() is a function that can be
available to all BSPs. Some CPU might not support this. I don't know :(. 

In either case, here's the function for the ERC32 board (I know, the names
can be improved):

 

#define INTERRUPT_PENDING_ADDRESS 0x01F80048

 

/**

 * RTC interrupt level

 **/

#define RTC_ISR_INTERRUPT_LEVEL 13

 

#define RTC_INTERRUPT_PENDING_MASK (1 << RTC_ISR_INTERRUPT_LEVEL)

 

uint32_t isClockInterruptPending() {

    uint32_t *pointer_pending = (uint32_t *) INTERRUPT_PENDING_ADDRESS;

    return pending (*pointer_pending ) & RTC_INTERRUPT_PENDING_MASK;

}

 

 

  _____  

From: aitorvs at googlemail.com [mailto:aitorvs at googlemail.com] On Behalf Of
Aitor Viana
Sent: Tuesday, March 31, 2009 10:09 AM
To: Manuel Coutinho
Cc: Joel Sherrill; rtems-users at rtems.com


Subject: Re: Bug with clock nanoseconds

 

Yep,

 

crystal clear. BTW, isClockInterruptPending() is not present in the RTEMS
distribution, it should be implemented for all BSPs I guess.

 

Best regards,

 

Aitor

 

On Tue, Mar 31, 2009 at 10:30 AM, Manuel Coutinho
<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] On Behalf Of
Aitor Viana
Sent: Tuesday, March 31, 2009 8:31 AM
To: Joel Sherrill
Cc: Manuel Coutinho; 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>
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
http://rtems.rtems.org/mailman/listinfo/rtems-users

 

 

 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://rtems.rtems.org/pipermail/rtems-users/attachments/20090331/71027894/attachment-0001.html