Date: Sat, 1 Oct 2011 14:39:14 +0200 From: Attilio Rao <attilio@freebsd.org> To: Kostik Belousov <kostikbel@gmail.com> Cc: Kirk McKusick <mckusick@mckusick.com>, Garrett Cooper <yanegomi@gmail.com>, Xin LI <delphij@freebsd.org>, freebsd-fs@freebsd.org Subject: Re: Need to force sync(2) before umounting UFS1 filesystems? Message-ID: <CAJ-FndDjLA8Gpq_W5FQF5YKukZnXxpAa-kzMaRUrEe5S-jzGBw@mail.gmail.com> In-Reply-To: <20110930201851.GB1511@deviant.kiev.zoral.com.ua> References: <CAJ-FndBDzv6af%2BAVq9wkLbN8V3wHDk3BGPuTFaXB7j8EXsrrhA@mail.gmail.com> <201109301820.p8UIKSGj039445@chez.mckusick.com> <20110930201851.GB1511@deviant.kiev.zoral.com.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
2011/9/30 Kostik Belousov <kostikbel@gmail.com>: > On Fri, Sep 30, 2011 at 11:20:28AM -0700, Kirk McKusick wrote: >> > Date: Fri, 30 Sep 2011 15:31:56 +0200 >> > Subject: Re: Need to force sync(2) before umounting UFS1 filesystems? >> > From: Attilio Rao <attilio@freebsd.org> >> > To: Kirk McKusick <mckusick@mckusick.com> >> > Cc: Konstantin Belousov <kib@freebsd.org>, >> > =C2=A0 =C2=A0 Garrett Cooper <yanegomi@gmail.com>, >> > =C2=A0 =C2=A0 freebsd-fs@freebsd.org, Xin LI <delphij@freebsd.org> >> > >> > 2011/9/30 Kirk McKusick <mckusick@mckusick.com>: >> > >> > > Here is my proposed fix. It does the unroll originally found in the >> > > non-FORCE case before sleeping waiting for the vfs_busy to clear. >> > > Is it acceptable to hold the mount mutex while calling VOP_UNLOCK? >> > > If not, then it needs to be released before the unlock, reacquired >> > > afterwards, and the check to see if the sleep is needed redone. >> > >> > I thought about this approach when sending the e-mail, but there is a >> > problem: you need to handle the MNTK_UNMOUNT flag checking and >> > subsequent setting after coveredvnode is held, otherwise at the first >> > looping you will just return EBUSY. >> > >> > You can avoid the unlock by passing PVFS | PDROP. >> > >> > Attilio >> >> Problem noted. I have updated the patch to clear the MNTK_UNMOUNT >> (and other flags set above it) after it returns from the sleep. >> Which means I cannot use the PDROP flag now, but it is good to >> know about it for future reference. >> >> Still not clear to me if it is acceptable to hold the mount mutex >> while calling VOP_UNLOCK. Should I drop the mount mutex around the >> VOP_UNLOCK(coveredvp)? Other than that possible problem, this patch >> appears to solve the EBUSY problem and avoid possible deadlocks. > I do not understand which deadlock is talked about there. > It seems thay Attilio concern was with acquiring covered vnode lock > after mounted fs is busied, but this is prohibited. > > See r166167 for more detailed description of the order. Ok, so that is the invariant I was forgetting, thanks Kostik. Kirk, you can make the 'forced unmount' behaviour by default for me, now, thanks. It would be great to have a comment on top of vfs_busy() or dounmount() check of mnt_ref on why this deadlock cannot happen, likely squeezing some good words from tegge's description of r166167. Kirk may be the best person to do it, but I can have his backs if he doesn't have time right now. Attilio --=20 Peace can only be achieved by understanding - A. Einstein
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-FndDjLA8Gpq_W5FQF5YKukZnXxpAa-kzMaRUrEe5S-jzGBw>