Re: [PATCH] btrfs: fix race between device add and chunk allocation leading to transaction abort

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

在 2026/10/24 08:01, Stanislav Zubenko via B4 Relay 写道:
> From: Stanislav Zubenko <zubenkos@external.gene.com>
> 
> While adding a device to a filesystem that was under heavy buffered write
> load, a chunk allocation done by writeback aborted the transaction with
> -ENOENT, which turned the filesystem read-only:
> 
>    BTRFS: Transaction aborted (error -2)
>    WARNING: CPU: 15 PID: 1149147 at fs/btrfs/block-group.c:3731 do_chunk_alloc.cold+0x162/0x1b7 [btrfs]
>    CPU: 15 PID: 1149147 Comm: kworker/u128:13 Tainted: G           OE      6.1.170-213.321.amzn2023.x86_64 #1
>    Hardware name: Amazon EC2 r6i.16xlarge/, BIOS 1.0 10/16/2017
>    Workqueue: writeback wb_workfn (flush-btrfs-1)
>    RIP: 0010:do_chunk_alloc.cold+0x162/0x1b7 [btrfs]
>    Call Trace:
>     <TASK>
>     btrfs_chunk_alloc+0x15d/0x2f0 [btrfs]
>     find_free_extent_update_loop+0x1f6/0x2d0 [btrfs]
>     find_free_extent+0x347/0x740 [btrfs]
>     btrfs_reserve_extent+0x147/0x290 [btrfs]
>     cow_file_range+0x15c/0x590 [btrfs]
>     btrfs_run_delalloc_range+0x111/0x1d0 [btrfs]
>     writepage_delalloc+0xc1/0x180 [btrfs]
>     __extent_writepage+0x10c/0x460 [btrfs]
>     extent_write_cache_pages+0x141/0x4e0 [btrfs]
>     extent_writepages+0x7b/0x140 [btrfs]
>     do_writepages+0xcd/0x1c0
>     __writeback_single_inode+0x41/0x270
>     writeback_sb_inodes+0x209/0x4c0
>     __writeback_inodes_wb+0x4c/0xe0
>     wb_writeback+0x1d7/0x2d0
>     wb_do_writeback+0x22a/0x2b0
>     wb_workfn+0x5d/0x250
>     process_one_work+0x1e5/0x3b0
>     worker_thread+0x50/0x3a0
>     kthread+0xd9/0x100
>     ret_from_fork+0x1f/0x30
>     </TASK>
>    BTRFS: error (device nvme1n1: state A) in do_chunk_alloc:3731: errno=-2 No such entry
>    BTRFS info (device nvme1n1: state EA): forced readonly
> 
> The filesystem had a single data profile, it was about 80% full and was
> receiving around 1GB/s of buffered writes. The abort happened about 11
> seconds after "btrfs device add" was started for a sixth device, and the
> add never completed (no "disk added" message was logged). The same
> happened on a second machine with the same workload on the same day.
> 
> The -ENOENT comes from btrfs_update_device(), which does not find the
> device item of the device being added. This is due to a race between
> adding a device and chunk allocation, which the following diagram
> illustrates:
> 
>           CPU 1                        CPU 2
> 
>   btrfs_init_new_device()
>     btrfs_start_transaction()
>     lock fs_devices->device_list_mutex
>     lock fs_info->chunk_mutex
>     adds the new device to
>     fs_devices->alloc_list
>     btrfs_clear_space_info_full()
>     unlock fs_info->chunk_mutex
> 
>                                cow_file_range()
>                                  btrfs_reserve_extent()
>                                    find_free_extent()
>                                      btrfs_join_transaction()
>                                        --> joins the transaction
>                                            started by CPU 1
>                                      btrfs_chunk_alloc()
>                                        lock fs_info->chunk_mutex
>                                        do_chunk_alloc()
>                                          btrfs_create_chunk()
>                                            --> picks the new device,
>                                                it is the one with
>                                                most free space
>                                          btrfs_chunk_alloc_add_chunk_item()
>                                            btrfs_update_device()
>                                              --> doesn't find the
>                                                  device item of the
>                                                  new device, returns
>                                                  -ENOENT
>                                          btrfs_abort_transaction()
>                                        unlock fs_info->chunk_mutex
>     btrfs_sysfs_add_device()
>     unlock fs_devices->device_list_mutex
>     btrfs_add_dev_item()
>       btrfs_reserve_chunk_metadata()
>         lock fs_info->chunk_mutex
>         unlock fs_info->chunk_mutex
>       inserts the device item in the
>       chunk btree, but it's too late
> 
> Before commit 79bd37120b14 ("btrfs: rework chunk allocation to avoid
> exhaustion of the system chunk array"), device items were only updated in
> phase 2 of the chunk allocation, at btrfs_finish_chunk_alloc(), and that
> was done while holding the device_list_mutex, which is taken there since
> commit 50460e37186a ("Btrfs: fix race when finishing dev replace leading
> to transaction abort"). Since adding a device holds the device_list_mutex
> until after it registers the device in sysfs, the race window was only the
> very short time between unlocking that mutex and inserting the device
> item. The chunk allocation rework moved the device item updates to phase
> 1, under the protection of the chunk_mutex only, so the window now also
> covers the sysfs registration and the system space reservation done by
> btrfs_add_dev_item(), which must take the chunk_mutex itself and so can
> end up waiting for the very chunk allocation that is going to fail.
> 
> Fix this by deferring everything that makes the new device usable by
> the chunk allocator until btrfs_add_dev_item() has inserted its device
> item in the chunk btree: adding it to the allocation list, accounting
> its space in free_chunk_space, clearing the full flags of the space
> infos and updating the per profile available space.
> 
> When sprouting a seed filesystem, keep doing this before inserting the
> device item, because init_first_rw_device() has to allocate chunks from
> the new device first. That is safe because a seed filesystem is
> read-only and we are holding sb->s_umount, so nothing else can be
> allocating chunks.
> 
> Fixes: 79bd37120b14 ("btrfs: rework chunk allocation to avoid exhaustion of the system chunk array")
> CC: stable@vger.kernel.org # 5.15+
> Assisted-by: LLM
> Signed-off-by: Stanislav Zubenko <zubenkos@external.gene.com>
> ---
>   fs/btrfs/volumes.c | 34 ++++++++++++++++++++++++----------
>   1 file changed, 24 insertions(+), 10 deletions(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 6574939e1..bc8ffa7bb 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -3066,15 +3066,12 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
>   
>   	mutex_lock(&fs_info->chunk_mutex);
>   	list_add_rcu(&device->dev_list, &fs_devices->devices);
> -	list_add(&device->dev_alloc_list, &fs_devices->alloc_list);
>   	fs_devices->num_devices++;
>   	fs_devices->open_devices++;
>   	fs_devices->rw_devices++;
>   	fs_devices->total_devices++;
>   	fs_devices->total_rw_bytes += device->total_bytes;
>   
> -	atomic64_add(device->total_bytes, &fs_info->free_chunk_space);
> -
>   	if (bdev_rot(device->bdev))
>   		fs_devices->rotating = true;
>   
> @@ -3088,12 +3085,20 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
>   				    orig_super_num_devices + 1);
>   
>   	/*
> -	 * we've got more storage, clear any full flags on the space
> -	 * infos
> +	 * The chunk allocator must not use the new device before its device
> +	 * item is in the chunk btree, otherwise allocating a chunk on it fails
> +	 * to update that item with -ENOENT and aborts the transaction.
> +	 * Sprouting is the exception: init_first_rw_device() must allocate from
> +	 * the new device before its item can be inserted, and nothing else can
> +	 * allocate chunks since a seed filesystem is read-only and we are
> +	 * holding sb->s_umount.
>   	 */
> -	btrfs_clear_space_info_full(fs_info);
> -
> -	btrfs_update_per_profile_avail(fs_info);
> +	if (seeding_dev) {
> +		list_add(&device->dev_alloc_list, &fs_devices->alloc_list);
> +		atomic64_add(device->total_bytes, &fs_info->free_chunk_space);
> +		btrfs_clear_space_info_full(fs_info);
> +		btrfs_update_per_profile_avail(fs_info);
> +	}
>   	mutex_unlock(&fs_info->chunk_mutex);
>   
>   	/* Add sysfs device entry */
> @@ -3134,6 +3139,13 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
>   		 * its fsid has been changed by btrfs_sprout_splice().
>   		 */
>   		btrfs_sysfs_update_sprout_fsid(fs_devices);
> +	} else {
> +		mutex_lock(&fs_info->chunk_mutex);

Sashiko mentioned a critical problem, that if the device is removed 
before reaching this branch?

Also sashiko has another high level problem exposed, I believe you 
should take care of both sashiko reviews:

https://sashiko.dev/#/patchset/20261023-btrfs-device-add-race-v1-1-90cf05e8128f%40external.gene.com

> +		list_add(&device->dev_alloc_list, &fs_devices->alloc_list);
> +		atomic64_add(device->total_bytes, &fs_info->free_chunk_space);
> +		btrfs_clear_space_info_full(fs_info);
> +		btrfs_update_per_profile_avail(fs_info);
> +		mutex_unlock(&fs_info->chunk_mutex);
>   	}
>   
>   	ret = btrfs_commit_transaction(trans);
> @@ -3190,13 +3202,15 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
>   	if (!list_empty(&device->post_commit_list))
>   		list_del_init(&device->post_commit_list);
>   	list_del_rcu(&device->dev_list);
> -	list_del(&device->dev_alloc_list);
> +	if (!list_empty(&device->dev_alloc_list)) {
> +		list_del_init(&device->dev_alloc_list);
> +		atomic64_sub(device->total_bytes, &fs_info->free_chunk_space);
> +	}
>   	fs_info->fs_devices->num_devices--;
>   	fs_info->fs_devices->open_devices--;
>   	fs_info->fs_devices->rw_devices--;
>   	fs_info->fs_devices->total_devices--;
>   	fs_info->fs_devices->total_rw_bytes -= device->total_bytes;
> -	atomic64_sub(device->total_bytes, &fs_info->free_chunk_space);
>   	btrfs_set_super_total_bytes(fs_info->super_copy,
>   				    orig_super_total_bytes);
>   	btrfs_set_super_num_devices(fs_info->super_copy,
> 
> ---
> base-commit: e17e64baf63f7c85bec209ed73f14c1b2a7e39fb
> change-id: 20261023-btrfs-device-add-race-d8c4f1c3ed84
> 
> Best regards,
> --
> Stanislav Zubenko <zubenkos@external.gene.com>