Re: [PATCH 3/4] block: move queue enter logic into blk_mq_submit_bio()

On 11/4/21 11:30 AM, Christoph Hellwig wrote:

On Thu, Nov 04, 2021 at 05:41:35AM -0600, Jens Axboe wrote:

quoted

quoted

quoted

 	if (!submit_bio_checks(bio) || !blk_crypto_bio_prep(&bio))
-		goto queue_exit;
+		return;

This is broken, we really ant the submit checks under freeze
protection to make sure the parameters can't be changed underneath
us.

Which parameters are you worried about in submit_bio_checks()? I don't
immediately see anything that would make me worry about it.

Mostly checks if certain operations are supported or not, as
revalidation could clear those.

quoted

quoted

This looks weird, as blk_try_enter_queue is already called by
bio_queue_enter.

It's just for avoiding a pointless call into bio_queue_enter(), which
isn't needed it blk_try_enter_queue() is successful. The latter is short
and small and can be inlined, while bio_queue_enter() is a lot bigger.

If this is so impotant let's operated with an inlined bio_queue_enter
that calls out of line into slow path instead of open coding it
like this.

Sure, I can do that instead.

quoted

quoted

quoted

 	} else {
 		struct blk_mq_alloc_data data = {
 			.q		= q,

@@ -2528,6 +2534,11 @@ void blk_mq_submit_bio(struct bio *bio)
 			.cmd_flags	= bio->bi_opf,
 		};
 
+		if (unlikely(!blk_mq_queue_enter(q, bio)))
+			return;
+
+		rq_qos_throttle(q, bio);
+

At some point the code in this !cached branch really needs to move
into a helper..

Like in the next patch?

No, I mean the !cached case which is a lot more convoluted.

Yeah, a helper there might be appropriate. I'll write it up.

-- 
Jens Axboe