[possible patch] fdescfs vop_lookup and parent vnode locking

[Kostik Belousov]

Is it correct that filesystem-specific vop_lookup is expected to
take the directory vnode (dvp) locked, and shall return with dvp locked ?
vnode_if.src specifies this behaviour as dependent on the flags in the cnp
arguments. My reading of the sources suggest that the statement in the first
sentence is true, and note from vnode_if.src applies to
sys/kern/vfs_lookup.c:lookup() procedure itself.

If it is true, the fdescfs break this rule, dropping the lock on the dvp. See
sys/fs/fdescfs/fdesc_vnops.c, line 196. As result, having fdescfs mounted,
most accesses to files on it cause warning from the lockmgr,
lockmgr: thread XXX unlocking unheld lock

I did some testing with the following patch

Index: sys/fs/fdescfs/fdesc_vnops.c
===================================================================
RCS file: /usr/local/arch/ncvs/src/sys/fs/fdescfs/fdesc_vnops.c,v
retrieving revision 1.100
diff -u -r1.100 fdesc_vnops.c
--- sys/fs/fdescfs/fdesc_vnops.c	10 Aug 2005 07:08:14 -0000	1.100
+++ sys/fs/fdescfs/fdesc_vnops.c	3 Feb 2006 14:35:11 -0000
@@ -193,11 +193,9 @@
 		goto bad;
 	}

-	VOP_UNLOCK(dvp, 0, td);
 	if (cnp->cn_namelen == 1 && *pname == '.') {
 		*vpp = dvp;
 		VREF(dvp);
-		vn_lock(dvp, LK_EXCLUSIVE | LK_RETRY, td);
 		return (0);
 	}

@@ -228,12 +226,12 @@
 	if (error)
 		goto bad;
 	VTOFDESC(fvp)->fd_fd = fd;
-	vn_lock(fvp, LK_EXCLUSIVE | LK_RETRY, td);
+	if (fvp != dvp)
+		vn_lock(fvp, LK_EXCLUSIVE | LK_RETRY, td);
 	*vpp = fvp;
 	return (0);

 bad:
-	vn_lock(dvp, LK_EXCLUSIVE | LK_RETRY, td);
 	*vpp = NULL;
 	return (error);
 }


and do not see the warnings anymore. The system does not deadlock.

What do you think about the problem ?

Best regards,
Kostik Belousov