diff mbox series

[RFC,3/6] block: Handle bio_split() errors in bio_submit_split()

Message ID 20240919092302.3094725-4-john.g.garry@oracle.com (mailing list archive)
State New, archived
Headers show
Series bio_split() error handling rework | expand

Commit Message

John Garry Sept. 19, 2024, 9:22 a.m. UTC 
bio_split() may error, so check this.

Signed-off-by: John Garry <john.g.garry@oracle.com>
---
Should we move the WARN_ON_ONCE(bio_zone_write_plugging(bio)) call (not
shown) in bio_submit_split() to bio_split()?

 block/blk-merge.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Christoph Hellwig Sept. 20, 2024, 2:09 p.m. UTC | #1 
On Thu, Sep 19, 2024 at 09:22:59AM +0000, John Garry wrote:
> +		if (IS_ERR(split)) {
> +			bio->bi_status = errno_to_blk_status(PTR_ERR(split));
> +			bio_endio(bio);
> +			return NULL;
> +		}

This could use a goto to have a single path that ends the bio and
return NULL instead of duplicating the logic.
John Garry Sept. 23, 2024, 10:33 a.m. UTC | #2 
On 20/09/2024 15:09, Christoph Hellwig wrote:
> On Thu, Sep 19, 2024 at 09:22:59AM +0000, John Garry wrote:
>> +		if (IS_ERR(split)) {
>> +			bio->bi_status = errno_to_blk_status(PTR_ERR(split));
>> +			bio_endio(bio);
>> +			return NULL;
>> +		}
> This could use a goto to have a single path that ends the bio and
> return NULL instead of duplicating the logic.

Sure, ok.

I was also considering adding a helper for these cases, similar to 
bio_io_error(), which accepts a bio and an int errorno or blk_status_t 
type, like:

void bio_end_error(struct bio* bio, int errno)
{
	bio->bi_status = errno_to_blk_status(errno);
	bio_endio(bio);
}

I didn't bother though. Sometimes we already have the blk_status_t 
value, which made this a half-useful API.
diff mbox series

Patch

diff --git a/block/blk-merge.c b/block/blk-merge.c
index ad763ec313b6..ec7be2031819 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -118,6 +118,11 @@  static struct bio *bio_submit_split(struct bio *bio, int split_sectors)
 
 		split = bio_split(bio, split_sectors, GFP_NOIO,
 				&bio->bi_bdev->bd_disk->bio_split);
+		if (IS_ERR(split)) {
+			bio->bi_status = errno_to_blk_status(PTR_ERR(split));
+			bio_endio(bio);
+			return NULL;
+		}
 		split->bi_opf |= REQ_NOMERGE;
 		blkcg_bio_issue_init(split);
 		bio_chain(split, bio);