Re: [RFC 00/12] bio cleanups

David Sterba <[email protected]>
Newsgroups gmane.linux.block,gmane.comp.file-systems.btrfs,gmane.linux.raid,gmane.linux.kernel
Message-ID <[email protected]>
On Mon, Dec 08, 2025 at 12:10:07PM +0000, Andreas Gruenbacher wrote:
> Hello,
> 
> we are not quite careful enough about setting bio->bi_status in all
> places (see BACKGROUND below).  This patch queue tries to fix this by
> systematically eliminating the direct assignments to bi_status sprinkled
> all throughout the code.  Please comment.
> 
> 
> The first patch ("bio: rename bio_chain arguments") is an loosely
> related cleanup.  The remaining changes are:
> 
> - Use bio_io_error() in more places.
> 
> - Add a bio_set_errno() helper for setting bi_status based on an errno.
>   Use this helper throughout the code.
> 
> - Add a bio_set_status() helper for setting bi_status to a blk_status_t
>   status code.  Use this helper in places in the code where it's
>   necessary, or at least useful without adding any overhead.
> 
> And on top of that, we have two more cleanups:
> 
> - Add a bio_endio_errno() helper that combines bio_set_errno() and
>   bio_endio().
> 
> - Add a bio_endio_status() helper that combines bio_set_status() and
>   bio_endio().
> 
> The patches are currently based on v6.18.
> 
> GIT tree:
> https://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git/log/?h=bio-cleanups
> 
> With these changes, only a few direct assignments to bio->bi_status
> remain, in BTRFS and in MD, and SOME OF THOSE MAY BE UNSAFE.  Could the
> maintainers of those subsystems please have a look?

The btrfs bits look good to me, we expect the same semantics, ie. not
overwrite existing error with 0. If there are racing writes to the
status like in btrfs_bio_end_io() we use cmpxchg() so we don't overwrite
it.

> Once the remaining direct assignments to bi_status are gone, we may want
> to think about "write protecting" bi_status to prevent unintended new
> direct assignments from creeping back in.

This makes sense, though I'm not sure if this takes into account the
mentioned cmpxchg pattern:

	if (status != BLK_STS_OK)
		cmpxchg(&bbio->status, BLK_STS_OK, status);
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.