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

[Christoph Hellwig]

On Wed, Dec 03, 2025 at 11:09:36AM +0800, Stephen Zhang wrote:
> 
> I’ve been reconsidering the two approaches for the upcoming patch revision.
> Essentially, we’re comparing two methods:
> A:
>         if (bio->bi_status)
>                    parent->bi_status = bio->bi_status;
> B:
>         if (bio->bi_status)
>                 cmpxchg(&parent->bi_status, 0, bio->bi_status);
> 
> Both appear correct, but B seems a little bit redundant here.

A is not correct.  You at least needs READ_ONCE/WRITE_ONCE here.

B solves all these issues.

> Upon further reflection, I’ve noticed a subtle difference:
> A unconditionally writes to parent->bi_status when bio->bi_status is non-zero,
> regardless of the current value of parent->bi_status.
> B uses cmpxchg to only update parent->bi_status if it is still zero.
> 
> Thus, B could avoid unnecessary writes in cases where parent->bi_status has
> already been set to a non-zero value.

The unnecessary writes don't really matter, we're in an error slow path
here.