From: Qu Wenruo <wqu@suse.com>
To: Josef Bacik <josef@toxicpanda.com>,
David Sterba <dsterba@suse.com>, Chris Mason <mason@kernel.org>,
Naohiro Aota <naohiro.aota@wdc.com>,
Johannes Thumshirn <johannes.thumshirn@wdc.com>,
Boris Burkov <boris@bur.io>, Filipe Manana <fdmanana@suse.com>
Cc: linux-btrfs@vger.kernel.org, linux-kernel@vger.kernel.org,
stable@vger.kernel.org
Subject: Re: [PATCH 1/5] btrfs: don't drop the error of the last split bio
Date: Sat, 31 Oct 2026 07:33:38 +1030 [thread overview]
Message-ID: <fab0bfc4-3c20-4058-8b39-585b14152f6e@suse.com> (raw)
In-Reply-To: <20261030-b4-btrfs-data-integrity-v1-1-f4d4196731e6@toxicpanda.com>
在 2026/10/31 01:08, Josef Bacik 写道:
> 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@vger.kernel.org # 6.12+
> Assisted-by: LLM
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: Qu Wenruo <wqu@suse.com>
Thanks,
Qu
> ---
> 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;
>
next prev parent reply other threads:[~2026-10-30 21:03 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-30 14:38 [PATCH 0/5] btrfs: data integrity fixes Josef Bacik
2026-10-30 14:38 ` [PATCH 1/5] btrfs: don't drop the error of the last split bio Josef Bacik
2026-10-30 21:03 ` Qu Wenruo [this message]
2026-10-30 14:38 ` [PATCH 2/5] btrfs: clear dirty blocks after a delalloc failure Josef Bacik
2026-10-30 20:57 ` Qu Wenruo
2026-10-30 14:38 ` [PATCH 3/5] btrfs: make fsync wait after pre-submission errors Josef Bacik
2026-10-30 21:06 ` Qu Wenruo
2026-10-30 14:38 ` [PATCH 4/5] btrfs: don't report success for a skipped log sync Josef Bacik
2026-10-30 14:38 ` [PATCH 5/5] btrfs: don't trim the target of a device replace Josef Bacik
2026-10-30 21:02 ` Qu Wenruo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=fab0bfc4-3c20-4058-8b39-585b14152f6e@suse.com \
--to=wqu@suse.com \
--cc=boris@bur.io \
--cc=dsterba@suse.com \
--cc=fdmanana@suse.com \
--cc=johannes.thumshirn@wdc.com \
--cc=josef@toxicpanda.com \
--cc=linux-btrfs@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mason@kernel.org \
--cc=naohiro.aota@wdc.com \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®