[PATCH] scsi: add attribute to set lun queue depth on all luns on shost

[PATCH] scsi: add attribute to set lun queue depth on all luns on shost

[James Smart]

There has been a desire to set the lun queue depth on all luns on
an shost. Today that is done by an external script looping through
discovered sdevs and set an sdev attribute.

This patch addes a shost attribute that will cycle through all
sdevs and set the lun queue depth.

Signed-off-by: James Smart <jsmart2021@gmail.com>
---
 drivers/scsi/scsi_sysfs.c | 49 ++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 46 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 677b5c5403d2..debb79be66c0 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -368,6 +368,50 @@ store_shost_eh_deadline(struct device *dev, struct device_attribute *attr,
 
 static DEVICE_ATTR(eh_deadline, S_IRUGO | S_IWUSR, show_shost_eh_deadline, store_shost_eh_deadline);
 
+static int
+sdev_set_queue_depth(struct scsi_device *sdev, int depth)
+{
+	struct scsi_host_template *sht = sdev->host->hostt;
+	int retval;
+
+	retval = sht->change_queue_depth(sdev, depth);
+	if (retval < 0)
+		return retval;
+
+	sdev->max_queue_depth = sdev->queue_depth;
+
+	return 0;
+}
+
+static ssize_t
+store_host_lun_queue_depth(struct device *dev, struct device_attribute *attr,
+		const char *buf, size_t count)
+{
+	struct Scsi_Host *shost = class_to_shost(dev);
+	struct scsi_host_template *sht = shost->hostt;
+	struct scsi_device *sdev;
+	int depth, retval;
+
+	if (!sht->change_queue_depth)
+		return -EINVAL;
+
+	depth = simple_strtoul(buf, NULL, 0);
+	if (depth < 1 || depth > shost->can_queue)
+		return -EINVAL;
+
+	shost_for_each_device(sdev, shost) {
+		retval = sdev_set_queue_depth(sdev, depth);
+		if (retval != 0)
+			sdev_printk(KERN_INFO, sdev,
+				"failed to set queue depth to %d (err %d)",
+				depth, retval);
+	}
+
+	return count;
+}
+
+static DEVICE_ATTR(lun_queue_depth, S_IWUSR, NULL, store_host_lun_queue_depth);
+
 shost_rd_attr(unique_id, "%u\n");
 shost_rd_attr(cmd_per_lun, "%hd\n");
 shost_rd_attr(can_queue, "%hd\n");
@@ -411,6 +455,7 @@ static struct attribute *scsi_sysfs_shost_attrs[] = {
 	&dev_attr_prot_guard_type.attr,
 	&dev_attr_host_reset.attr,
 	&dev_attr_eh_deadline.attr,
+	&dev_attr_lun_queue_depth.attr,
 	NULL
 };
 
@@ -992,12 +1037,10 @@ sdev_store_queue_depth(struct device *dev, struct device_attribute *attr,
 	if (depth < 1 || depth > sdev->host->can_queue)
 		return -EINVAL;
 
-	retval = sht->change_queue_depth(sdev, depth);
+	retval = sdev_set_queue_depth(sdev, depth);
 	if (retval < 0)
 		return retval;
 
-	sdev->max_queue_depth = sdev->queue_depth;
-
 	return count;
 }
 sdev_show_function(queue_depth, "%d\n");
-- 
2.13.7

Re: [PATCH] scsi: add attribute to set lun queue depth on all luns on shost

[Bart Van Assche]

On 2020-01-21 15:53, James Smart wrote:
> +static int
> +sdev_set_queue_depth(struct scsi_device *sdev, int depth)
> +{
> +	struct scsi_host_template *sht = sdev->host->hostt;
> +	int retval;
> +
> +	retval = sht->change_queue_depth(sdev, depth);
> +	if (retval < 0)
> +		return retval;
> +
> +	sdev->max_queue_depth = sdev->queue_depth;
> +
> +	return 0;
> +}

'sht' is only used once, so I'm not sure whether it is worth it to keep
that local variable.

> +static ssize_t
> +store_host_lun_queue_depth(struct device *dev, struct device_attribute *attr,
> +		const char *buf, size_t count)
> +{
> +	struct Scsi_Host *shost = class_to_shost(dev);
> +	struct scsi_host_template *sht = shost->hostt;
> +	struct scsi_device *sdev;
> +	int depth, retval;
> +
> +	if (!sht->change_queue_depth)
> +		return -EINVAL;
> +
> +	depth = simple_strtoul(buf, NULL, 0);
> +	if (depth < 1 || depth > shost->can_queue)
> +		return -EINVAL;
> +
> +	shost_for_each_device(sdev, shost) {
> +		retval = sdev_set_queue_depth(sdev, depth);
> +		if (retval != 0)
> +			sdev_printk(KERN_INFO, sdev,
> +				"failed to set queue depth to %d (err %d)",
> +				depth, retval);
> +	}
> +
> +	return count;
> +}

Returning -EINVAL from all error paths makes it impossible to tell the
difference between a value that is out of range and queue depth changes
not being supported. Should another error code be returned if
sht->change_queue_depth == NULL?

> @@ -992,12 +1037,10 @@ sdev_store_queue_depth(struct device *dev, struct device_attribute *attr,
>  	if (depth < 1 || depth > sdev->host->can_queue)
>  		return -EINVAL;
>  
> -	retval = sht->change_queue_depth(sdev, depth);
> +	retval = sdev_set_queue_depth(sdev, depth);
>  	if (retval < 0)
>  		return retval;
>  
> -	sdev->max_queue_depth = sdev->queue_depth;
> -
>  	return count;
>  }

How about splitting this patch into two patches: one that introduces the
function sdev_set_queue_depth() and a second patch that introduces the
lun_queue_depth sysfs attribute?

Thanks,

Bart.

Re: [PATCH] scsi: add attribute to set lun queue depth on all luns on shost

[James Smart]

Thanks Bart - please see the v2 patch set just posted.

-- james