[PATCH RESEND v5 2/7] blk-mq-sched: unify elevators checking for async requests

Yu Kuai posted 7 patches 2 months, 3 weeks ago
There is a newer version of this series
[PATCH RESEND v5 2/7] blk-mq-sched: unify elevators checking for async requests
Posted by Yu Kuai 2 months, 3 weeks ago
bfq and mq-deadline consider sync writes as async requests and only
resver tags for sync reads by async_depth, however, kyber doesn't
consider sync writes as async requests for now.

Consider the case there are lots of dirty pages, and user use fsync to
flush dirty pages. In this case sched_tags can be exhausted by sync writes
and sync reads can stuck waiting for tag. Hence let kyber follow what
mq-deadline and bfq did, and unify async requests checking for all
elevators.

Signed-off-by: Yu Kuai <yukuai@fnnas.com>
Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
---
 block/bfq-iosched.c   | 2 +-
 block/blk-mq-sched.h  | 5 +++++
 block/kyber-iosched.c | 2 +-
 block/mq-deadline.c   | 2 +-
 4 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 4a8d3d96bfe4..63452e791f98 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -697,7 +697,7 @@ static void bfq_limit_depth(blk_opf_t opf, struct blk_mq_alloc_data *data)
 	unsigned int limit, act_idx;
 
 	/* Sync reads have full depth available */
-	if (op_is_sync(opf) && !op_is_write(opf))
+	if (blk_mq_sched_sync_request(opf))
 		limit = data->q->nr_requests;
 	else
 		limit = bfqd->async_depths[!!bfqd->wr_busy_queues][op_is_sync(opf)];
diff --git a/block/blk-mq-sched.h b/block/blk-mq-sched.h
index 8e21a6b1415d..ae747f9053c7 100644
--- a/block/blk-mq-sched.h
+++ b/block/blk-mq-sched.h
@@ -103,4 +103,9 @@ static inline void blk_mq_set_min_shallow_depth(struct request_queue *q,
 						depth);
 }
 
+static inline bool blk_mq_sched_sync_request(blk_opf_t opf)
+{
+	return op_is_sync(opf) && !op_is_write(opf);
+}
+
 #endif
diff --git a/block/kyber-iosched.c b/block/kyber-iosched.c
index 18efd6ef2a2b..cf243a457175 100644
--- a/block/kyber-iosched.c
+++ b/block/kyber-iosched.c
@@ -544,7 +544,7 @@ static void kyber_limit_depth(blk_opf_t opf, struct blk_mq_alloc_data *data)
 	 * We use the scheduler tags as per-hardware queue queueing tokens.
 	 * Async requests can be limited at this stage.
 	 */
-	if (!op_is_sync(opf)) {
+	if (!blk_mq_sched_sync_request(opf)) {
 		struct kyber_queue_data *kqd = data->q->elevator->elevator_data;
 
 		data->shallow_depth = kqd->async_depth;
diff --git a/block/mq-deadline.c b/block/mq-deadline.c
index 3e3719093aec..1aef6ce2e78e 100644
--- a/block/mq-deadline.c
+++ b/block/mq-deadline.c
@@ -495,7 +495,7 @@ static void dd_limit_depth(blk_opf_t opf, struct blk_mq_alloc_data *data)
 	struct deadline_data *dd = data->q->elevator->elevator_data;
 
 	/* Do not throttle synchronous reads. */
-	if (op_is_sync(opf) && !op_is_write(opf))
+	if (blk_mq_sched_sync_request(opf))
 		return;
 
 	/*
-- 
2.51.0
Re: [PATCH RESEND v5 2/7] blk-mq-sched: unify elevators checking for async requests
Posted by Bart Van Assche 2 months, 3 weeks ago
On 11/15/25 7:52 PM, Yu Kuai wrote:
> +static inline bool blk_mq_sched_sync_request(blk_opf_t opf)
> +{
> +	return op_is_sync(opf) && !op_is_write(opf);
> +}

The name of this function suggests that it performs an action while it
only performs a test. Please consider renaming this function into e.g.
blk_mq_is_sync_read(). I think the suggested name reflects much more
clearly what this function does than "blk_mq_sched_sync_request()".

Thanks,

Bart.
Re: [PATCH RESEND v5 2/7] blk-mq-sched: unify elevators checking for async requests
Posted by Yu Kuai 2 months, 3 weeks ago
Hi,

在 2025/11/18 7:35, Bart Van Assche 写道:
> On 11/15/25 7:52 PM, Yu Kuai wrote:
>> +static inline bool blk_mq_sched_sync_request(blk_opf_t opf)
>> +{
>> +    return op_is_sync(opf) && !op_is_write(opf);
>> +}
>
> The name of this function suggests that it performs an action while it
> only performs a test. Please consider renaming this function into e.g.
> blk_mq_is_sync_read(). I think the suggested name reflects much more
> clearly what this function does than "blk_mq_sched_sync_request()".
>
Yes, this sounds good.

> Thanks,
>
> Bart.
>

-- 
Thanks,
Kuai