From 5a6ac4cffcd0255ca6f730bcf356e12891fd18af Mon Sep 17 00:00:00 2001 From: Pavel Snajdr Date: Mon, 11 Nov 2019 18:34:21 +0100 Subject: [PATCH] Remove zpl_revalidate This patch removes the need for zpl_revalidate altogether. There were 3 main reasons why we used d_revalidate: 1. periodic automounted snapshots umount deferral 2. negative dentries created before snapshot rollback 3. stale inodes referenced by dentry cache after snapshot rollback Periodic snapshots deferral solution introduces zfs_exit_fs function, which is called as a part of ZFS_EXIT(zfsvfs_t) macro. Negative dentries and stale inodes are solved by flushing the dcache for the particular dataset on zfs_resume_fs call. This patch also removes now unused HAVE_S_D_OP configure test. Reviewed-by: Aleksa Sarai Reviewed-by: Brian Behlendorf Signed-off-by: Pavel Snajdr Closes #8774 Closes #9549 --- config/kernel-dentry-operations.m4 | 25 ---------- include/os/linux/zfs/sys/zfs_vfsops.h | 1 + include/os/linux/zfs/sys/zfs_znode_impl.h | 1 + include/os/linux/zfs/sys/zpl.h | 1 - module/os/linux/zfs/zfs_vfsops.c | 33 +++++++++++-- module/os/linux/zfs/zpl_inode.c | 60 ----------------------- 6 files changed, 32 insertions(+), 89 deletions(-) diff --git a/config/kernel-dentry-operations.m4 b/config/kernel-dentry-operations.m4 index 2dfd2ac554cf..de7482801b7b 100644 --- a/config/kernel-dentry-operations.m4 +++ b/config/kernel-dentry-operations.m4 @@ -150,29 +150,6 @@ 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) - AC_DEFINE(HAVE_S_D_OP, 1, [struct super_block has s_d_op]) - ], [ - AC_MSG_RESULT(no) - ]) -]) - AC_DEFUN([ZFS_AC_KERNEL_SRC_DENTRY], [ ZFS_AC_KERNEL_SRC_D_MAKE_ROOT ZFS_AC_KERNEL_SRC_D_OBTAIN_ALIAS @@ -180,7 +157,6 @@ 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], [ @@ -190,5 +166,4 @@ 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/zfs_vfsops.h b/include/os/linux/zfs/sys/zfs_vfsops.h index 3a959996f3cd..26e49d7b497c 100644 --- a/include/os/linux/zfs/sys/zfs_vfsops.h +++ b/include/os/linux/zfs/sys/zfs_vfsops.h @@ -197,6 +197,7 @@ typedef struct zfid_long { extern int zfs_suspend_fs(zfsvfs_t *zfsvfs); extern int zfs_resume_fs(zfsvfs_t *zfsvfs, struct dsl_dataset *ds); extern int zfs_end_fs(zfsvfs_t *zfsvfs, struct dsl_dataset *ds); +extern void zfs_exit_fs(zfsvfs_t *zfsvfs); extern int zfs_userspace_one(zfsvfs_t *zfsvfs, zfs_userquota_prop_t type, const char *domain, uint64_t rid, uint64_t *valuep); extern int zfs_userspace_many(zfsvfs_t *zfsvfs, zfs_userquota_prop_t type, diff --git a/include/os/linux/zfs/sys/zfs_znode_impl.h b/include/os/linux/zfs/sys/zfs_znode_impl.h index 0ecfb0a90a4f..65abf4ca9d79 100644 --- a/include/os/linux/zfs/sys/zfs_znode_impl.h +++ b/include/os/linux/zfs/sys/zfs_znode_impl.h @@ -84,6 +84,7 @@ do { \ /* Must be called before exiting the operation. */ #define ZFS_EXIT(zfsvfs) \ do { \ + zfs_exit_fs(zfsvfs); \ rrm_exit(&(zfsvfs)->z_teardown_lock, FTAG); \ } while (0) #define ZPL_EXIT(zfsvfs) ZFS_EXIT(zfsvfs) diff --git a/include/os/linux/zfs/sys/zpl.h b/include/os/linux/zfs/sys/zpl.h index 2766269f31c8..01f0f6f38c6e 100644 --- a/include/os/linux/zfs/sys/zpl.h +++ b/include/os/linux/zfs/sys/zpl.h @@ -45,7 +45,6 @@ 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 d0771fa6fedb..75840dfe71fb 100644 --- a/module/os/linux/zfs/zfs_vfsops.c +++ b/module/os/linux/zfs/zfs_vfsops.c @@ -1931,9 +1931,6 @@ 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; -#ifdef HAVE_S_D_OP - sb->s_d_op = &zpl_dentry_operations; -#endif /* HAVE_S_D_OP */ /* Set features for file system. */ zfs_set_fuid_feature(zfsvfs); @@ -2303,6 +2300,16 @@ zfs_resume_fs(zfsvfs_t *zfsvfs, dsl_dataset_t *ds) zfs_unlinked_drain(zfsvfs); } + /* + * Most of the time zfs_suspend_fs is used for changing the contents + * of the underlying dataset. ZFS rollback and receive operations + * might create files for which negative dentries are present in + * the cache. Since walking the dcache would require a lot of GPL-only + * code duplication, it's much easier on these rather rare occasions + * just to flush the whole dcache for the given dataset/filesystem. + */ + shrink_dcache_sb(zfsvfs->z_sb); + bail: if (err != 0) zfsvfs->z_unmounted = B_TRUE; @@ -2353,6 +2360,26 @@ zfs_end_fs(zfsvfs_t *zfsvfs, dsl_dataset_t *ds) return (0); } +/* + * Automounted snapshots rely on periodic revalidation + * to defer snapshots from being automatically unmounted. + */ + +inline void +zfs_exit_fs(zfsvfs_t *zfsvfs) +{ + if (!zfsvfs->z_issnap) + return; + + if (time_after(jiffies, zfsvfs->z_snap_defer_time + + MAX(zfs_expire_snapshot * HZ / 2, HZ))) { + zfsvfs->z_snap_defer_time = jiffies; + zfsctl_snapshot_unmount_delay(zfsvfs->z_os->os_spa, + dmu_objset_id(zfsvfs->z_os), + zfs_expire_snapshot); + } +} + int zfs_set_version(zfsvfs_t *zfsvfs, uint64_t newvers) { diff --git a/module/os/linux/zfs/zpl_inode.c b/module/os/linux/zfs/zpl_inode.c index 3f3b2e2dc53c..4c628f60898c 100644 --- a/module/os/linux/zfs/zpl_inode.c +++ b/module/os/linux/zfs/zpl_inode.c @@ -70,9 +70,6 @@ zpl_lookup(struct inode *dir, struct dentry *dentry, unsigned int flags) spin_lock(&dentry->d_lock); dentry->d_time = jiffies; -#ifndef HAVE_S_D_OP - d_set_d_op(dentry, &zpl_dentry_operations); -#endif /* HAVE_S_D_OP */ spin_unlock(&dentry->d_lock); if (error) { @@ -655,59 +652,6 @@ zpl_fallocate(struct inode *ip, int mode, loff_t offset, loff_t len) } #endif /* HAVE_INODE_FALLOCATE */ -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); - - /* - * Automounted snapshots rely on periodic dentry revalidation - * to defer snapshots from being automatically unmounted. - */ - if (zfsvfs->z_issnap) { - if (time_after(jiffies, zfsvfs->z_snap_defer_time + - MAX(zfs_expire_snapshot * HZ / 2, HZ))) { - zfsvfs->z_snap_defer_time = jiffies; - zfsctl_snapshot_unmount_delay(zfsvfs->z_os->os_spa, - dmu_objset_id(zfsvfs->z_os), zfs_expire_snapshot); - } - } - - /* - * 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, @@ -820,7 +764,3 @@ const struct inode_operations zpl_special_inode_operations = { #endif /* HAVE_GET_ACL | HAVE_CHECK_ACL | HAVE_PERMISSION */ #endif /* CONFIG_FS_POSIX_ACL */ }; - -dentry_operations_t zpl_dentry_operations = { - .d_revalidate = zpl_revalidate, -};