svn commit: r280308 - head/sys/fs/devfs

Bruce Evans brde at optusnet.com.au
Mon Mar 30 05:03:55 UTC 2015


On Mon, 30 Mar 2015, Bruce Evans wrote:

> On Sun, 29 Mar 2015, Konstantin Belousov wrote:
>
>> Interesting complication with the devfs timestamp update is that
>> devfs_read_f() and devfs_write_f() do not lock the vnode. So whatever
>> update method is used, stat(2) on devfs might return inconsistent value,
>> since tv_src/tv_nsec cannot be updated or read by single op, without
>> locking.
>
> Urk.
> ...
>> +static void
>> +devfs_timestamp(struct timespec *tsp)
>> +{
>> +	time_t ts;
>> +
>> +	if (devfs_dotimes) {
>> +		vfs_timestamp(tsp);
>> +	} else {
>> +		ts = time_second;
>> +		if (tsp->tv_sec < ts) {
>> +			tsp->tv_sec = ts;
>> +			tsp->tv_nsec = 0;
>> +		}
> ...
> I think you only want to do a null update if tv_nsec is nonzero due to a
> previous setting with vfs_timestamp(), and the new second hasn't arrived
> yet.  Something like:
> ...

Further problems:
- all changes to vfs.timestamp_precision to a lower precision can give
   non-monotonic timestamps.  I wouldn't bother fixing this only here.
- time_t is bogusly 64 bits on some 32-bit arches (32-bit arm and 32-bit
   mips).  Thus direct accesses to time_second are racy and should not
   be used in MI code.  This bug is harmless for the same reason that
   64-bit time_t is bogus -- 32-bit unsigned time_t works until 2106.
   The first race will occur slightly before then.  Except for testing
   timestamps far in the future.  With 32-bit time_t, you just can't
   do such tests, but with 64-bit time_t you can do them to find races
   like this one.

Bruce


More information about the svn-src-head mailing list