gettimeofday seconds rollover problem?

[Till Straumann]

Eric Norum wrote:
> On Feb 24, 2006, at 1:16 AM, Andrew Sinclair wrote:
> 
>>
>> 1) interesting...
>> 2) I think an alternative here is adding all the registers to the
>> clobber list in the "barrier", we can force the compiler to reload any
>> data into registers, and perform stores within the critical sections.
>>
>> 3) Yes, on second thoughts I think you have something here. It is  safer
>> as one single asm volatile statement.
>>
>>
>> For example on the Coldfire...
>>
>> #define m68k_disable_interrupts( _level ) \
>>     do { register unsigned32   _tmpsr = 0x0700; \
>>          asm __volatile__ ( "move.w %%sr,%0\n\t" \
>>                  "or.l   %0,%1\n\t" \
>>                  "move.w %1,%%sr" \
>>                  : "=d" (_level), "=d"(_tmpsr) : "1"(_tmpsr) \
>>                  :"cc","d0","d1","d2","d3","d4","d5", \
>>                 "a0","a1","a2","a3","a4","a5","a6","memory" ); \
>>     } while( 0 )
>>
>> #define m68k_enable_interrupts( _level ) \
>>     do { asm __volatile__ ( "move.w  %0,%%sr " : : "d" (_level) \
>>        :"cc","d0","d1","d2","d3","d4","d5", \
>>        "a0","a1","a2","a3","a4","a5","a6","memory" ); \
>>     } while( 0 )
>>
>> #define m68k_flash_interrupts( _level ) \
>>    do { register unsigned32 _tmpsr = 0x0700; \
>>         asm __volatile__ ( "move.w %2,%%sr\n\t" \
>>                "or.l   %2,%1\n\t" \
>>                "move.w %1,%%sr" \
>>                : "=d"(_tmpsr) : "0"(_tmpsr), "d"(_level) \
>>                 :"cc","d0","d1","d2","d3","d4","d5", \
>>                 "a0","a1","a2","a3","a4","a5","a6","memory" ); \
>>    } while( 0 )
> 
> 
> I *really* don't like this.
> Given that the assembly instructions in question don't affect d0-d5/ 
> a0-a6 why would you want to place them in the clobber list?
> 
> To summarize my position:
> 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).

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.

-- Till

>  and together 
> they should be sufficient.
>