Re: [PATCH] Fixing soft NFS umount -f, round 1

[Chuck Silvers <chuq%chuq.com@localhost>]

On Fri, Jun 26, 2015 at 09:10:59AM +0000, Emmanuel Dreyfus wrote:
> Hi
> 
> It took me some time to get it working, but here is a patch that fixes
> soft NFS umount -f in NetBSD-current. 
> http://ftp.espci.fr/shadow/manu/umount_f1.patch
> 
> The problem to fix is that a soft mount is supposed to be allowed to 
> fail, but when the server is gone, even a umount -f cannot get rid of
> the mount.  The only way out is reboot -n.
> 
> This hapened to be caused by several issues that are fixed bu the 
> patch:
> 
> 1) In umount(8), we called sync(2) before attempting a forced unmount(2), 
>    but sync(2) does not return before data is sent to storage, and 
>    therefore we never had the opportunity to attempt the forced unmount
>    when using -f

the sync() is never necessary, you can just remove it.


> 2) When trying to unmount, we first try vinvalbuf() with V_SAVE in 
>    order to push data to storage, but when it fails, we call it
>    again without V_SAVE, to get rid of the vnode's buffer. In that
>    case, we need PGO_BUSYFAIL in order to avoid being trapped in 
>    genfs_gop_write(), on UVM_UNLOCK_AND_WAIT(pg, slock, 0, "genput", 0);

that's not safe.  we need to make sure all pages are gone before
we can get rid of the vnodes they are attached to, which is exactly
what the non-V_SAVE call is trying to do.  but that still needs to
wait for any busy pages to become not-busy.


> 3) In NFS code, the 3 occurences of cv_timedwait() must always have a
>    a timeout for soft mounts so that we have an opportunity to detect
>    and report a failure. I chose to report EIO. ENOTCONN could be
>    more explicit, but it is not an errno POSIX write(2) is supposed to
>    return.

that part is fine.


> 4) When unmount is in progress, prevent nfs_connect() to start a new
>    connexion, otherwise we will have a thread looping in nfs_reconnect()
>    with a RW_READER held on nmp->nm_writeverflock and we cannot unmount.

that should be only for soft mounts.


> 5) In genfs code, report VOP_STRATEGY errors to higher layers instead
>    of hiding it, so that we can detect error. Display a message for the
>    administrator.

VOP_STRATEGY() reports errors via the buffer's B_ERROR/b_error mechanism,
please use that instead of the function return value to detect
failed I/O requests.

also, genfs should not be printing on the console.  if you want a message
on the console for I/O that fails due to NFS soft-mount timeouts then
you should do that in the NFS code at the point where the decision is made
that the I/O will not be retried again.


> 6) In genfs code, make sure genfs_do_putpages() do not wait I/O completion 
>    forever when it hits an error in GOP_WRITE: the write being partial,
>    it will never complete, hence we should report error now.

as I said before, this is wrong.  I/O that fails should still complete
eventually (with an error), so it's fine for genfs to wait indefinitely
for that.  it's up to the underlying "disk" driver (NFS in this case)
to decide when to give up.  if failed NFS writes are never being reported as done
(ie. biodone() is not being called on buffers which will not be retried anymore)
then that's what you need to fix.

I'm not sure what you mean about a partial write, could you explain?

-Chuck