Re: [PATCH] scsi_debug: fix cmd_per_lun, set to max_queue

[Ming Lei <ming.lei@redhat.com>]

On Thu, Apr 15, 2021 at 10:15:27AM +0100, John Garry wrote:
> This looks ok.
> 
> Apart from this, I tested linux-next (without this patch) - which includes
> Ming's changes to replace sdev-->device_busy with sbitmap - and, as
> expected, it has the issue.
> 
> So I think it is also worth having this to stop this happening elsewhere:
> 
> ------>8-------
> 
> Subject: [PATCH] scsi: core: Cap initial sdev queue depth at Shost.can_queue
> 
> Function sdev_store_queue_depth() enforces that the sdev queue depth cannot
> exceed Shost.can_queue.
> 
> However, the LLDD may still set cmd_per_lun > can_queue, which would lead to
> an initial sdev queue depth greater than can_queue.
> 
> Stop this happened by capping initial sdev queue depth at can_queue.
> 
> <insert credits>
> Signed-off-by: John Garry <john.garry@huawei.com>
> 
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index 9af50e6f94c4..fec6c17ff37c 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -218,6 +218,7 @@ static struct scsi_device *scsi_alloc_sdev(struct
> scsi_target *starget,
>  	struct scsi_device *sdev;
>  	int display_failure_msg = 1, ret;
>  	struct Scsi_Host *shost = dev_to_shost(starget->dev.parent);
> +	int depth;
> 
>  	sdev = kzalloc(sizeof(*sdev) + shost->transportt->device_size,
>  		       GFP_KERNEL);
> @@ -276,8 +277,13 @@ static struct scsi_device *scsi_alloc_sdev(struct
> scsi_target *starget,
>  	WARN_ON_ONCE(!blk_get_queue(sdev->request_queue));
>  	sdev->request_queue->queuedata = sdev;
> 
> -	scsi_change_queue_depth(sdev, sdev->host->cmd_per_lun ?
> -					sdev->host->cmd_per_lun : 1);
> +	if (sdev->host->cmd_per_lun)
> +		depth = min_t(int, sdev->host->cmd_per_lun,
> +			      sdev->host->can_queue);
> +	else
> +		depth = 1;
> +
> +	scsi_change_queue_depth(sdev, depth);

'cmd_per_lun' should have been set as correct from the beginning instead
of capping it for changing queue depth:

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 697c09ef259b..0d9954eabbb8 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -414,7 +414,7 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
 	shost->can_queue = sht->can_queue;
 	shost->sg_tablesize = sht->sg_tablesize;
 	shost->sg_prot_tablesize = sht->sg_prot_tablesize;
-	shost->cmd_per_lun = sht->cmd_per_lun;
+	shost->cmd_per_lun = min_t(int, sht->cmd_per_lun, shost->can_queue);
 	shost->no_write_same = sht->no_write_same;
 	shost->host_tagset = sht->host_tagset;
 

Thanks,
Ming