Re: [PATCH V4 11/12] scsi: make sure sdev->queue_depth is <= max(shost->can_queue, 1024)

[Hannes Reinecke <hare@suse.de>]

On 11/16/20 10:07 AM, Ming Lei wrote:
> Limit scsi device's queue depth is less than max(host->can_queue, 1024)
> in scsi_change_queue_depth(), and 1024 is big enough for saturating
> current fast SCSI LUN(SSD, or raid volume on multiple SSDs).
> 
> We need this patch for replacing sdev->device_busy with sbitmap which
> has to be pre-allocated with reasonable max depth.
> 
> Cc: Omar Sandoval <osandov@fb.com>
> Cc: Kashyap Desai <kashyap.desai@broadcom.com>
> Cc: Sumanesh Samanta <sumanesh.samanta@broadcom.com>
> Cc: Ewan D. Milne <emilne@redhat.com>
> Tested-by: Sumanesh Samanta <sumanesh.samanta@broadcom.com>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
>   drivers/scsi/scsi.c | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> index 24619c3bebd5..a28d48c850cf 100644
> --- a/drivers/scsi/scsi.c
> +++ b/drivers/scsi/scsi.c
> @@ -214,6 +214,15 @@ void scsi_finish_command(struct scsi_cmnd *cmd)
>   	scsi_io_completion(cmd, good_bytes);
>   }
>   
> +
> +/*
> + * 1024 is big enough for saturating the fast scsi LUN now
> + */
> +static int scsi_device_max_queue_depth(struct scsi_device *sdev)
> +{
> +	return max_t(int, sdev->host->can_queue, 1024);
> +}
> +

Shouldn't this rather be initialized with scsi_host->can_queue?
These 'should be enough' settings inevitable turn out to be not enough 
in the long run ...

>   /**
>    * scsi_change_queue_depth - change a device's queue depth
>    * @sdev: SCSI Device in question
> @@ -223,6 +232,8 @@ void scsi_finish_command(struct scsi_cmnd *cmd)
>    */
>   int scsi_change_queue_depth(struct scsi_device *sdev, int depth)
>   {
> +	depth = min_t(int, depth, scsi_device_max_queue_depth(sdev));
> +
>   	if (depth > 0) {
>   		sdev->queue_depth = depth;
>   		wmb();
> 

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer