I think I've tracked down my problem with nullfs layered over NFS causing
deadlocks on 4.10. The nullfs code blindly trusts the lower layer to
correctly set the PDIRUNLOCK flag to know whether it has released the lock
on the parent directory, and NFS doesn't seem to have any code that
manipulates this flag. The result is that nullfs's VOP_LOOKUP doesn't
release the lock on the upper layer.
Everything else that I've looked at (FFS, SMBFS, MSDOSFS, etc.) seems to
play nice with PDIRUNLOCK, though I haven't actually tested with anything
other than FFS and NFS. NFS in -CURRENT also appears to use PDIRUNLOCK.
I thought I was missing something here, wondering "how did NFS ever
work?!", until I discovered that NFS in -STABLE just isn't locked. Oh.
But why do we have PDIRUNLOCK, anyway? Shouldn't everything be following
the locking protocol described in VOP_LOOKUP(9)?
Also, there appears to be a separate logic bug in the -CURRENT nullfs code
(and tjr's nullfs patch) that results in the same deadlock if the lower
layer did not export a lock.
Here's a patch against -STABLE that should make things better. This
is obviously not the right solution, but it is easier than hacking
on NFS.
nullfs in -CURRENT could use the "... dvp->v_vnlock == NULL || ..."
part
of the fix, too, to fix the no-exported-lock bug. Of course, everything
in current might be exporting a lock by now.
Index: null_vnops.c
==================================================================RCS file:
/home/ncvs/src/sys/miscfs/nullfs/Attic/null_vnops.c,v
retrieving revision 1.38.2.6
diff -u -r1.38.2.6 null_vnops.c
--- null_vnops.c 31 Jul 2002 00:32:28 -0000 1.38.2.6
+++ null_vnops.c 1 Aug 2004 18:08:49 -0000
@@ -376,17 +376,26 @@
ldvp = NULLVPTOLOWERVP(dvp);
vp = lvp = NULL;
error = VOP_LOOKUP(ldvp, &lvp, cnp);
+
+ /*
+ * We should be relying on PDIRUNLOCK here, but we can't
+ * currently trust the underlying filesystem to set it.
+ * Instead, we'll determine whether the lower vnode should
+ * have been unlocked per VOP_LOOKUP(9).
+ *
+ * Also, only release our lock if it is not shared with
+ * the lower vnode.
+ */
+ if ((error == 0 || error == EJUSTRETURN) &&
+ !((flags & ISLASTCN) && (flags & LOCKPARENT)) &&
+ (dvp->v_vnlock == NULL || dvp->v_vnlock != ldvp->v_vnlock))
+ VOP_UNLOCK(dvp, LK_THISLAYER, p);
+
if (error == EJUSTRETURN && (flags & ISLASTCN) &&
(dvp->v_mount->mnt_flag & MNT_RDONLY) &&
(cnp->cn_nameiop == CREATE || cnp->cn_nameiop == RENAME))
error = EROFS;
- /*
- * Rely only on the PDIRUNLOCK flag which should be carefully
- * tracked by underlying filesystem.
- */
- if (cnp->cn_flags & PDIRUNLOCK)
- VOP_UNLOCK(dvp, LK_THISLAYER, p);
if ((error == 0 || error == EJUSTRETURN) && lvp != NULL) {
if (ldvp == lvp) {
*ap->a_vpp = dvp;
--
Brian Buchanan, CISSP bwb@holo.org
--------------------------------------------------------------------------
FreeBSD - The Power to Serve http://www.freebsd.org