mirror of
https://github.com/torvalds/linux.git
synced 2026-09-23 05:04:02 +02:00
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 <glass.su@suse.com>
Link: https://lore.kernel.org/linux-btrfs/DC0C775E-13B3-47D9-9AB2-895BB11C029D@suse.com/
Fixes: 83f7e52b7e ("btrfs: warn about extent buffer that can not be released")
Reviewed-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
This commit is contained in:
parent
5376c9db45
commit
690c2accac
|
|
@ -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();
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user