svn commit: r222466 - head/sbin/umount

[Bruce Evans]

On Mon, 30 May 2011, Rick Macklem wrote:

>> On Mon, May 30, 2011 at 01:48:53PM +0100, Robert Watson wrote:
>>> On Sun, 29 May 2011, Rick Macklem wrote:
>>>
>>>> Modify the umount(8) command so that it doesn't do
>>>> a sync(2) syscall before unmount(2) for the "-f" case.
>>>> This avoids a forced dismount from getting stuck for
>>>> an NFS mountpoint in sync() when the server is not
>>>> responsive. With this commit, forced dismounts should
>>>> normally work for the NFS clients, but can take up to
>>>> about 1minute to complete.
>>>
>>> I'm actually a bit confused about why umount(8) calls sync(2) at
>>> all:
>>> surely it's the responsibility of the file system, rather than the
>>> userland
>>> tool, to ensure consistency subject to file system configuration and
>>> unmount-time flags?
>> This call is from the same department as triple-sync before reboot,
>> IMO.
> Hehe. I'm so old, I do two syncs, as required by 6th Edition.:-)

I am apparently not so old, since my reboot script only has 1 sync :-).

> I assumed the sync() was meant to be an optimization (given the comment
> for it) in the sense that it would get the writes of dirty blocks started
> "right away". However, given the short period of time from the the sync(2)

It is only an optimization.  Any number of syncs are useless for
actually syncing the system, since sync(2) only does an async sync (it
returns without waiting for most writes to complete).  As you pointed
out later in this thread, unmount(2) does a sync that works -- a sync
sync -- before doing the unmount proper.  It does this irrespective of
the force flag:

% 	if (((mp->mnt_flag & MNT_RDONLY) ||
% 	     (error = VFS_SYNC(mp, MNT_WAIT)) == 0) || (flags & MNT_FORCE) != 0)
% 		error = VFS_UNMOUNT(mp, flags);

The force flag doesn't mean that i/o's are forced to complete.  It
only means that open files are forced to be closed (and a few related
things).  This can be seen here.  We wait (potentially forever) for
any existing writes to complete.  Only then do we look at the force
flag and bale out if the sync failed and the force flag is _not_ set.
Most i/o problems will cause hangs in this sync, and the force flag
won't help.  But if we somehow get past this sync, and have i/o
problems, then honoring the force flag and continuing gives even more
fragility, since we have almost passed the point of being able to give
up after an error (this point is typically very early in VFS_UNMOUNT()).

Many file systems have had bugs in this area.  Before 1999/01/22
(vfs_bio.c 1.196), b*write() gave up after an i/o error, and pretended
to succeed.  Both recoverable and unrecoverable errors were mishandled.
This avoided many hangs in VFS_SYNC().  When this was partially fixed,
many VFS_SYNC()s couldn't handle the errors, and paniced when they
should have looped endlessly.  Now I think they mostly loop endlessly,
even for unrecoverable errors when they shouldn't.  My version attempts
to fix vfs_bio.c 1.196 by still giving up after an unrecoverable i/o
error.  This reduced endless loops but gave more panics in other places
that can't handle the errors, mainly in the VFS_UMOUNT() call in the
above -- many file systems saw the errors after the point where it is
posible to either recover from them or loop endlessly on them, and then
a panic occurred soon after the VFS_UMOUNT() returned with a half-complete
unmount(), since neither a success or a failure return can indicate this
state.

> call to the unmount(2) call, I'm not convinced it makes a significant
> difference. (I thought of just getting rid of it, but figured it was
> harmless for the non "-f" case and might matter for a buggy fs that doesn't
> get the unmount(2) quite right. ie. Same argument as doing the triple-sync,
> just to be sure.)

I think you shouldn't have touched umount(8).  The sync can still hang or
just be in progress when unmount(2) is called, and unmount(2) still does
its own sync, so nfs_unmount() must still handle hanging syncs in some way.

My reboot script has more details related to this:

%%%
sh <<EOF
sync
#umount -Af -t nfs
/sbin/reboot
EOF
%%%

This used to unmount the nfs file systems explicitly because I _wanted_
their unmounts to hang if there is a problem (hopefullly while they could
be aborted by ^C).  Otherwise, I wanted the reboot to be fairly forceful
(no messing around with shutdown or even umount).  reboot(8) and reboot(2)
do a fairly clean shutdown, including unmounting all file systems with
the force flag set (in vfs_unmountall(9)), although their man pages don't
mention unmounting.  reboot(8) doesn't even mention reboot(2)!, although
it bogusly references sync(8).  reboot(8) actually uses sync(2) (unless
nflag), but this is fairly bogus too, since reboot(2) does another sync
(unless RB_NOSYNC).  Perhaps there was a problem with the force flag being
too forceful especially for nfs.  vfs_bio.c 1.196 might have fixed this.

Bruce