Date: Mon, 8 Dec 2014 15:29:02 -0500 (EST) From: Benjamin Kaduk <kaduk@MIT.EDU> To: Konstantin Belousov <kostikbel@gmail.com> Cc: fs@freebsd.org Subject: Re: VFS_SYNC() call in dounmount() Message-ID: <alpine.GSO.1.10.1412081525320.23489@multics.mit.edu> In-Reply-To: <20141208181906.GW97072@kib.kiev.ua> References: <20141208181906.GW97072@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 8 Dec 2014, Konstantin Belousov wrote: > 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. It looks like VFS_SYNC is a no-op for net/openafs, so we should not be affected by this change. -Ben
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?alpine.GSO.1.10.1412081525320.23489>