xfs: introduce CONFIG_XFS_WARN

Running a CONFIG_XFS_DEBUG kernel in production environments is not
the best idea as it introduces significant overhead, can change
the behaviour of algorithms (such as allocation) to improve test
coverage, and (most importantly) panic the machine on non-fatal
errors.

There are many cases where all we want to do is run a
kernel with more bounds checking enabled, such as is provided by the
ASSERT() statements throughout the code, but without all the
potential overhead and drawbacks.

This patch converts all the ASSERT statements to evaluate as
WARN_ON(1) statements and hence if they fail dump a warning and a
stack trace to the log. This has minimal overhead and does not
change any algorithms, and will allow us to find strange "out of
bounds" problems more easily on production machines.

There are a few places where assert statements contain debug only
code. These are converted to be debug-or-warn only code so that we
still get all the assert checks in the code.

Signed-off-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Signed-off-by: Ben Myers <bpm@sgi.com>

diff --git a/fs/xfs/Kconfig b/fs/xfs/Kconfig
index cc33aaf219f15fc30cc2df86ee01a8553b3148e4..399e8cec6e603bb84df41b3d062e2a88e99a98a3 100644 (file)
--- a/fs/xfs/Kconfig
+++ b/fs/xfs/Kconfig
@@ -69,6 +69,19 @@ config XFS_RT
 
          If unsure, say N.
 
+config XFS_WARN
+       bool "XFS Verbose Warnings"
+       depends on XFS_FS && !XFS_DEBUG
+       help
+         Say Y here to get an XFS build with many additional warnings.
+         It converts ASSERT checks to WARN, so will log any out-of-bounds
+         conditions that occur that would otherwise be missed. It is much
+         lighter weight than XFS_DEBUG and does not modify algorithms and will
+         not cause the kernel to panic on non-fatal errors.
+
+         However, similar to XFS_DEBUG, it is only advisable to use this if you
+         are debugging a particular problem.
+
 config XFS_DEBUG
        bool "XFS Debugging support"
        depends on XFS_FS

diff --git a/fs/xfs/mrlock.h b/fs/xfs/mrlock.h
index ff6a19873e5cfb6a2ac45dd38514d951a45cd105..e3c92d19e54079e5ffc950adf8974c35eee0703e 100644 (file)
--- a/fs/xfs/mrlock.h
+++ b/fs/xfs/mrlock.h
@@ -22,12 +22,12 @@
 
 typedef struct {
        struct rw_semaphore     mr_lock;
-#ifdef DEBUG
+#if defined(DEBUG) || defined(XFS_WARN)
        int                     mr_writer;
 #endif
 } mrlock_t;
 
-#ifdef DEBUG
+#if defined(DEBUG) || defined(XFS_WARN)
 #define mrinit(mrp, name)      \
        do { (mrp)->mr_writer = 0; init_rwsem(&(mrp)->mr_lock); } while (0)
 #else
@@ -46,7 +46,7 @@ static inline void mraccess_nested(mrlock_t *mrp, int subclass)
 static inline void mrupdate_nested(mrlock_t *mrp, int subclass)
 {
        down_write_nested(&mrp->mr_lock, subclass);
-#ifdef DEBUG
+#if defined(DEBUG) || defined(XFS_WARN)
        mrp->mr_writer = 1;
 #endif
 }
@@ -60,7 +60,7 @@ static inline int mrtryupdate(mrlock_t *mrp)
 {
        if (!down_write_trylock(&mrp->mr_lock))
                return 0;
-#ifdef DEBUG
+#if defined(DEBUG) || defined(XFS_WARN)
        mrp->mr_writer = 1;
 #endif
        return 1;
@@ -68,7 +68,7 @@ static inline int mrtryupdate(mrlock_t *mrp)
 
 static inline void mrunlock_excl(mrlock_t *mrp)
 {
-#ifdef DEBUG
+#if defined(DEBUG) || defined(XFS_WARN)
        mrp->mr_writer = 0;
 #endif
        up_write(&mrp->mr_lock);
@@ -81,7 +81,7 @@ static inline void mrunlock_shared(mrlock_t *mrp)
 
 static inline void mrdemote(mrlock_t *mrp)
 {
-#ifdef DEBUG
+#if defined(DEBUG) || defined(XFS_WARN)
        mrp->mr_writer = 0;
 #endif
        downgrade_write(&mrp->mr_lock);

diff --git a/fs/xfs/xfs.h b/fs/xfs/xfs.h
index d8b11b7f94aae5bf7f591037d1fc07eaf79e7c25..a742c47f7d5a00cc7bd38cd80a153eea3d4ffa7e 100644 (file)
--- a/fs/xfs/xfs.h
+++ b/fs/xfs/xfs.h
@@ -24,6 +24,11 @@
 #define XFS_BUF_LOCK_TRACKING 1
 #endif
 
+#ifdef CONFIG_XFS_WARN
+#define XFS_WARN 1
+#endif
+
+
 #include "xfs_linux.h"
 
 #endif /* __XFS_H__ */

diff --git a/fs/xfs/xfs_alloc_btree.c b/fs/xfs/xfs_alloc_btree.c
index 30c4c1434faf1dd2f2440eaf3ac27fe0fd191dd9..cafc90251d1993e76c10d0b6c6dae9d875dcd21e 100644 (file)
--- a/fs/xfs/xfs_alloc_btree.c
+++ b/fs/xfs/xfs_alloc_btree.c
@@ -386,7 +386,7 @@ const struct xfs_buf_ops xfs_allocbt_buf_ops = {
 };
 
 
-#ifdef DEBUG
+#if defined(DEBUG) || defined(XFS_WARN)
 STATIC int
 xfs_allocbt_keys_inorder(
        struct xfs_btree_cur    *cur,
@@ -442,7 +442,7 @@ static const struct xfs_btree_ops xfs_allocbt_ops = {
        .init_ptr_from_cur      = xfs_allocbt_init_ptr_from_cur,
        .key_diff               = xfs_allocbt_key_diff,
        .buf_ops                = &xfs_allocbt_buf_ops,
-#ifdef DEBUG
+#if defined(DEBUG) || defined(XFS_WARN)
        .keys_inorder           = xfs_allocbt_keys_inorder,
        .recs_inorder           = xfs_allocbt_recs_inorder,
 #endif

diff --git a/fs/xfs/xfs_bmap_btree.c b/fs/xfs/xfs_bmap_btree.c
index 3a86c3fa6de1bd3be963e3615a29cdf038744857..0c61a22be6fd630668a16d0f92b3db625fa03173 100644 (file)
--- a/fs/xfs/xfs_bmap_btree.c
+++ b/fs/xfs/xfs_bmap_btree.c
@@ -813,7 +813,7 @@ const struct xfs_buf_ops xfs_bmbt_buf_ops = {
 };
 
 
-#ifdef DEBUG
+#if defined(DEBUG) || defined(XFS_WARN)
 STATIC int
 xfs_bmbt_keys_inorder(
        struct xfs_btree_cur    *cur,
@@ -853,7 +853,7 @@ static const struct xfs_btree_ops xfs_bmbt_ops = {
        .init_ptr_from_cur      = xfs_bmbt_init_ptr_from_cur,
        .key_diff               = xfs_bmbt_key_diff,
        .buf_ops                = &xfs_bmbt_buf_ops,
-#ifdef DEBUG
+#if defined(DEBUG) || defined(XFS_WARN)
        .keys_inorder           = xfs_bmbt_keys_inorder,
        .recs_inorder           = xfs_bmbt_recs_inorder,
 #endif

diff --git a/fs/xfs/xfs_btree.h b/fs/xfs/xfs_btree.h
index 6e6c915673feb398adfe82bd1b0b25a157a73428..55e3c7cc3c3d3f22178fb1feb8aab44679821172 100644 (file)
--- a/fs/xfs/xfs_btree.h
+++ b/fs/xfs/xfs_btree.h
@@ -215,7 +215,7 @@ struct xfs_btree_ops {
 
        const struct xfs_buf_ops        *buf_ops;
 
-#ifdef DEBUG
+#if defined(DEBUG) || defined(XFS_WARN)
        /* check that k1 is lower than k2 */
        int     (*keys_inorder)(struct xfs_btree_cur *cur,
                                union xfs_btree_key *k1,

diff --git a/fs/xfs/xfs_dir2_node.c b/fs/xfs/xfs_dir2_node.c
index ecc6c661064c8b09011b7f46dfce40b8b94d817e..5246de4912d4b07d3fda2940bfe30a15d77a09f1 100644 (file)
--- a/fs/xfs/xfs_dir2_node.c
+++ b/fs/xfs/xfs_dir2_node.c
@@ -993,7 +993,7 @@ xfs_dir2_leafn_rebalance(
        xfs_dir2_leaf_t         *leaf1;         /* first leaf structure */
        xfs_dir2_leaf_t         *leaf2;         /* second leaf structure */
        int                     mid;            /* midpoint leaf index */
-#ifdef DEBUG
+#if defined(DEBUG) || defined(XFS_WARN)
        int                     oldstale;       /* old count of stale leaves */
 #endif
        int                     oldsum;         /* old total leaf count */
@@ -1022,7 +1022,7 @@ xfs_dir2_leafn_rebalance(
        ents2 = xfs_dir3_leaf_ents_p(leaf2);
 
        oldsum = hdr1.count + hdr2.count;
-#ifdef DEBUG
+#if defined(DEBUG) || defined(XFS_WARN)
        oldstale = hdr1.stale + hdr2.stale;
 #endif
        mid = oldsum >> 1;

diff --git a/fs/xfs/xfs_ialloc_btree.c b/fs/xfs/xfs_ialloc_btree.c
index c82ac88674217bb8a41872244a4fa1b64c8f2cae..5448eb6b8c12ad1acdf9d621750ec39e6c280a80 100644 (file)
--- a/fs/xfs/xfs_ialloc_btree.c
+++ b/fs/xfs/xfs_ialloc_btree.c
@@ -272,7 +272,7 @@ const struct xfs_buf_ops xfs_inobt_buf_ops = {
        .verify_write = xfs_inobt_write_verify,
 };
 
-#ifdef DEBUG
+#if defined(DEBUG) || defined(XFS_WARN)
 STATIC int
 xfs_inobt_keys_inorder(
        struct xfs_btree_cur    *cur,
@@ -310,7 +310,7 @@ static const struct xfs_btree_ops xfs_inobt_ops = {
        .init_ptr_from_cur      = xfs_inobt_init_ptr_from_cur,
        .key_diff               = xfs_inobt_key_diff,
        .buf_ops                = &xfs_inobt_buf_ops,
-#ifdef DEBUG
+#if defined(DEBUG) || defined(XFS_WARN)
        .keys_inorder           = xfs_inobt_keys_inorder,
        .recs_inorder           = xfs_inobt_recs_inorder,
 #endif

diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index 558ef494720675aeb44bbfcf69792f2a8eae34eb..efbe1accb6ca0b5e433e87df5fe0976469764e3d 100644 (file)
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -287,7 +287,7 @@ xfs_ilock_demote(
        trace_xfs_ilock_demote(ip, lock_flags, _RET_IP_);
 }
 
-#ifdef DEBUG
+#if defined(DEBUG) || defined(XFS_WARN)
 int
 xfs_isilocked(
        xfs_inode_t             *ip,

diff --git a/fs/xfs/xfs_linux.h b/fs/xfs/xfs_linux.h
index 14e59d953b7bb24b65d0b31f32b737551b2eea44..800f896a6cc48cdffc19f85d6a9dd4c30ef897d5 100644 (file)
--- a/fs/xfs/xfs_linux.h
+++ b/fs/xfs/xfs_linux.h
@@ -293,22 +293,34 @@ static inline __uint64_t howmany_64(__uint64_t x, __uint32_t y)
 #define ASSERT_ALWAYS(expr)    \
        (unlikely(expr) ? (void)0 : assfail(#expr, __FILE__, __LINE__))
 
-#ifndef DEBUG
-#define ASSERT(expr)   ((void)0)
+#ifdef DEBUG
+#define ASSERT(expr)   \
+       (unlikely(expr) ? (void)0 : assfail(#expr, __FILE__, __LINE__))
 
 #ifndef STATIC
-# define STATIC static noinline
+# define STATIC noinline
 #endif
 
-#else /* DEBUG */
+#else  /* !DEBUG */
+
+#ifdef XFS_WARN
 
 #define ASSERT(expr)   \
-       (unlikely(expr) ? (void)0 : assfail(#expr, __FILE__, __LINE__))
+       (unlikely(expr) ? (void)0 : asswarn(#expr, __FILE__, __LINE__))
 
 #ifndef STATIC
-# define STATIC noinline
+# define STATIC static noinline
+#endif
+
+#else  /* !DEBUG && !XFS_WARN */
+
+#define ASSERT(expr)   ((void)0)
+
+#ifndef STATIC
+# define STATIC static noinline
 #endif
 
+#endif /* XFS_WARN */
 #endif /* DEBUG */
 
 #endif /* __XFS_LINUX__ */

diff --git a/fs/xfs/xfs_message.c b/fs/xfs/xfs_message.c
index 331cd9f83a7fe04b8089cdcf4952a3186ad7f037..9163dc14053244c7ff6a70f60e91271995bbe87c 100644 (file)
--- a/fs/xfs/xfs_message.c
+++ b/fs/xfs/xfs_message.c
@@ -92,6 +92,14 @@ xfs_alert_tag(
        BUG_ON(do_panic);
 }
 
+void
+asswarn(char *expr, char *file, int line)
+{
+       xfs_warn(NULL, "Assertion failed: %s, file: %s, line: %d",
+               expr, file, line);
+       WARN_ON(1);
+}
+
 void
 assfail(char *expr, char *file, int line)
 {

diff --git a/fs/xfs/xfs_message.h b/fs/xfs/xfs_message.h
index 76c81982f9640049e0b8573bdc1dd9bb013f55f4..85401155750eaa5e95868597bc1e97e7b24dcbab 100644 (file)
--- a/fs/xfs/xfs_message.h
+++ b/fs/xfs/xfs_message.h
@@ -57,6 +57,7 @@ do {                                                                  \
        xfs_printk_ratelimited(xfs_debug, dev, fmt, ##__VA_ARGS__)
 
 extern void assfail(char *expr, char *f, int l);
+extern void asswarn(char *expr, char *f, int l);
 
 extern void xfs_hex_dump(void *p, int length);
 

diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
index cd29f617102165c9e518088faf701ab14df52cd4..a44dba5b2cdb1665d5e70e3dc59477632730a8f5 100644 (file)
--- a/fs/xfs/xfs_trans.h
+++ b/fs/xfs/xfs_trans.h
@@ -405,7 +405,7 @@ typedef struct xfs_trans {
        int64_t                 t_res_fdblocks_delta; /* on-disk only chg */
        int64_t                 t_frextents_delta;/* superblock freextents chg*/
        int64_t                 t_res_frextents_delta; /* on-disk only chg */
-#ifdef DEBUG
+#if defined(DEBUG) || defined(XFS_WARN)
        int64_t                 t_ag_freeblks_delta; /* debugging counter */
        int64_t                 t_ag_flist_delta; /* debugging counter */
        int64_t                 t_ag_btree_delta; /* debugging counter */
@@ -433,7 +433,7 @@ typedef struct xfs_trans {
 #define        xfs_trans_get_block_res(tp)     ((tp)->t_blk_res)
 #define        xfs_trans_set_sync(tp)          ((tp)->t_flags |= XFS_TRANS_SYNC)
 
-#ifdef DEBUG
+#if defined(DEBUG) || defined(XFS_WARN)
 #define        xfs_trans_agblocks_delta(tp, d) ((tp)->t_ag_freeblks_delta += (int64_t)d)
 #define        xfs_trans_agflist_delta(tp, d)  ((tp)->t_ag_flist_delta += (int64_t)d)
 #define        xfs_trans_agbtree_delta(tp, d)  ((tp)->t_ag_btree_delta += (int64_t)d)