[src/trunk]: src/sys/nfs Adjust sillyrename cleanup code to deal with the par...

[wrstuden <wrstuden%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/3e9321d588ab
branches:  trunk
changeset: 557973:3e9321d588ab
user:      wrstuden <wrstuden%NetBSD.org@localhost>
date:      Fri Jan 23 22:20:20 2004 +0000

description:
Adjust sillyrename cleanup code to deal with the parent vnode
already being locked by our thread. VOP_INACTIVATE() makes no
statement as to the lock state of the parent, yet this code assumed
we had it unlocked.

With this change, we let vn_lock() fail with EDEADLK if we already
have the parent locked. We then handle the rename cleanup, and on
the way out just vrele() the parent vnode, not vput() it.

Fixes a case seen by Steve Woodford at Wasabisystems dot com where
we'd panic while running a pkgsrc configure test that verified
fork() functionality. I expect the problem is a result of the recent
exit() changes and the performance of the machines he tested on.

Specifically we would crash during an nfs_remove(). As best I can
tell, when nfs_remove() tested to see if we should rename or we
should remove, v_usecount was > 1 and vattr.va_nlink was 1. Thus
we did the sillyrename in nfs_remove(). However by the time we got
down to the vput(vp), v_usecount had dropped to one and thus vput()
triggered the VOP_INACTIVATE() code path. nfs_inactive() tries to
lock the parent to undo the sillyrename, and deadlocks as we still
have it locked.

diffstat:

 sys/nfs/nfs_node.c |  16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)

diffs (48 lines):

diff -r 0e3661af55a0 -r 3e9321d588ab sys/nfs/nfs_node.c
--- a/sys/nfs/nfs_node.c        Fri Jan 23 21:12:07 2004 +0000
+++ b/sys/nfs/nfs_node.c        Fri Jan 23 22:20:20 2004 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: nfs_node.c,v 1.71 2003/12/07 21:15:46 fvdl Exp $       */
+/*     $NetBSD: nfs_node.c,v 1.72 2004/01/23 22:20:20 wrstuden Exp $   */
 
 /*
  * Copyright (c) 1989, 1993
@@ -35,7 +35,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: nfs_node.c,v 1.71 2003/12/07 21:15:46 fvdl Exp $");
+__KERNEL_RCSID(0, "$NetBSD: nfs_node.c,v 1.72 2004/01/23 22:20:20 wrstuden Exp $");
 
 #include "opt_nfs.h"
 
@@ -238,6 +238,7 @@
        struct vnode *vp = ap->a_vp;
        struct nfsmount *nmp = VFSTONFS(vp->v_mount);
        boolean_t removed;
+       int err;
 
        np = VTONFS(vp);
        if (prtactive && vp->v_usecount != 0)
@@ -257,12 +258,19 @@
 
                /*
                 * Remove the silly file that was rename'd earlier
+                *
+                * Just in case our thread also has the parent node locked,
+                * we let vn_lock() fail.
                 */
 
-               vn_lock(sp->s_dvp, LK_EXCLUSIVE | LK_RETRY);
+               err = vn_lock(sp->s_dvp, LK_EXCLUSIVE | LK_RETRY
+                                       | LK_RECURSEFAIL);
                nfs_removeit(sp);
                crfree(sp->s_cred);
-               vput(sp->s_dvp);
+               if (err != EDEADLK)
+                       vput(sp->s_dvp);
+               else
+                       vrele(sp->s_dvp);
                FREE(sp, M_NFSREQ);
        }