Linux block layer
 help / color / mirror / Atom feed
* [PATCH] blk-mq: set RQF_USE_SCHED when the operation is known
@ 2026-09-21 21:26 Keith Busch
  2026-09-22  5:48 ` Hannes Reinecke
  2026-09-22 12:48 ` Christoph Hellwig
  0 siblings, 2 replies; 3+ messages in thread
From: Keith Busch @ 2026-09-21 21:26 UTC (permalink / raw)
  To: axboe, linux-block; +Cc: Keith Busch, stable, Henry Hu

From: Keith Busch <kbusch@kernel.org>

The cached requests are allocated for one operation but can be handed
out for another. A passthrough command has RQF_USE_SCHED cleared, so
using those flags for a subsequent read/write bio will insert it into
the scheduler without ->prepare_request() and frees it without
->finish_request(). For kyber, this leaks the domain token acquired at
dispatch and stalls the queue.

Don't set RQF_USE_SCHED based on the first operation the batch happened
to be allocated for. Instead, set it after the request is claimed by an
operation.

Fixes: 4b6a5d9cea91 ("block: enable batched allocation for blk_mq_alloc_request()")
Cc: stable@vger.kernel.org
Reported-by: Henry Hu <huhai@kylinos.cn>
Signed-off-by: Keith Busch <kbusch@kernel.org>
---
 block/blk-mq.c | 51 +++++++++++++++++++++++++++++---------------------
 1 file changed, 30 insertions(+), 21 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index a26a11c73ee3e..b1e1b9dea3c0f 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -447,16 +447,6 @@ static struct request *blk_mq_rq_ctx_init(struct blk_mq_alloc_data *data,
 	WRITE_ONCE(rq->deadline, 0);
 	req_ref_set(rq, 1);
 
-	if (rq->rq_flags & RQF_USE_SCHED) {
-		struct elevator_queue *e = data->q->elevator;
-
-		INIT_HLIST_NODE(&rq->hash);
-		RB_CLEAR_NODE(&rq->rb_node);
-
-		if (e->type->ops.prepare_request)
-			e->type->ops.prepare_request(rq);
-	}
-
 	return rq;
 }
 
@@ -498,6 +488,12 @@ __blk_mq_alloc_requests_batch(struct blk_mq_alloc_data *data)
 	return rq_list_pop(data->cached_rqs);
 }
 
+static bool blk_op_bypass_sched(blk_opf_t opf)
+{
+	return (opf & REQ_OP_MASK) == REQ_OP_FLUSH ||
+	       blk_op_is_passthrough(opf);
+}
+
 static void blk_mq_limit_depth(struct blk_mq_alloc_data *data)
 {
 	struct elevator_mq_ops *ops;
@@ -518,12 +514,10 @@ static void blk_mq_limit_depth(struct blk_mq_alloc_data *data)
 	 * Flush/passthrough requests are special and go directly to the
 	 * dispatch list, they are not subject to the async_depth limit.
 	 */
-	if ((data->cmd_flags & REQ_OP_MASK) == REQ_OP_FLUSH ||
-	    blk_op_is_passthrough(data->cmd_flags))
+	if (blk_op_bypass_sched(data->cmd_flags))
 		return;
 
 	WARN_ON_ONCE(data->flags & BLK_MQ_REQ_RESERVED);
-	data->rq_flags |= RQF_USE_SCHED;
 
 	/*
 	 * By default, sync requests have no limit, and async requests are
@@ -534,6 +528,23 @@ static void blk_mq_limit_depth(struct blk_mq_alloc_data *data)
 		ops->limit_depth(data->cmd_flags, data);
 }
 
+static void blk_mq_set_rq_sched(struct request *rq)
+{
+	struct elevator_queue *e;
+
+	if (!(rq->rq_flags & RQF_SCHED_TAGS) ||
+	    blk_op_bypass_sched(rq->cmd_flags))
+		return;
+
+	rq->rq_flags |= RQF_USE_SCHED;
+	INIT_HLIST_NODE(&rq->hash);
+	RB_CLEAR_NODE(&rq->rb_node);
+
+	e = rq->q->elevator;
+	if (e->type->ops.prepare_request)
+		e->type->ops.prepare_request(rq);
+}
+
 static struct request *__blk_mq_alloc_requests(struct blk_mq_alloc_data *data)
 {
 	struct request_queue *q = data->q;
@@ -563,6 +574,7 @@ static struct request *__blk_mq_alloc_requests(struct blk_mq_alloc_data *data)
 		rq = __blk_mq_alloc_requests_batch(data);
 		if (rq) {
 			blk_mq_rq_time_init(rq, alloc_time_ns);
+			blk_mq_set_rq_sched(rq);
 			return rq;
 		}
 		data->nr_tags = 1;
@@ -591,6 +603,7 @@ static struct request *__blk_mq_alloc_requests(struct blk_mq_alloc_data *data)
 		blk_mq_inc_active_requests(data->hctx);
 	rq = blk_mq_rq_ctx_init(data, blk_mq_tags_from_data(data), tag);
 	blk_mq_rq_time_init(rq, alloc_time_ns);
+	blk_mq_set_rq_sched(rq);
 	return rq;
 }
 
@@ -637,8 +650,6 @@ static struct request *blk_mq_alloc_cached_request(struct request_queue *q,
 		if (plug->nr_ios == 1)
 			return NULL;
 		rq = blk_mq_rq_cache_fill(q, plug, opf, flags);
-		if (!rq)
-			return NULL;
 	} else {
 		rq = rq_list_peek(&plug->cached_rqs);
 		if (!rq || rq->q != q)
@@ -646,15 +657,14 @@ static struct request *blk_mq_alloc_cached_request(struct request_queue *q,
 
 		if (blk_mq_get_hctx_type(opf) != rq->mq_hctx->type)
 			return NULL;
-		if (op_is_flush(rq->cmd_flags) != op_is_flush(opf))
-			return NULL;
 
 		rq_list_pop(&plug->cached_rqs);
 		blk_mq_rq_time_init(rq, blk_time_get_ns());
+		rq->cmd_flags = opf;
+		INIT_LIST_HEAD(&rq->queuelist);
+		blk_mq_set_rq_sched(rq);
 	}
 
-	rq->cmd_flags = opf;
-	INIT_LIST_HEAD(&rq->queuelist);
 	return rq;
 }
 
@@ -3060,8 +3070,6 @@ static struct request *blk_mq_get_cached_request(struct blk_plug *plug,
 	if (type != rq->mq_hctx->type &&
 	    (type != HCTX_TYPE_READ || rq->mq_hctx->type != HCTX_TYPE_DEFAULT))
 		return NULL;
-	if (op_is_flush(rq->cmd_flags) != op_is_flush(opf))
-		return NULL;
 	rq_list_pop(&plug->cached_rqs);
 	return rq;
 }
@@ -3166,6 +3174,7 @@ void blk_mq_submit_bio(struct bio *bio)
 		blk_mq_rq_time_init(rq, blk_time_get_ns());
 		rq->cmd_flags = bio->bi_opf;
 		INIT_LIST_HEAD(&rq->queuelist);
+		blk_mq_set_rq_sched(rq);
 	} else {
 		rq = blk_mq_get_new_requests(q, plug, bio);
 		if (unlikely(!rq)) {
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] blk-mq: set RQF_USE_SCHED when the operation is known
  2026-09-21 21:26 [PATCH] blk-mq: set RQF_USE_SCHED when the operation is known Keith Busch
@ 2026-09-22  5:48 ` Hannes Reinecke
  2026-09-22 12:48 ` Christoph Hellwig
  1 sibling, 0 replies; 3+ messages in thread
From: Hannes Reinecke @ 2026-09-22  5:48 UTC (permalink / raw)
  To: Keith Busch, axboe, linux-block; +Cc: Keith Busch, stable, Henry Hu

On 9/21/26 11:26 PM, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
> 
> The cached requests are allocated for one operation but can be handed
> out for another. A passthrough command has RQF_USE_SCHED cleared, so
> using those flags for a subsequent read/write bio will insert it into
> the scheduler without ->prepare_request() and frees it without
> ->finish_request(). For kyber, this leaks the domain token acquired at
> dispatch and stalls the queue.
> 
> Don't set RQF_USE_SCHED based on the first operation the batch happened
> to be allocated for. Instead, set it after the request is claimed by an
> operation.
> 
> Fixes: 4b6a5d9cea91 ("block: enable batched allocation for blk_mq_alloc_request()")
> Cc: stable@vger.kernel.org
> Reported-by: Henry Hu <huhai@kylinos.cn>
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
>   block/blk-mq.c | 51 +++++++++++++++++++++++++++++---------------------
>   1 file changed, 30 insertions(+), 21 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@kernel.org>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] blk-mq: set RQF_USE_SCHED when the operation is known
  2026-09-21 21:26 [PATCH] blk-mq: set RQF_USE_SCHED when the operation is known Keith Busch
  2026-09-22  5:48 ` Hannes Reinecke
@ 2026-09-22 12:48 ` Christoph Hellwig
  1 sibling, 0 replies; 3+ messages in thread
From: Christoph Hellwig @ 2026-09-22 12:48 UTC (permalink / raw)
  To: Keith Busch; +Cc: axboe, linux-block, Keith Busch, stable, Henry Hu

On Mon, Sep 21, 2026 at 02:26:24PM -0700, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
> 
> The cached requests are allocated for one operation but can be handed
> out for another. A passthrough command has RQF_USE_SCHED cleared, so
> using those flags for a subsequent read/write bio will insert it into
> the scheduler without ->prepare_request() and frees it without
> ->finish_request(). For kyber, this leaks the domain token acquired at
> dispatch and stalls the queue.
> 
> Don't set RQF_USE_SCHED based on the first operation the batch happened
> to be allocated for. Instead, set it after the request is claimed by an
> operation.

This looks generally good, but also a bit hard to follow.

Notes:

 - mq-deadline refers to blk_mq_rq_ctx_init in a comment that needs
   to be updated or removed.
 - blk_mq_rq_time_init is always called right next to
   blk_mq_set_rq_sched.  Should these be comined in a single late
   init helper?
 - AFAICS the op_is_flush check removal is enabled by this, but not
   required, so maybe move it in a well-documened follow on patch?

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-09-22 12:48 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-21 21:26 [PATCH] blk-mq: set RQF_USE_SCHED when the operation is known Keith Busch
2026-09-22  5:48 ` Hannes Reinecke
2026-09-22 12:48 ` Christoph Hellwig

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox