Linux: Fix ZFS_ENTER/ZFS_EXIT/ZFS_VERFY_ZP usage

The ZFS_ENTER/ZFS_EXIT/ZFS_VERFY_ZP macros should not be used in the Linux zpl_*.c source files. They return a positive error value which is correct for the common code, but not for the Linux specific kernel code which expects a negative return value. The ZPL_ENTER/ZPL_EXIT/ZPL_VERFY_ZP macros should be used instead. Furthermore, the ZPL_EXIT macro has been updated to not call the zfs_exit_fs() function. This prevents a possible deadlock which can occur when a snapshot is automatically unmounted because the zpl_show_devname() must never wait on in progress automatic snapshot unmounts. Reviewed-by: Adam Moss <[email protected]> Reviewed-by: Ryan Moeller <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes #11169 Closes #11201
openzfs · Nov 14, 2020 · d02fc15 · d02fc15
1 parent 435dc4b
commit d02fc15
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 19 deletions.
diff --git a/include/os/linux/zfs/sys/zfs_znode_impl.h b/include/os/linux/zfs/sys/zfs_znode_impl.h
@@ -90,7 +90,11 @@ do {								\
 	zfs_exit_fs(zfsvfs);					\
 	rrm_exit(&(zfsvfs)->z_teardown_lock, FTAG);		\
 } while (0)
-#define	ZPL_EXIT(zfsvfs)	ZFS_EXIT(zfsvfs)
+
+#define	ZPL_EXIT(zfsvfs)					\
+do {								\
+	rrm_exit(&(zfsvfs)->z_teardown_lock, FTAG);		\
+} while (0)
 
 /* Verifies the znode is valid. */
 #define	ZFS_VERIFY_ZP_ERROR(zp, error)				\

diff --git a/module/os/linux/zfs/zpl_ctldir.c b/module/os/linux/zfs/zpl_ctldir.c
@@ -55,7 +55,7 @@ zpl_root_iterate(struct file *filp, zpl_dir_context_t *ctx)
 	zfsvfs_t *zfsvfs = ITOZSB(file_inode(filp));
 	int error = 0;
 
-	ZFS_ENTER(zfsvfs);
+	ZPL_ENTER(zfsvfs);
 
 	if (!zpl_dir_emit_dots(filp, ctx))
 		goto out;
@@ -76,7 +76,7 @@ zpl_root_iterate(struct file *filp, zpl_dir_context_t *ctx)
 		ctx->pos++;
 	}
 out:
-	ZFS_EXIT(zfsvfs);
+	ZPL_EXIT(zfsvfs);
 
 	return (error);
 }
@@ -242,7 +242,7 @@ zpl_snapdir_iterate(struct file *filp, zpl_dir_context_t *ctx)
 	uint64_t id, pos;
 	int error = 0;
 
-	ZFS_ENTER(zfsvfs);
+	ZPL_ENTER(zfsvfs);
 	cookie = spl_fstrans_mark();
 
 	if (!zpl_dir_emit_dots(filp, ctx))
@@ -266,7 +266,7 @@ zpl_snapdir_iterate(struct file *filp, zpl_dir_context_t *ctx)
 	}
 out:
 	spl_fstrans_unmark(cookie);
-	ZFS_EXIT(zfsvfs);
+	ZPL_EXIT(zfsvfs);
 
 	if (error == -ENOENT)
 		return (0);
@@ -369,13 +369,13 @@ zpl_snapdir_getattr_impl(const struct path *path, struct kstat *stat,
 	struct inode *ip = path->dentry->d_inode;
 	zfsvfs_t *zfsvfs = ITOZSB(ip);
 
-	ZFS_ENTER(zfsvfs);
+	ZPL_ENTER(zfsvfs);
 	generic_fillattr(ip, stat);
 
 	stat->nlink = stat->size = 2;
 	stat->ctime = stat->mtime = dmu_objset_snap_cmtime(zfsvfs->z_os);
 	stat->atime = current_time(ip);
-	ZFS_EXIT(zfsvfs);
+	ZPL_EXIT(zfsvfs);
 
 	return (0);
 }
@@ -453,7 +453,7 @@ zpl_shares_iterate(struct file *filp, zpl_dir_context_t *ctx)
 	znode_t *dzp;
 	int error = 0;
 
-	ZFS_ENTER(zfsvfs);
+	ZPL_ENTER(zfsvfs);
 	cookie = spl_fstrans_mark();
 
 	if (zfsvfs->z_shares_dir == 0) {
@@ -472,7 +472,7 @@ zpl_shares_iterate(struct file *filp, zpl_dir_context_t *ctx)
 	iput(ZTOI(dzp));
 out:
 	spl_fstrans_unmark(cookie);
-	ZFS_EXIT(zfsvfs);
+	ZPL_EXIT(zfsvfs);
 	ASSERT3S(error, <=, 0);
 
 	return (error);
@@ -503,13 +503,13 @@ zpl_shares_getattr_impl(const struct path *path, struct kstat *stat,
 	znode_t *dzp;
 	int error;
 
-	ZFS_ENTER(zfsvfs);
+	ZPL_ENTER(zfsvfs);
 
 	if (zfsvfs->z_shares_dir == 0) {
 		generic_fillattr(path->dentry->d_inode, stat);
 		stat->nlink = stat->size = 2;
 		stat->atime = current_time(ip);
-		ZFS_EXIT(zfsvfs);
+		ZPL_EXIT(zfsvfs);
 		return (0);
 	}
 
@@ -519,7 +519,7 @@ zpl_shares_getattr_impl(const struct path *path, struct kstat *stat,
 		iput(ZTOI(dzp));
 	}
 
-	ZFS_EXIT(zfsvfs);
+	ZPL_EXIT(zfsvfs);
 	ASSERT3S(error, <=, 0);
 
 	return (error);

diff --git a/module/os/linux/zfs/zpl_file.c b/module/os/linux/zfs/zpl_file.c
@@ -675,10 +675,10 @@ zpl_writepages(struct address_space *mapping, struct writeback_control *wbc)
 	enum writeback_sync_modes sync_mode;
 	int result;
 
-	ZFS_ENTER(zfsvfs);
+	ZPL_ENTER(zfsvfs);
 	if (zfsvfs->z_os->os_sync == ZFS_SYNC_ALWAYS)
 		wbc->sync_mode = WB_SYNC_ALL;
-	ZFS_EXIT(zfsvfs);
+	ZPL_EXIT(zfsvfs);
 	sync_mode = wbc->sync_mode;
 
 	/*
@@ -691,11 +691,11 @@ zpl_writepages(struct address_space *mapping, struct writeback_control *wbc)
 	wbc->sync_mode = WB_SYNC_NONE;
 	result = write_cache_pages(mapping, wbc, zpl_putpage, mapping);
 	if (sync_mode != wbc->sync_mode) {
-		ZFS_ENTER(zfsvfs);
-		ZFS_VERIFY_ZP(zp);
+		ZPL_ENTER(zfsvfs);
+		ZPL_VERIFY_ZP(zp);
 		if (zfsvfs->z_log != NULL)
 			zil_commit(zfsvfs->z_log, zp->z_id);
-		ZFS_EXIT(zfsvfs);
+		ZPL_EXIT(zfsvfs);
 
 		/*
 		 * We need to call write_cache_pages() again (we can't just

diff --git a/module/os/linux/zfs/zpl_super.c b/module/os/linux/zfs/zpl_super.c
@@ -185,7 +185,7 @@ zpl_remount_fs(struct super_block *sb, int *flags, char *data)
 static int
 __zpl_show_devname(struct seq_file *seq, zfsvfs_t *zfsvfs)
 {
-	ZFS_ENTER(zfsvfs);
+	ZPL_ENTER(zfsvfs);
 
 	char *fsname = kmem_alloc(ZFS_MAX_DATASET_NAME_LEN, KM_SLEEP);
 	dmu_objset_name(zfsvfs->z_os, fsname);
@@ -205,7 +205,7 @@ __zpl_show_devname(struct seq_file *seq, zfsvfs_t *zfsvfs)
 
 	kmem_free(fsname, ZFS_MAX_DATASET_NAME_LEN);
 
-	ZFS_EXIT(zfsvfs);
+	ZPL_EXIT(zfsvfs);
 
 	return (0);
 }