From d02fc15ba1739903afe273c4c1ce064954050652 Mon Sep 17 00:00:00 2001 From: Brian Behlendorf Date: Sat, 14 Nov 2020 10:19:00 -0800 Subject: [PATCH] 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 Reviewed-by: Ryan Moeller Signed-off-by: Brian Behlendorf Closes #11169 Closes #11201 --- include/os/linux/zfs/sys/zfs_znode_impl.h | 6 +++++- module/os/linux/zfs/zpl_ctldir.c | 22 +++++++++++----------- module/os/linux/zfs/zpl_file.c | 10 +++++----- module/os/linux/zfs/zpl_super.c | 4 ++-- 4 files changed, 23 insertions(+), 19 deletions(-) diff --git a/include/os/linux/zfs/sys/zfs_znode_impl.h b/include/os/linux/zfs/sys/zfs_znode_impl.h index 39bbd135ac06..145d7605297f 100644 --- 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 index b503962cd678..e6420f19ed87 100644 --- 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 index 51e189a87272..09159db42a3a 100644 --- 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 index 88ced77c005d..c2fd3fee1401 100644 --- 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); }