Date: Sat, 1 Oct 2011 12:44:04 -0700 From: Garrett Cooper <yanegomi@gmail.com> To: Attilio Rao <attilio@freebsd.org> Cc: Kirk McKusick <mckusick@mckusick.com>, Xin LI <delphij@freebsd.org>, freebsd-fs@freebsd.org Subject: Re: Need to force sync(2) before umounting UFS1 filesystems? Message-ID: <CAGH67wSYmcxJCbTMVL%2BqWzbLojiCiBmRF98yaNL4b3d3LbvbYw@mail.gmail.com> In-Reply-To: <CAJ-FndDjLA8Gpq_W5FQF5YKukZnXxpAa-kzMaRUrEe5S-jzGBw@mail.gmail.com> References: <CAJ-FndBDzv6af%2BAVq9wkLbN8V3wHDk3BGPuTFaXB7j8EXsrrhA@mail.gmail.com> <201109301820.p8UIKSGj039445@chez.mckusick.com> <20110930201851.GB1511@deviant.kiev.zoral.com.ua> <CAJ-FndDjLA8Gpq_W5FQF5YKukZnXxpAa-kzMaRUrEe5S-jzGBw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, Oct 1, 2011 at 5:39 AM, Attilio Rao <attilio@freebsd.org> wrote: > 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>, >>> > =A0 =A0 Garrett Cooper <yanegomi@gmail.com>, >>> > =A0 =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. Ok. Now that I know this is the direction you guys want to go, I'll start testing the change. Thanks! -Garrett
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAGH67wSYmcxJCbTMVL%2BqWzbLojiCiBmRF98yaNL4b3d3LbvbYw>