On Mon, Sep 02, 2019 at 06:29:34PM +0200, Christoph Hellwig wrote:
> On Sun, Sep 01, 2019 at 03:08:22PM -0500, Goldwyn Rodrigues wrote:
> > From: Goldwyn Rodrigues <rgold...@suse.com>
> > 
> > To improve debugging abilities, especially invalid option
> > asserts.
> 
> Looking at the code I'd much rather have unconditional WARN_ON_ONCE
> statements in most places.  Including returning an error when we see
> something invalid in most cases.

Yeah, I was thinking something like this, which has the advantage that
the report format is familiar to XFS developers and will get picked up
by the automated error collection stuff I put in xfstests to complain
about any XFS assertion failures:

iomap: Introduce CONFIG_FS_IOMAP_DEBUG

To improve debugging abilities, especially invalid option
asserts.

Signed-off-by: Goldwyn Rodrigues <rgold...@suse.com>
[darrick: restructure it to follow what xfs does]
Reviewed-by: Darrick J. Wong <darrick.w...@oracle.com>
Signed-off-by: Darrick J. Wong <darrick.w...@oracle.com>
---
 fs/Kconfig            |    3 +++
 fs/iomap/apply.c      |   10 ++++++++++
 include/linux/iomap.h |   14 ++++++++++++++
 3 files changed, 27 insertions(+)

diff --git a/fs/Kconfig b/fs/Kconfig
index bfb1c6095c7a..4bed5df9b55f 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -19,6 +19,9 @@ if BLOCK
 
 config FS_IOMAP
        bool
+config FS_IOMAP_DEBUG
+       bool "Debugging for the iomap code"
+       depends on FS_IOMAP
 
 source "fs/ext2/Kconfig"
 source "fs/ext4/Kconfig"
diff --git a/fs/iomap/apply.c b/fs/iomap/apply.c
index 54c02aecf3cd..95cc3a2cadd5 100644
--- a/fs/iomap/apply.c
+++ b/fs/iomap/apply.c
@@ -8,6 +8,16 @@
 #include <linux/fs.h>
 #include <linux/iomap.h>
 
+#ifdef CONFIG_FS_IOMAP_DEBUG
+void
+iomap_assertion_failed(const char *expr, const char *file, int line)
+{
+       printk(KERN_EMERG "IOMAP: Assertion failed: %s, file: %s, line: %d",
+              expr, file, line);
+       WARN_ON_ONCE(1);
+}
+#endif
+
 /*
  * Execute a iomap write on a segment of the mapping that spans a
  * contiguous range of pages that have identical block mapping state.
diff --git a/include/linux/iomap.h b/include/linux/iomap.h
index 834d3923e2f2..b3d5d6f323cf 100644
--- a/include/linux/iomap.h
+++ b/include/linux/iomap.h
@@ -20,6 +20,20 @@ struct page;
 struct vm_area_struct;
 struct vm_fault;
 
+#ifdef CONFIG_FS_IOMAP_DEBUG
+
+extern void iomap_assertion_failed(const char *expr, const char *f, int l);
+
+#define IOMAP_ASSERT(expr) \
+       do { \
+               if (unlikely(!(expr))) \
+                       iomap_assertion_failed(#expr, __FILE__, __LINE__); \
+       } while(0)
+
+#else
+#define IOMAP_ASSERT(expr)     ((void)0)
+#endif
+
 /*
  * Types of block ranges for iomap mappings:
  */

Reply via email to