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

gettimeofday seconds rollover problem?



On Feb 24, 2006, at 11:36 AM, Till Straumann wrote:

> Eric Norum wrote:
>>
>> 1) The _TOD_* variables must be declared as volatile since their   
>> values can change in a fashion (Deus ex machina) not apparent to   
>> compiler.
>> 2) The M68K_BARRIER macro, or its equivalent addition to the  
>> enable/ disable macros, is required to keep the optimizer from  
>> hoisting/ sinking any code beyond the interrupt disable/enable  
>> operations.   My  feeling is that it's clearer and simpler to just  
>> add the "memory",  "cc" to the disable/enable macros, but I'm not  
>> dogmatic about this.
>> These two steps seem very clear to me since they impart to the   
>> compiler exactly the information needed.  Both steps are necessary
>
> 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).

I'm not sure what you're trying to get across here.  It seems to me  
that any global variable shared amongst threads (or amongst threads  
and interrupt handlers) has to be declared volatile regardless of  
whether it's accessed from inside an inlined protection mechanism or  
accessed from anywhere else.

>
> 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.

Agreed.

>
>
> 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.
>
> -- Till

Excellent solution!
Who's going to commit this?

-- 
Eric Norum <norume at aps.anl.gov>
Advanced Photon Source
Argonne National Laboratory
(630) 252-4793