Hi sagi,

On 2021/11/5 9:34, liruozhu wrote:

On 2021/11/4 20:26, Sagi Grimberg wrote:

quoted

quoted

A crash happens when I try to disconnect a reconnecting ctrl:

1) The network was cut off when the connection was just established,
scan work hang there waiting for some IOs complete.Those IOs were
retrying because we return BLK_STS_RESOURCE to blk in reconnecting.

2) After a while, I tried to disconnect this connection.This procedure
also hung because it tried to obtain ctrl->scan_lock.It should be noted
that now we have switched the controller state to NVME_CTRL_DELETING.

3) In nvme_check_ready(), we always return true when ctrl->state is
NVME_CTRL_DELETING, so those retrying IOs were issued to the bottom
device which was already freed.

To fix this, when ctrl->state is NVME_CTRL_DELETING, issue cmd to 
bottom
device only when queue state is live.If not, return host path error 
to blk.

Signed-off-by: Ruozhu Li <redacted>
---
  drivers/nvme/host/core.c | 1 +
  drivers/nvme/host/nvme.h | 2 +-
  2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 838b5e2058be..752203ad7639 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c

@@ -666,6 +666,7 @@ blk_status_t nvme_fail_nonready_command(struct 

nvme_ctrl *ctrl,
          struct request *rq)
  {
      if (ctrl->state != NVME_CTRL_DELETING_NOIO &&
+        ctrl->state != NVME_CTRL_DELETING &&

Please explain why you need this change? As suggested by the name
only DELETING_NOIO does not accept I/O, and if we return
BLK_STS_RESOURCE we can get into an endless loop of resubmission.

I just added the handling of the DELETING state here, did not modify 
the DELETING_NOIO case.

Thanks,
Ruozhu

I'm not sure if I explained it clearly, my English is not very good.

If you think there is still a problem with this patch, please tell me.

Thanks,
Ruozhu

quoted

quoted

          ctrl->state != NVME_CTRL_DEAD &&
          !test_bit(NVME_CTRL_FAILFAST_EXPIRED, &ctrl->flags) &&
          !blk_noretry_request(rq) && !(rq->cmd_flags & 
REQ_NVME_MPATH))

diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index b334af8aa264..9b095ee01364 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h

@@ -709,7 +709,7 @@ static inline bool nvme_check_ready(struct 

nvme_ctrl *ctrl, struct request *rq,
          return true;
      if (ctrl->ops->flags & NVME_F_FABRICS &&
          ctrl->state == NVME_CTRL_DELETING)
-        return true;
+        return queue_live;

I agree with this change. I thought I've already seen this change from
James in the past.
.