Skip to content

Commit 9dd8dc3

Browse files
sagigrimberggregkh
authored andcommitted
nvme: fix controller removal race with scan work
[ Upstream commit 0157ec8 ] With multipath enabled, nvme_scan_work() can read from the device (through nvme_mpath_add_disk()) and hang [1]. However, with fabrics, once ctrl->state is set to NVME_CTRL_DELETING, the reads will hang (see nvmf_check_ready()) and the mpath stack device make_request will block if head->list is not empty. However, when the head->list consistst of only DELETING/DEAD controllers, we should actually not block, but rather fail immediately. In addition, before we go ahead and remove the namespaces, make sure to clear the current path and kick the requeue list so that the request will fast fail upon requeuing. [1]: -- INFO: task kworker/u4:3:166 blocked for more than 120 seconds. Not tainted 5.2.0-rc6-vmlocalyes-00005-g808c8c2dc0cf #316 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. kworker/u4:3 D 0 166 2 0x80004000 Workqueue: nvme-wq nvme_scan_work Call Trace: __schedule+0x851/0x1400 schedule+0x99/0x210 io_schedule+0x21/0x70 do_read_cache_page+0xa57/0x1330 read_cache_page+0x4a/0x70 read_dev_sector+0xbf/0x380 amiga_partition+0xc4/0x1230 check_partition+0x30f/0x630 rescan_partitions+0x19a/0x980 __blkdev_get+0x85a/0x12f0 blkdev_get+0x2a5/0x790 __device_add_disk+0xe25/0x1250 device_add_disk+0x13/0x20 nvme_mpath_set_live+0x172/0x2b0 nvme_update_ns_ana_state+0x130/0x180 nvme_set_ns_ana_state+0x9a/0xb0 nvme_parse_ana_log+0x1c3/0x4a0 nvme_mpath_add_disk+0x157/0x290 nvme_validate_ns+0x1017/0x1bd0 nvme_scan_work+0x44d/0x6a0 process_one_work+0x7d7/0x1240 worker_thread+0x8e/0xff0 kthread+0x2c3/0x3b0 ret_from_fork+0x35/0x40 INFO: task kworker/u4:1:1034 blocked for more than 120 seconds. Not tainted 5.2.0-rc6-vmlocalyes-00005-g808c8c2dc0cf #316 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. kworker/u4:1 D 0 1034 2 0x80004000 Workqueue: nvme-delete-wq nvme_delete_ctrl_work Call Trace: __schedule+0x851/0x1400 schedule+0x99/0x210 schedule_timeout+0x390/0x830 wait_for_completion+0x1a7/0x310 __flush_work+0x241/0x5d0 flush_work+0x10/0x20 nvme_remove_namespaces+0x85/0x3d0 nvme_do_delete_ctrl+0xb4/0x1e0 nvme_delete_ctrl_work+0x15/0x20 process_one_work+0x7d7/0x1240 worker_thread+0x8e/0xff0 kthread+0x2c3/0x3b0 ret_from_fork+0x35/0x40 -- Reported-by: Logan Gunthorpe <[email protected]> Tested-by: Logan Gunthorpe <[email protected]> Reviewed-by: Logan Gunthorpe <[email protected]> Reviewed-by: Ming Lei <[email protected]> Signed-off-by: Sagi Grimberg <[email protected]> Signed-off-by: Sasha Levin <[email protected]>
1 parent be2e81d commit 9dd8dc3

File tree

3 files changed

+54
-8
lines changed

3 files changed

+54
-8
lines changed

drivers/nvme/host/core.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3529,6 +3529,13 @@ void nvme_remove_namespaces(struct nvme_ctrl *ctrl)
35293529
struct nvme_ns *ns, *next;
35303530
LIST_HEAD(ns_list);
35313531

3532+
/*
3533+
* make sure to requeue I/O to all namespaces as these
3534+
* might result from the scan itself and must complete
3535+
* for the scan_work to make progress
3536+
*/
3537+
nvme_mpath_clear_ctrl_paths(ctrl);
3538+
35323539
/* prevent racing with ns scanning */
35333540
flush_work(&ctrl->scan_work);
35343541

drivers/nvme/host/multipath.c

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -134,18 +134,34 @@ static const char *nvme_ana_state_names[] = {
134134
[NVME_ANA_CHANGE] = "change",
135135
};
136136

137-
void nvme_mpath_clear_current_path(struct nvme_ns *ns)
137+
bool nvme_mpath_clear_current_path(struct nvme_ns *ns)
138138
{
139139
struct nvme_ns_head *head = ns->head;
140+
bool changed = false;
140141
int node;
141142

142143
if (!head)
143-
return;
144+
goto out;
144145

145146
for_each_node(node) {
146-
if (ns == rcu_access_pointer(head->current_path[node]))
147+
if (ns == rcu_access_pointer(head->current_path[node])) {
147148
rcu_assign_pointer(head->current_path[node], NULL);
149+
changed = true;
150+
}
148151
}
152+
out:
153+
return changed;
154+
}
155+
156+
void nvme_mpath_clear_ctrl_paths(struct nvme_ctrl *ctrl)
157+
{
158+
struct nvme_ns *ns;
159+
160+
mutex_lock(&ctrl->scan_lock);
161+
list_for_each_entry(ns, &ctrl->namespaces, list)
162+
if (nvme_mpath_clear_current_path(ns))
163+
kblockd_schedule_work(&ns->head->requeue_work);
164+
mutex_unlock(&ctrl->scan_lock);
149165
}
150166

151167
static struct nvme_ns *__nvme_find_path(struct nvme_ns_head *head, int node)
@@ -248,6 +264,24 @@ inline struct nvme_ns *nvme_find_path(struct nvme_ns_head *head)
248264
return ns;
249265
}
250266

267+
static bool nvme_available_path(struct nvme_ns_head *head)
268+
{
269+
struct nvme_ns *ns;
270+
271+
list_for_each_entry_rcu(ns, &head->list, siblings) {
272+
switch (ns->ctrl->state) {
273+
case NVME_CTRL_LIVE:
274+
case NVME_CTRL_RESETTING:
275+
case NVME_CTRL_CONNECTING:
276+
/* fallthru */
277+
return true;
278+
default:
279+
break;
280+
}
281+
}
282+
return false;
283+
}
284+
251285
static blk_qc_t nvme_ns_head_make_request(struct request_queue *q,
252286
struct bio *bio)
253287
{
@@ -274,14 +308,14 @@ static blk_qc_t nvme_ns_head_make_request(struct request_queue *q,
274308
disk_devt(ns->head->disk),
275309
bio->bi_iter.bi_sector);
276310
ret = direct_make_request(bio);
277-
} else if (!list_empty_careful(&head->list)) {
278-
dev_warn_ratelimited(dev, "no path available - requeuing I/O\n");
311+
} else if (nvme_available_path(head)) {
312+
dev_warn_ratelimited(dev, "no usable path - requeuing I/O\n");
279313

280314
spin_lock_irq(&head->requeue_lock);
281315
bio_list_add(&head->requeue_list, bio);
282316
spin_unlock_irq(&head->requeue_lock);
283317
} else {
284-
dev_warn_ratelimited(dev, "no path - failing I/O\n");
318+
dev_warn_ratelimited(dev, "no available path - failing I/O\n");
285319

286320
bio->bi_status = BLK_STS_IOERR;
287321
bio_endio(bio);

drivers/nvme/host/nvme.h

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -490,7 +490,8 @@ void nvme_mpath_remove_disk(struct nvme_ns_head *head);
490490
int nvme_mpath_init(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id);
491491
void nvme_mpath_uninit(struct nvme_ctrl *ctrl);
492492
void nvme_mpath_stop(struct nvme_ctrl *ctrl);
493-
void nvme_mpath_clear_current_path(struct nvme_ns *ns);
493+
bool nvme_mpath_clear_current_path(struct nvme_ns *ns);
494+
void nvme_mpath_clear_ctrl_paths(struct nvme_ctrl *ctrl);
494495
struct nvme_ns *nvme_find_path(struct nvme_ns_head *head);
495496

496497
static inline void nvme_mpath_check_last_path(struct nvme_ns *ns)
@@ -538,7 +539,11 @@ static inline void nvme_mpath_add_disk(struct nvme_ns *ns,
538539
static inline void nvme_mpath_remove_disk(struct nvme_ns_head *head)
539540
{
540541
}
541-
static inline void nvme_mpath_clear_current_path(struct nvme_ns *ns)
542+
static inline bool nvme_mpath_clear_current_path(struct nvme_ns *ns)
543+
{
544+
return false;
545+
}
546+
static inline void nvme_mpath_clear_ctrl_paths(struct nvme_ctrl *ctrl)
542547
{
543548
}
544549
static inline void nvme_mpath_check_last_path(struct nvme_ns *ns)

0 commit comments

Comments
 (0)