linux-kernel - Re: [RFC 06/12] bio: don't check target->bi

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAHc6FU7H7gFFRw5FCc58ki85Pbedfs8NegjWQbpN7otzfsJNpg@mail.gmail.com>
Date: Fri, 19 Dec 2025 21:14:12 +0100
From: Andreas Gruenbacher <agruenba@...hat.com>
To: Christoph Hellwig <hch@...radead.org>
Cc: Jens Axboe <axboe@...nel.dk>, Chris Mason <clm@...com>, David Sterba <dsterba@...e.com>, 
	linux-block@...r.kernel.org, linux-btrfs@...r.kernel.org, 
	linux-raid@...r.kernel.org, dm-devel@...ts.linux.dev, 
	linux-kernel@...r.kernel.org
Subject: Re: [RFC 06/12] bio: don't check target->bi_status on error

On Thu, Dec 18, 2025 at 10:08 AM Christoph Hellwig <hch@...radead.org> 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