Re: netbsd-5 NFS(?) lock up

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

On Sun, Apr 05, 2009 at 03:29:37AM +0000, David Holland wrote:
>  > I can't see what's causing this to happen, although with all the macros the
>  > NFS source can be a bit of a minefield (lots going on behind the scenes).
>  > Sadly the "last locked" address is a bit useless because it's always going
>  > to be genfs_renamelock_enter().
>  > 
>  > David does NFS really need this lock? I think not...
> 
> It certainly does, unless you want to reintroduce the silent fs
> corruption.
> 
> The trace suggests that it's being left dangling somewhere; does
> lockdebug remember where each lock was acquired?

I found a code path that could exit from nfsrv_rename() without releasing
the lock:
nfsrv_rename() calls nfsm_srvmtofh(&tnsfh) which is a macro. This macro
can set error = EBADRPC and call nfsm_reply(0).
nfsm_reply() is also a macro, which can (especially when error == EBADRPC)
do a return(0). This will exit nfsrv_rename() without doing the needed
cleanups.

Changing the return(0) to a "error = 0; goto nfsmout" could be a 
solution; but I don't know if this will do the needed cleanup.
Someone familiar with the VFS stuff should confirm.
At first look it would need a
VOP_ABORTOP(fromnd.ni_dvp, &fromnd.ni_cnd);
vrele(fromnd.ni_dvp);
vrele(fvp);
It's possible that the code at nfsmout: will do it; but I don't know
if (fromnd.ni_cnd.cn_nameiop) is true at this point.
This would also need some change in the out1: path, and review
of all other uses of the nfs macros.

Another option is to move nfsm_srvmtofh(&tnsfh) before the
VFS_RENAMELOCK_ENTER(). It looks like the code between VFS_RENAMELOCK_ENTER()
and this nfsm_srvmtofh() is only VFS stuff, and the nfsm_srvmtofh(&tnsfh)
doens't depend on it.

-- 
Manuel Bouyer <bouyer%antioche.eu.org@localhost>
     NetBSD: 26 ans d'experience feront toujours la difference
--