Re: [PATCH 2/2] blk-mq: ensure a q_usage_counter reference is held when splitting bios

[Jens Axboe <axboe@kernel.dk>]

On 1/11/24 10:24 AM, Christoph Hellwig wrote:
> On Thu, Jan 11, 2024 at 10:18:31AM -0700, Jens Axboe wrote:
>> This also highlights a potential inefficiency in the patch, as now we're
>> grabbing+dropping references when we don't need to. May not be a big
>> deal, but it's one of the things that cached requests got rid of. Though
>> I'm not quite sure how to refactor to get rid of that, as we'd need to
>> shuffle the splitting and request get for that.
>>
>> Could you take another look at the series with that in mind?
> 
> I thought about it, but it gets pretty ugly quickly.  bio_queue_enter
> needs to move back into blk_mq_submit_bio, and then we'd skip it
> initially if bio_may_exceed_limits is false, and then we later need
> to add it back.  (we'll probably also need to special case
> blk_queue_bounce as that setting could change to.  I wish we could
> finally kill that)

Something like this? Not super pretty with the duplication, but...
Should suffice for a fix, and then we can refactor it on top of that.
ioprio is inherited when cloning, so we don't need to do that post the
split.

For the bounce side, how would these settings change at runtime? Should
be set at init time and then never change. And agree would be so nice to
kill that code...

diff --git a/block/blk-mq.c b/block/blk-mq.c
index aa9a05fdd023..01134e845d8d 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -2945,12 +2945,6 @@ void blk_mq_submit_bio(struct bio *bio)
 	blk_status_t ret;
 
 	bio = blk_queue_bounce(bio, q);
-	if (bio_may_exceed_limits(bio, &q->limits)) {
-		bio = __bio_split_to_limits(bio, &q->limits, &nr_segs);
-		if (!bio)
-			return;
-	}
-
 	bio_set_ioprio(bio);
 
 	if (plug) {
@@ -2959,6 +2953,11 @@ void blk_mq_submit_bio(struct bio *bio)
 			rq = NULL;
 	}
 	if (rq) {
+		if (unlikely(bio_may_exceed_limits(bio, &q->limits))) {
+			bio = __bio_split_to_limits(bio, &q->limits, &nr_segs);
+			if (!bio)
+				return;
+		}
 		if (!bio_integrity_prep(bio))
 			return;
 		if (blk_mq_attempt_bio_merge(q, bio, nr_segs))
@@ -2969,6 +2968,11 @@ void blk_mq_submit_bio(struct bio *bio)
 	} else {
 		if (unlikely(bio_queue_enter(bio)))
 			return;
+		if (unlikely(bio_may_exceed_limits(bio, &q->limits))) {
+			bio = __bio_split_to_limits(bio, &q->limits, &nr_segs);
+			if (!bio)
+				goto fail;
+		}
 		if (!bio_integrity_prep(bio))
 			goto fail;
 	}

-- 
Jens Axboe