Subject: Re: [solution] vnode refcount panic, perhaps due to kern/vfs_lookup.c:lookup()
To: Greg Troxel <gdt@ir.bbn.com>
From: Jaromir Dolecek <jdolecek@netbsd.org>
List: tech-kern
Date: 03/19/2003 10:30:33
Greg Troxel wrote:
> I think I have found the problem, but I'm not 100% sure since the
> crashes were not repeatable by a fixed number of operations.  It used
> to crash after 3-5 'modify buffer, ^X u' operations, and now I've gone
> 20 or so.  (duh this is emacs so off goes a keyboard macro to do '
> ^X^S' 200 times, causing massive coda lock printouts
> (coda_lockdebug=1).  After, repeating with ' ^Xu' 200 times, I'm
> pretty convinced that at the very least the code is closer to correct
> 
> In coda/coda_vnops.c:coda_symlink, vput is called before lookup on the
> parent vnode.  This seemed wrong to me - dropping a reference and then
> using the vnode.  I took out the vput and got 'locking against
> myself'.  After reading the code more, I came to the conclusion:

Good analysis, I think this is correct.

Can you please try following patch? It just uses VOP_LOOKUP() directly
to find the vnode rather than lookup(), otherwise it should be same
as your version.

Jaromir

Index: coda_vnops.c
===================================================================
RCS file: /cvsroot/src/sys/coda/coda_vnops.c,v
retrieving revision 1.33
diff -u -p -r1.33 coda_vnops.c
--- coda_vnops.c	2003/01/06 20:32:42	1.33
+++ coda_vnops.c	2003/03/19 09:30:04
@@ -1642,26 +1642,9 @@ coda_symlink(v)
     /* Invalidate the parent's attr cache, the modification time has changed */
     tdcp->c_flags &= ~C_VATTR;
 
-    if (!error)
-    {
-	struct nameidata nd;
-	NDINIT(&nd, LOOKUP, FOLLOW|LOCKLEAF, UIO_SYSSPACE, nm, p);
-	nd.ni_cnd.cn_cred = cred;
-	nd.ni_loopcnt = 0;
-	nd.ni_startdir = tdvp;
-	nd.ni_cnd.cn_pnbuf = (char *)nm;
-	nd.ni_cnd.cn_nameptr = nd.ni_cnd.cn_pnbuf;
-	nd.ni_pathlen = len;
-	vput(tdvp);
-	error = lookup(&nd);
-	*ap->a_vpp = nd.ni_vp;
-    }
-
-    /* 
-     * Free the name buffer 
-     */
-    if ((cnp->cn_flags & SAVESTART) == 0) {
-	PNBUF_PUT(cnp->cn_pnbuf);
+    if (!error) {
+	VOP_UNLOCK(tdvp, 0);
+	error = VOP_LOOKUP(tdvp, ap->a_vpp, cnp);
     }
 
  exit:    
-- 
Jaromir Dolecek <jdolecek@NetBSD.org>            http://www.NetBSD.org/
-=- We should be mindful of the potential goal, but as the tantric    -=-
-=- Buddhist masters say, ``You may notice during meditation that you -=-
-=- sometimes levitate or glow.   Do not let this distract you.''     -=-