[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]>