git: 225c0eda7efb - stable/12 - nvme: fix a race between failing the controller and failing requests

[Warner Losh]

The branch stable/12 has been updated by imp:

URL: https://cgit.FreeBSD.org/src/commit/?id=225c0eda7efb73c9af75fc54c434b0ec694f506a

commit 225c0eda7efb73c9af75fc54c434b0ec694f506a
Author:     Warner Losh <imp at FreeBSD.org>
AuthorDate: 2021-05-29 05:01:52 +0000
Commit:     Warner Losh <imp at FreeBSD.org>
CommitDate: 2021-07-31 00:02:53 +0000

    nvme: fix a race between failing the controller and failing requests
    
    Part of the nvme recovery process for errors is to reset the
    card. Sometimes, this results in failing the entire controller. When nda
    is in use, we free the sim, which will sleep until all the I/O has
    completed. However, with only one thread, the request fail task never
    runs once the reset thread sleeps here. Create two threads to allow I/O
    to fail until it's all processed and the reset task can proceed.
    
    This is a temporary kludge until I can work out questions that arose
    during the review, not least is what was the race that queueing to a
    failure task solved. The original commit is vague and other error paths
    in the same context do a direct failure. I'll investigate that more
    completely before committing changing that to a direct failure. mav@
    raised this issue during the review, but didn't otherwise object.
    
    Multiple threads, though, solve the problem in the mean time until other
    such means can be perfected.
    
    Reviewed by:            jhb@
    Sponsored by:           Netflix
    Differential Revision:  https://reviews.freebsd.org/D30366
    
    (cherry picked from commit f0f47121653e88197d8537572294b90f5aef7f17)
---
 sys/dev/nvme/nvme_ctrlr.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/sys/dev/nvme/nvme_ctrlr.c b/sys/dev/nvme/nvme_ctrlr.c
index 007e0758023a..0cd2c31ab086 100644
--- a/sys/dev/nvme/nvme_ctrlr.c
+++ b/sys/dev/nvme/nvme_ctrlr.c
@@ -1442,9 +1442,20 @@ nvme_ctrlr_construct(struct nvme_controller *ctrlr, device_t dev)
 	if (nvme_ctrlr_construct_admin_qpair(ctrlr) != 0)
 		return (ENXIO);
 
+	/*
+	 * Create 2 threads for the taskqueue. The reset thread will block when
+	 * it detects that the controller has failed until all I/O has been
+	 * failed up the stack. The fail_req task needs to be able to run in
+	 * this case to finish the request failure for some cases.
+	 *
+	 * We could partially solve this race by draining the failed requeust
+	 * queue before proceding to free the sim, though nothing would stop
+	 * new I/O from coming in after we do that drain, but before we reach
+	 * cam_sim_free, so this big hammer is used instead.
+	 */
 	ctrlr->taskqueue = taskqueue_create("nvme_taskq", M_WAITOK,
 	    taskqueue_thread_enqueue, &ctrlr->taskqueue);
-	taskqueue_start_threads(&ctrlr->taskqueue, 1, PI_DISK, "nvme taskq");
+	taskqueue_start_threads(&ctrlr->taskqueue, 2, PI_DISK, "nvme taskq");
 
 	ctrlr->is_resetting = 0;
 	ctrlr->is_initialized = 0;