On 1/10/23 2:20 AM, Christoph Hellwig wrote: > On Wed, Jan 04, 2023 at 09:09:37AM -0700, Jens Axboe wrote: >> This can't happen right now, but in preparation for allowing >> bio_split_to_limits() returning NULL if it ended the bio, check for it >> in all the callers. >> >> Signed-off-by: Jens Axboe <axboe@xxxxxxxxx> >> --- >> block/blk-merge.c | 4 +++- >> block/blk-mq.c | 5 ++++- >> drivers/block/drbd/drbd_req.c | 2 ++ >> drivers/block/ps3vram.c | 2 ++ >> drivers/md/dm.c | 2 ++ >> drivers/md/md.c | 2 ++ >> drivers/nvme/host/multipath.c | 2 ++ >> drivers/s390/block/dcssblk.c | 2 ++ >> 8 files changed, 19 insertions(+), 2 deletions(-) >> >> diff --git a/block/blk-merge.c b/block/blk-merge.c >> index 35a8f75cc45d..071c5f8cf0cf 100644 >> --- a/block/blk-merge.c >> +++ b/block/blk-merge.c >> @@ -358,11 +358,13 @@ struct bio *__bio_split_to_limits(struct bio *bio, >> default: >> split = bio_split_rw(bio, lim, nr_segs, bs, >> get_max_io_size(bio, lim) << SECTOR_SHIFT); >> + if (IS_ERR(split)) >> + return NULL; > > Can we decide on either passing an ERR_PTR or NULL and do it through > the whole stack? We need the error return here as we could just return NULL and it not be an error, but for further down the stack I feel like returning an error that you can't do anything with (as it's already been dealt with) would be confusing. That's why I did it that way. >> diff --git a/drivers/block/drbd/drbd_req.c b/drivers/block/drbd/drbd_req.c >> index eb14ec8ec04c..e36216d50753 100644 >> --- a/drivers/block/drbd/drbd_req.c >> +++ b/drivers/block/drbd/drbd_req.c >> @@ -1607,6 +1607,8 @@ void drbd_submit_bio(struct bio *bio) >> struct drbd_device *device = bio->bi_bdev->bd_disk->private_data; >> >> bio = bio_split_to_limits(bio); >> + if (!bio) >> + return; > > So for the callers in drivers, do we need thee checks for drivers > that don't even support REQ_NOWAIT? Good point, probably not, we should be erroring these out before they reach the driver. Doesn't hurt though, it would not necessarily be obvious that you'd now need to start checking bio_split_to_limits() return values when you set NOWAIT on the queue. -- Jens Axboe
