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

gettimeofday seconds rollover problem?



Hello All,

On Friday 24 February 2006 18:36, Till Straumann wrote:
> This is not true. Only the 'volatile' declaration of global variables
> that are accessed during any kind of 'inlined' protection mechanism
> (be it interrupts, thread-dispatching, mutex, ...)  is necessary
> (and sufficient).

Only notice: this is still not SMP safe, etc. If it should be really safe
some other barriers or mutexes mechanisms are required to be explicitly
stated around variable accesses.
On UP calling of non-inlined mutex function is enough. On real SMP
mutex code has to contain barriers anyway and result is safe again.
If mutex is inlined, than it still should contain at least local
"memory" barrier anyway.

> Nobody has addressed my point # 1: adding a memory barrier
> to rtems_interrupt_disable/enable does not address thread-dispatch-disable
> protected code. If we're going to add this safe-guard, it should
> be introduced to thread_dispatch_disable/enable, too.
>
>
> BTW: I don't think we should modify all the ports! We should just
>       change score/isr.h:
>
>       #define _ISR_Disable( _level ) \
>               _CPU_ISR_Disable( _level ) \
>               asm volatile("":::"memory")
>
>       #define _ISR_Enable( _level ) \
>               asm volatile("":::"memory") \
>               _CPU_ISR_Enable()
>
>       #define _ISR_Flash( _level ) \
>               asm volatile("":::"memory") \
>               _CPU_ISR_Flash( _level ) \
>               asm volatile("":::"memory")
>
> _Thread_Disable_dispatch() should be treated alike.

Great.

I have only concern about syntactical correctness of this construct
and behavior in if() x; else y; flow. I would modify it next way

      #define _ISR_Disable( _level ) \
              do { _CPU_ISR_Disable( _level ); \
              asm volatile("":::"memory"); } while(0)

      #define _ISR_Enable( _level ) \
              do { asm volatile("":::"memory"); \
              _CPU_ISR_Enable(); } while(0)

      #define _ISR_Flash( _level ) \
              do { asm volatile("":::"memory"); \
              _CPU_ISR_Flash( _level ); \
              asm volatile("":::"memory"); } while(0)

By the way, I really like that RTEMS is so precisely layered
(seems to me as military culture heritage :-), because possibility
so simply introduce such patch to all ports is great.

Best wishes

             Pavel Pisa