btrfs: propagate a split bio's error when the failing split completes last
btrfs_bio_end_io() saves the first error of any split bio in the original
btrfs_bio's ->status, then loads it into ->bio.bi_status when the last
split completes -- but only if that last completion itself succeeded:
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);
The condition assumes that a failing final completion has already stored
its error in the original bio at the top of the function. That holds only
when the bio was never split. For a split, "bbio->bio.bi_status = status"
is applied to the clone, which is then freed and bbio switched to the
original; the original's bi_status keeps whatever an earlier, successful
split left in it, and the error never reaches the caller.
The ordering that loses the error is not rare. A read that fails checksum
verification completes only after read repair has tried every mirror,
which takes far longer than the healthy splits of the same bio, so the
failing split completes last essentially every time.
On a degraded raid56 array the result is silent corruption. Reads are
split per stripe, so for any read of BTRFS_STRIPE_LEN or more, a sector
whose reconstruction from a torn stripe fails its checksum is returned to
userspace as zeros with no error -- while btrfs logs the checksum failure
and increments the device corruption counter. A 4 KiB read of the same
sector is not split and correctly returns EIO. Profiles that do not split
at 64 KiB (single, dup, raid1) are unaffected, which is how this has gone
unnoticed.
Reproducer: on a raid5 filesystem, tear one stripe row's parity (leaving
the data intact), withhold one data member of that row and mount degraded.
Reconstructing the missing column then yields wrong bytes for that column
while every other column still verifies. Reads of 64 KiB or more over the
affected range return zeros and succeed; 4 KiB reads return EIO.
Load the saved status unconditionally. It is the first error seen by any
split, including this one, which is the value the caller wants in either
case.
projects / linux / commit