mbox series

[v4,0/6] bio_split() error handling rework

Message ID 20241111112150.3756529-1-john.g.garry@oracle.com (mailing list archive)
Headers show
Series bio_split() error handling rework | expand

Message

John Garry Nov. 11, 2024, 11:21 a.m. UTC 
bio_split() error handling could be improved as follows:
- Instead of returning NULL for an error - which is vague - return a
  PTR_ERR, which may hint what went wrong.
- Remove BUG_ON() calls - which are generally not preferred - and instead
  WARN and pass an error code back to the caller. Many callers of
  bio_split() don't check the return code. As such, for an error we would
  be getting a crash still from an invalid pointer dereference.

Most bio_split() callers don't check the return value. However, it could
be argued the bio_split() calls should not fail. So far I have just
fixed up the md RAID code to handle these errors, as that is my interest
now.

The motivator for this series was initial md RAID atomic write support in
https://lore.kernel.org/linux-block/20241030094912.3960234-1-john.g.garry@oracle.com/T/#m5859ee900de8e6554d5bb027c0558f0147c32df8

There I wanted to ensure that we don't split an atomic write bio, and it
made more sense to handle this in bio_split() (instead of the bio_split()
caller).

Based on (block/for-6.13/block) 5fcfcd51ea1c zram: fix NULL pointer in comp_algorithm_show()

Changes since v3:
- Rebase
- Add RB tags from Hannes and Kuai (thanks!)

Changes since v2:
- Drop "block: Use BLK_STS_OK in bio_init()" change (Christoph)
- Use proper rdev indexing in raid10_write_request() (Kuai)
- Decrement rdev nr_pending in raid1 read error path (Kuai)
- Add RB tags from Christoph, Johannes, and Kuai (thanks!)

John Garry (6):
  block: Rework bio_split() return value
  block: Error an attempt to split an atomic write in bio_split()
  block: Handle bio_split() errors in bio_submit_split()
  md/raid0: Handle bio_split() errors
  md/raid1: Handle bio_split() errors
  md/raid10: Handle bio_split() errors

 block/bio.c                 | 14 +++++++----
 block/blk-crypto-fallback.c |  2 +-
 block/blk-merge.c           | 15 ++++++++----
 drivers/md/raid0.c          | 12 ++++++++++
 drivers/md/raid1.c          | 33 ++++++++++++++++++++++++--
 drivers/md/raid10.c         | 47 ++++++++++++++++++++++++++++++++++++-
 6 files changed, 110 insertions(+), 13 deletions(-)

Comments

Jens Axboe Nov. 11, 2024, 3:35 p.m. UTC | #1 
On Mon, 11 Nov 2024 11:21:44 +0000, John Garry wrote:
> bio_split() error handling could be improved as follows:
> - Instead of returning NULL for an error - which is vague - return a
>   PTR_ERR, which may hint what went wrong.
> - Remove BUG_ON() calls - which are generally not preferred - and instead
>   WARN and pass an error code back to the caller. Many callers of
>   bio_split() don't check the return code. As such, for an error we would
>   be getting a crash still from an invalid pointer dereference.
> 
> [...]

Applied, thanks!

[1/6] block: Rework bio_split() return value
      commit: e546fe1da9bd47a6fddce6b37c17b1aa1811f7d3
[2/6] block: Error an attempt to split an atomic write in bio_split()
      commit: 27b26f09a7e6ae3ecae460299349b31fe0b5452f
[3/6] block: Handle bio_split() errors in bio_submit_split()
      commit: 6eb09685885a4445da31097aa6418ee1875f9cec
[4/6] md/raid0: Handle bio_split() errors
      commit: 74538fdac3e85aae55eb4ed786478ed2384cb85d
[5/6] md/raid1: Handle bio_split() errors
      commit: b1a7ad8b5c4fa28325ee7b369a2d545d3e16ccde
[6/6] md/raid10: Handle bio_split() errors
      commit: 4cf58d9529097328b669e3c8693ed21e3a041903

Best regards,