PR kern/161016 Need to force sync(2) before umounting UFS1 filesystems?

[Kostik Belousov]

On Tue, Sep 27, 2011 at 05:19:31PM -0700, Kirk McKusick wrote:
> > Date: Sun, 25 Sep 2011 12:07:18 -0700
> > From: Garrett Cooper <yanegomi at gmail.com>
> > To: lev at freebsd.org
> > Cc: freebsd-fs at freebsd.org, Xin LI <delphij at freebsd.org>, current at freebsd.org
> > Subject: Re: Need to force sync(2) before umounting UFS1 filesystems?
> > 
> > 2011/9/25 Lev Serebryakov <lev at freebsd.org>:
> > > Hello, Garrett.
> > > You wrote 25 =D3=C5=CE=D4=D1=C2=D2=D1 2011 =C7., 12:06:05:
> > >
> > >> =9A =9A Talking to Xin yesterday, he was convinced that this was a
> > >> filesystem//kern bug. Before I file a PR, I'm wondering if anyone else
> > >> has seen this issue..
> > > =9AYes, and I posted message about it in embedded@ (Message-ID
> > > <1175277342.20110821215629 at serebryakov.spb.ru>), I've got additional
> > > question from Warner Losh about base (underlying) file system, without
> > > any additional reaction.
> > 
> > Thanks for the comments Adrian and Lev! I've filed PR 161016 to track
> > the issue, because it might be due to changes in the SU code, md, or a
> > subtle race condition in umount (highly unlikely, but it's been
> > noted).
> > -Garrett
> > _______________________________________________
> > freebsd-fs at freebsd.org mailing list
> > http://lists.freebsd.org/mailman/listinfo/freebsd-fs
> > To unsubscribe, send any mail to "freebsd-fs-unsubscribe at freebsd.org"
> 
> I have taken responsibility for working on this bug report (PR kern/161016).
> 
> I propose the following change to correct it:
> 
> Index: sys/kern/vfs_mount.c
> ===================================================================
> --- sys/kern/vfs_mount.c	(revision 225807)
> +++ sys/kern/vfs_mount.c	(working copy)
> @@ -1227,18 +1227,6 @@
>  		mp->mnt_kern_flag |= MNTK_UNMOUNTF;
>  	error = 0;
>  	if (mp->mnt_lockref) {
> -		if ((flags & MNT_FORCE) == 0) {
> -			mp->mnt_kern_flag &= ~(MNTK_UNMOUNT | MNTK_NOINSMNTQ |
> -			    MNTK_UNMOUNTF);
> -			if (mp->mnt_kern_flag & MNTK_MWAIT) {
> -				mp->mnt_kern_flag &= ~MNTK_MWAIT;
> -				wakeup(mp);
> -			}
> -			MNT_IUNLOCK(mp);
> -			if (coveredvp)
> -				VOP_UNLOCK(coveredvp, 0);
> -			return (EBUSY);
> -		}
>  		mp->mnt_kern_flag |= MNTK_DRAINING;
>  		error = msleep(&mp->mnt_lockref, MNT_MTX(mp), PVFS,
>  		    "mount drain", 0);
> 
> The things to check for are:
> 
> 1) That it fixes the EBUSY on unmount.
> 
> 2) That it does not cause unmount to hang.
> 
> I would appreciate feedback as to whether this fix helps.

I think the item 2) should be tested mostly on the hung NFS server.

I understand what you are doing, you do not want a transient mount point
busy caller to fail the unmount. But my belief is that this is the
intended mode of operation for non-forced unmounts.

As I compare the original bug report and your change, the reason that
UFS gives spurious EBUSY on soft unmounts is that SU code busies mp
around some processing. Is my guess right ? Then, restoring some amount
of sync(2) before the unmount would be useful, please see r222466 for
the most likely reason why the issue appeared.

Might be, the best route would be to add a kludge mnt_flag that request
dounmount() to do a VFS_SYNC() before checking for the busy holder ?

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-fs/attachments/20110929/c0d792a6/attachment.pgp