[RTEMS Project] #2180: _TOD_Get_with_nanoseconds() is broken on SMP

[RTEMS trac]

#2180: _TOD_Get_with_nanoseconds() is broken on SMP
-----------------------------+------------------------------
 Reporter:  sebastian.huber  |       Owner:  sebastian.huber
     Type:  defect           |      Status:  assigned
 Priority:  normal           |   Milestone:  5.0
Component:  cpukit           |     Version:  4.11
 Severity:  normal           |  Resolution:
 Keywords:                   |
-----------------------------+------------------------------
Changes (by sebastian.huber):

 * owner:  joel.sherrill => sebastian.huber
 * status:  new => assigned
 * milestone:  4.11 => 5.0


Old description:

> We have
>
> Timestamp_Control *_TOD_Get_with_nanoseconds(
>   Timestamp_Control *snapshot,
>   const Timestamp_Control *clock
> )
> {
>   TOD_Control      *tod = &_TOD;
>   ISR_lock_Context  lock_context;
>   Timestamp_Control offset;
>   Timestamp_Control now;
>   uint32_t          nanoseconds;
>
>   _TOD_Acquire( tod, &lock_context );
>     nanoseconds = ( *tod->nanoseconds_since_last_tick )();
>     now = *clock;
>   _TOD_Release( tod, &lock_context );
>
>   _Timestamp_Set( &offset, 0, nanoseconds );
>   _Timestamp_Add_to( &now, &offset );
>
>   *snapshot = now;
>
>   return snapshot;
> }
>
> and
>
> void _TOD_Tickle_ticks( void )
> {
>   TOD_Control       *tod = &_TOD;
>   ISR_lock_Context   lock_context;
>   Timestamp_Control  tick;
>   uint32_t           nanoseconds_per_tick;
>
>   nanoseconds_per_tick = rtems_configuration_get_nanoseconds_per_tick();
>
>   /* Convert the tick quantum to a timestamp */
>   _Timestamp_Set( &tick, 0, nanoseconds_per_tick );
>
>   /* Update the counter of ticks since boot */
>   _Watchdog_Ticks_since_boot += 1;
>
>   _TOD_Acquire( tod, &lock_context );
>
>   /* Update the uptime */
>   _Timestamp_Add_to( &tod->uptime, &tick );
>
>   /* Update the current TOD */
>   _Timestamp_Add_to( &tod->now, &tick );
>
>   _TOD_Release( tod, &lock_context );
>
>   _TOD.seconds_trigger += nanoseconds_per_tick;
>   if ( _TOD.seconds_trigger >= 1000000000UL ) {
>     _TOD.seconds_trigger -= 1000000000UL;
>     _Watchdog_Tickle_seconds();
>   }
> }
>
> and (standard Clock driver)
>
> #if defined(BSP_FEATURE_IRQ_EXTENSION) || \
>     (CPU_SIMPLE_VECTORED_INTERRUPTS != TRUE)
> void Clock_isr(void *arg)
> {
> #else
> rtems_isr Clock_isr(rtems_vector_number vector);
> rtems_isr Clock_isr(
>   rtems_vector_number vector
> )
> {
> #endif
>   /*
>    *  Accurate count of ISRs
>    */
>   Clock_driver_ticks += 1;
>
>   #if CLOCK_DRIVER_USE_FAST_IDLE
>     do {
>       rtems_clock_tick();
>     } while (
>       _Thread_Heir == _Thread_Executing
>         && _Thread_Executing->Start.entry_point
>           == rtems_configuration_get_idle_task()
>     );
>
>     Clock_driver_support_at_tick();
>     return;
>   #else
>     /*
>      *  Do the hardware specific per-tick action.
>      *
>      *  The counter/timer may or may not be set to automatically reload.
>      */
>     Clock_driver_support_at_tick();
>
>     #if CLOCK_DRIVER_ISRS_PER_TICK
>       /*
>        *  The driver is multiple ISRs per clock tick.
>        */
>       if ( !Clock_driver_isrs ) {
>         rtems_clock_tick();
>
>         Clock_driver_isrs = CLOCK_DRIVER_ISRS_PER_TICK;
>       }
>       Clock_driver_isrs--;
>     #else
>       /*
>        *  The driver is one ISR per clock tick.
>        */
>       rtems_clock_tick();
>     #endif
>   #endif
> }
>

> Suppose we are between Clock_driver_support_at_tick() and
> _TOD_Tickle_ticks().  Now call _TOD_Get_with_nanoseconds() on another
> processor.  With most nanoseconds extensions we observe now a serviced
> hardware clock interrupt and the old _TOD.uptime value.

New description:

 We have

 Timestamp_Control *_TOD_Get_with_nanoseconds(
   Timestamp_Control *snapshot,
   const Timestamp_Control *clock
 )
 {
   TOD_Control      *tod = &_TOD;
   ISR_lock_Context  lock_context;
   Timestamp_Control offset;
   Timestamp_Control now;
   uint32_t          nanoseconds;

   _TOD_Acquire( tod, &lock_context );
     nanoseconds = ( *tod->nanoseconds_since_last_tick )();
     now = *clock;
   _TOD_Release( tod, &lock_context );

   _Timestamp_Set( &offset, 0, nanoseconds );
   _Timestamp_Add_to( &now, &offset );

   *snapshot = now;

   return snapshot;
 }

 and

 void _TOD_Tickle_ticks( void )
 {
   TOD_Control       *tod = &_TOD;
   ISR_lock_Context   lock_context;
   Timestamp_Control  tick;
   uint32_t           nanoseconds_per_tick;

   nanoseconds_per_tick = rtems_configuration_get_nanoseconds_per_tick();

   /* Convert the tick quantum to a timestamp */
   _Timestamp_Set( &tick, 0, nanoseconds_per_tick );

   /* Update the counter of ticks since boot */
   _Watchdog_Ticks_since_boot += 1;

   _TOD_Acquire( tod, &lock_context );

   /* Update the uptime */
   _Timestamp_Add_to( &tod->uptime, &tick );

   /* Update the current TOD */
   _Timestamp_Add_to( &tod->now, &tick );

   _TOD_Release( tod, &lock_context );

   _TOD.seconds_trigger += nanoseconds_per_tick;
   if ( _TOD.seconds_trigger >= 1000000000UL ) {
     _TOD.seconds_trigger -= 1000000000UL;
     _Watchdog_Tickle_seconds();
   }
 }

 and (standard Clock driver)

 #if defined(BSP_FEATURE_IRQ_EXTENSION) || \
     (CPU_SIMPLE_VECTORED_INTERRUPTS != TRUE)
 void Clock_isr(void *arg)
 {
 #else
 rtems_isr Clock_isr(rtems_vector_number vector);
 rtems_isr Clock_isr(
   rtems_vector_number vector
 )
 {
 #endif
   /*
    *  Accurate count of ISRs
    */
   Clock_driver_ticks += 1;

   #if CLOCK_DRIVER_USE_FAST_IDLE
     do {
       rtems_clock_tick();
     } while (
       _Thread_Heir == _Thread_Executing
         && _Thread_Executing->Start.entry_point
           == rtems_configuration_get_idle_task()
     );

     Clock_driver_support_at_tick();
     return;
   #else
     /*
      *  Do the hardware specific per-tick action.
      *
      *  The counter/timer may or may not be set to automatically reload.
      */
     Clock_driver_support_at_tick();

     #if CLOCK_DRIVER_ISRS_PER_TICK
       /*
        *  The driver is multiple ISRs per clock tick.
        */
       if ( !Clock_driver_isrs ) {
         rtems_clock_tick();

         Clock_driver_isrs = CLOCK_DRIVER_ISRS_PER_TICK;
       }
       Clock_driver_isrs--;
     #else
       /*
        *  The driver is one ISR per clock tick.
        */
       rtems_clock_tick();
     #endif
   #endif
 }


 Suppose we are between Clock_driver_support_at_tick() and
 _TOD_Tickle_ticks().  Now call _TOD_Get_with_nanoseconds() on another
 processor.  With most nanoseconds extensions we observe now a serviced
 hardware clock interrupt and the old _TOD.uptime value.

--

--
Ticket URL: <http://devel.rtems.org/ticket/2180#comment:2>
RTEMS Project <http://www.rtems.org/>
RTEMS Project