diff mbox series

[v2,2/3] scsi: introduce 'blocked' sysfs api

Message ID 20230328143442.2684167-3-yebin@huaweicloud.com (mailing list archive)
State Changes Requested
Headers show
Series limit set the host state by sysfs | expand

Commit Message

Ye Bin March 28, 2023, 2:34 p.m. UTC
From: Ye Bin <yebin10@huawei.com>

Introduce 'blocked' sysfs api to control scsi host blocking IO.
Use this founction for test. Perhaps we can use this to do some fault
recovery or firmware upgrades, as long as the driver support is good,
it may be insensitive to the upper layer.

Signed-off-by: Ye Bin <yebin10@huawei.com>
---
 drivers/scsi/scsi_sysfs.c | 32 ++++++++++++++++++++++++++++++++
 include/scsi/scsi_host.h  |  3 +++
 2 files changed, 35 insertions(+)

Comments

Mike Christie March 28, 2023, 4:06 p.m. UTC | #1
On 3/28/23 9:34 AM, Ye Bin wrote:
> From: Ye Bin <yebin10@huawei.com>
> 
> Introduce 'blocked' sysfs api to control scsi host blocking IO.
> Use this founction for test. Perhaps we can use this to do some fault
> recovery or firmware upgrades, as long as the driver support is good,

If it's for testing only then do people like a debugfs type of interface
is better?

If it's for actual production use like firmware upgrades and they can't
handle IO while they are doing the upgrade, then I think you need to do
more than just set a bit to prevent new IO. You also need to handle cmds
that have passed the scsi_queue_rq ready checks and have not been processed
by the driver's queuecommand. Also there are some issues like cmds can
still timeout and so you will get scsi_host_template->eh* calls still.


>  
> diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
> index 587cc767bb67..3e916dbac1cb 100644
> --- a/include/scsi/scsi_host.h
> +++ b/include/scsi/scsi_host.h
> @@ -659,6 +659,9 @@ struct Scsi_Host {
>  	/* The transport requires the LUN bits NOT to be stored in CDB[1] */
>  	unsigned no_scsi2_lun_in_cdb:1;
>  
> +	/* True host will blocking IO */
> +	unsigned host_blockio:1;
> +

Maybe rename to host_user_blocked to match the host_self_blocked naming.

I would make the comment similar to host_self_blocked's comment but
instead of the host requesting it userspace did.

>  	/*
>  	 * Optional work queue to be utilized by the transport
>  	 */
diff mbox series

Patch

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 903aa9de46e5..cad1981ab528 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -345,6 +345,37 @@  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 ssize_t
+store_shost_blocked(struct device *dev, struct device_attribute *attr,
+		    const char *buf, size_t count)
+{
+	int err;
+	bool blocked;
+	struct Scsi_Host *shost = class_to_shost(dev);
+
+	err = kstrtobool(buf, &blocked);
+	if (err)
+		return err;
+
+	if (shost->host_blockio != blocked) {
+		shost->host_blockio = blocked;
+		if (!blocked)
+			scsi_run_host_queues(shost);
+	}
+
+	return count;
+}
+
+static ssize_t
+show_shost_blocked(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct Scsi_Host *shost = class_to_shost(dev);
+
+	return snprintf(buf, 20, "%d\n", shost->host_blockio);
+}
+static DEVICE_ATTR(blocked, S_IRUGO | S_IWUSR,
+		   show_shost_blocked, store_shost_blocked);
+
 shost_rd_attr(unique_id, "%u\n");
 shost_rd_attr(cmd_per_lun, "%hd\n");
 shost_rd_attr(can_queue, "%d\n");
@@ -397,6 +428,7 @@  static struct attribute *scsi_sysfs_shost_attrs[] = {
 	&dev_attr_host_reset.attr,
 	&dev_attr_eh_deadline.attr,
 	&dev_attr_nr_hw_queues.attr,
+	&dev_attr_blocked.attr,
 	NULL
 };
 
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 587cc767bb67..3e916dbac1cb 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -659,6 +659,9 @@  struct Scsi_Host {
 	/* The transport requires the LUN bits NOT to be stored in CDB[1] */
 	unsigned no_scsi2_lun_in_cdb:1;
 
+	/* True host will blocking IO */
+	unsigned host_blockio:1;
+
 	/*
 	 * Optional work queue to be utilized by the transport
 	 */