[PATCH] btrfs: add EM_WARN_ON_ONCE() to dump the extent map on a warning

Jeff Layton <[email protected]>
Newsgroups org.kernel.vger.linux-btrfs,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
When an extent_map trips a consistency check in the free path, the bare
WARN_ON() only gives a stack trace.  For the checks in
btrfs_free_extent_map():

	WARN_ON(btrfs_extent_map_in_tree(em));
	WARN_ON(!list_empty(&em->list));

that means the report tells us an extent map was freed while still in the
tree, or while still linked onto a list (e.g. an inode's modified_extents
list), but nothing about the map itself: its flags, refcount, or its
geometry.

Borrow the pattern the mm code uses with VM_WARN_ON_ONCE_FOLIO() and
friends, which dump the offending object via dump_page() before the
warning.  Add btrfs_dump_extent_map(), a helper that prints an extent
map's geometry, flags, refcount and list/tree membership, and an
EM_WARN_ON_ONCE(cond, em) macro that dumps the map before triggering the
warning and stack trace.  As with the VM_*_ONCE helpers it fires only
once, which keeps these checks from spamming the log on a filesystem
that is repeatedly hitting them.

Unlike the existing dump_extent_map() helper (which is gated behind
CONFIG_BTRFS_DEBUG and calls ASSERT(0)) and unlike the VM_* helpers
(gated behind CONFIG_DEBUG_VM), EM_WARN_ON_ONCE() is always compiled in,
since the conditions it guards can be hit on production kernels and the
extra context is what makes those reports actionable.  No btrfs_fs_info
is available at the call site, so the helper uses the plain printk helpers
rather than the device-aware btrfs_*() ones.

Convert both checks in btrfs_free_extent_map() to use it.

Signed-off-by: Jeff Layton <[email protected]>
---
We've seen a number of crashes in our fleet that have shown WARN_ON()
pops from the list_empty() check here. I'm hoping this might give us a
bit more info.
---
 fs/btrfs/extent_map.c | 20 ++++++++++++++++++--
 fs/btrfs/extent_map.h | 22 ++++++++++++++++++++++
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/extent_map.c b/fs/btrfs/extent_map.c
index 6b79bff241f2..670143f7d473 100644
--- a/fs/btrfs/extent_map.c
+++ b/fs/btrfs/extent_map.c
@@ -63,8 +63,8 @@ void btrfs_free_extent_map(struct extent_map *em)
 	if (!em)
 		return;
 	if (refcount_dec_and_test(&em->refs)) {
-		WARN_ON(btrfs_extent_map_in_tree(em));
-		WARN_ON(!list_empty(&em->list));
+		EM_WARN_ON_ONCE(btrfs_extent_map_in_tree(em), em);
+		EM_WARN_ON_ONCE(!list_empty(&em->list), em);
 		kmem_cache_free(extent_map_cache, em);
 	}
 }
@@ -316,6 +316,22 @@ static void dump_extent_map(struct btrfs_fs_info *fs_info, const char *prefix,
 	ASSERT(0);
 }
 
+/*
+ * Dump the contents of an extent map to the kernel log.  Used by
+ * EM_WARN_ON_ONCE() to print context about the offending map before its
+ * warning and stack trace.
+ */
+void btrfs_dump_extent_map(const struct extent_map *em, const char *prefix)
+{
+	pr_crit(
+"BTRFS: %s: em=%p start=%llu len=%llu disk_bytenr=%llu disk_num_bytes=%llu offset=%llu ram_bytes=%llu generation=%llu flags=0x%x refs=%u in_tree=%d list_empty=%d list=(next=%p prev=%p)\n",
+		prefix, em, em->start, em->len, em->disk_bytenr,
+		em->disk_num_bytes, em->offset, em->ram_bytes, em->generation,
+		em->flags, refcount_read(&em->refs),
+		btrfs_extent_map_in_tree(em), list_empty(&em->list),
+		em->list.next, em->list.prev);
+}
+
 /* Internal sanity checks for btrfs debug builds. */
 static void validate_extent_map(struct btrfs_fs_info *fs_info, struct extent_map *em)
 {
diff --git a/fs/btrfs/extent_map.h b/fs/btrfs/extent_map.h
index 6f685f3c9327..1d2efd4a5665 100644
--- a/fs/btrfs/extent_map.h
+++ b/fs/btrfs/extent_map.h
@@ -8,6 +8,8 @@
 #include <linux/rbtree.h>
 #include <linux/list.h>
 #include <linux/refcount.h>
+#include <linux/bug.h>
+#include <linux/stringify.h>
 #include "fs.h"
 
 struct btrfs_inode;
@@ -175,6 +177,26 @@ int btrfs_split_extent_map(struct btrfs_inode *inode, u64 start, u64 len, u64 pr
 
 struct extent_map *btrfs_alloc_extent_map(void);
 void btrfs_free_extent_map(struct extent_map *em);
+void btrfs_dump_extent_map(const struct extent_map *em, const char *prefix);
+
+/*
+ * Warn once about a condition involving an extent map, dumping the offending
+ * map's contents (geometry, flags, refcount and list/tree membership) before
+ * the warning and stack trace, much like VM_WARN_ON_ONCE_FOLIO() does for
+ * folios.
+ */
+#define EM_WARN_ON_ONCE(cond, em) ({					\
+	static bool __section(".data..once") __warned;			\
+	int __ret_warn_once = !!(cond);					\
+									\
+	if (unlikely(__ret_warn_once && !__warned)) {			\
+		btrfs_dump_extent_map((em),				\
+			"EM_WARN_ON_ONCE(" __stringify(cond) ")");	\
+		__warned = true;					\
+		WARN_ON(1);						\
+	}								\
+	unlikely(__ret_warn_once);					\
+})
 int __init btrfs_extent_map_init(void);
 void __cold btrfs_extent_map_exit(void);
 int btrfs_unpin_extent_cache(struct btrfs_inode *inode, u64 start, u64 len, u64 gen);

---
base-commit: eae9071be4d8d386e6530120bdaee42f6da1f70f
change-id: 20260626-em-warn-on-dump-e82922269f25

Best regards,
-- 
Jeff Layton <[email protected]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.