svn commit: r280308 - head/sys/fs/devfs
Bruce Evans
brde at optusnet.com.au
Mon Mar 30 04:14:14 UTC 2015
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.
> That said, we could either:
> - ignore the race. Then the patch below might be good enough.
> - Trim the precision of futimens() for devfs to always set tv_nsec to 0.
> Then, the race can be handled satisfactory with atomics.
> - Add a lock (IMO this does not make sense).
Locks aren't that expensive, and are easier to use than atomics.
> diff --git a/sys/fs/devfs/devfs_vnops.c b/sys/fs/devfs/devfs_vnops.c
> index 941f92c..e199d52 100644
> --- a/sys/fs/devfs/devfs_vnops.c
> +++ b/sys/fs/devfs/devfs_vnops.c
> @@ -83,8 +83,24 @@ MTX_SYSINIT(cdevpriv_mtx, &cdevpriv_mtx, "cdevpriv lock", MTX_DEF);
> SYSCTL_DECL(_vfs_devfs);
>
> static int devfs_dotimes;
> -SYSCTL_INT(_vfs_devfs, OID_AUTO, dotimes, CTLFLAG_RW,
> - &devfs_dotimes, 0, "Update timestamps on DEVFS");
> +SYSCTL_INT(_vfs_devfs, OID_AUTO, dotimes, CTLFLAG_RW, &devfs_dotimes, 0,
> + "Update timestamps on DEVFS with default precision");
This fixes 1 style bug (the tab) but adds another. The normal formatting
is to split the declaration after CTLFLAG*. Then preferably use a less
verbose discription so as to not need to split the line. The old
description was a bit cryptic (but better than the sysctl name).
> +
> +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;
> + }
File timestamps use CLOCK_REALTIME, so they are supposed to go backwards
sometimes. More importantly, if the time is set to a future time (by
utimes(), etc., not due to a clock step), this prevents it being correted.
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:
if (tsp->tv_sec != ts) ...
If '<', then as above. If '>', it means a correction by >= 1 second
(strictly greater if tv_nsec != 0). The initial value tv_nsec is
irrelevant in both cases.
> + }
> +}
Bruce
More information about the svn-src-head
mailing list