From owner-dev-commits-src-all@freebsd.org Mon Jul 12 19:33:34 2021 Return-Path: Delivered-To: dev-commits-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id E2EB1665796; Mon, 12 Jul 2021 19:33:34 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4GNv7y35h5z3QTW; Mon, 12 Jul 2021 19:33:34 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4616514C9E; Mon, 12 Jul 2021 19:33:34 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 16CJXYJb016987; Mon, 12 Jul 2021 19:33:34 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 16CJXYvM016986; Mon, 12 Jul 2021 19:33:34 GMT (envelope-from git) Date: Mon, 12 Jul 2021 19:33:34 GMT Message-Id: <202107121933.16CJXYvM016986@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Warner Losh Subject: git: 1109ce0da292 - stable/13 - nvme: fix a race between failing the controller and failing requests MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: imp X-Git-Repository: src X-Git-Refname: refs/heads/stable/13 X-Git-Reftype: branch X-Git-Commit: 1109ce0da292fca8952301086beef63ecaed8649 Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for all branches of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 12 Jul 2021 19:33:35 -0000 The branch stable/13 has been updated by imp: URL: https://cgit.FreeBSD.org/src/commit/?id=1109ce0da292fca8952301086beef63ecaed8649 commit 1109ce0da292fca8952301086beef63ecaed8649 Author: Warner Losh AuthorDate: 2021-05-29 05:01:52 +0000 Commit: Warner Losh CommitDate: 2021-07-12 19:30:55 +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 b6befb2c88d0..d8760158a75f 100644 --- a/sys/dev/nvme/nvme_ctrlr.c +++ b/sys/dev/nvme/nvme_ctrlr.c @@ -1429,9 +1429,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;