Re: [PATCH 1/5] btrfs: don't drop the error of the last split bio

[Filipe Manana]

On Fri, Oct 30, 2026 at 2:38 PM Josef Bacik <josef@xxxxxxxxxxxxxx> wrote:
>
> When a btrfs_bio had to be split, btrfs_bio_end_io() saves the first error
> of any of its parts in the status field of the original btrfs_bio. The
> part that completes last is supposed to load it into bi_status of the
> original bio before calling the end_io handler, but that is only done if
> that last part was successful:
>
>         if (atomic_dec_and_test(&bbio->pending_ios)) {
>                 /* Load split bio's error which might be set above. */
>                 if (status == BLK_STS_OK)
>                         bbio->bio.bi_status = READ_ONCE(bbio->status);
>
> If the last part failed, the assumption is that bi_status was set at the
> top of the function. That is true if the last part is what is left of the
> original bio. If it is a split bio, the assignment was done to the split
> bio, which was freed right after. If the original bio has completed before
> without an error, its bi_status is BLK_STS_OK and stays like that:
>
>   original bio (tail)                 split bio (head)
>
>   btrfs_bio_end_io(orig, OK)
>     orig->bio.bi_status = OK
>     pending_ios 2 -> 1
>                                       btrfs_bio_end_io(split, IOERR)
>                                         split->bio.bi_status = IOERR
>                                         bio_put(split)
>                                         cmpxchg(&orig->status, OK, IOERR)
>                                         pending_ios 1 -> 0
>                                         orig->end_io(), bi_status is OK
>
> The end_io handler covers the whole range, so for a buffered read all the
> folios are marked uptodate, including the ones that were never filled, and
> read(2) returns their content instead of -EIO. For a write the ordered
> extent is completed as if all the data had reached the device.
>
> This can be reproduced with raid0 data on two devices, one of them slower
> than the other and with a bad block, by reading a file whose last stripe
> is on the fast device.
>
> The saved status has the first error of all the parts, or BLK_STS_OK if
> there was none, so always use it.
>
> Fixes: d48e1dea3931 ("btrfs: fix error propagation of split bios")
> CC: stable@xxxxxxxxxxxxxxx # 6.12+
> Assisted-by: LLM
> Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>

Reviewed-by: Filipe Manana <fdmanana@xxxxxxxx>

Looks good, thanks.

> ---
>  fs/btrfs/bio.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/fs/btrfs/bio.c b/fs/btrfs/bio.c
> index 2c6234f6182f..fc0b021cdefb 100644
> --- a/fs/btrfs/bio.c
> +++ b/fs/btrfs/bio.c
> @@ -127,9 +127,12 @@ void btrfs_bio_end_io(struct btrfs_bio *bbio, blk_status_t status)
>                 cmpxchg(&bbio->status, BLK_STS_OK, status);
>
>         if (atomic_dec_and_test(&bbio->pending_ios)) {
> -               /* Load split bio's error which might be set above. */
> -               if (status == BLK_STS_OK)
> -                       bbio->bio.bi_status = READ_ONCE(bbio->status);
> +               /*
> +                * Load the first error, if any. It may come from a split bio,
> +                * including the one we were called for, in which case nothing
> +                * stored it in the original bio yet.
> +                */
> +               bbio->bio.bi_status = READ_ONCE(bbio->status);
>
>                 if (bbio_has_ordered_extent(bbio)) {
>                         struct btrfs_ordered_extent *ordered = bbio->ordered;
>
> --
> 2.55.0
>
>