gettimeofday seconds rollover problem?
Thomas Doerfler
Thomas.Doerfler at imd-systems.de
Thu Feb 23 09:11:23 UTC 2006
Hi,
wow, we really got a problem here. According to Andrews quoting, GCC can
move around "rtems_disable_interrupt" and "rtems_enable_interrupt" in
the current implementation (a macro that contains "volatile" asm), and
therefore the usual way to block a short critical section does not work
reliable.
Even making the _TOD* vairables volatile (which should be done anyway, I
think), will not fix this problem, because their usage might still be
moved out of the irq-disabled section.
One solution might be to reimplement rtems_dis/enable_interrupt as a
(non-inline) function. Then GCC will have no idea what is going on in
these functions, they might even change e.g. the global _TOD* variables
(even when they are not volatile), and therefore GCC would be forced to
read these variables exactly at the location specified in source code.
The advantage of this solution would be, that ALL critical sections
useing rtems_dis/enable_interrupt would be save again. The disadvantage
would be, that these statements will be executed slower due to the
subroutine call and the register save/restore operations needed.
Any comments on my sugestion?
wkr,
Thomas.
Andrew Sinclair wrote:
> It seems to me that the assembler statements in m68k.h for enabling and
> disabling interrupts are never going to work with gcc optimisation.
>
> Have a look at this article.
>
> http://gcc.gnu.org/onlinedocs/gcc/Extended-Asm.html#Extended-Asm
>
> Quoting
> "
> The volatile keyword indicates that the instruction has important
> side-effects. GCC will not delete a volatile asm if it is reachable.
> (The instruction can still be deleted if GCC can prove that control-flow
> will never reach the location of the instruction.) Note that even a
> volatile asm instruction can be moved relative to other code, including
> across jump instructions. For example, on many targets there is a system
> register which can be set to control the rounding mode of floating point
> operations. You might try setting it with a volatile asm, like this
> PowerPC example:
>
> asm volatile("mtfsf 255,%0" : : "f" (fpenv));
> sum = x + y;
>
> This will not work reliably, as the compiler may move the addition back
> before the volatile asm. To make it work you need to add an artificial
> dependency to the asm referencing a variable in the code you don't want
> moved, for example:
>
> asm volatile ("mtfsf 255,%1" : "=X"(sum): "f"(fpenv));
> sum = x + y;
> "
>
> Nice trick, but how can we make it work for our situation?
>
> I am looking at setting values (eg. "memory") in the clobber field to
> prevent movement of the instruction.
>
> -----Original Message-----
> From: Ian Caddy [mailto:ianc at goanna.iinet.net.au]
> Sent: Thursday, 23 February 2006 3:01 PM
> To: Andrew Sinclair
> Cc: Chris Johns; Joel Sherrill; rtems-users
> Subject: Re: gettimeofday seconds rollover problem?
>
> Hmmm... looks like I will have to look deeper into our application. I
> had a quick look through this morning, but on some functions such as the
> one you mentioned, the optimisation makes it a bit hard to follow on a
> quick run through.
>
> I confirm the problem for rtems_clock_get, although in our case, one of
> the parameters is inside the disable interrupts, while the other part is
> not. This is just as bad as both not being in there. Again, I would
> say that changing tmp to be a volatile will fix the problem.
>
> Throughout the rest of today I will try to look thoroughly through our
> application to see if I can find anymore and compile a list.
>
> regards,
>
> Ian Caddy
>
>
> Andrew Sinclair wrote:
>
>>It looks like a similar issue exists with rtems_clock_get.
>>
>>I just wonder where else it is an issue...
>>
>>
>>-----Original Message-----
>>From: Ian Caddy [mailto:ianc at goanna.iinet.net.au]
>>Sent: Thursday, 23 February 2006 1:46 PM
>>To: Chris Johns
>>Cc: Joel Sherrill; Andrew Sinclair; rtems-users
>>Subject: Re: gettimeofday seconds rollover problem?
>>
>>On further analysis, it looks like the compiler is optimising out the
>>two local variables, seconds and microseconds in gettimeofday.
>>
>>The reason for this is that the interrupt macros do not use these
>>variables and so the compiler has no reason to ensure they are created
>
>
>>in the interrupt disabled state, since the compiler does not know that
>
>
>>was what we were trying to do.
>>
>>The simple solution is to make these two local variables volatile
>>which ensures no optimisation of them:
>>
>>int gettimeofday(
>> struct timeval *tp,
>> struct timezone *tzp
>>)
>>{
>> rtems_interrupt_level level;
>> volatile rtems_unsigned32 seconds;
>> volatile rtems_unsigned32 microseconds;
>>
>>
>>This then creates the correct code, when -O3 is used:
>>
>> rtems_interrupt_disable(level);
>> 8083f70: 203c 0000 0700 movel #1792,%d0
>> 8083f76: 40c1 movew %sr,%d1
>> 8083f78: 8081 orl %d1,%d0
>> 8083f7a: 46c0 movew %d0,%sr
>> seconds = _TOD_Seconds_since_epoch;
>> 8083f7c: 2279 0810 f4e2 moveal 810f4e2
>><_TOD_Seconds_since_epoch>,%a1
>> 8083f82: 2f49 0004 movel %a1,%sp@(4)
>> microseconds = _TOD_Current.ticks;
>> 8083f86: 2eb9 0810 f5c6 movel 810f5c6 <_TOD_Current+0x18>,%sp@
>> rtems_interrupt_enable(level);
>> 8083f8c: 46c1 movew %d1,%sr
>>
>> tp->tv_sec = seconds + POSIX_TIME_SECONDS_1970_THROUGH_1988;
>> 8083f8e: 222f 0004 movel %sp@(4),%d1
>> 8083f92: 0681 21da e500 addil #567993600,%d1
>> 8083f98: 2081 movel %d1,%a0@
>>
>>
>>I hope this helps.
>>
>>regards,
>>
>>Ian Caddy
>>
>>
>>
>>
>>
>>Chris Johns wrote:
>>
>>>Joel Sherrill wrote:
>>>
>>>>The seconds and nanoseconds values are grabbed/computed with
>>>>interrupts disabled so I don't think that would be the problem.
>>>
>>>No they are not and it is the problem. The interrupts are being
>>>enable
>>
>>>just after they are being disabled. Here is the code:
>>>
>>>00000000 <gettimeofday>:
>>>gettimeofday():
>>>../../../../../head/cpukit/libcsupport/src/__gettod.c:50
>>> 0: 4e56 0000 linkw %fp,#0
>>> 4: 206e 0008 moveal %fp@(8),%a0
>>>../../../../../head/cpukit/libcsupport/src/__gettod.c:55
>>> 8: 4a88 tstl %a0
>>> a: 6736 beqs 42 <gettimeofday+0x42>
>>>../../../../../head/cpukit/libcsupport/src/__gettod.c:67
>>> c: 203c 0000 0700 movel #1792,%d0
>>> 12: 2200 movel %d0,%d1
>>> 14: 40c0 movew %sr,%d0
>>> 16: 8280 orl %d0,%d1
>>> 18: 46c1 movew %d1,%sr
>>> ^^^^^^^^^^^^^ DISABLED
>>>../../../../../head/cpukit/libcsupport/src/__gettod.c:70
>>> 1a: 46c0 movew %d0,%sr
>>> ^^^^^^^^^^^^^ ENABLED
>>>
>>>The remainder of the code is the reading of the time:
>>>
>>>../../../../../head/cpukit/libcsupport/src/__gettod.c:72
>>> 1c: 2039 0000 0000 movel 0 <gettimeofday>,%d0
>>> 22: 0680 21da e500 addil #567993600,%d0
>>> 28: 2080 movel %d0,%a0@
>>>../../../../../head/cpukit/libcsupport/src/__gettod.c:73
>>> 2a: 2239 0000 0000 movel 0 <gettimeofday>,%d1
>>> 30: 2039 0000 0000 movel 0 <gettimeofday>,%d0
>>> 36: 4c00 1800 mulsl %d0,%d1
>>> 3a: 2141 0004 movel %d1,%a0@(4)
>>> 3e: 4280 clrl %d0
>>> 40: 6010 bras 52 <gettimeofday+0x52>
>>>../../../../../head/cpukit/libcsupport/src/__gettod.c:56
>>> 42: 4eb9 0000 0000 jsr 0 <gettimeofday>
>>> 48: 2040 moveal %d0,%a0
>>> 4a: 20bc 0000 000e movel #14,%a0@
>>> 50: 70ff moveq #-1,%d0
>>> 52: 4e5e unlk %fp
>>> 54: 4e75 rts
>>>
>>>I wonder where else in the kernel this is happening and to which
>>>other
>>
>>>targets. I see the i386 also has problems. In the i386 only one value
>
>
>>>is being read with interrupts masked:
>>>
>>> rtems_interrupt_disable(level);
>>> c: 9c pushf
>>> d: fa cli
>>> e: 58 pop %eax
>>> cpukit/libcsupport/src/__gettod.c:69
>>> seconds = _TOD_Seconds_since_epoch;
>>> microseconds = _TOD_Current.ticks;
>>> f: 8b 15 18 00 00 00 mov 0x18,%edx
>>> ^^^^^^^^^^^^^^^^ ONLY ONE READ
>>>cpukit/libcsupport/src/__gettod.c:70
>>> rtems_interrupt_enable(level);
>>> 15: 50 push %eax
>>> 16: 9d popf
>>>cpukit/libcsupport/src/__gettod.c:72
>>> tp->tv_sec = seconds + POSIX_TIME_SECONDS_1970_THROUGH_1988;
>>> 17: a1 00 00 00 00 mov 0x0,%eax
>>> ^^^^^^^^^^^^^^^ THE OTHER READ
>>> 1c: 05 00 e5 da 21 add $0x21dae500,%eax
>>> 21: 89 01 mov %eax,(%ecx)
>>>
>>>
>>>>The asm volatile statements
>>>>should provide sync points for the compiler if a partial read of
>>>>memory were an issue.
>>>
>>>If this is the case it would seem we have a problem with our asm
>>>statements or the compiler has a bug.
>>>
>>>Regards
>>>Chris
>>>
>>
>>--
>>Ian Caddy
>>Goanna Technologies Pty Ltd
>>+61 8 9221 1860
>>
>>
>>
>
>
> --
> Ian Caddy
> Goanna Technologies Pty Ltd
> +61 8 9221 1860
>
>
>
--
--------------------------------------------
IMD Ingenieurbuero fuer Microcomputertechnik
Thomas Doerfler Herbststrasse 8
D-82178 Puchheim Germany
email: Thomas.Doerfler at imd-systems.de
PGP public key available at:
http://www.imd-systems.de/pgpkey_en.html
More information about the users
mailing list