Re: [PATCH v4 3/3] block: prevent race condition on bi_status in __bio_chain_endio
Christoph Hellwig <[email protected]> Wed, 3 Dec 2025 07:14:03 +0100
| Newsgroups | org.kernel.vger.linux-bcache,org.kernel.vger.linux-block,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
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.