Date: Wed, 28 Sep 2011 23:20:16 -0700 From: Kirk McKusick <mckusick@mckusick.com> To: attilio@freebsd.org Cc: Garrett Cooper <yanegomi@gmail.com>, freebsd-fs@freebsd.org, Xin LI <delphij@freebsd.org> Subject: Re: Need to force sync(2) before umounting UFS1 filesystems? Message-ID: <201109290620.p8T6KGCl057169@chez.mckusick.com>
next in thread | raw e-mail | index | archive | help
Hi Attilio,
I have been looking into the problem described below and since you
appear to be the person that put in the change in question, I would
like to get you opinion on what (if anything) should be changed here.
A bit of background. Historically (i.e., since UNIX was written and
up until this change went in) the unmount command would always fully
sync out the filesystem and return with it unmounted. The exception
was if some file or directory in the filesystem was actively being
used. In this case, the unmount would fail with EBUSY. After this
change, an unmount will fail with EBUSY if there are dirty blocks
that need to be synced, even if there are no active users of the
filesystem. This affects UFS (where the soft-updates code may be
doing background tasks), NFS (where the NFS daemon may be doing
background tasks), and ZFS (where its syncer may be doing background
tasks). The only way to reliably unmount an idle filesystem is to loop
doing sync's and unmount attempts until it succeeds. Now it is possible
to get the unmount to succeed by doing a forcible unmount, but that
is often not what is desired as that will kill any legitimate users
on the filesystem. My argument below is that we should revert to the
historic semantics of unmount which is to always succeed unless there
are active users on the filesystem.
So, please look over my suggested change and let me know what you
think.
Kirk McKusick
=-=-=
Date: Thu, 29 Sep 2011 04:36:35 +0300
From: Kostik Belousov <kostikbel@gmail.com>
To: Kirk McKusick <mckusick@mckusick.com>
Cc: Garrett Cooper <yanegomi@gmail.com>, freebsd-fs@freebsd.org,
Xin LI <delphij@freebsd.org>
Subject: Re: PR kern/161016 Need to force sync(2) before umounting UFS1 filesystems?
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@gmail.com>
> > To: lev@freebsd.org
> > Cc: freebsd-fs@freebsd.org, Xin LI <delphij@freebsd.org>, current@freebsd.org
> > Subject: Re: Need to force sync(2) before umounting UFS1 filesystems?
> >
> > 2011/9/25 Lev Serebryakov <lev@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@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@freebsd.org mailing list
> > http://lists.freebsd.org/mailman/listinfo/freebsd-fs
> > To unsubscribe, send any mail to "freebsd-fs-unsubscribe@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 ?
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201109290620.p8T6KGCl057169>