Date: Fri, 6 Sep 1996 08:33:19 +1000 From: Bruce Evans <bde@zeta.org.au> To: current@freebsd.org Subject: fixing accesses to volatile variable `time' Message-ID: <199609052233.IAA07391@godzilla.zeta.org.au>
next in thread | raw e-mail | index | archive | help
`time' is now declared volatile. This doesn't do much except to cause
annoying warnings for -Wcast-qual when &time is cast to a pointer to a
non-volatile when it is passed to utility functions like timevaladd().
The utility functions are not prepared to deal with volatile times;
however, there is no problem for the cases where warnings are generated,
because the code runs at splclock(). splclock() has the effect of making
`time' nonvolatile. Accesses to `time' outside of regions protected
by splclock() are usually bugs, and declaring time as volatile usually
doesn't help. I think the correct way to handle this is to declare
`time' as nonvolatile and almost never access it outside of regions
protected by splclock().
Most of the invalid accesses to `time' are of the form `time.tv_sec'
in file systems code. There are several small problems with this:
1. time.tv_sec is long, and accesses to longs are not guaranteed to be
atomic. They happen to be atomic on i386's.
2. time.tv_sec may be 1 less than the `true' seconds value reported by
microtime(). The worst case is approximately when time.tv_usec
== 999999. The true time.tv_sec is then ahead of the reported
time.tv_sec by 1 second for almost a whole clock tick every second.
Applications can see the difference if they mix gettimeofday()
calls with file system calls. utimes() uses microtime() and gives
results inconsistent with most other fs calls. Possible fixes:
(1) use microtime() everywhere. This is probably too expensive.
(2) keep track of (time.tv_usec % 10000) at each clock tick and
adjust the current time to match.
Both of these changes require replacing hundreds of direct accesses
to time.tv_sec. I started replacing time.tv_sec by macrotime()
but gave up after about 100 accesses.
There are larger problems accessing the full `time'. Most file systems
sort of support timespecs and would need to access the full time for
complete support.
3. Accesses to `time' aren't atomic even on i386's.
4. The problems in (2) are more obvious if the nanoseconds fields in
file times are set to nonzero.
I changed only all the accesses to the full `time' to used a new
get_time() inline functions. ext2fs already calls such a function
in the !__FreeBSD__ case.
Bruce
diff -c2 src/sys/sys/kernel.h~ src/sys/sys/kernel.h
*** src/sys/sys/kernel.h~ Wed Sep 4 17:32:00 1996
--- src/sys/sys/kernel.h Tue Sep 3 03:05:01 1996
***************
*** 60,64 ****
extern struct timeval boottime;
extern struct timeval runtime;
! extern volatile struct timeval time;
extern struct timezone tz; /* XXX */
--- 60,64 ----
extern struct timeval boottime;
extern struct timeval runtime;
! extern struct timeval time; /* nonvolatile at ipl >= splclock() */
extern struct timezone tz; /* XXX */
***************
*** 72,75 ****
--- 72,86 ----
extern int tickdelta;
extern long timedelta;
+
+ static __inline void
+ get_time(struct timeval *tvp)
+ {
+ int s;
+
+ s = splclock();
+ /* XXX should use microtime() iff tv_usec is used. */
+ *tvp = time;
+ splx(s);
+ }
/*
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199609052233.IAA07391>