public inbox for linux-btrfs@vger.kernel.org
 help / color / mirror / Atom feed
* [RFC 00/12] bio cleanups
@ 2025-12-08 12:10 Andreas Gruenbacher
  2025-12-08 12:10 ` [RFC 01/12] bio: rename bio_chain arguments Andreas Gruenbacher
                   ` (12 more replies)
  0 siblings, 13 replies; 27+ messages in thread
From: Andreas Gruenbacher @ 2025-12-08 12:10 UTC (permalink / raw)
  To: Christoph Hellwig, Jens Axboe, Chris Mason, David Sterba,
	Satya Tangirala
  Cc: Andreas Gruenbacher, linux-block, linux-btrfs, linux-raid,
	dm-devel, linux-kernel

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?

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.


BACKGROUND

'struct bio' objects start out with their bi_status field initialized to
BLK_STS_OK (0).  When the bio completes, that field needs to be left
unchanged in case of success, and set to a BLK_STS_* error code
otherwise.

This is important when bios are chained (bio_chain()) because then,
multiple execution contexts will race updating the same bi_status field.
When an execution context resets bi_status to BLK_STS_OK (0) during bio
completion, this could hide the error code of the adjacent bio in the
chain.

When more than a single bio fails in a chain, we know that the resulting
bi_status will not be BLK_STS_OK, but we don't know which of the status
error codes will win.


CRYPTO FALLBACK (SATYA TANGIRALA?)

Related to chained bios but unrelated to setting bio->bi_status,
blk_crypto_fallback_encrypt_bio() in block/blk-crypto-fallback.c swaps
out bi_private and bi_end_io and reuses the same bio for downcalls, then
restores those fields in blk_crypto_fallback_decrypt_endio() before
calling bio_endio() again on the same bio.  This will at the very least
break with chained bios because it will mess up __bi_remaining.


Thanks,
Andreas

Andreas Gruenbacher (12):
  bio: rename bio_chain arguments
  bio: use bio_io_error more often
  bio: add bio_set_errno
  bio: use bio_set_errno in more places
  bio: add bio_set_status
  bio: don't check target->bi_status on error
  bio: use bio_set_status for BLK_STS_* status codes
  bio: use bio_set_status in some more places
  bio: switch to bio_set_status in submit_bio_noacct
  bio: never set bi_status to BLK_STS_OK during completion
  bio: add bio_endio_errno
  bio: add bio_endio_status

 block/bio-integrity-auto.c       |  3 +--
 block/bio.c                      | 25 ++++++++++++-------------
 block/blk-core.c                 | 10 ++++------
 block/blk-crypto-fallback.c      | 22 +++++++++++-----------
 block/blk-crypto-internal.h      |  2 +-
 block/blk-crypto.c               |  4 ++--
 block/blk-merge.c                |  6 ++----
 block/blk-mq.c                   | 11 ++++-------
 block/fops.c                     |  6 ++----
 block/t10-pi.c                   |  2 +-
 drivers/block/aoe/aoecmd.c       |  8 ++++----
 drivers/block/aoe/aoedev.c       |  2 +-
 drivers/block/drbd/drbd_int.h    |  3 +--
 drivers/block/drbd/drbd_req.c    |  9 +++------
 drivers/block/ps3vram.c          |  3 +--
 drivers/block/zram/zram_drv.c    |  4 ++--
 drivers/md/bcache/bcache.h       |  3 +--
 drivers/md/bcache/request.c      |  8 +++-----
 drivers/md/dm-cache-target.c     |  9 +++++----
 drivers/md/dm-ebs-target.c       |  2 +-
 drivers/md/dm-flakey.c           |  2 +-
 drivers/md/dm-integrity.c        | 30 +++++++++++-------------------
 drivers/md/dm-mpath.c            |  6 ++----
 drivers/md/dm-pcache/dm_pcache.c |  3 +--
 drivers/md/dm-raid1.c            |  7 +++----
 drivers/md/dm-thin.c             |  5 ++---
 drivers/md/dm-vdo/data-vio.c     |  3 +--
 drivers/md/dm-verity-target.c    |  2 +-
 drivers/md/dm-writecache.c       |  7 +++----
 drivers/md/dm-zoned-target.c     |  2 +-
 drivers/md/dm.c                  |  4 +---
 drivers/md/md.c                  |  8 +++-----
 drivers/md/raid1-10.c            |  3 +--
 drivers/md/raid1.c               |  2 +-
 drivers/md/raid10.c              | 18 +++++++-----------
 drivers/md/raid5.c               |  4 ++--
 drivers/nvdimm/btt.c             |  4 ++--
 drivers/nvdimm/pmem.c            |  7 ++-----
 fs/btrfs/bio.c                   |  8 ++++----
 fs/btrfs/direct-io.c             |  2 +-
 fs/btrfs/raid56.c                |  6 ++----
 fs/crypto/bio.c                  |  2 +-
 fs/erofs/fileio.c                |  2 +-
 fs/erofs/fscache.c               |  4 ++--
 fs/f2fs/data.c                   |  6 +++---
 fs/f2fs/segment.c                |  3 +--
 fs/iomap/ioend.c                 |  3 +--
 fs/verity/verify.c               |  2 +-
 fs/xfs/xfs_aops.c                |  3 +--
 fs/xfs/xfs_zone_alloc.c          |  2 +-
 include/linux/bio.h              | 30 +++++++++++++++++++++++++++---
 51 files changed, 153 insertions(+), 179 deletions(-)

-- 
2.51.0


^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2025-12-19 20:14 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-08 12:10 [RFC 00/12] bio cleanups Andreas Gruenbacher
2025-12-08 12:10 ` [RFC 01/12] bio: rename bio_chain arguments Andreas Gruenbacher
2025-12-16  7:57   ` Christoph Hellwig
2025-12-08 12:10 ` [RFC 02/12] bio: use bio_io_error more often Andreas Gruenbacher
2025-12-16  7:57   ` Christoph Hellwig
2025-12-08 12:10 ` [RFC 03/12] bio: add bio_set_errno Andreas Gruenbacher
2025-12-16  7:58   ` Christoph Hellwig
2025-12-08 12:10 ` [RFC 04/12] bio: use bio_set_errno in more places Andreas Gruenbacher
2025-12-08 12:10 ` [RFC 05/12] bio: add bio_set_status Andreas Gruenbacher
2025-12-16  7:59   ` Christoph Hellwig
2025-12-08 12:10 ` [RFC 06/12] bio: don't check target->bi_status on error Andreas Gruenbacher
2025-12-16  7:59   ` Christoph Hellwig
2025-12-16  8:41     ` Andreas Gruenbacher
2025-12-16 10:44       ` Christoph Hellwig
2025-12-16 11:20         ` Andreas Gruenbacher
2025-12-18  8:47           ` Christoph Hellwig
2025-12-19 20:14             ` Andreas Gruenbacher
2025-12-08 12:10 ` [RFC 07/12] bio: use bio_set_status for BLK_STS_* status codes Andreas Gruenbacher
2025-12-08 12:10 ` [RFC 08/12] bio: use bio_set_status in some more places Andreas Gruenbacher
2025-12-08 12:10 ` [RFC 09/12] bio: switch to bio_set_status in submit_bio_noacct Andreas Gruenbacher
2025-12-08 12:10 ` [RFC 10/12] bio: never set bi_status to BLK_STS_OK during completion Andreas Gruenbacher
2025-12-08 12:10 ` [RFC 11/12] bio: add bio_endio_errno Andreas Gruenbacher
2025-12-16  8:00   ` Christoph Hellwig
2025-12-08 12:10 ` [RFC 12/12] bio: add bio_endio_status Andreas Gruenbacher
2025-12-16  8:01   ` Christoph Hellwig
2025-12-08 19:37 ` [RFC 00/12] bio cleanups David Sterba
2025-12-08 21:16   ` Andreas Gruenbacher

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox