On Thu, Dec 18, 2025 at 10:08 AM Christoph Hellwig <[email protected]> wrote: > On Tue, Dec 16, 2025 at 12:20:07PM +0100, Andreas Gruenbacher wrote: > > > I still don't understand what you're saying here at all, or what this is > > > trying to fix or optimize. > > > > When we have this construct in the code and we know that status is not 0: > > > > if (!bio->bi_status) > > bio->bi_status = status; > > > > we can just do this instead: > > > > bio>bi_status = status; > > But this now overrides the previous status instead of preserving the > first error?
This is exactly my point: we already don't preserve the first error, it only looks like we do. Here are the possible cases: (1) A single bio A: there are no competing completions. A->bi_status is set before calling bio_endio(), and it can be set to any value including BLK_STS_OK (0) with a simple assignment. (2) An A -> B chain: there are two competing completions, and B->bi_status is the resulting status of the bio chain. Both completions will immediately update B->bi_status. When B->bi_status is updated, it must not be set to BLK_STS_OK (0) or else a previous non-zero status code could be wiped out. But for such a non-zero status code, a construct like 'if (B->status != BLK_STS_OK) B->status = status' is no better than a simple 'B->status = status' because the if is not atomic. But we only care about preserving an error code, not the first error code that occurred, so that's fine. (3) An A -> B -> C chain: There are three competing completions, and C->bi_status is the resulting status of the bio chain. Not all completions will immediately update C->bi_status, but if at least one of the completions fails, we know that we will always end up with a non-zero error code in C->bi_status eventually. And again, switching to cmpxchg() would not always preserve the first error, either: for example, in case (3), if the bios complete in the order A, C, B and they all fail, C->bi_status would end up with the error code of C instead of A even though A completed before C. This could be fixed by chaining all consecutive bios to bio A (the first one) and reversing the pointer direction so that all cmpxchg() operations will target A->bi_status. But again, I don't think preserving the first error is actually needed. And it certainly isn't being done today. Thanks, Andreas
