| Message ID | 20240919092302.3094725-4-john.g.garry@oracle.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | bio_split() error handling rework | expand |
| Context | Check | Description |
|---|---|---|
| mdraidci/vmtest-md-6_12-PR | fail | merge-conflict |
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.
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 --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);
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(+)