Commit ecca390e authored by Sagi Grimberg's avatar Sagi Grimberg Committed by Christoph Hellwig

nvme: fix deadlock in disconnect during scan_work and/or ana_work

A deadlock happens in the following scenario with multipath:
1) scan_work(nvme0) detects a new nsid while nvme0
    is an optimized path to it, path nvme1 happens to be
    inaccessible.

2) Before scan_work is complete nvme0 disconnect is initiated
    nvme_delete_ctrl_sync() sets nvme0 state to NVME_CTRL_DELETING

3) scan_work(1) attempts to submit IO,
    but nvme_path_is_optimized() observes nvme0 is not LIVE.
    Since nvme1 is a possible path IO is requeued and scan_work hangs.

--
Workqueue: nvme-wq nvme_scan_work [nvme_core]
kernel: Call Trace:
kernel:  __schedule+0x2b9/0x6c0
kernel:  schedule+0x42/0xb0
kernel:  io_schedule+0x16/0x40
kernel:  do_read_cache_page+0x438/0x830
kernel:  read_cache_page+0x12/0x20
kernel:  read_dev_sector+0x27/0xc0
kernel:  read_lba+0xc1/0x220
kernel:  efi_partition+0x1e6/0x708
kernel:  check_partition+0x154/0x244
kernel:  rescan_partitions+0xae/0x280
kernel:  __blkdev_get+0x40f/0x560
kernel:  blkdev_get+0x3d/0x140
kernel:  __device_add_disk+0x388/0x480
kernel:  device_add_disk+0x13/0x20
kernel:  nvme_mpath_set_live+0x119/0x140 [nvme_core]
kernel:  nvme_update_ns_ana_state+0x5c/0x60 [nvme_core]
kernel:  nvme_set_ns_ana_state+0x1e/0x30 [nvme_core]
kernel:  nvme_parse_ana_log+0xa1/0x180 [nvme_core]
kernel:  nvme_mpath_add_disk+0x47/0x90 [nvme_core]
kernel:  nvme_validate_ns+0x396/0x940 [nvme_core]
kernel:  nvme_scan_work+0x24f/0x380 [nvme_core]
kernel:  process_one_work+0x1db/0x380
kernel:  worker_thread+0x249/0x400
kernel:  kthread+0x104/0x140
--

4) Delete also hangs in flush_work(ctrl->scan_work)
    from nvme_remove_namespaces().

Similiarly a deadlock with ana_work may happen: if ana_work has started
and calls nvme_mpath_set_live and device_add_disk, it will
trigger I/O. When we trigger disconnect I/O will block because
our accessible (optimized) path is disconnecting, but the alternate
path is inaccessible, so I/O blocks. Then disconnect tries to flush
the ana_work and hangs.

[  605.550896] Workqueue: nvme-wq nvme_ana_work [nvme_core]
[  605.552087] Call Trace:
[  605.552683]  __schedule+0x2b9/0x6c0
[  605.553507]  schedule+0x42/0xb0
[  605.554201]  io_schedule+0x16/0x40
[  605.555012]  do_read_cache_page+0x438/0x830
[  605.556925]  read_cache_page+0x12/0x20
[  605.557757]  read_dev_sector+0x27/0xc0
[  605.558587]  amiga_partition+0x4d/0x4c5
[  605.561278]  check_partition+0x154/0x244
[  605.562138]  rescan_partitions+0xae/0x280
[  605.563076]  __blkdev_get+0x40f/0x560
[  605.563830]  blkdev_get+0x3d/0x140
[  605.564500]  __device_add_disk+0x388/0x480
[  605.565316]  device_add_disk+0x13/0x20
[  605.566070]  nvme_mpath_set_live+0x5e/0x130 [nvme_core]
[  605.567114]  nvme_update_ns_ana_state+0x2c/0x30 [nvme_core]
[  605.568197]  nvme_update_ana_state+0xca/0xe0 [nvme_core]
[  605.569360]  nvme_parse_ana_log+0xa1/0x180 [nvme_core]
[  605.571385]  nvme_read_ana_log+0x76/0x100 [nvme_core]
[  605.572376]  nvme_ana_work+0x15/0x20 [nvme_core]
[  605.573330]  process_one_work+0x1db/0x380
[  605.574144]  worker_thread+0x4d/0x400
[  605.574896]  kthread+0x104/0x140
[  605.577205]  ret_from_fork+0x35/0x40
[  605.577955] INFO: task nvme:14044 blocked for more than 120 seconds.
[  605.579239]       Tainted: G           OE     5.3.5-050305-generic #201910071830
[  605.580712] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[  605.582320] nvme            D    0 14044  14043 0x00000000
[  605.583424] Call Trace:
[  605.583935]  __schedule+0x2b9/0x6c0
[  605.584625]  schedule+0x42/0xb0
[  605.585290]  schedule_timeout+0x203/0x2f0
[  605.588493]  wait_for_completion+0xb1/0x120
[  605.590066]  __flush_work+0x123/0x1d0
[  605.591758]  __cancel_work_timer+0x10e/0x190
[  605.593542]  cancel_work_sync+0x10/0x20
[  605.594347]  nvme_mpath_stop+0x2f/0x40 [nvme_core]
[  605.595328]  nvme_stop_ctrl+0x12/0x50 [nvme_core]
[  605.596262]  nvme_do_delete_ctrl+0x3f/0x90 [nvme_core]
[  605.597333]  nvme_sysfs_delete+0x5c/0x70 [nvme_core]
[  605.598320]  dev_attr_store+0x17/0x30

Fix this by introducing a new state: NVME_CTRL_DELETE_NOIO, which will
indicate the phase of controller deletion where I/O cannot be allowed
to access the namespace. NVME_CTRL_DELETING still allows mpath I/O to
be issued to the bottom device, and only after we flush the ana_work
and scan_work (after nvme_stop_ctrl and nvme_prep_remove_namespaces)
we change the state to NVME_CTRL_DELETING_NOIO. Also we prevent ana_work
from re-firing by aborting early if we are not LIVE, so we should be safe
here.

In addition, change the transport drivers to follow the updated state
machine.

Fixes: 0d0b660f ("nvme: add ANA support")
Reported-by: default avatarAnton Eidelman <anton@lightbitslabs.com>
Signed-off-by: default avatarSagi Grimberg <sagi@grimberg.me>
Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
parent 4212f4e9
...@@ -366,6 +366,16 @@ bool nvme_change_ctrl_state(struct nvme_ctrl *ctrl, ...@@ -366,6 +366,16 @@ bool nvme_change_ctrl_state(struct nvme_ctrl *ctrl,
break; break;
} }
break; break;
case NVME_CTRL_DELETING_NOIO:
switch (old_state) {
case NVME_CTRL_DELETING:
case NVME_CTRL_DEAD:
changed = true;
/* FALLTHRU */
default:
break;
}
break;
case NVME_CTRL_DEAD: case NVME_CTRL_DEAD:
switch (old_state) { switch (old_state) {
case NVME_CTRL_DELETING: case NVME_CTRL_DELETING:
...@@ -403,6 +413,7 @@ static bool nvme_state_terminal(struct nvme_ctrl *ctrl) ...@@ -403,6 +413,7 @@ static bool nvme_state_terminal(struct nvme_ctrl *ctrl)
case NVME_CTRL_CONNECTING: case NVME_CTRL_CONNECTING:
return false; return false;
case NVME_CTRL_DELETING: case NVME_CTRL_DELETING:
case NVME_CTRL_DELETING_NOIO:
case NVME_CTRL_DEAD: case NVME_CTRL_DEAD:
return true; return true;
default: default:
...@@ -3476,6 +3487,7 @@ static ssize_t nvme_sysfs_show_state(struct device *dev, ...@@ -3476,6 +3487,7 @@ static ssize_t nvme_sysfs_show_state(struct device *dev,
[NVME_CTRL_RESETTING] = "resetting", [NVME_CTRL_RESETTING] = "resetting",
[NVME_CTRL_CONNECTING] = "connecting", [NVME_CTRL_CONNECTING] = "connecting",
[NVME_CTRL_DELETING] = "deleting", [NVME_CTRL_DELETING] = "deleting",
[NVME_CTRL_DELETING_NOIO]= "deleting (no IO)",
[NVME_CTRL_DEAD] = "dead", [NVME_CTRL_DEAD] = "dead",
}; };
...@@ -4112,6 +4124,9 @@ void nvme_remove_namespaces(struct nvme_ctrl *ctrl) ...@@ -4112,6 +4124,9 @@ void nvme_remove_namespaces(struct nvme_ctrl *ctrl)
if (ctrl->state == NVME_CTRL_DEAD) if (ctrl->state == NVME_CTRL_DEAD)
nvme_kill_queues(ctrl); nvme_kill_queues(ctrl);
/* this is a no-op when called from the controller reset handler */
nvme_change_ctrl_state(ctrl, NVME_CTRL_DELETING_NOIO);
down_write(&ctrl->namespaces_rwsem); down_write(&ctrl->namespaces_rwsem);
list_splice_init(&ctrl->namespaces, &ns_list); list_splice_init(&ctrl->namespaces, &ns_list);
up_write(&ctrl->namespaces_rwsem); up_write(&ctrl->namespaces_rwsem);
......
...@@ -547,7 +547,7 @@ static struct nvmf_transport_ops *nvmf_lookup_transport( ...@@ -547,7 +547,7 @@ static struct nvmf_transport_ops *nvmf_lookup_transport(
blk_status_t nvmf_fail_nonready_command(struct nvme_ctrl *ctrl, blk_status_t nvmf_fail_nonready_command(struct nvme_ctrl *ctrl,
struct request *rq) struct request *rq)
{ {
if (ctrl->state != NVME_CTRL_DELETING && if (ctrl->state != NVME_CTRL_DELETING_NOIO &&
ctrl->state != NVME_CTRL_DEAD && ctrl->state != NVME_CTRL_DEAD &&
!blk_noretry_request(rq) && !(rq->cmd_flags & REQ_NVME_MPATH)) !blk_noretry_request(rq) && !(rq->cmd_flags & REQ_NVME_MPATH))
return BLK_STS_RESOURCE; return BLK_STS_RESOURCE;
......
...@@ -182,7 +182,8 @@ bool nvmf_ip_options_match(struct nvme_ctrl *ctrl, ...@@ -182,7 +182,8 @@ bool nvmf_ip_options_match(struct nvme_ctrl *ctrl,
static inline bool nvmf_check_ready(struct nvme_ctrl *ctrl, struct request *rq, static inline bool nvmf_check_ready(struct nvme_ctrl *ctrl, struct request *rq,
bool queue_live) bool queue_live)
{ {
if (likely(ctrl->state == NVME_CTRL_LIVE)) if (likely(ctrl->state == NVME_CTRL_LIVE ||
ctrl->state == NVME_CTRL_DELETING))
return true; return true;
return __nvmf_check_ready(ctrl, rq, queue_live); return __nvmf_check_ready(ctrl, rq, queue_live);
} }
......
...@@ -826,6 +826,7 @@ nvme_fc_ctrl_connectivity_loss(struct nvme_fc_ctrl *ctrl) ...@@ -826,6 +826,7 @@ nvme_fc_ctrl_connectivity_loss(struct nvme_fc_ctrl *ctrl)
break; break;
case NVME_CTRL_DELETING: case NVME_CTRL_DELETING:
case NVME_CTRL_DELETING_NOIO:
default: default:
/* no action to take - let it delete */ /* no action to take - let it delete */
break; break;
......
...@@ -167,9 +167,18 @@ void nvme_mpath_clear_ctrl_paths(struct nvme_ctrl *ctrl) ...@@ -167,9 +167,18 @@ void nvme_mpath_clear_ctrl_paths(struct nvme_ctrl *ctrl)
static bool nvme_path_is_disabled(struct nvme_ns *ns) static bool nvme_path_is_disabled(struct nvme_ns *ns)
{ {
return ns->ctrl->state != NVME_CTRL_LIVE || /*
test_bit(NVME_NS_ANA_PENDING, &ns->flags) || * We don't treat NVME_CTRL_DELETING as a disabled path as I/O should
test_bit(NVME_NS_REMOVING, &ns->flags); * still be able to complete assuming that the controller is connected.
* Otherwise it will fail immediately and return to the requeue list.
*/
if (ns->ctrl->state != NVME_CTRL_LIVE &&
ns->ctrl->state != NVME_CTRL_DELETING)
return true;
if (test_bit(NVME_NS_ANA_PENDING, &ns->flags) ||
test_bit(NVME_NS_REMOVING, &ns->flags))
return true;
return false;
} }
static struct nvme_ns *__nvme_find_path(struct nvme_ns_head *head, int node) static struct nvme_ns *__nvme_find_path(struct nvme_ns_head *head, int node)
...@@ -563,6 +572,9 @@ static void nvme_ana_work(struct work_struct *work) ...@@ -563,6 +572,9 @@ static void nvme_ana_work(struct work_struct *work)
{ {
struct nvme_ctrl *ctrl = container_of(work, struct nvme_ctrl, ana_work); struct nvme_ctrl *ctrl = container_of(work, struct nvme_ctrl, ana_work);
if (ctrl->state != NVME_CTRL_LIVE)
return;
nvme_read_ana_log(ctrl); nvme_read_ana_log(ctrl);
} }
......
...@@ -190,6 +190,11 @@ static inline u16 nvme_req_qid(struct request *req) ...@@ -190,6 +190,11 @@ static inline u16 nvme_req_qid(struct request *req)
* @NVME_CTRL_CONNECTING: Controller is disconnected, now connecting the * @NVME_CTRL_CONNECTING: Controller is disconnected, now connecting the
* transport * transport
* @NVME_CTRL_DELETING: Controller is deleting (or scheduled deletion) * @NVME_CTRL_DELETING: Controller is deleting (or scheduled deletion)
* @NVME_CTRL_DELETING_NOIO: Controller is deleting and I/O is not
* disabled/failed immediately. This state comes
* after all async event processing took place and
* before ns removal and the controller deletion
* progress
* @NVME_CTRL_DEAD: Controller is non-present/unresponsive during * @NVME_CTRL_DEAD: Controller is non-present/unresponsive during
* shutdown or removal. In this case we forcibly * shutdown or removal. In this case we forcibly
* kill all inflight I/O as they have no chance to * kill all inflight I/O as they have no chance to
...@@ -201,6 +206,7 @@ enum nvme_ctrl_state { ...@@ -201,6 +206,7 @@ enum nvme_ctrl_state {
NVME_CTRL_RESETTING, NVME_CTRL_RESETTING,
NVME_CTRL_CONNECTING, NVME_CTRL_CONNECTING,
NVME_CTRL_DELETING, NVME_CTRL_DELETING,
NVME_CTRL_DELETING_NOIO,
NVME_CTRL_DEAD, NVME_CTRL_DEAD,
}; };
......
...@@ -1102,11 +1102,12 @@ static int nvme_rdma_setup_ctrl(struct nvme_rdma_ctrl *ctrl, bool new) ...@@ -1102,11 +1102,12 @@ static int nvme_rdma_setup_ctrl(struct nvme_rdma_ctrl *ctrl, bool new)
changed = nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_LIVE); changed = nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_LIVE);
if (!changed) { if (!changed) {
/* /*
* state change failure is ok if we're in DELETING state, * state change failure is ok if we started ctrl delete,
* unless we're during creation of a new controller to * unless we're during creation of a new controller to
* avoid races with teardown flow. * avoid races with teardown flow.
*/ */
WARN_ON_ONCE(ctrl->ctrl.state != NVME_CTRL_DELETING); WARN_ON_ONCE(ctrl->ctrl.state != NVME_CTRL_DELETING &&
ctrl->ctrl.state != NVME_CTRL_DELETING_NOIO);
WARN_ON_ONCE(new); WARN_ON_ONCE(new);
ret = -EINVAL; ret = -EINVAL;
goto destroy_io; goto destroy_io;
...@@ -1159,8 +1160,9 @@ static void nvme_rdma_error_recovery_work(struct work_struct *work) ...@@ -1159,8 +1160,9 @@ static void nvme_rdma_error_recovery_work(struct work_struct *work)
blk_mq_unquiesce_queue(ctrl->ctrl.admin_q); blk_mq_unquiesce_queue(ctrl->ctrl.admin_q);
if (!nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_CONNECTING)) { if (!nvme_change_ctrl_state(&ctrl->ctrl, NVME_CTRL_CONNECTING)) {
/* state change failure is ok if we're in DELETING state */ /* state change failure is ok if we started ctrl delete */
WARN_ON_ONCE(ctrl->ctrl.state != NVME_CTRL_DELETING); WARN_ON_ONCE(ctrl->ctrl.state != NVME_CTRL_DELETING &&
ctrl->ctrl.state != NVME_CTRL_DELETING_NOIO);
return; return;
} }
......
...@@ -1950,11 +1950,12 @@ static int nvme_tcp_setup_ctrl(struct nvme_ctrl *ctrl, bool new) ...@@ -1950,11 +1950,12 @@ static int nvme_tcp_setup_ctrl(struct nvme_ctrl *ctrl, bool new)
if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE)) { if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_LIVE)) {
/* /*
* state change failure is ok if we're in DELETING state, * state change failure is ok if we started ctrl delete,
* unless we're during creation of a new controller to * unless we're during creation of a new controller to
* avoid races with teardown flow. * avoid races with teardown flow.
*/ */
WARN_ON_ONCE(ctrl->state != NVME_CTRL_DELETING); WARN_ON_ONCE(ctrl->state != NVME_CTRL_DELETING &&
ctrl->state != NVME_CTRL_DELETING_NOIO);
WARN_ON_ONCE(new); WARN_ON_ONCE(new);
ret = -EINVAL; ret = -EINVAL;
goto destroy_io; goto destroy_io;
...@@ -2010,8 +2011,9 @@ static void nvme_tcp_error_recovery_work(struct work_struct *work) ...@@ -2010,8 +2011,9 @@ static void nvme_tcp_error_recovery_work(struct work_struct *work)
blk_mq_unquiesce_queue(ctrl->admin_q); blk_mq_unquiesce_queue(ctrl->admin_q);
if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_CONNECTING)) { if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_CONNECTING)) {
/* state change failure is ok if we're in DELETING state */ /* state change failure is ok if we started ctrl delete */
WARN_ON_ONCE(ctrl->state != NVME_CTRL_DELETING); WARN_ON_ONCE(ctrl->state != NVME_CTRL_DELETING &&
ctrl->state != NVME_CTRL_DELETING_NOIO);
return; return;
} }
...@@ -2046,8 +2048,9 @@ static void nvme_reset_ctrl_work(struct work_struct *work) ...@@ -2046,8 +2048,9 @@ static void nvme_reset_ctrl_work(struct work_struct *work)
nvme_tcp_teardown_ctrl(ctrl, false); nvme_tcp_teardown_ctrl(ctrl, false);
if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_CONNECTING)) { if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_CONNECTING)) {
/* state change failure is ok if we're in DELETING state */ /* state change failure is ok if we started ctrl delete */
WARN_ON_ONCE(ctrl->state != NVME_CTRL_DELETING); WARN_ON_ONCE(ctrl->state != NVME_CTRL_DELETING &&
ctrl->state != NVME_CTRL_DELETING_NOIO);
return; return;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment