From b5053f07a122e16188935b3af50684ec770b41f9 Mon Sep 17 00:00:00 2001 From: wrstuden Date: Fri, 23 Jan 2004 22:20:20 +0000 Subject: [PATCH] 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. --- sys/nfs/nfs_node.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/sys/nfs/nfs_node.c b/sys/nfs/nfs_node.c index 56a305857440..bbb4732f0394 100644 --- a/sys/nfs/nfs_node.c +++ b/sys/nfs/nfs_node.c @@ -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 -__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 @@ nfs_inactive(v) 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 @@ nfs_inactive(v) /* * 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); }