From 4ec15b8dcf8038aeb15c7877c50d0fa500b468c6 Mon Sep 17 00:00:00 2001 From: Brian Behlendorf Date: Thu, 26 Feb 2015 15:29:33 -0800 Subject: [PATCH 1/3] Use MUTEX_FSTRANS mutex type There are regions in the ZFS code where it is desirable to be able to be set PF_FSTRANS while a specific mutex is held. The ZFS code could be updated to set/clear this flag in all the correct places, but this is undesirable for a few reasons. 1) It would require changes to a significant amount of the ZFS code. This would complicate applying patches from upstream. 2) It would be easy to accidentally miss a critical region in the initial patch or to have an future change introduce a new one. Both of these concerns can be addressed by using a new mutex type which is responsible for managing PF_FSTRANS, support for which was added to the SPL in commit zfsonlinux/spl@9099312 - Merge branch 'kmem-rework'. Signed-off-by: Brian Behlendorf Signed-off-by: Tim Chase Closes #3050 Closes #3055 Closes #3062 Closes #3132 Closes #3142 Closes #2983 --- include/sys/zfs_context.h | 1 + module/zfs/arc.c | 2 +- module/zfs/dbuf.c | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/include/sys/zfs_context.h b/include/sys/zfs_context.h index 3dc54f1d7d90..b8eff58bc615 100644 --- a/include/sys/zfs_context.h +++ b/include/sys/zfs_context.h @@ -273,6 +273,7 @@ typedef struct kmutex { } kmutex_t; #define MUTEX_DEFAULT 0 +#define MUTEX_FSTRANS MUTEX_DEFAULT #define MUTEX_HELD(m) ((m)->m_owner == curthread) #define MUTEX_NOT_HELD(m) (!MUTEX_HELD(m)) diff --git a/module/zfs/arc.c b/module/zfs/arc.c index 800394c21653..070d85aafed6 100644 --- a/module/zfs/arc.c +++ b/module/zfs/arc.c @@ -928,7 +928,7 @@ buf_init(void) for (i = 0; i < BUF_LOCKS; i++) { mutex_init(&buf_hash_table.ht_locks[i].ht_lock, - NULL, MUTEX_DEFAULT, NULL); + NULL, MUTEX_FSTRANS, NULL); } } diff --git a/module/zfs/dbuf.c b/module/zfs/dbuf.c index 9be69b5ae2cf..277e5439e3df 100644 --- a/module/zfs/dbuf.c +++ b/module/zfs/dbuf.c @@ -331,7 +331,7 @@ dbuf_init(void) 0, dbuf_cons, dbuf_dest, NULL, NULL, NULL, 0); for (i = 0; i < DBUF_MUTEXES; i++) - mutex_init(&h->hash_mutexes[i], NULL, MUTEX_DEFAULT, NULL); + mutex_init(&h->hash_mutexes[i], NULL, MUTEX_FSTRANS, NULL); dbuf_stats_init(h); } From 8c45def24a5c640a3b44ce38cc3482b9c89a3b1d Mon Sep 17 00:00:00 2001 From: Brian Behlendorf Date: Fri, 27 Feb 2015 16:09:52 -0800 Subject: [PATCH 2/3] Linux 4.0 compat: bdi_setup_and_register() The 'capabilities' argument which was passed to bdi_setup_and_register() has been removed. File systems should no longer pass BDI_CAP_MAP_COPY. For our purposes this means there are now three different interfaces which must be handled. A zpl_bdi_setup_and_register() wrapper function has been introduced to provide a single interface to the ZPL code. * 2.6.32 - 2.6.33, bdi_setup_and_register() is not exported. * 2.6.34 - 3.19, bdi_setup_and_register() takes 3 arguments. * 4.0 - x.y, bdi_setup_and_register() takes 2 arguments. I've also taken this opportunity to remove HAVE_BDI because kernels older then 2.6.32 are no longer supported. All kernels newer than this will have one of the above interfaces. Signed-off-by: Brian Behlendorf Signed-off-by: Chunwei Chen Closes #3128 --- config/kernel-bdi-setup-and-register.m4 | 36 +++++++++++++++++-------- config/kernel-bdi.m4 | 21 --------------- config/kernel.m4 | 1 - include/linux/vfs_compat.h | 30 ++++++++++++++------- module/zfs/zfs_vfsops.c | 21 +++------------ 5 files changed, 49 insertions(+), 60 deletions(-) delete mode 100644 config/kernel-bdi.m4 diff --git a/config/kernel-bdi-setup-and-register.m4 b/config/kernel-bdi-setup-and-register.m4 index 6369409b8b39..f13d1fe53dda 100644 --- a/config/kernel-bdi-setup-and-register.m4 +++ b/config/kernel-bdi-setup-and-register.m4 @@ -1,22 +1,36 @@ dnl # -dnl # 2.6.34 API change -dnl # The bdi_setup_and_register() helper function is avaliable and -dnl # exported by the kernel. This is a trivial helper function but -dnl # using it significantly simplifies the code surrounding setting -dnl # up and tearing down the bdi structure. +dnl # 2.6.32 - 2.6.33, bdi_setup_and_register() is not exported. +dnl # 2.6.34 - 3.19, bdi_setup_and_register() takes 3 arguments. +dnl # 4.0 - x.y, bdi_setup_and_register() takes 2 arguments. dnl # -AC_DEFUN([ZFS_AC_KERNEL_BDI_SETUP_AND_REGISTER], - [AC_MSG_CHECKING([whether bdi_setup_and_register() is available]) +AC_DEFUN([ZFS_AC_KERNEL_BDI_SETUP_AND_REGISTER], [ + AC_MSG_CHECKING([whether bdi_setup_and_register() wants 2 args]) ZFS_LINUX_TRY_COMPILE_SYMBOL([ #include ], [ - int r = bdi_setup_and_register(NULL, NULL, 0); - r = *(&r); + struct backing_dev_info bdi; + char *name = "bdi"; + (void) bdi_setup_and_register(&bdi, name); ], [bdi_setup_and_register], [mm/backing-dev.c], [ AC_MSG_RESULT(yes) - AC_DEFINE(HAVE_BDI_SETUP_AND_REGISTER, 1, - [bdi_setup_and_register() is available]) + AC_DEFINE(HAVE_2ARGS_BDI_SETUP_AND_REGISTER, 1, + [bdi_setup_and_register() wants 2 args]) ], [ AC_MSG_RESULT(no) + AC_MSG_CHECKING([whether bdi_setup_and_register() wants 3 args]) + ZFS_LINUX_TRY_COMPILE_SYMBOL([ + #include + ], [ + struct backing_dev_info bdi; + char *name = "bdi"; + unsigned int cap = BDI_CAP_MAP_COPY; + (void) bdi_setup_and_register(&bdi, name, cap); + ], [bdi_setup_and_register], [mm/backing-dev.c], [ + AC_MSG_RESULT(yes) + AC_DEFINE(HAVE_3ARGS_BDI_SETUP_AND_REGISTER, 1, + [bdi_setup_and_register() wants 3 args]) + ], [ + AC_MSG_RESULT(no) + ]) ]) ]) diff --git a/config/kernel-bdi.m4 b/config/kernel-bdi.m4 deleted file mode 100644 index 00bd37539cbe..000000000000 --- a/config/kernel-bdi.m4 +++ /dev/null @@ -1,21 +0,0 @@ -dnl # -dnl # 2.6.32 API change -dnl # Private backing_device_info interfaces available -dnl # -AC_DEFUN([ZFS_AC_KERNEL_BDI], [ - AC_MSG_CHECKING([whether super_block has s_bdi]) - ZFS_LINUX_TRY_COMPILE([ - #include - - static const struct super_block - sb __attribute__ ((unused)) = { - .s_bdi = NULL, - }; - ],[ - ],[ - AC_MSG_RESULT(yes) - AC_DEFINE(HAVE_BDI, 1, [struct super_block has s_bdi]) - ],[ - AC_MSG_RESULT(no) - ]) -]) diff --git a/config/kernel.m4 b/config/kernel.m4 index e0b7954224ca..ce10707e4954 100644 --- a/config/kernel.m4 +++ b/config/kernel.m4 @@ -90,7 +90,6 @@ AC_DEFUN([ZFS_AC_CONFIG_KERNEL], [ ZFS_AC_KERNEL_SHRINK ZFS_AC_KERNEL_S_INSTANCES_LIST_HEAD ZFS_AC_KERNEL_S_D_OP - ZFS_AC_KERNEL_BDI ZFS_AC_KERNEL_BDI_SETUP_AND_REGISTER ZFS_AC_KERNEL_SET_NLINK ZFS_AC_KERNEL_ELEVATOR_CHANGE diff --git a/include/linux/vfs_compat.h b/include/linux/vfs_compat.h index 6f5a9dc968f8..e8f844841076 100644 --- a/include/linux/vfs_compat.h +++ b/include/linux/vfs_compat.h @@ -65,25 +65,35 @@ truncate_setsize(struct inode *ip, loff_t new) } #endif /* HAVE_TRUNCATE_SETSIZE */ -#if defined(HAVE_BDI) && !defined(HAVE_BDI_SETUP_AND_REGISTER) /* - * 2.6.34 API change, - * Add bdi_setup_and_register() function if not yet provided by kernel. - * It is used to quickly initialize and register a BDI for the filesystem. + * 2.6.32 - 2.6.33, bdi_setup_and_register() is not available. + * 2.6.34 - 3.19, bdi_setup_and_register() takes 3 arguments. + * 4.0 - x.y, bdi_setup_and_register() takes 2 arguments. */ +#if defined(HAVE_2ARGS_BDI_SETUP_AND_REGISTER) +static inline int +zpl_bdi_setup_and_register(struct backing_dev_info *bdi, char *name) +{ + return (bdi_setup_and_register(bdi, name)); +} +#elif defined(HAVE_3ARGS_BDI_SETUP_AND_REGISTER) +static inline int +zpl_bdi_setup_and_register(struct backing_dev_info *bdi, char *name) +{ + return (bdi_setup_and_register(bdi, name, BDI_CAP_MAP_COPY)); +} +#else extern atomic_long_t zfs_bdi_seq; static inline int -bdi_setup_and_register( - struct backing_dev_info *bdi, - char *name, - unsigned int cap) +zpl_bdi_setup_and_register(struct backing_dev_info *bdi, char *name) { char tmp[32]; int error; bdi->name = name; - bdi->capabilities = cap; + bdi->capabilities = BDI_CAP_MAP_COPY; + error = bdi_init(bdi); if (error) return (error); @@ -98,7 +108,7 @@ bdi_setup_and_register( return (error); } -#endif /* HAVE_BDI && !HAVE_BDI_SETUP_AND_REGISTER */ +#endif /* * 2.6.38 API change, diff --git a/module/zfs/zfs_vfsops.c b/module/zfs/zfs_vfsops.c index a2dea89b7f66..4df324a68f88 100644 --- a/module/zfs/zfs_vfsops.c +++ b/module/zfs/zfs_vfsops.c @@ -1198,9 +1198,10 @@ zfs_sb_teardown(zfs_sb_t *zsb, boolean_t unmounting) } EXPORT_SYMBOL(zfs_sb_teardown); -#if defined(HAVE_BDI) && !defined(HAVE_BDI_SETUP_AND_REGISTER) +#if !defined(HAVE_2ARGS_BDI_SETUP_AND_REGISTER) && \ + !defined(HAVE_3ARGS_BDI_SETUP_AND_REGISTER) atomic_long_t zfs_bdi_seq = ATOMIC_LONG_INIT(0); -#endif /* HAVE_BDI && !HAVE_BDI_SETUP_AND_REGISTER */ +#endif int zfs_domount(struct super_block *sb, void *data, int silent) @@ -1227,23 +1228,12 @@ zfs_domount(struct super_block *sb, void *data, int silent) sb->s_time_gran = 1; sb->s_blocksize = recordsize; sb->s_blocksize_bits = ilog2(recordsize); - -#ifdef HAVE_BDI - /* - * 2.6.32 API change, - * Added backing_device_info (BDI) per super block interfaces. A BDI - * must be configured when using a non-device backed filesystem for - * proper writeback. This is not required for older pdflush kernels. - * - * NOTE: Linux read-ahead is disabled in favor of zfs read-ahead. - */ zsb->z_bdi.ra_pages = 0; sb->s_bdi = &zsb->z_bdi; - error = -bdi_setup_and_register(&zsb->z_bdi, "zfs", BDI_CAP_MAP_COPY); + error = -zpl_bdi_setup_and_register(&zsb->z_bdi, "zfs"); if (error) goto out; -#endif /* HAVE_BDI */ /* Set callback operations for the file system. */ sb->s_op = &zpl_super_operations; @@ -1336,10 +1326,7 @@ zfs_umount(struct super_block *sb) VERIFY(zfs_sb_teardown(zsb, B_TRUE) == 0); os = zsb->z_os; - -#ifdef HAVE_BDI bdi_destroy(sb->s_bdi); -#endif /* HAVE_BDI */ /* * z_os will be NULL if there was an error in From 989fd514b1053d5443b4e6155af9c8d863f5f0f2 Mon Sep 17 00:00:00 2001 From: Brian Behlendorf Date: Fri, 27 Feb 2015 12:53:35 -0800 Subject: [PATCH 3/3] Change ASSERT(!"...") to cmn_err(CE_PANIC, ...) There are a handful of ASSERT(!"...")'s throughout the code base for cases which should be impossible. This patch converts them to use cmn_err(CE_PANIC, ...) to ensure they are always enabled and so that additional debugging is logged if they were to occur. Signed-off-by: Brian Behlendorf Issue #1445 --- module/zcommon/zfs_deleg.c | 2 +- module/zfs/arc.c | 2 +- module/zfs/dmu_tx.c | 3 ++- module/zfs/zap_leaf.c | 9 ++++++--- module/zfs/zap_micro.c | 2 +- 5 files changed, 11 insertions(+), 7 deletions(-) diff --git a/module/zcommon/zfs_deleg.c b/module/zcommon/zfs_deleg.c index a152b4e76e0f..f6e41da9d7ea 100644 --- a/module/zcommon/zfs_deleg.c +++ b/module/zcommon/zfs_deleg.c @@ -227,7 +227,7 @@ zfs_deleg_whokey(char *attr, zfs_deleg_who_type_t type, ZFS_DELEG_FIELD_SEP_CHR); break; default: - ASSERT(!"bad zfs_deleg_who_type_t"); + cmn_err(CE_PANIC, "bad zfs_deleg_who_type_t %d", type); } } diff --git a/module/zfs/arc.c b/module/zfs/arc.c index 070d85aafed6..9a81b4c59944 100644 --- a/module/zfs/arc.c +++ b/module/zfs/arc.c @@ -2944,7 +2944,7 @@ arc_access(arc_buf_hdr_t *buf, kmutex_t *hash_lock) DTRACE_PROBE1(new_state__mfu, arc_buf_hdr_t *, buf); arc_change_state(arc_mfu, buf, hash_lock); } else { - ASSERT(!"invalid arc state"); + cmn_err(CE_PANIC, "invalid arc state 0x%p", buf->b_state); } } diff --git a/module/zfs/dmu_tx.c b/module/zfs/dmu_tx.c index 890aecc1d2c1..cdf5a6d0fcfa 100644 --- a/module/zfs/dmu_tx.c +++ b/module/zfs/dmu_tx.c @@ -925,7 +925,8 @@ dmu_tx_dirty_buf(dmu_tx_t *tx, dmu_buf_impl_t *db) match_object = TRUE; break; default: - ASSERT(!"bad txh_type"); + cmn_err(CE_PANIC, "bad txh_type %d", + txh->txh_type); } } if (match_object && match_offset) { diff --git a/module/zfs/zap_leaf.c b/module/zfs/zap_leaf.c index 78f05d7a7e37..9578048250e2 100644 --- a/module/zfs/zap_leaf.c +++ b/module/zfs/zap_leaf.c @@ -79,8 +79,9 @@ stv(int len, void *addr, uint64_t value) case 8: *(uint64_t *)addr = value; return; + default: + cmn_err(CE_PANIC, "bad int len %d", len); } - ASSERT(!"bad int len"); } static uint64_t @@ -95,8 +96,9 @@ ldv(int len, const void *addr) return (*(uint32_t *)addr); case 8: return (*(uint64_t *)addr); + default: + cmn_err(CE_PANIC, "bad int len %d", len); } - ASSERT(!"bad int len"); return (0xFEEDFACEDEADBEEFULL); } @@ -147,7 +149,8 @@ zap_leaf_byteswap(zap_leaf_phys_t *buf, int size) /* la_array doesn't need swapping */ break; default: - ASSERT(!"bad leaf type"); + cmn_err(CE_PANIC, "bad leaf type %d", + lc->l_free.lf_type); } } } diff --git a/module/zfs/zap_micro.c b/module/zfs/zap_micro.c index 0c2e76319772..dfa7c6615659 100644 --- a/module/zfs/zap_micro.c +++ b/module/zfs/zap_micro.c @@ -965,7 +965,7 @@ mzap_addent(zap_name_t *zn, uint64_t value) start = 0; goto again; } - ASSERT(!"out of entries!"); + cmn_err(CE_PANIC, "out of entries!"); } int