mq-deadline: the dd->dispatch queue follows a FIFO policy

[PATCH] mq-deadline: the dd->dispatch queue follows a FIFO policy

Posted by chengkaitao 1 week, 3 days ago

From: Chengkaitao <chengkaitao@kylinos.cn>

In the initial implementation, the 'list_add(&rq->queuelist, ...' statement
added to the dd_insert_request function was designed to differentiate
priorities among various IO-requests within the same linked list. For
example, 'Commit 945ffb60c11d ("mq-deadline: add blk-mq adaptation of the
deadline IO scheduler")', introduced this 'list_add' operation to ensure
that requests with the at_head flag would always be dispatched before
requests without the REQ_TYPE_FS flag.

Since 'Commit 7687b38ae470 ("bfq/mq-deadline: remove redundant check for
passthrough request")', removed blk_rq_is_passthrough, the dd->dispatch
list now contains only requests with the at_head flag. In this context,
all at_head requests should be treated as having equal priority, and a
first-in-first-out (FIFO) policy better aligns with the current situation.
Therefore, replacing list_add with list_add_tail is more appropriate.

Signed-off-by: Chengkaitao <chengkaitao@kylinos.cn>
---
 block/mq-deadline.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/mq-deadline.c b/block/mq-deadline.c
index 3e3719093aec..dcd7f4f1ecd2 100644
--- a/block/mq-deadline.c
+++ b/block/mq-deadline.c
@@ -661,7 +661,7 @@ static void dd_insert_request(struct blk_mq_hw_ctx *hctx, struct request *rq,
 	trace_block_rq_insert(rq);
 
 	if (flags & BLK_MQ_INSERT_AT_HEAD) {
-		list_add(&rq->queuelist, &dd->dispatch);
+		list_add_tail(&rq->queuelist, &dd->dispatch);
 		rq->fifo_time = jiffies;
 	} else {
 		deadline_add_rq_rb(per_prio, rq);
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] mq-deadline: the dd->dispatch queue follows a FIFO policy

Posted by Bart Van Assche 1 week, 3 days ago

On 12/8/25 4:02 AM, chengkaitao wrote:
> From: Chengkaitao <chengkaitao@kylinos.cn>
> 
> In the initial implementation, the 'list_add(&rq->queuelist, ...' statement
> added to the dd_insert_request function was designed to differentiate
> priorities among various IO-requests within the same linked list. For
> example, 'Commit 945ffb60c11d ("mq-deadline: add blk-mq adaptation of the
> deadline IO scheduler")', introduced this 'list_add' operation to ensure
> that requests with the at_head flag would always be dispatched before
> requests without the REQ_TYPE_FS flag.
> 
> Since 'Commit 7687b38ae470 ("bfq/mq-deadline: remove redundant check for
> passthrough request")', removed blk_rq_is_passthrough, the dd->dispatch
> list now contains only requests with the at_head flag. In this context,
> all at_head requests should be treated as having equal priority, and a
> first-in-first-out (FIFO) policy better aligns with the current situation.
> Therefore, replacing list_add with list_add_tail is more appropriate.
> 
> Signed-off-by: Chengkaitao <chengkaitao@kylinos.cn>
> ---
>   block/mq-deadline.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block/mq-deadline.c b/block/mq-deadline.c
> index 3e3719093aec..dcd7f4f1ecd2 100644
> --- a/block/mq-deadline.c
> +++ b/block/mq-deadline.c
> @@ -661,7 +661,7 @@ static void dd_insert_request(struct blk_mq_hw_ctx *hctx, struct request *rq,
>   	trace_block_rq_insert(rq);
>   
>   	if (flags & BLK_MQ_INSERT_AT_HEAD) {
> -		list_add(&rq->queuelist, &dd->dispatch);
> +		list_add_tail(&rq->queuelist, &dd->dispatch);
>   		rq->fifo_time = jiffies;
>   	} else {
>   		deadline_add_rq_rb(per_prio, rq);

I think the current behavior (LIFO) is on purpose and also that it only
should be changed if there is a strong reason to change it. I don't see
a strong reason being mentioned in the patch description.

Bart.

Re: [PATCH] mq-deadline: the dd->dispatch queue follows a FIFO policy

Posted by Tao pilgrim 1 week, 3 days ago

On Tue, Dec 9, 2025 at 12:24 AM Bart Van Assche <bvanassche@acm.org> wrote:
>
> On 12/8/25 4:02 AM, chengkaitao wrote:
> > From: Chengkaitao <chengkaitao@kylinos.cn>
> >
> > In the initial implementation, the 'list_add(&rq->queuelist, ...' statement
> > added to the dd_insert_request function was designed to differentiate
> > priorities among various IO-requests within the same linked list. For
> > example, 'Commit 945ffb60c11d ("mq-deadline: add blk-mq adaptation of the
> > deadline IO scheduler")', introduced this 'list_add' operation to ensure
> > that requests with the at_head flag would always be dispatched before
> > requests without the REQ_TYPE_FS flag.
> >
> > Since 'Commit 7687b38ae470 ("bfq/mq-deadline: remove redundant check for
> > passthrough request")', removed blk_rq_is_passthrough, the dd->dispatch
> > list now contains only requests with the at_head flag. In this context,
> > all at_head requests should be treated as having equal priority, and a
> > first-in-first-out (FIFO) policy better aligns with the current situation.
> > Therefore, replacing list_add with list_add_tail is more appropriate.
> >
> > Signed-off-by: Chengkaitao <chengkaitao@kylinos.cn>
> > ---
> >   block/mq-deadline.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/block/mq-deadline.c b/block/mq-deadline.c
> > index 3e3719093aec..dcd7f4f1ecd2 100644
> > --- a/block/mq-deadline.c
> > +++ b/block/mq-deadline.c
> > @@ -661,7 +661,7 @@ static void dd_insert_request(struct blk_mq_hw_ctx *hctx, struct request *rq,
> >       trace_block_rq_insert(rq);
> >
> >       if (flags & BLK_MQ_INSERT_AT_HEAD) {
> > -             list_add(&rq->queuelist, &dd->dispatch);
> > +             list_add_tail(&rq->queuelist, &dd->dispatch);
> >               rq->fifo_time = jiffies;
> >       } else {
> >               deadline_add_rq_rb(per_prio, rq);
>
> I think the current behavior (LIFO) is on purpose and also that it only
> should be changed if there is a strong reason to change it. I don't see
> a strong reason being mentioned in the patch description.
>
Previously, the dd->dispatch queue contained both requests with the
BLK_MQ_INSERT_AT_HEAD flag and those without it. The implementation
placed requests with the BLK_MQ_INSERT_AT_HEAD flag at the head of
the dd->dispatch queue, while requests without this flag were placed
at the tail. However, now all requests in the dd->dispatch list carry
the BLK_MQ_INSERT_AT_HEAD flag, and I can't find any justification for
continuing to use a LIFO (last-in-first-out) policy.

Additionally, the dispatch queue has recently been moved from struct
dd_per_prio to struct deadline_data, switching back to a single dispatch
list. This means more requests will queue in the dispatch list, and
continuing with the LIFO policy makes earlier-arriving requests more
susceptible to starvation. Could anyone explain the rationale behind
maintaining this approach?

-- 
Yours,
Kaitao Cheng

Re: [PATCH] mq-deadline: the dd->dispatch queue follows a FIFO policy

Posted by Jens Axboe 1 week, 2 days ago

On 12/8/25 7:06 PM, Tao pilgrim wrote:
> However, now all requests in the dd->dispatch list carry
> the BLK_MQ_INSERT_AT_HEAD flag, and I can't find any justification for
> continuing to use a LIFO (last-in-first-out) policy.

What if a head insertion requires another TUR or similar before being
able to be processed?

I agree with notion that AT_HEAD should generally be issued in order,
but let's not make blanket changes like this just because you can't find
any immediate justification for why the current behavior is as it is.
It'd be different if you were fixing a bug and came across this. But
that does not seem to be the case.

-- 
Jens Axboe