[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <bf84ef70-9715-4722-b693-6e8519595b0c@suse.de>
Date: Fri, 21 Nov 2025 08:14:16 +0100
From: Hannes Reinecke <hare@...e.de>
To: Yu Kuai <yukuai@...as.com>, axboe@...nel.dk, nilay@...ux.ibm.com,
bvanassche@....org, linux-block@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 4/8] blk-mq: add a new queue sysfs attribute
async_depth
On 11/21/25 06:28, Yu Kuai wrote:
> Add a new field async_depth to request_queue and related APIs, this is
> currently not used, following patches will convert elevators to use
> this instead of internal async_depth.
>
> Signed-off-by: Yu Kuai <yukuai@...as.com>
> Reviewed-by: Nilay Shroff <nilay@...ux.ibm.com>
> ---
> block/blk-core.c | 1 +
> block/blk-mq.c | 6 ++++++
> block/blk-sysfs.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> block/elevator.c | 1 +
> include/linux/blkdev.h | 1 +
> 5 files changed, 51 insertions(+)
>
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 14ae73eebe0d..cc5c9ced8e6f 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -463,6 +463,7 @@ struct request_queue *blk_alloc_queue(struct queue_limits *lim, int node_id)
> fs_reclaim_release(GFP_KERNEL);
>
> q->nr_requests = BLKDEV_DEFAULT_RQ;
> + q->async_depth = BLKDEV_DEFAULT_RQ;
>
> return q;
>
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 6c505ebfab65..ae6ce68f4786 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -4628,6 +4628,7 @@ int blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
> spin_lock_init(&q->requeue_lock);
>
> q->nr_requests = set->queue_depth;
> + q->async_depth = set->queue_depth;
>
> blk_mq_init_cpu_queues(q, set->nr_hw_queues);
> blk_mq_map_swqueue(q);
> @@ -4994,6 +4995,11 @@ struct elevator_tags *blk_mq_update_nr_requests(struct request_queue *q,
> q->elevator->et = et;
> }
>
> + /*
> + * Preserve relative value, both nr and async_depth are at most 16 bit
> + * value, no need to worry about overflow.
> + */
> + q->async_depth = max(q->async_depth * nr / q->nr_requests, 1);
> q->nr_requests = nr;
> if (q->elevator && q->elevator->type->ops.depth_updated)
> q->elevator->type->ops.depth_updated(q);
> diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
> index 8684c57498cc..5c2d29ac6570 100644
> --- a/block/blk-sysfs.c
> +++ b/block/blk-sysfs.c
> @@ -127,6 +127,46 @@ queue_requests_store(struct gendisk *disk, const char *page, size_t count)
> return ret;
> }
>
> +static ssize_t queue_async_depth_show(struct gendisk *disk, char *page)
> +{
> + guard(mutex)(&disk->queue->elevator_lock);
> +
> + return queue_var_show(disk->queue->async_depth, page);
> +}
> +
> +static ssize_t
> +queue_async_depth_store(struct gendisk *disk, const char *page, size_t count)
> +{
> + struct request_queue *q = disk->queue;
> + unsigned int memflags;
> + unsigned long nr;
> + int ret;
> +
> + if (!queue_is_mq(q))
> + return -EINVAL;
> +
> + ret = queue_var_store(&nr, page, count);
> + if (ret < 0)
> + return ret;
> +
> + if (nr == 0)
> + return -EINVAL;
> +
> + memflags = blk_mq_freeze_queue(q);
> + scoped_guard(mutex, &q->elevator_lock) {
> + if (q->elevator) {
> + q->async_depth = min(q->nr_requests, nr);
> + if (q->elevator->type->ops.depth_updated)
> + q->elevator->type->ops.depth_updated(q);
> + } else {
> + ret = -EINVAL;
> + }
> + }
> + blk_mq_unfreeze_queue(q, memflags);
> +
> + return ret;
> +}
> +
> static ssize_t queue_ra_show(struct gendisk *disk, char *page)
> {
> ssize_t ret;
> @@ -532,6 +572,7 @@ static struct queue_sysfs_entry _prefix##_entry = { \
> }
>
> QUEUE_RW_ENTRY(queue_requests, "nr_requests");
> +QUEUE_RW_ENTRY(queue_async_depth, "async_depth");
> QUEUE_RW_ENTRY(queue_ra, "read_ahead_kb");
> QUEUE_LIM_RW_ENTRY(queue_max_sectors, "max_sectors_kb");
> QUEUE_LIM_RO_ENTRY(queue_max_hw_sectors, "max_hw_sectors_kb");
> @@ -754,6 +795,7 @@ static struct attribute *blk_mq_queue_attrs[] = {
> */
> &elv_iosched_entry.attr,
> &queue_requests_entry.attr,
> + &queue_async_depth_entry.attr,
> #ifdef CONFIG_BLK_WBT
> &queue_wb_lat_entry.attr,
> #endif
> diff --git a/block/elevator.c b/block/elevator.c
> index 5b37ef44f52d..5ff21075a84a 100644
> --- a/block/elevator.c
> +++ b/block/elevator.c
> @@ -589,6 +589,7 @@ static int elevator_switch(struct request_queue *q, struct elv_change_ctx *ctx)
> blk_queue_flag_clear(QUEUE_FLAG_SQ_SCHED, q);
> q->elevator = NULL;
> q->nr_requests = q->tag_set->queue_depth;
> + q->async_depth = q->tag_set->queue_depth;
> }
> blk_add_trace_msg(q, "elv switch: %s", ctx->name);
>
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index cdc68c41fa96..edddf17f8304 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -552,6 +552,7 @@ struct request_queue {
> * queue settings
> */
> unsigned int nr_requests; /* Max # of requests */
> + unsigned int async_depth; /* Max # of async requests */
>
> #ifdef CONFIG_BLK_INLINE_ENCRYPTION
> struct blk_crypto_profile *crypto_profile;
Hmm. Makes me wonder: async_depth is only used within the elevators, so
maybe we should restrict visibility of that attribute when an elevator
is selected?
It feels kinda awkward having an attribute which does nothing if no
elevator is loaded ...
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@...e.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
Powered by blists - more mailing lists