Allow zfs_purgedir() to skip inodes undergoing eviction

When destroying a file which contains xattrs the xattr directory inode
and its child inodes may be acquired with zfs_zget().  This can result
in a deadlock if these inodes are part of the same disposal list.  This
is only possible in zfs_purgedir() because it is called from evict()
while processing this disposal list.

Prevent this deadlock by allowing zfs_zget() to fail in zfs_purgedir().
The object will be left in the unlinked set and processing of it will
be deferred via the existing mechanisms.

Signed-off-by: Brian Behlendorf <behlendorf1@llnl.gov>
Issue openzfs#4816

Author: behlendorf

include/sys/zfs_znode.h

@@ -305,6 +305,7 @@ extern void	zfs_znode_init(void);
 extern void	zfs_znode_fini(void);
 extern int	zfs_znode_hold_compare(const void *, const void *);
 extern int	zfs_zget(zfs_sb_t *, uint64_t, znode_t **);
+extern int	zfs_zget_retry(zfs_sb_t *, uint64_t, znode_t **, boolean_t);
 extern int	zfs_rezget(znode_t *);
 extern void	zfs_zinactive(znode_t *);
 extern void	zfs_znode_delete(znode_t *, dmu_tx_t *);

module/zfs/zfs_dir.c

@@ -518,7 +518,7 @@ zfs_unlinked_drain(zfs_sb_t *zsb)
 		 * We need to re-mark these list entries for deletion,
 		 * so we pull them back into core and set zp->z_unlinked.
 		 */
-		error = zfs_zget(zsb, zap.za_first_integer, &zp);
+		error = zfs_zget_retry(zsb, zap.za_first_integer, &zp, B_FALSE);
 
 		/*
 		 * We may pick up znodes that are already marked for deletion.
@@ -561,8 +561,8 @@ zfs_purgedir(znode_t *dzp)
 	for (zap_cursor_init(&zc, zsb->z_os, dzp->z_id);
 	    (error = zap_cursor_retrieve(&zc, &zap)) == 0;
 	    zap_cursor_advance(&zc)) {
-		error = zfs_zget(zsb,
-		    ZFS_DIRENT_OBJ(zap.za_first_integer), &xzp);
+		error = zfs_zget_retry(zsb,
+		    ZFS_DIRENT_OBJ(zap.za_first_integer), &xzp, B_FALSE);
 		if (error) {
 			skipped += 1;
 			continue;

module/zfs/zfs_znode.c

@@ -1061,8 +1061,8 @@ zfs_xvattr_set(znode_t *zp, xvattr_t *xvap, dmu_tx_t *tx)
 	}
 }
 
-int
-zfs_zget(zfs_sb_t *zsb, uint64_t obj_num, znode_t **zpp)
+static int
+zfs_zget_impl(zfs_sb_t *zsb, uint64_t obj_num, znode_t **zpp, boolean_t retry)
 {
 	dmu_object_info_t doi;
 	dmu_buf_t	*db;
@@ -1116,28 +1116,25 @@ zfs_zget(zfs_sb_t *zsb, uint64_t obj_num, znode_t **zpp)
 			 * called iput_final() to start the eviction process.
 			 * The SA handle is still valid but because the VFS
 			 * requires that the eviction succeed we must drop
-			 * our locks and references to allow the eviction to
-			 * complete.  The zfs_zget() may then be retried.
-			 *
-			 * This unlikely case could be optimized by registering
-			 * a sops->drop_inode() callback.  The callback would
-			 * need to detect the active SA hold thereby informing
-			 * the VFS that this inode should not be evicted.
+			 * our locks and references and attempt to allow the
+			 * eviction to complete.
 			 */
-			if (igrab(ZTOI(zp)) == NULL) {
-				mutex_exit(&zp->z_lock);
-				sa_buf_rele(db, NULL);
-				zfs_znode_hold_exit(zsb, zh);
-				/* inode might need this to finish evict */
-				cond_resched();
-				goto again;
+			if (igrab(ZTOI(zp)) != NULL) {
+				*zpp = zp;
+				err = 0;
+			} else {
+				err = SET_ERROR(EAGAIN);
 			}
-			*zpp = zp;
-			err = 0;
 		}
 		mutex_exit(&zp->z_lock);
 		sa_buf_rele(db, NULL);
 		zfs_znode_hold_exit(zsb, zh);
+
+		if (err == EAGAIN && retry == B_TRUE) {
+			cond_resched();
+			goto again;
+		}
+
 		return (err);
 	}
 
@@ -1162,6 +1159,18 @@ zfs_zget(zfs_sb_t *zsb, uint64_t obj_num, znode_t **zpp)
 	return (err);
 }
 
+int
+zfs_zget(zfs_sb_t *zsb, uint64_t obj_num, znode_t **zpp)
+{
+	return (zfs_zget_impl(zsb, obj_num, zpp, B_TRUE));
+}
+
+int
+zfs_zget_retry(zfs_sb_t *zsb, uint64_t obj_num, znode_t **zpp, boolean_t retry)
+{
+	return (zfs_zget_impl(zsb, obj_num, zpp, retry));
+}
+
 int
 zfs_rezget(znode_t *zp)
 {