Date: Mon, 8 Dec 2014 20:19:06 +0200 From: Konstantin Belousov <kostikbel@gmail.com> To: fs@freebsd.org Subject: VFS_SYNC() call in dounmount() Message-ID: <20141208181906.GW97072@kib.kiev.ua>
next in thread | raw e-mail | index | archive | help
Right now, dounmount() has the following code:
if (((mp->mnt_flag & MNT_RDONLY) ||
(error = VFS_SYNC(mp, MNT_WAIT)) == 0) || (flags & MNT_FORCE) != 0)
error = VFS_UNMOUNT(mp, flags);
In other words, if the filesystem is mounted rw, we try VFS_SYNC().
If the unmount request if forced, VFS_UNMOUNT() is called unconditionally,
otherwise, VFS_UNMOUNT() is only performed when VFS_SYNC() succeeded.
Apparently, the sync call is problematic, both for UFS and NFS. It
was demonstrated by Peter Holm that sufficient fsx load prevents sync
from finishing for unbounded amount of time. The ffs_unmount() makes
neccessary measures to stop writers and to sync filesystem to the stable
state before destroying the mount structures, so removal of VFS_SYNC()
above fixed the test.
More, NFS client just ignores the VFS_SYNC() call for forced unmount,
to work around the hung nfs requests.
Andrey Gapon assured me that ZFS handles unmount correctly and does
not need help in the form of sync before unmount. The only major
writeable filesystem which apparently did not correctly synced on
unmount is msdosfs.
Note that relying on VFS_SYNC() before VFS_UNMOUNT() to flush all caches
to permanent storage is racy, since VFS does not (and cannot) stop
other threads from writing to fs meantime. UFS and TMPFS suspend
filesystem in VFS_UNMOUNT(), handling the race in VFS_UNMOUNT().
I propose to only call VFS_SYNC() before VFS_UNMOUNT() for non-forced
unmount. As I explained, the call for forced case is mostly pointless.
For non-forced unmount, this is needed for KBI compatibility for NFS
(not important), and to increase the chances of unmount succeedeing
(again not important). I still prefer to keep the call around for
non-forced case for some time.
diff --git a/sys/fs/msdosfs/msdosfs_vfsops.c b/sys/fs/msdosfs/msdosfs_vfsops.c
index 213dd00..d14cdef 100644
--- a/sys/fs/msdosfs/msdosfs_vfsops.c
+++ b/sys/fs/msdosfs/msdosfs_vfsops.c
@@ -797,11 +797,15 @@ msdosfs_unmount(struct mount *mp, int mntflags)
int error, flags;
flags = 0;
- if (mntflags & MNT_FORCE)
+ error = msdosfs_sync(mp, MNT_WAIT);
+ if ((mntflags & MNT_FORCE) != 0) {
flags |= FORCECLOSE;
+ } else if (error != 0) {
+ return (error);
+ }
error = vflush(mp, 0, flags, curthread);
- if (error && error != ENXIO)
- return error;
+ if (error != 0 && error != ENXIO)
+ return (error);
pmp = VFSTOMSDOSFS(mp);
if ((pmp->pm_flags & MSDOSFSMNT_RONLY) == 0) {
error = markvoldirty(pmp, 0);
diff --git a/sys/kern/vfs_mount.c b/sys/kern/vfs_mount.c
index c407699..b2b4969 100644
--- a/sys/kern/vfs_mount.c
+++ b/sys/kern/vfs_mount.c
@@ -1305,8 +1305,8 @@ dounmount(mp, flags, td)
}
vput(fsrootvp);
}
- if (((mp->mnt_flag & MNT_RDONLY) ||
- (error = VFS_SYNC(mp, MNT_WAIT)) == 0) || (flags & MNT_FORCE) != 0)
+ if ((mp->mnt_flag & MNT_RDONLY) != 0 || (flags & MNT_FORCE) != 0 ||
+ (error = VFS_SYNC(mp, MNT_WAIT)) == 0)
error = VFS_UNMOUNT(mp, flags);
vn_finished_write(mp);
/*
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20141208181906.GW97072>