Need to force sync(2) before umounting UFS1 filesystems?

[Attilio Rao]

2011/9/30 Kostik Belousov <kostikbel at 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 at freebsd.org>
>> > To: Kirk McKusick <mckusick at mckusick.com>
>> > Cc: Konstantin Belousov <kib at freebsd.org>,
>> >     Garrett Cooper <yanegomi at gmail.com>,
>> >     freebsd-fs at freebsd.org, Xin LI <delphij at freebsd.org>
>> >
>> > 2011/9/30 Kirk McKusick <mckusick at 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


-- 
Peace can only be achieved by understanding - A. Einstein