From 690c2accacb1aca91ab8186d15dee56da8723f31 Mon Sep 17 00:00:00 2001 From: Qu Wenruo Date: Sat, 20 Jun 2026 14:37:02 +0930 Subject: [PATCH] btrfs: make sure EXTENT_BUFFER_READING is cleared under refs_lock [FALSE ALERTS] There is a bug report that the warning inside invalidate_and_check_btree_folios() got triggered during btrfs/298: BTRFS info (device sdd): first mount of filesystem f9bf732a-a19b-44b9-99a7-614ddff168e2 BTRFS info (device sdd): using crc32c checksum algorithm BTRFS error (device sdd): failed to find fsid cb2fdb42-b638-4f2f-badd-4127467ba674 when attempting to open seed devices BTRFS error (device sdd): failed to read chunk tree: -2 ------------[ cut here ]------------ WARNING: disk-io.c:3342 at invalidate_and_check_btree_folios+0x260/0x3c0 [btrfs], CPU#4: mount/125993 CPU: 4 UID: 0 PID: 125993 Comm: mount Tainted: G W OE 7.1.0-rc7-custom+ #1 PREEMPT(full) Hardware name: QEMU KVM Virtual Machine, BIOS edk2-20250812-19.fc42 08/12/2025 Call trace: invalidate_and_check_btree_folios+0x260/0x3c0 [btrfs] (P) open_ctree+0x1f50/0x23b0 [btrfs] btrfs_get_tree+0x89c/0xc48 [btrfs] vfs_get_tree+0x30/0x110 vfs_cmd_create+0x58/0xe8 __arm64_sys_fsconfig+0x39c/0x518 invoke_syscall.constprop.0+0x48/0x120 el0_svc_common.constprop.0+0x40/0xe8 do_el0_svc+0x24/0x38 el0_svc+0x50/0x310 el0t_64_sync_handler+0xa0/0xe8 el0t_64_sync+0x198/0x1a0 ---[ end trace 0000000000000000 ]--- BTRFS warning (device sdd): unable to release extent buffer 365985792 owner 3 gen 17 refs 3 flags 0x5 [CAUSE] In that invalidate_and_check_btree_folios() we wait for the eb to finish its read, then check if it's only held by us and the btree inode. If not, then do a warning as it may be still held, and could cause problems. But there is a small window where the check can lead to false alerts: Thread A (Read endio) | Thread B (Unmount) ----------------------------------+------------------------------------- end_bbio_meta_read() | | The eb has one extra ref held | | by the reader, and has | | EXTENT_BUFFER_READING flag set | invalidate_and_check_btree_folios() | | | |- clear_extent_buffer_reading() | | | | |- wait_on_bit_io(); | | | The EXTENT_BUFFER_READING flag is | | | cleared | | |- if (refcount_read(eb->refs) > 2) | | The eb is held by the read, us | | and btree inode, thus it | | will trigger the warning |- free_extent_buffer() | [FIX] Introduce a helper, free_extent_buffer_clear_reading(). If the new parameter, @clear_reading, is set, we will hold the spinlock at the beginning of free_extent_buffer_clear_reading() to make sure the EXTENT_BUFFER_READING flag is cleared inside the same critical section of decreasing refs. Now free_extent_buffer() will just call free_extent_buffer_clear_reading() with @clear_reading set to false, so no behavior change. But for end_bbio_meta_read(), it will not clear_extent_buffer_reading() directly, but pass @clear_reading as true. Then inside invalidate_and_check_btree_folios(), hold the refs_lock before reading refs. So that we eliminate the race window completely. Reported-by: Su Yue Link: https://lore.kernel.org/linux-btrfs/DC0C775E-13B3-47D9-9AB2-895BB11C029D@suse.com/ Fixes: 83f7e52b7ed1 ("btrfs: warn about extent buffer that can not be released") Reviewed-by: Filipe Manana Signed-off-by: Qu Wenruo Signed-off-by: David Sterba --- fs/btrfs/disk-io.c | 16 ++++++++++++++-- fs/btrfs/extent_io.c | 43 +++++++++++++++++++++++++++++++++---------- 2 files changed, 47 insertions(+), 12 deletions(-) diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index db3f5d3e3e04..d30c1d02d994 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -3313,6 +3313,8 @@ static void invalidate_and_check_btree_folios(struct btrfs_fs_info *fs_info) */ rcu_read_lock(); xa_for_each(&fs_info->buffer_tree, index, eb) { + unsigned int refs; + /* Increase the ref so that the eb won't disappear. */ if (!refcount_inc_not_zero(&eb->refs)) continue; @@ -3322,17 +3324,27 @@ static void invalidate_and_check_btree_folios(struct btrfs_fs_info *fs_info) if (test_bit(EXTENT_BUFFER_READING, &eb->bflags)) wait_on_bit_io(&eb->bflags, EXTENT_BUFFER_READING, TASK_UNINTERRUPTIBLE); + /* + * We hold the spinlock to make sure above + * EXTENT_BUFFER_READING flag is cleared with the held + * ref dropped. + * Or we can hit a race window and lead to false alerts. + */ + spin_lock(&eb->refs_lock); + refs = refcount_read(&eb->refs); + spin_unlock(&eb->refs_lock); + /* * The refs threshold is 2, one held by us at the beginning * of the loop, one for the ownership in the buffer tree. */ - if (unlikely(refcount_read(&eb->refs) > 2 || extent_buffer_under_io(eb))) { + if (unlikely(refs > 2 || extent_buffer_under_io(eb))) { WARN_ON_ONCE(IS_ENABLED(CONFIG_BTRFS_DEBUG)); btrfs_warn(fs_info, "unable to release extent buffer %llu owner %llu gen %llu refs %u flags 0x%lx", eb->start, btrfs_header_owner(eb), btrfs_header_generation(eb), - refcount_read(&eb->refs), eb->bflags); + refs, eb->bflags); } free_extent_buffer(eb); rcu_read_lock(); diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c index 80e6aaf72e5a..6a516cd1c18b 100644 --- a/fs/btrfs/extent_io.c +++ b/fs/btrfs/extent_io.c @@ -3853,12 +3853,31 @@ static int release_extent_buffer(struct extent_buffer *eb) return 0; } -void free_extent_buffer(struct extent_buffer *eb) +static void clear_extent_buffer_reading(struct extent_buffer *eb) +{ + clear_and_wake_up_bit(EXTENT_BUFFER_READING, &eb->bflags); +} + +static void free_extent_buffer_clear_reading(struct extent_buffer *eb, + bool clear_reading) { int refs; + if (!eb) return; + /* + * We want to clear EXTENT_BUFFER_READING flag and decrease refs + * in the same critical section. + * This will make sure invalidate_and_check_btree_folios() won't + * see an eb with EXTENT_BUFFER_READING cleared but refs not yet + * decreased. + */ + if (clear_reading) { + spin_lock(&eb->refs_lock); + clear_extent_buffer_reading(eb); + } + refs = refcount_read(&eb->refs); while (1) { if (test_bit(EXTENT_BUFFER_UNMAPPED, &eb->bflags)) { @@ -3869,11 +3888,16 @@ void free_extent_buffer(struct extent_buffer *eb) } /* Optimization to avoid locking eb->refs_lock. */ - if (atomic_try_cmpxchg(&eb->refs.refs, &refs, refs - 1)) + if (atomic_try_cmpxchg(&eb->refs.refs, &refs, refs - 1)) { + if (clear_reading) + spin_unlock(&eb->refs_lock); return; + } } - spin_lock(&eb->refs_lock); + if (!clear_reading) + spin_lock(&eb->refs_lock); + if (refcount_read(&eb->refs) == 2 && test_bit(EXTENT_BUFFER_STALE, &eb->bflags) && !extent_buffer_under_io(eb) && @@ -3887,6 +3911,11 @@ void free_extent_buffer(struct extent_buffer *eb) release_extent_buffer(eb); } +void free_extent_buffer(struct extent_buffer *eb) +{ + return free_extent_buffer_clear_reading(eb, false); +} + void free_extent_buffer_stale(struct extent_buffer *eb) { if (!eb) @@ -4012,11 +4041,6 @@ void set_extent_buffer_uptodate(struct extent_buffer *eb) btrfs_meta_folio_set_uptodate(eb->folios[i], eb); } -static void clear_extent_buffer_reading(struct extent_buffer *eb) -{ - clear_and_wake_up_bit(EXTENT_BUFFER_READING, &eb->bflags); -} - static void end_bbio_meta_read(struct btrfs_bio *bbio) { struct extent_buffer *eb = bbio->private; @@ -4040,8 +4064,7 @@ static void end_bbio_meta_read(struct btrfs_bio *bbio) else clear_extent_buffer_uptodate(eb); - clear_extent_buffer_reading(eb); - free_extent_buffer(eb); + free_extent_buffer_clear_reading(eb, true); bio_put(&bbio->bio); }