* [PATCH 0/5] limit set the host state by sysfs
@ 2023-03-25 1:17 Ye Bin
2023-03-25 1:17 ` [PATCH 1/5] scsi: fix switch host state race between by sysfs and others Ye Bin
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Ye Bin @ 2023-03-25 1:17 UTC (permalink / raw)
To: jejb, martin.petersen, linux-scsi, linux-kernel; +Cc: Ye Bin
From: Ye Bin <yebin10@huawei.com>
Now, we can set the host state by sysfs with any value. Actually, it
doesn't make sense. May cause some functional issues.
This patchset introduce 'blocked' state to blocking IO, we can use this
state for testing. 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.
Ye Bin (5):
scsi: fix switch host state race between by sysfs and others
scsi: introduce SHOST_BLOCKED state to support blocking IO
scsi: limit to set the host state
scsi: blocking IO when host is blocked
scsi: run queue after set host state from blocked to running
drivers/scsi/hosts.c | 11 +++++++++++
drivers/scsi/scsi_lib.c | 4 ++++
drivers/scsi/scsi_sysfs.c | 18 +++++++++++++++++-
include/scsi/scsi_host.h | 6 ++++++
4 files changed, 38 insertions(+), 1 deletion(-)
--
2.31.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/5] scsi: fix switch host state race between by sysfs and others
2023-03-25 1:17 [PATCH 0/5] limit set the host state by sysfs Ye Bin
@ 2023-03-25 1:17 ` Ye Bin
2023-03-27 21:26 ` Bart Van Assche
2023-03-25 1:17 ` [PATCH 2/5] scsi: introduce SHOST_BLOCKED state to support blocking IO Ye Bin
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Ye Bin @ 2023-03-25 1:17 UTC (permalink / raw)
To: jejb, martin.petersen, linux-scsi, linux-kernel; +Cc: Ye Bin
From: Ye Bin <yebin10@huawei.com>
Now, switch host state by sysfs isn't hold 'shost->host_lock' lock.
It may race with other process, lead to host mixed state.
Signed-off-by: Ye Bin <yebin10@huawei.com>
---
drivers/scsi/scsi_sysfs.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index ee28f73af4d4..cc0ae5e3def3 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -202,6 +202,7 @@ store_shost_state(struct device *dev, struct device_attribute *attr,
int i;
struct Scsi_Host *shost = class_to_shost(dev);
enum scsi_host_state state = 0;
+ unsigned long flags;
for (i = 0; i < ARRAY_SIZE(shost_states); i++) {
const int len = strlen(shost_states[i].name);
@@ -214,8 +215,13 @@ store_shost_state(struct device *dev, struct device_attribute *attr,
if (!state)
return -EINVAL;
- if (scsi_host_set_state(shost, state))
+ spin_lock_irqsave(shost->host_lock, flags);
+ if (scsi_host_set_state(shost, state)) {
+ spin_unlock_irqrestore(shost->host_lock, flags);
return -EINVAL;
+ }
+ spin_unlock_irqrestore(shost->host_lock, flags);
+
return count;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/5] scsi: introduce SHOST_BLOCKED state to support blocking IO
2023-03-25 1:17 [PATCH 0/5] limit set the host state by sysfs Ye Bin
2023-03-25 1:17 ` [PATCH 1/5] scsi: fix switch host state race between by sysfs and others Ye Bin
@ 2023-03-25 1:17 ` Ye Bin
2023-03-27 21:34 ` Bart Van Assche
2023-03-25 1:17 ` [PATCH 3/5] scsi: limit to set the host state Ye Bin
` (2 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Ye Bin @ 2023-03-25 1:17 UTC (permalink / raw)
To: jejb, martin.petersen, linux-scsi, linux-kernel; +Cc: Ye Bin
From: Ye Bin <yebin10@huawei.com>
SHOST_BLOCKED state to blocking io in block layer. This state use for
test, Only running state and blocked state can be switched to each
other.
Signed-off-by: Ye Bin <yebin10@huawei.com>
---
drivers/scsi/hosts.c | 11 +++++++++++
drivers/scsi/scsi_sysfs.c | 1 +
include/scsi/scsi_host.h | 1 +
3 files changed, 13 insertions(+)
diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 9b6fbbe15d92..3b497fd4d329 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -90,6 +90,7 @@ int scsi_host_set_state(struct Scsi_Host *shost, enum scsi_host_state state)
switch (oldstate) {
case SHOST_CREATED:
case SHOST_RECOVERY:
+ case SHOST_BLOCKED:
break;
default:
goto illegal;
@@ -99,6 +100,7 @@ int scsi_host_set_state(struct Scsi_Host *shost, enum scsi_host_state state)
case SHOST_RECOVERY:
switch (oldstate) {
case SHOST_RUNNING:
+ case SHOST_BLOCKED:
break;
default:
goto illegal;
@@ -109,6 +111,7 @@ int scsi_host_set_state(struct Scsi_Host *shost, enum scsi_host_state state)
switch (oldstate) {
case SHOST_CREATED:
case SHOST_RUNNING:
+ case SHOST_BLOCKED:
case SHOST_CANCEL_RECOVERY:
break;
default:
@@ -144,6 +147,14 @@ int scsi_host_set_state(struct Scsi_Host *shost, enum scsi_host_state state)
goto illegal;
}
break;
+ case SHOST_BLOCKED:
+ switch (oldstate) {
+ case SHOST_RUNNING:
+ break;
+ default:
+ goto illegal;
+ }
+ break;
}
shost->shost_state = state;
return 0;
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index cc0ae5e3def3..b14f95ac594e 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -69,6 +69,7 @@ static const struct {
{ SHOST_RECOVERY, "recovery" },
{ SHOST_CANCEL_RECOVERY, "cancel/recovery" },
{ SHOST_DEL_RECOVERY, "deleted/recovery", },
+ { SHOST_BLOCKED, "blocked", },
};
const char *scsi_host_state_name(enum scsi_host_state state)
{
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 587cc767bb67..9e99317b11fa 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -527,6 +527,7 @@ enum scsi_host_state {
SHOST_RECOVERY,
SHOST_CANCEL_RECOVERY,
SHOST_DEL_RECOVERY,
+ SHOST_BLOCKED,
};
struct Scsi_Host {
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/5] scsi: limit to set the host state
2023-03-25 1:17 [PATCH 0/5] limit set the host state by sysfs Ye Bin
2023-03-25 1:17 ` [PATCH 1/5] scsi: fix switch host state race between by sysfs and others Ye Bin
2023-03-25 1:17 ` [PATCH 2/5] scsi: introduce SHOST_BLOCKED state to support blocking IO Ye Bin
@ 2023-03-25 1:17 ` Ye Bin
2023-03-27 21:25 ` Bart Van Assche
2023-03-25 1:17 ` [PATCH 4/5] scsi: blocking IO when host is blocked Ye Bin
2023-03-25 1:17 ` [PATCH 5/5] scsi: run queue after set host state from blocked to running Ye Bin
4 siblings, 1 reply; 10+ messages in thread
From: Ye Bin @ 2023-03-25 1:17 UTC (permalink / raw)
To: jejb, martin.petersen, linux-scsi, linux-kernel; +Cc: Ye Bin
From: Ye Bin <yebin10@huawei.com>
Now, we can set the host state with any value. Actually, it doesn't
make sense. As previous patch introduce SHOST_BLOCKED state, set this
state, it will blocking IO. So this patch limit to set the host with
running/blocked state.
Signed-off-by: Ye Bin <yebin10@huawei.com>
---
drivers/scsi/scsi_sysfs.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index b14f95ac594e..42c5936c7711 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -203,6 +203,7 @@ store_shost_state(struct device *dev, struct device_attribute *attr,
int i;
struct Scsi_Host *shost = class_to_shost(dev);
enum scsi_host_state state = 0;
+ enum scsi_host_state old_state;
unsigned long flags;
for (i = 0; i < ARRAY_SIZE(shost_states); i++) {
@@ -216,8 +217,13 @@ store_shost_state(struct device *dev, struct device_attribute *attr,
if (!state)
return -EINVAL;
+ if (state != SHOST_RUNNING && state != SHOST_BLOCKED)
+ return -EINVAL;
+
spin_lock_irqsave(shost->host_lock, flags);
- if (scsi_host_set_state(shost, state)) {
+ old_state = shost->shost_state;
+ if ((old_state != SHOST_RUNNING && old_state != SHOST_BLOCKED) ||
+ scsi_host_set_state(shost, state)) {
spin_unlock_irqrestore(shost->host_lock, flags);
return -EINVAL;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/5] scsi: blocking IO when host is blocked
2023-03-25 1:17 [PATCH 0/5] limit set the host state by sysfs Ye Bin
` (2 preceding siblings ...)
2023-03-25 1:17 ` [PATCH 3/5] scsi: limit to set the host state Ye Bin
@ 2023-03-25 1:17 ` Ye Bin
2023-03-27 21:37 ` Bart Van Assche
2023-03-25 1:17 ` [PATCH 5/5] scsi: run queue after set host state from blocked to running Ye Bin
4 siblings, 1 reply; 10+ messages in thread
From: Ye Bin @ 2023-03-25 1:17 UTC (permalink / raw)
To: jejb, martin.petersen, linux-scsi, linux-kernel; +Cc: Ye Bin
From: Ye Bin <yebin10@huawei.com>
Unlike recovery state may block other process, blocking IO only
when host is blocked.
Signed-off-by: Ye Bin <yebin10@huawei.com>
---
drivers/scsi/scsi_lib.c | 4 ++++
include/scsi/scsi_host.h | 5 +++++
2 files changed, 9 insertions(+)
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index b7c569a42aa4..492487717f41 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1731,6 +1731,10 @@ static blk_status_t scsi_queue_rq(struct blk_mq_hw_ctx *hctx,
ret = BLK_STS_OFFLINE;
goto out_dec_target_busy;
}
+
+ if (unlikely(scsi_host_blocked(shost)))
+ goto out_dec_target_busy;
+
if (!scsi_host_queue_ready(q, shost, sdev, cmd))
goto out_dec_target_busy;
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 9e99317b11fa..571321bbb706 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -745,6 +745,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->shost_state == SHOST_BLOCKED;
+}
+
extern int scsi_queue_work(struct Scsi_Host *, struct work_struct *);
extern void scsi_flush_work(struct Scsi_Host *);
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 5/5] scsi: run queue after set host state from blocked to running
2023-03-25 1:17 [PATCH 0/5] limit set the host state by sysfs Ye Bin
` (3 preceding siblings ...)
2023-03-25 1:17 ` [PATCH 4/5] scsi: blocking IO when host is blocked Ye Bin
@ 2023-03-25 1:17 ` Ye Bin
4 siblings, 0 replies; 10+ messages in thread
From: Ye Bin @ 2023-03-25 1:17 UTC (permalink / raw)
To: jejb, martin.petersen, linux-scsi, linux-kernel; +Cc: Ye Bin
From: Ye Bin <yebin10@huawei.com>
As when host is blocked, all request is blocked. After set the host
state with running, e will need to ensure that these requests are
started.
Signed-off-by: Ye Bin <yebin10@huawei.com>
---
drivers/scsi/scsi_sysfs.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 42c5936c7711..202d58f4f267 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -229,6 +229,9 @@ store_shost_state(struct device *dev, struct device_attribute *attr,
}
spin_unlock_irqrestore(shost->host_lock, flags);
+ if (old_state == SHOST_BLOCKED && state == SHOST_RUNNING)
+ scsi_run_host_queues(shost);
+
return count;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/5] scsi: limit to set the host state
2023-03-25 1:17 ` [PATCH 3/5] scsi: limit to set the host state Ye Bin
@ 2023-03-27 21:25 ` Bart Van Assche
0 siblings, 0 replies; 10+ messages in thread
From: Bart Van Assche @ 2023-03-27 21:25 UTC (permalink / raw)
To: Ye Bin, jejb, martin.petersen, linux-scsi, linux-kernel; +Cc: Ye Bin
On 3/24/23 18:17, Ye Bin wrote:
> From: Ye Bin <yebin10@huawei.com>
>
> Now, we can set the host state with any value. Actually, it doesn't
> make sense. As previous patch introduce SHOST_BLOCKED state, set this
> state, it will blocking IO. So this patch limit to set the host with
> running/blocked state.
>
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
> drivers/scsi/scsi_sysfs.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> index b14f95ac594e..42c5936c7711 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -203,6 +203,7 @@ store_shost_state(struct device *dev, struct device_attribute *attr,
> int i;
> struct Scsi_Host *shost = class_to_shost(dev);
> enum scsi_host_state state = 0;
> + enum scsi_host_state old_state;
> unsigned long flags;
>
> for (i = 0; i < ARRAY_SIZE(shost_states); i++) {
> @@ -216,8 +217,13 @@ store_shost_state(struct device *dev, struct device_attribute *attr,
> if (!state)
> return -EINVAL;
>
> + if (state != SHOST_RUNNING && state != SHOST_BLOCKED)
> + return -EINVAL;
> +
> spin_lock_irqsave(shost->host_lock, flags);
> - if (scsi_host_set_state(shost, state)) {
> + old_state = shost->shost_state;
> + if ((old_state != SHOST_RUNNING && old_state != SHOST_BLOCKED) ||
> + scsi_host_set_state(shost, state)) {
> spin_unlock_irqrestore(shost->host_lock, flags);
> return -EINVAL;
> }
Please make sure that the "state != SHOST_RUNNING && state !=
SHOST_BLOCKED" check occurs only once and also that there is one
spin_lock_irqsave() call in this function and only one
spin_unlock_irqrestore() call.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/5] scsi: fix switch host state race between by sysfs and others
2023-03-25 1:17 ` [PATCH 1/5] scsi: fix switch host state race between by sysfs and others Ye Bin
@ 2023-03-27 21:26 ` Bart Van Assche
0 siblings, 0 replies; 10+ messages in thread
From: Bart Van Assche @ 2023-03-27 21:26 UTC (permalink / raw)
To: Ye Bin, jejb, martin.petersen, linux-scsi, linux-kernel; +Cc: Ye Bin
On 3/24/23 18:17, Ye Bin wrote:
> From: Ye Bin <yebin10@huawei.com>
>
> Now, switch host state by sysfs isn't hold 'shost->host_lock' lock.
> It may race with other process, lead to host mixed state.
>
> Signed-off-by: Ye Bin <yebin10@huawei.com>
> ---
> drivers/scsi/scsi_sysfs.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> index ee28f73af4d4..cc0ae5e3def3 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -202,6 +202,7 @@ store_shost_state(struct device *dev, struct device_attribute *attr,
> int i;
> struct Scsi_Host *shost = class_to_shost(dev);
> enum scsi_host_state state = 0;
> + unsigned long flags;
>
> for (i = 0; i < ARRAY_SIZE(shost_states); i++) {
> const int len = strlen(shost_states[i].name);
> @@ -214,8 +215,13 @@ store_shost_state(struct device *dev, struct device_attribute *attr,
> if (!state)
> return -EINVAL;
>
> - if (scsi_host_set_state(shost, state))
> + spin_lock_irqsave(shost->host_lock, flags);
> + if (scsi_host_set_state(shost, state)) {
> + spin_unlock_irqrestore(shost->host_lock, flags);
> return -EINVAL;
> + }
> + spin_unlock_irqrestore(shost->host_lock, flags);
> +
> return count;
> }
Please make sure that there is only one spin_unlock_irqrestore() call in
this function.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/5] scsi: introduce SHOST_BLOCKED state to support blocking IO
2023-03-25 1:17 ` [PATCH 2/5] scsi: introduce SHOST_BLOCKED state to support blocking IO Ye Bin
@ 2023-03-27 21:34 ` Bart Van Assche
0 siblings, 0 replies; 10+ messages in thread
From: Bart Van Assche @ 2023-03-27 21:34 UTC (permalink / raw)
To: Ye Bin, jejb, martin.petersen, linux-scsi, linux-kernel; +Cc: Ye Bin
On 3/24/23 18:17, Ye Bin wrote:
> diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
> index 9b6fbbe15d92..3b497fd4d329 100644
> --- a/drivers/scsi/hosts.c
> +++ b/drivers/scsi/hosts.c
> @@ -90,6 +90,7 @@ int scsi_host_set_state(struct Scsi_Host *shost, enum scsi_host_state state)
> switch (oldstate) {
> case SHOST_CREATED:
> case SHOST_RECOVERY:
> + case SHOST_BLOCKED:
> break;
> default:
> goto illegal;
> @@ -99,6 +100,7 @@ int scsi_host_set_state(struct Scsi_Host *shost, enum scsi_host_state state)
> case SHOST_RECOVERY:
> switch (oldstate) {
> case SHOST_RUNNING:
> + case SHOST_BLOCKED:
> break;
> default:
> goto illegal;
> @@ -109,6 +111,7 @@ int scsi_host_set_state(struct Scsi_Host *shost, enum scsi_host_state state)
> switch (oldstate) {
> case SHOST_CREATED:
> case SHOST_RUNNING:
> + case SHOST_BLOCKED:
> case SHOST_CANCEL_RECOVERY:
> break;
> default:
> @@ -144,6 +147,14 @@ int scsi_host_set_state(struct Scsi_Host *shost, enum scsi_host_state state)
> goto illegal;
> }
> break;
> + case SHOST_BLOCKED:
> + switch (oldstate) {
> + case SHOST_RUNNING:
> + break;
> + default:
> + goto illegal;
> + }
> + break;
> }
If a host is blocked, error recovery happens and completes, the host
will be unblocked. I don't think that is acceptable.
The "blocked" property is orthogonal to the host state so a new boolean
member variable should be introduced in struct Scsi_Host instead of
introducing a new SCSI host state.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 4/5] scsi: blocking IO when host is blocked
2023-03-25 1:17 ` [PATCH 4/5] scsi: blocking IO when host is blocked Ye Bin
@ 2023-03-27 21:37 ` Bart Van Assche
0 siblings, 0 replies; 10+ messages in thread
From: Bart Van Assche @ 2023-03-27 21:37 UTC (permalink / raw)
To: Ye Bin, jejb, martin.petersen, linux-scsi, linux-kernel; +Cc: Ye Bin
On 3/24/23 18:17, Ye Bin wrote:
> + if (unlikely(scsi_host_blocked(shost)))
> + goto out_dec_target_busy;
I this check would be moved earlier then the atomic_inc() and
atomic_dec() of scsi_target(sdev)->target_busy could be skipped.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-03-27 21:37 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-25 1:17 [PATCH 0/5] limit set the host state by sysfs Ye Bin
2023-03-25 1:17 ` [PATCH 1/5] scsi: fix switch host state race between by sysfs and others Ye Bin
2023-03-27 21:26 ` Bart Van Assche
2023-03-25 1:17 ` [PATCH 2/5] scsi: introduce SHOST_BLOCKED state to support blocking IO Ye Bin
2023-03-27 21:34 ` Bart Van Assche
2023-03-25 1:17 ` [PATCH 3/5] scsi: limit to set the host state Ye Bin
2023-03-27 21:25 ` Bart Van Assche
2023-03-25 1:17 ` [PATCH 4/5] scsi: blocking IO when host is blocked Ye Bin
2023-03-27 21:37 ` Bart Van Assche
2023-03-25 1:17 ` [PATCH 5/5] scsi: run queue after set host state from blocked to running Ye Bin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.