Re: [PATCH 5/5] btrfs: don't trim the target of a device replace

[Filipe Manana]

On Fri, Oct 30, 2026 at 2:40 PM Josef Bacik <josef@xxxxxxxxxxxxxx> wrote:
>
> FITRIM also discards the space of every device that is not allocated to a
> chunk. btrfs_trim_free_extents() goes over fs_devices->devices and, for
> each device that is writeable, discards the ranges that have neither
> CHUNK_ALLOCATED nor CHUNK_TRIMMED set in device->alloc_state.
>
> The target device of a running replace operation is on that list and it is
> writeable. But its alloc_state is empty. The allocated ranges of the
> source device are only copied to it by btrfs_set_target_alloc_state(),
> when the replace finishes. Until then the whole device looks unallocated,
> so an fstrim discards everything that was copied to it so far, and what is
> being written to it in parallel to the source device.
>
> The replace finishes without any error and the source device is dropped.
> With a single device filesystem, 700M of data and fstrim called in a loop
> while the replace runs:
>
>   BTRFS info: dev_replace from /dev/vdb (devid 1) to /dev/vdc finished
>   BTRFS error: bad tree block start, mirror 1 want 30638080 have 0
>   BTRFS error: bad tree block start, mirror 2 want 30638080 have 0
>   BTRFS info: scrub: not finished on devid 1 with status: -5
>
> Skip the target device. The flag is cleared with the device list mutex
> held, after the alloc_state was set up, and the trimming code holds that
> mutex while it works on a device.
>
> Fixes: 499f377f49f0 ("btrfs: iterate over unused chunk space in FITRIM")
> CC: stable@xxxxxxxxxxxxxxx
> Assisted-by: LLM
> Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>

Reviewed-by: Filipe Manana <fdmanana@xxxxxxxx>

Looks good, thanks.

> ---
>  fs/btrfs/extent-tree.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 9b0a5c6b96f3..22155507f905 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -6641,6 +6641,14 @@ static int btrfs_trim_free_extents_throttle(struct btrfs_device *device,
>         if (!test_bit(BTRFS_DEV_STATE_WRITEABLE, &device->dev_state) || !device->bdev)
>                 return 0;
>
> +       /*
> +        * The target of a running device replace has nothing in its alloc_state
> +        * until the replace finishes, all of it would look unallocated and we
> +        * would discard what was already copied to it.
> +        */
> +       if (test_bit(BTRFS_DEV_STATE_REPLACE_TGT, &device->dev_state))
> +               return 0;
> +
>         /* Discard not supported = nothing to do. */
>         if (!bdev_max_discard_sectors(device->bdev))
>                 return 0;
>
> --
> 2.55.0
>
>