Re: [PATCH v4 6/6] scsi: core: Improve IOPS in case of host-wide tags

[Damien Le Moal <dlemoal@kernel.org>]

On 12/17/25 07:30, Bart Van Assche wrote:
> The SCSI core uses the budget map to restrict the number of commands
> that are in flight per logical unit. That limit check can be left out if
> host->cmd_per_lun >= host->can_queue and if the host tag set is shared
> across all hardware queues or if there is only one hardware queue  Since

Missing a period at the end of the sentence (before Since). But more
importantly, this does not explain why the above is true, and frankly, I do not
see it...

> scsi_mq_get_budget() shows up in all CPU profiles for fast SCSI devices,
> do not allocate a budget map if cmd_per_lun >= can_queue and if the host
> tag set is shared across all hardware queues.
> 
> For the following test this patch increases IOPS by 5%:
> 
> modprobe scsi_debug delay=0 no_rwlock=1 host_max_queue=192 submit_queues=$(nproc)
> 
> fio --bs=4096 --disable_clat=1 --disable_slat=1 --group_reporting=1 \
>   --gtod_reduce=1 --invalidate=1 --ioengine=io_uring --ioscheduler=none \
>   --norandommap --runtime=60 --rw=randread --thread --time_based=1 \
>   --buffered=0 --numjobs=1 --iodepth=192 --iodepth_batch=24 --name=/dev/sda \
>   --filename=/dev/sda
> 
> Cc: Jens Axboe <axboe@kernel.dk>
> Cc: Christoph Hellwig <hch@infradead.org>
> Cc: Ming Lei <ming.lei@redhat.com>
> Cc: John Garry <john.g.garry@oracle.com>
> Cc: Hannes Reinecke <hare@suse.de>
> Signed-off-by: Bart Van Assche <bvanassche@acm.org>
> ---
>  drivers/scsi/scsi.c      |  6 ++----
>  drivers/scsi/scsi_scan.c | 20 +++++++++++++++++++-
>  2 files changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> index 76cdad063f7b..3dc93dd9fda2 100644
> --- a/drivers/scsi/scsi.c
> +++ b/drivers/scsi/scsi.c
> @@ -216,9 +216,6 @@ int scsi_device_max_queue_depth(struct scsi_device *sdev)
>   */
>  int scsi_change_queue_depth(struct scsi_device *sdev, int depth)
>  {
> -	if (!sdev->budget_map.map)
> -		return -EINVAL;
> -
>  	depth = min_t(int, depth, scsi_device_max_queue_depth(sdev));
>  
>  	if (depth > 0) {
> @@ -229,7 +226,8 @@ int scsi_change_queue_depth(struct scsi_device *sdev, int depth)
>  	if (sdev->request_queue)
>  		blk_set_queue_depth(sdev->request_queue, depth);
>  
> -	sbitmap_resize(&sdev->budget_map, sdev->queue_depth);
> +	if (sdev->budget_map.map)
> +		sbitmap_resize(&sdev->budget_map, sdev->queue_depth);
>  
>  	return sdev->queue_depth;
>  }
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index 7acbfcfc2172..35bfc118e048 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -215,9 +215,19 @@ static void scsi_unlock_floptical(struct scsi_device *sdev,
>  			 SCSI_TIMEOUT, 3, NULL);
>  }
>  
> +static bool scsi_needs_budget_map(struct Scsi_Host *shost, unsigned int depth)
> +{
> +	if (shost->needs_budget_token)
> +		return true;
> +	if (shost->host_tagset || shost->tag_set.nr_hw_queues == 1)
> +		return depth < shost->can_queue;
> +	return true;
> +}
> +
>  static int scsi_realloc_sdev_budget_map(struct scsi_device *sdev,
>  					unsigned int depth)
>  {
> +	struct Scsi_Host *shost = sdev->host;
>  	int new_shift = sbitmap_calculate_shift(depth);
>  	bool need_alloc = !sdev->budget_map.map;
>  	bool need_free = false;
> @@ -225,6 +235,13 @@ static int scsi_realloc_sdev_budget_map(struct scsi_device *sdev,
>  	int ret;
>  	struct sbitmap sb_backup;
>  
> +	if (!scsi_needs_budget_map(shost, depth)) {
> +		memflags = blk_mq_freeze_queue(sdev->request_queue);
> +		sbitmap_free(&sdev->budget_map);
> +		blk_mq_unfreeze_queue(sdev->request_queue, memflags);
> +		return 0;
> +	}
> +
>  	depth = min_t(unsigned int, depth, scsi_device_max_queue_depth(sdev));
>  
>  	/*
> @@ -1120,7 +1137,8 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result,
>  	scsi_cdl_check(sdev);
>  
>  	sdev->max_queue_depth = sdev->queue_depth;
> -	WARN_ON_ONCE(sdev->max_queue_depth > sdev->budget_map.depth);
> +	WARN_ON_ONCE(sdev->budget_map.map &&
> +		     sdev->max_queue_depth > sdev->budget_map.depth);
>  
>  	/*
>  	 * Ok, the device is now all set up, we can


-- 
Damien Le Moal
Western Digital Research