Fix zfs_xattr_owner_unlinked on FreeBSD and comment

Explain FreeBSD VFS' unfortunate idiosyncratic locking requirements. There is no functional change for other platforms. Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Matt Macy <[email protected]> Closes openzfs#9720
loli10K · Dec 16, 2019 · ba434b1 · ba434b1
1 parent ddb4e69
commit ba434b1
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 0 deletions.
diff --git a/module/zfs/zfs_log.c b/module/zfs/zfs_log.c
@@ -232,7 +232,13 @@ zfs_xattr_owner_unlinked(znode_t *zp)
 	int unlinked = 0;
 	znode_t *dzp;
 
+	/*
+	 * zrele drops the vnode lock which violates the VOP locking contract
+	 * on FreeBSD. See comment at the top of zfs_replay.c for more detail.
+	 */
+#ifndef __FreeBSD__
 	zhold(zp);
+#endif
 	/*
 	 * if zp is XATTR node, keep walking up via z_xattr_parent until we
 	 * get the owner
@@ -247,7 +253,9 @@ zfs_xattr_owner_unlinked(znode_t *zp)
 		zp = dzp;
 		unlinked = zp->z_unlinked;
 	}
+#ifndef __FreeBSD__
 	zrele(zp);
+#endif
 	return (unlinked);
 }
 

diff --git a/module/zfs/zfs_replay.c b/module/zfs/zfs_replay.c
@@ -48,6 +48,16 @@
 #include <sys/cred.h>
 #include <sys/zpl.h>
 
+/*
+ * NB: FreeBSD expects to be able to do vnode locking in lookup and
+ * hold the locks across all subsequent VOPs until vput is called.
+ * This means that its zfs vnops routines can't do any internal locking.
+ * In order to have the same contract as the Linux vnops there would
+ * needed to be duplicate locked vnops. If the vnops were used more widely
+ * in common code this would likely be preferable. However, currently
+ * this is the only file where this is the case.
+ */
+
 /*
  * Functions to replay ZFS intent log (ZIL) records
  * The functions are called through a function vector (zfs_replay_vector)