fixing "umount -f" for the NFS client

[Konstantin Belousov]

On Thu, Aug 29, 2013 at 06:21:41PM -0400, Rick Macklem wrote:
> Kostik wrote:
> > On Wed, Aug 28, 2013 at 08:15:27PM -0400, Rick Macklem wrote:
> > > I've been doing a little more testing of "umount -f" for NFS
> > > mounts and they seem to be working unless some other process/thread
> > > has busied the file system via vfs_busy().
> > > 
> > > Unfortunately, it is pretty easy to vfs_busy() the file system
> > > by using a command like "df" that is stuck on the unresponsive
> > > NFS server.
> > > 
> > > The problem seems to be that dounmount() msleep()s while
> > > mnt_lockref != 0 before calling VFS_UNMOUNT().
> > > 
> > > If some call into the NFS client was done before this
> > > while (mp->mnt_lockref) loop with msleep() in it, it
> > > can easily kill off RPCs in progress. (It currently
> > > does this in nfs_unmount() using the newnfs_nmcancelreqs()
> > > call.
> > > 
> > > In summary:
> > > - Would it be appropriate to add a new vfs_XXX method that
> > >   dounmount() would call before the while() loop for the
> > >   forced dismount case?
> > >   (The default would be a no-op and I have no idea if any
> > >    file system other than NFS would have a use for it?)
> > >   Alternately, there could be a function pointer set non-NULL
> > >   that would specifically be used by the NFS client for this.
> > >   This would avoid adding a vfs_XXX() method, but would mean
> > >   an NFS specific call ends up in the generic dounmount() code.
> > > 
> > > Anyone have comments on this?
> > > 
> > Yes, I do.  I agree with adding the pre-unmount vfs method.
> > This seems to be the cleanest solution possible.
> > 
> I've attached a patch. It is also at
>   http://people.freebsd.org/~rmacklem/forced-dism.patch
> in case the attachment gets lost.
> I don't really like doing the MNT_IUNLOCK(), MNT_ILOCK() before/after
> the VFS_KILLIO() call, but I couldn't see any better way to do it and
> it looks safe to do so, at least for the forced case.
Might be, call it VFS_PURGE() ?

I suggest to move the call to the VFS_KILLIO after the MNTK_DRAINING is
set, to avoid getting new references after the current i/o transactions
are stopped. You would need to set MNTK_DRAINING unconditionally. Also,
it probably makes sense to replace the if (mnt_lockref) with while ().

> 
> I assume I would also need to bump __FreeBSD_version (and maybe VFS_VERSION?).
I think you could avoid it.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 834 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/freebsd-fs/attachments/20130830/42ad12f5/attachment.sig>