gettimeofday seconds rollover problem?
Andrew Sinclair
Andrew.Sinclair at elprotech.com
Thu Feb 23 07:55:01 UTC 2006
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
More information about the users
mailing list