From: Ye Bin <[email protected]>
Diff v2 vs v1:
1. Remove set scsi host state by sysfs, as set host state by sysfs
may lead to functional issue. Scsi host state has it's own running
state machine.
2. Introduce 'blocked' sysfs api for set scsi host blocking IO. Use
this function we can do some test.
Ye Bin (3):
scsi: forbid to set scsi host state by sysfs
scsi: introduce 'blocked' sysfs api
scsi: blocking IO when host is set blocked
drivers/scsi/scsi_lib.c | 2 ++
drivers/scsi/scsi_sysfs.c | 58 ++++++++++++++++++++++-----------------
include/scsi/scsi_host.h | 8 ++++++
3 files changed, 43 insertions(+), 25 deletions(-)
--
2.31.1
From: Ye Bin <[email protected]>
Actually, set scsi host state by sysfs may lead to functional issues.
So forbid to set scsi host state.
Signed-off-by: Ye Bin <[email protected]>
---
drivers/scsi/scsi_sysfs.c | 26 +-------------------------
1 file changed, 1 insertion(+), 25 deletions(-)
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index ee28f73af4d4..903aa9de46e5 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -195,30 +195,6 @@ store_scan(struct device *dev, struct device_attribute *attr,
};
static DEVICE_ATTR(scan, S_IWUSR, NULL, store_scan);
-static ssize_t
-store_shost_state(struct device *dev, struct device_attribute *attr,
- const char *buf, size_t count)
-{
- int i;
- struct Scsi_Host *shost = class_to_shost(dev);
- enum scsi_host_state state = 0;
-
- for (i = 0; i < ARRAY_SIZE(shost_states); i++) {
- const int len = strlen(shost_states[i].name);
- if (strncmp(shost_states[i].name, buf, len) == 0 &&
- buf[len] == '\n') {
- state = shost_states[i].value;
- break;
- }
- }
- if (!state)
- return -EINVAL;
-
- if (scsi_host_set_state(shost, state))
- return -EINVAL;
- return count;
-}
-
static ssize_t
show_shost_state(struct device *dev, struct device_attribute *attr, char *buf)
{
@@ -233,7 +209,7 @@ show_shost_state(struct device *dev, struct device_attribute *attr, char *buf)
/* DEVICE_ATTR(state) clashes with dev_attr_state for sdev */
static struct device_attribute dev_attr_hstate =
- __ATTR(state, S_IRUGO | S_IWUSR, show_shost_state, store_shost_state);
+ __ATTR(state, S_IRUGO, show_shost_state, NULL);
static ssize_t
show_shost_mode(unsigned int mode, char *buf)
--
2.31.1
From: Ye Bin <[email protected]>
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 <[email protected]>
---
drivers/scsi/scsi_sysfs.c | 32 ++++++++++++++++++++++++++++++++
include/scsi/scsi_host.h | 3 +++
2 files changed, 35 insertions(+)
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
*/
--
2.31.1
From: Ye Bin <[email protected]>
As previous patch introduce 'blocked' sysfs api to set 'host_blockio'.
If 'host_blockio' is true will blocking IO.
Signed-off-by: Ye Bin <[email protected]>
---
drivers/scsi/scsi_lib.c | 2 ++
include/scsi/scsi_host.h | 5 +++++
2 files changed, 7 insertions(+)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index b7c569a42aa4..20d618300a46 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1724,6 +1724,8 @@ static blk_status_t scsi_queue_rq(struct blk_mq_hw_ctx *hctx,
}
ret = BLK_STS_RESOURCE;
+ if (unlikely(scsi_host_blocked(shost)))
+ goto out_put_budget;
if (!scsi_target_queue_ready(shost, sdev))
goto out_put_budget;
if (unlikely(scsi_host_in_recovery(shost))) {
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 3e916dbac1cb..9fc30d0c48de 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -747,6 +747,11 @@ static inline int scsi_host_in_recovery(struct Scsi_Host *shost)
shost->tmf_in_progress;
}
+static inline int scsi_host_blocked(struct Scsi_Host *shost)
+{
+ return shost->host_blockio;
+}
+
extern int scsi_queue_work(struct Scsi_Host *, struct work_struct *);
extern void scsi_flush_work(struct Scsi_Host *);
--
2.31.1
On 3/28/23 9:34 AM, Ye Bin wrote:
> From: Ye Bin <[email protected]>
>
> As previous patch introduce 'blocked' sysfs api to set 'host_blockio'.
> If 'host_blockio' is true will blocking IO.
>
> Signed-off-by: Ye Bin <[email protected]>
> ---
> drivers/scsi/scsi_lib.c | 2 ++
> include/scsi/scsi_host.h | 5 +++++
> 2 files changed, 7 insertions(+)
>
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index b7c569a42aa4..20d618300a46 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1724,6 +1724,8 @@ static blk_status_t scsi_queue_rq(struct blk_mq_hw_ctx *hctx,
> }
>
> ret = BLK_STS_RESOURCE;
> + if (unlikely(scsi_host_blocked(shost)))
> + goto out_put_budget;
You can just put this check in scsi_host_queue_ready with the
host_self_blocked check.
On 3/28/23 9:34 AM, Ye Bin wrote:
> From: Ye Bin <[email protected]>
>
> 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
> */