Skip to content

Commit d069236

Browse files
shroffnikeithbusch
authored andcommitted
nvme: make keep-alive synchronous operation
The nvme keep-alive operation, which executes at a periodic interval, could potentially sneak in while shutting down a fabric controller. This may lead to a race between the fabric controller admin queue destroy code path (invoked while shutting down controller) and hw/hctx queue dispatcher called from the nvme keep-alive async request queuing operation. This race could lead to the kernel crash shown below: Call Trace: autoremove_wake_function+0x0/0xbc (unreliable) __blk_mq_sched_dispatch_requests+0x114/0x24c blk_mq_sched_dispatch_requests+0x44/0x84 blk_mq_run_hw_queue+0x140/0x220 nvme_keep_alive_work+0xc8/0x19c [nvme_core] process_one_work+0x200/0x4e0 worker_thread+0x340/0x504 kthread+0x138/0x140 start_kernel_thread+0x14/0x18 While shutting down fabric controller, if nvme keep-alive request sneaks in then it would be flushed off. The nvme_keep_alive_end_io function is then invoked to handle the end of the keep-alive operation which decrements the admin->q_usage_counter and assuming this is the last/only request in the admin queue then the admin->q_usage_counter becomes zero. If that happens then blk-mq destroy queue operation (blk_mq_destroy_ queue()) which could be potentially running simultaneously on another cpu (as this is the controller shutdown code path) would forward progress and deletes the admin queue. So, now from this point onward we are not supposed to access the admin queue resources. However the issue here's that the nvme keep-alive thread running hw/hctx queue dispatch operation hasn't yet finished its work and so it could still potentially access the admin queue resource while the admin queue had been already deleted and that causes the above crash. This fix helps avoid the observed crash by implementing keep-alive as a synchronous operation so that we decrement admin->q_usage_counter only after keep-alive command finished its execution and returns the command status back up to its caller (blk_execute_rq()). This would ensure that fabric shutdown code path doesn't destroy the fabric admin queue until keep-alive request finished execution and also keep-alive thread is not running hw/hctx queue dispatch operation. Reviewed-by: Christoph Hellwig <hch@lst.de> Signed-off-by: Nilay Shroff <nilay@linux.ibm.com> Signed-off-by: Keith Busch <kbusch@kernel.org>
1 parent c199fac commit d069236

File tree

1 file changed

+7
-10
lines changed

1 file changed

+7
-10
lines changed

drivers/nvme/host/core.c

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1292,10 +1292,9 @@ static void nvme_queue_keep_alive_work(struct nvme_ctrl *ctrl)
12921292
queue_delayed_work(nvme_wq, &ctrl->ka_work, delay);
12931293
}
12941294

1295-
static enum rq_end_io_ret nvme_keep_alive_end_io(struct request *rq,
1296-
blk_status_t status)
1295+
static void nvme_keep_alive_finish(struct request *rq,
1296+
blk_status_t status, struct nvme_ctrl *ctrl)
12971297
{
1298-
struct nvme_ctrl *ctrl = rq->end_io_data;
12991298
unsigned long flags;
13001299
bool startka = false;
13011300
unsigned long rtt = jiffies - (rq->deadline - rq->timeout);
@@ -1313,13 +1312,11 @@ static enum rq_end_io_ret nvme_keep_alive_end_io(struct request *rq,
13131312
delay = 0;
13141313
}
13151314

1316-
blk_mq_free_request(rq);
1317-
13181315
if (status) {
13191316
dev_err(ctrl->device,
13201317
"failed nvme_keep_alive_end_io error=%d\n",
13211318
status);
1322-
return RQ_END_IO_NONE;
1319+
return;
13231320
}
13241321

13251322
ctrl->ka_last_check_time = jiffies;
@@ -1331,7 +1328,6 @@ static enum rq_end_io_ret nvme_keep_alive_end_io(struct request *rq,
13311328
spin_unlock_irqrestore(&ctrl->lock, flags);
13321329
if (startka)
13331330
queue_delayed_work(nvme_wq, &ctrl->ka_work, delay);
1334-
return RQ_END_IO_NONE;
13351331
}
13361332

13371333
static void nvme_keep_alive_work(struct work_struct *work)
@@ -1340,6 +1336,7 @@ static void nvme_keep_alive_work(struct work_struct *work)
13401336
struct nvme_ctrl, ka_work);
13411337
bool comp_seen = ctrl->comp_seen;
13421338
struct request *rq;
1339+
blk_status_t status;
13431340

13441341
ctrl->ka_last_check_time = jiffies;
13451342

@@ -1362,9 +1359,9 @@ static void nvme_keep_alive_work(struct work_struct *work)
13621359
nvme_init_request(rq, &ctrl->ka_cmd);
13631360

13641361
rq->timeout = ctrl->kato * HZ;
1365-
rq->end_io = nvme_keep_alive_end_io;
1366-
rq->end_io_data = ctrl;
1367-
blk_execute_rq_nowait(rq, false);
1362+
status = blk_execute_rq(rq, false);
1363+
nvme_keep_alive_finish(rq, status, ctrl);
1364+
blk_mq_free_request(rq);
13681365
}
13691366

13701367
static void nvme_start_keep_alive(struct nvme_ctrl *ctrl)

0 commit comments

Comments
 (0)