Date: Wed, 28 Sep 2011 19:19:52 -0700 From: Garrett Cooper <yanegomi@gmail.com> To: Kostik Belousov <kostikbel@gmail.com> Cc: Kirk McKusick <mckusick@mckusick.com>, freebsd-fs@freebsd.org, Xin LI <delphij@freebsd.org>, bug-followup@freebsd.org Subject: Re: PR kern/161016 Need to force sync(2) before umounting UFS1 filesystems? Message-ID: <CAGH67wR4CcOuM8xKZMAX6uaZyanTt726Z7uey_i7KM00EG%2B%2BtA@mail.gmail.com> In-Reply-To: <20110929013635.GG1511@deviant.kiev.zoral.com.ua> References: <CAGH67wSvpmdmCFKxAsgLD5cGc=WcYpX=dXBkhqkePNxVNjR4=g@mail.gmail.com> <201109280019.p8S0JVUW067163@chez.mckusick.com> <20110929013635.GG1511@deviant.kiev.zoral.com.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Sep 28, 2011 at 6:36 PM, Kostik Belousov <kostikbel@gmail.com> wrot= e: > 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@free= bsd.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 =3DD3=3DC5=3DCE=3DD4=3DD1=3DC2=3DD2=3DD1 2011 =3DC7., 1= 2:06:05: >> > > >> > >> =3D9A =3D9A Talking to Xin yesterday, he was convinced that this wa= s a >> > >> filesystem//kern bug. Before I file a PR, I'm wondering if anyone e= lse >> > >> has seen this issue.. >> > > =3D9AYes, 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, witho= ut >> > > 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/1610= 16). >> >> I propose the following change to correct it: >> >> Index: sys/kern/vfs_mount.c >> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >> --- sys/kern/vfs_mount.c =A0 =A0 =A0(revision 225807) >> +++ sys/kern/vfs_mount.c =A0 =A0 =A0(working copy) >> @@ -1227,18 +1227,6 @@ >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 mp->mnt_kern_flag |=3D MNTK_UNMOUNTF; >> =A0 =A0 =A0 error =3D 0; >> =A0 =A0 =A0 if (mp->mnt_lockref) { >> - =A0 =A0 =A0 =A0 =A0 =A0 if ((flags & MNT_FORCE) =3D=3D 0) { >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mp->mnt_kern_flag &=3D ~(MNTK_= UNMOUNT | MNTK_NOINSMNTQ | >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 MNTK_UNMOUNTF); >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (mp->mnt_kern_flag & MNTK_M= WAIT) { >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mp->mnt_kern_f= lag &=3D ~MNTK_MWAIT; >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 wakeup(mp); >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 } >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 MNT_IUNLOCK(mp); >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (coveredvp) >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 VOP_UNLOCK(cov= eredvp, 0); >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (EBUSY); >> - =A0 =A0 =A0 =A0 =A0 =A0 } >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 mp->mnt_kern_flag |=3D MNTK_DRAINING; >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 error =3D msleep(&mp->mnt_lockref, MNT_MTX(m= p), PVFS, >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "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 ? This would undo some of the changes attillio added for the locking stuff in r184554. Not sure what the prior behavior was because I only traced back the change a little bit. Thanks, -Garrett 1. http://svnweb.freebsd.org/base/head/sys/kern/vfs_mount.c?revision=3D1845= 54&view=3Dmarkup
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAGH67wR4CcOuM8xKZMAX6uaZyanTt726Z7uey_i7KM00EG%2B%2BtA>