On Mon, Dec 8, 2025 at 8:43 PM David Sterba <[email protected]> wrote: > On Mon, Dec 08, 2025 at 12:10:07PM +0000, Andreas Gruenbacher wrote: > > With these changes, only a few direct assignments to bio->bi_status > > remain, in BTRFS and in MD, and SOME OF THOSE MAY BE UNSAFE. Could the > > maintainers of those subsystems please have a look? > > The btrfs bits look good to me, we expect the same semantics, ie. not > overwrite existing error with 0. If there are racing writes to the > status like in btrfs_bio_end_io() we use cmpxchg() so we don't overwrite > it.
Really? I'm not talking about the status field in struct btrfs_bio but about the bi_status field in struct bio. The first mention of bi_status I can find in the btrfs code is right at the beginning of btrfs_bio_end_io(): bbio->bio.bi_status = status; If status is ever BLK_STS_OK (0) here and bbio->bio is a chained bio, things are already not great. I believe we should eliminate all direct assignments to bi_status and use bio_set_status() instead. I'm not familiar enough with the btrfs code to make that replacement for you, though. A cursory look at struct btrfs_bio suggests that those objects cannot be chained like plain old bios (bio_chain()). This means that cmpxchg() may actually work for catching the first error that occurs. For chained regular bios, cmpxchg() won't catch the first error, at least not if the length of the chain is greater than two. Thanks, Andreas
