Re: [PATCH v6 3/3] block: prevent race condition on bi_status in __bio_chain_endio

[Andreas Gruenbacher]

On Sun, Dec 7, 2025 at 1:22 PM zhangshida <starzhangzsd@xxxxxxxxx> wrote:
> From: Shida Zhang <zhangshida@xxxxxxxxxx>
>
> Andreas point out that multiple completions can race setting
> bi_status.
>
> If __bio_chain_endio() is called concurrently from multiple threads
> accessing the same parent bio, it should use WRITE_ONCE()/READ_ONCE()
> to access parent->bi_status and avoid data races.
>
> On x86 and ARM, these macros compile to the same instruction as a
> normal write, but they may be required on other architectures to
> prevent tearing, and to ensure the compiler does not add or remove
> memory accesses under the assumption that the values are not accessed
> concurrently.
>
> Adopting a cmpxchg approach, as used in other code paths, resolves all
> these issues, as suggested by Christoph.
>
> Suggested-by: Andreas Gruenbacher <agruenba@xxxxxxxxxx>
> Suggested-by: Christoph Hellwig <hch@xxxxxxxxxxxxx>
> Suggested-by: Caleb Sander Mateos <csander@xxxxxxxxxxxxxxx>
> Reviewed-by: Christoph Hellwig <hch@xxxxxx>
> Signed-off-by: Shida Zhang <zhangshida@xxxxxxxxxx>
> ---
>  block/bio.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/block/bio.c b/block/bio.c
> index d236ca35271..8b4b6b4e210 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -314,8 +314,9 @@ static struct bio *__bio_chain_endio(struct bio *bio)
>  {
>         struct bio *parent = bio->bi_private;
>
> -       if (bio->bi_status && !parent->bi_status)
> -               parent->bi_status = bio->bi_status;
> +       if (bio->bi_status)
> +               cmpxchg(&parent->bi_status, 0, bio->bi_status);
> +
>         bio_put(bio);
>         return parent;
>  }
> --
> 2.34.1
>

I thought you were going to drop this??

Andreas