From 7ae3f8dc8f1e075108f91ddf6fb16471d4a0f044 Mon Sep 17 00:00:00 2001 From: Brian Behlendorf Date: Mon, 18 Nov 2019 13:05:56 -0800 Subject: [PATCH] Partially revert 5a6ac4c Reinstate the zpl_revalidate() functionality to resolve a regression where dentries for open files during a rollback are not invalidated. The unrelated functionality for automatically unmounting .zfs/snapshots was not reverted. Nor was the addition of shrink_dcache_sb() to the zfs_resume_fs() function. This issue was not immediately caught by the CI because the test case intended to catch it was included in the list of ZTS tests which may occasionally fail for unrelated reasons. Remove all of the rollback tests from this list to help identify the frequency of any spurious failures. The rollback_003_pos.ksh test case exposes a real issue with the long standing code which needs to be investigated. Regardless, it has been enable with a small workaround in the test case itself. Reviewed-by: Matt Ahrens Reviewed-by: Pavel Snajdr Signed-off-by: Brian Behlendorf Closes #9587 Closes #9592 --- config/kernel-dentry-operations.m4 | 24 ++++++++++ include/os/linux/zfs/sys/zpl.h | 1 + module/os/linux/zfs/zfs_vfsops.c | 1 + module/os/linux/zfs/zpl_inode.c | 44 +++++++++++++++++++ tests/test-runner/bin/zts-report.py | 3 -- .../functional/snapshot/rollback_003_pos.ksh | 29 ++++++------ 6 files changed, 85 insertions(+), 17 deletions(-) diff --git a/config/kernel-dentry-operations.m4 b/config/kernel-dentry-operations.m4 index 660d6260b..dd470d760 100644 --- a/config/kernel-dentry-operations.m4 +++ b/config/kernel-dentry-operations.m4 @@ -147,6 +147,28 @@ AC_DEFUN([ZFS_AC_KERNEL_CONST_DENTRY_OPERATIONS], [ ]) ]) +dnl # +dnl # 2.6.38 API change +dnl # Added sb->s_d_op default dentry_operations member +dnl # +AC_DEFUN([ZFS_AC_KERNEL_SRC_S_D_OP], [ + ZFS_LINUX_TEST_SRC([super_block_s_d_op], [ + #include + ],[ + struct super_block sb __attribute__ ((unused)); + sb.s_d_op = NULL; + ]) +]) + +AC_DEFUN([ZFS_AC_KERNEL_S_D_OP], [ + AC_MSG_CHECKING([whether super_block has s_d_op]) + ZFS_LINUX_TEST_RESULT([super_block_s_d_op], [ + AC_MSG_RESULT(yes) + ], [ + ZFS_LINUX_TEST_ERROR([super_block s_d_op]) + ]) +]) + AC_DEFUN([ZFS_AC_KERNEL_SRC_DENTRY], [ ZFS_AC_KERNEL_SRC_D_MAKE_ROOT ZFS_AC_KERNEL_SRC_D_OBTAIN_ALIAS @@ -154,6 +176,7 @@ AC_DEFUN([ZFS_AC_KERNEL_SRC_DENTRY], [ ZFS_AC_KERNEL_SRC_D_SET_D_OP ZFS_AC_KERNEL_SRC_D_REVALIDATE_NAMEIDATA ZFS_AC_KERNEL_SRC_CONST_DENTRY_OPERATIONS + ZFS_AC_KERNEL_SRC_S_D_OP ]) AC_DEFUN([ZFS_AC_KERNEL_DENTRY], [ @@ -163,4 +186,5 @@ AC_DEFUN([ZFS_AC_KERNEL_DENTRY], [ ZFS_AC_KERNEL_D_SET_D_OP ZFS_AC_KERNEL_D_REVALIDATE_NAMEIDATA ZFS_AC_KERNEL_CONST_DENTRY_OPERATIONS + ZFS_AC_KERNEL_S_D_OP ]) diff --git a/include/os/linux/zfs/sys/zpl.h b/include/os/linux/zfs/sys/zpl.h index c5b347728..20a3dc674 100644 --- a/include/os/linux/zfs/sys/zpl.h +++ b/include/os/linux/zfs/sys/zpl.h @@ -45,6 +45,7 @@ extern const struct inode_operations zpl_inode_operations; extern const struct inode_operations zpl_dir_inode_operations; extern const struct inode_operations zpl_symlink_inode_operations; extern const struct inode_operations zpl_special_inode_operations; +extern dentry_operations_t zpl_dentry_operations; /* zpl_file.c */ extern ssize_t zpl_read_common(struct inode *ip, const char *buf, diff --git a/module/os/linux/zfs/zfs_vfsops.c b/module/os/linux/zfs/zfs_vfsops.c index c0acaab0b..154933a4c 100644 --- a/module/os/linux/zfs/zfs_vfsops.c +++ b/module/os/linux/zfs/zfs_vfsops.c @@ -1926,6 +1926,7 @@ zfs_domount(struct super_block *sb, zfs_mnt_t *zm, int silent) sb->s_op = &zpl_super_operations; sb->s_xattr = zpl_xattr_handlers; sb->s_export_op = &zpl_export_operations; + sb->s_d_op = &zpl_dentry_operations; /* Set features for file system. */ zfs_set_fuid_feature(zfsvfs); diff --git a/module/os/linux/zfs/zpl_inode.c b/module/os/linux/zfs/zpl_inode.c index 7912a5b3d..124021166 100644 --- a/module/os/linux/zfs/zpl_inode.c +++ b/module/os/linux/zfs/zpl_inode.c @@ -602,6 +602,46 @@ out: return (error); } +static int +#ifdef HAVE_D_REVALIDATE_NAMEIDATA +zpl_revalidate(struct dentry *dentry, struct nameidata *nd) +{ + unsigned int flags = (nd ? nd->flags : 0); +#else +zpl_revalidate(struct dentry *dentry, unsigned int flags) +{ +#endif /* HAVE_D_REVALIDATE_NAMEIDATA */ + /* CSTYLED */ + zfsvfs_t *zfsvfs = dentry->d_sb->s_fs_info; + int error; + + if (flags & LOOKUP_RCU) + return (-ECHILD); + + /* + * After a rollback negative dentries created before the rollback + * time must be invalidated. Otherwise they can obscure files which + * are only present in the rolled back dataset. + */ + if (dentry->d_inode == NULL) { + spin_lock(&dentry->d_lock); + error = time_before(dentry->d_time, zfsvfs->z_rollback_time); + spin_unlock(&dentry->d_lock); + + if (error) + return (0); + } + + /* + * The dentry may reference a stale inode if a mounted file system + * was rolled back to a point in time where the object didn't exist. + */ + if (dentry->d_inode && ITOZ(dentry->d_inode)->z_is_stale) + return (0); + + return (1); +} + const struct inode_operations zpl_inode_operations = { .setattr = zpl_setattr, .getattr = zpl_getattr, @@ -690,3 +730,7 @@ const struct inode_operations zpl_special_inode_operations = { .get_acl = zpl_get_acl, #endif /* CONFIG_FS_POSIX_ACL */ }; + +dentry_operations_t zpl_dentry_operations = { + .d_revalidate = zpl_revalidate, +}; diff --git a/tests/test-runner/bin/zts-report.py b/tests/test-runner/bin/zts-report.py index 65233c41b..4c3c29384 100755 --- a/tests/test-runner/bin/zts-report.py +++ b/tests/test-runner/bin/zts-report.py @@ -189,7 +189,6 @@ known = { 'removal/removal_with_zdb': ['SKIP', known_reason], 'rootpool/setup': ['SKIP', na_reason], 'rsend/rsend_008_pos': ['SKIP', '6066'], - 'snapshot/rollback_003_pos': ['SKIP', '6143'], 'vdev_zaps/vdev_zaps_007_pos': ['FAIL', known_reason], 'xattr/xattr_008_pos': ['SKIP', na_reason], 'xattr/xattr_009_neg': ['SKIP', na_reason], @@ -222,8 +221,6 @@ maybe = { 'cli_root/zdb/zdb_006_pos': ['FAIL', known_reason], 'cli_root/zfs_get/zfs_get_004_pos': ['FAIL', known_reason], 'cli_root/zfs_get/zfs_get_009_pos': ['SKIP', '5479'], - 'cli_root/zfs_rollback/zfs_rollback_001_pos': ['FAIL', '6415'], - 'cli_root/zfs_rollback/zfs_rollback_002_pos': ['FAIL', '6416'], 'cli_root/zfs_share/setup': ['SKIP', share_reason], 'cli_root/zfs_snapshot/zfs_snapshot_002_neg': ['FAIL', known_reason], 'cli_root/zfs_unshare/setup': ['SKIP', share_reason], diff --git a/tests/zfs-tests/tests/functional/snapshot/rollback_003_pos.ksh b/tests/zfs-tests/tests/functional/snapshot/rollback_003_pos.ksh index 342e7df58..59e7c110d 100755 --- a/tests/zfs-tests/tests/functional/snapshot/rollback_003_pos.ksh +++ b/tests/zfs-tests/tests/functional/snapshot/rollback_003_pos.ksh @@ -48,10 +48,6 @@ verify_runnable "both" -if is_linux; then - log_unsupported "Test case is known to fail on Linux" -fi - function cleanup { typeset snap="" @@ -61,18 +57,16 @@ function cleanup log_must zfs mount -a unset __ZFS_POOL_RESTRICT - for snap in "$SNAPPOOL.1" "$SNAPPOOL" - do - snapexists $snap - [[ $? -eq 0 ]] && \ - log_must zfs destroy $snap + for snap in "$SNAPPOOL.1" "$SNAPPOOL"; do + if snapexists $snap; then + destroy_snapshot $snap + fi done - for fs in "$TESTPOOL/$TESTFILE/$TESTFILE.1" "$TESTPOOL/$TESTFILE" - do - datasetexists $fs - [[ $? -eq 0 ]] && \ - log_must zfs destroy -r $fs + for fs in "$TESTPOOL/$TESTFILE/$TESTFILE.1" "$TESTPOOL/$TESTFILE"; do + if datasetexists $fs; then + destroy_dataset $fs -r + fi done [[ -e /$TESTPOOL ]] && \ @@ -107,4 +101,11 @@ log_must touch /$TESTPOOL/$TESTFILE/$TESTFILE.1 log_must zfs rollback $SNAPPOOL.1 +# +# Workaround for issue #6143. Issuing a `df` seems to properly force any +# negative dcache entries to be invalidated preventing subsequent failures +# when accessing the mount point. Additional investigation required. +# +log_must df + log_pass "Rollbacks succeed when nested file systems are present."