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
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
