From owner-dev-commits-src-all@freebsd.org Fri Oct 1 17:10:23 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 9C0E16B2D85; Fri, 1 Oct 2021 17:10:23 +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 4HLc7M3q0Jz4lMM; Fri, 1 Oct 2021 17:10:23 +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 5F245230C7; Fri, 1 Oct 2021 17:10:23 +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 191HANEK026522; Fri, 1 Oct 2021 17:10:23 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 191HANo4026521; Fri, 1 Oct 2021 17:10:23 GMT (envelope-from git) Date: Fri, 1 Oct 2021 17:10:23 GMT Message-Id: <202110011710.191HANo4026521@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Warner Losh Subject: git: e5e26e4a24a1 - main - nvme: Remove pause while resetting 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/main X-Git-Reftype: branch X-Git-Commit: e5e26e4a24a1142e02a9b477877e13ed0c194f36 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: Fri, 01 Oct 2021 17:10:23 -0000 The branch main has been updated by imp: URL: https://cgit.FreeBSD.org/src/commit/?id=e5e26e4a24a1142e02a9b477877e13ed0c194f36 commit e5e26e4a24a1142e02a9b477877e13ed0c194f36 Author: Warner Losh AuthorDate: 2021-10-01 17:09:05 +0000 Commit: Warner Losh CommitDate: 2021-10-01 17:09:05 +0000 nvme: Remove pause while resetting After some study of the code and the standard, I think we can just drop the pause(), unconditionally. If we're not initialized, then there's nothing to wait for from a software perspective. If we are initialized, then there might be outstanding I/O. If so, then the qpair 'recovery state' will transition to WAITING in nvme_ctrlr_disable_qpairs, which will ignore any interrupts for items that complete before we complete the reset by setting cc.en=0. If we go on to fail the controller, we'll cancel the outstanding I/O transactions. If we reset the controller, the hardware throws away pending transactions and we retry all the pending I/O transactions. Any transactions that happend to complete before cc.en=0 will have the same effect in the end (doing the same transaction twice is just inefficient, it won't affect the state of the device any differently than having done it once). The standard imposes no wait times here, so it isn't needed from that perspective. Unanswered Question: Do we may need to disable interrupts while we disable in legacy mode since those are level-sensitive. Sponsored by: Netflix Reviewed by: mav Differential Revision: https://reviews.freebsd.org/D32248 --- sys/dev/nvme/nvme_ctrlr.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/sys/dev/nvme/nvme_ctrlr.c b/sys/dev/nvme/nvme_ctrlr.c index eb95fa33b90b..d0be0da39902 100644 --- a/sys/dev/nvme/nvme_ctrlr.c +++ b/sys/dev/nvme/nvme_ctrlr.c @@ -410,13 +410,13 @@ nvme_ctrlr_hw_reset(struct nvme_controller *ctrlr) int err; TSENTER(); - nvme_ctrlr_disable_qpairs(ctrlr); - pause("nvmehwreset", hz / 10); + nvme_ctrlr_disable_qpairs(ctrlr); err = nvme_ctrlr_disable(ctrlr); if (err != 0) return err; + err = nvme_ctrlr_enable(ctrlr); TSEXIT(); return (err); @@ -1653,13 +1653,10 @@ nvme_ctrlr_suspend(struct nvme_controller *ctrlr) * flushes any metadata the drive may have stored so it can survive * having its power removed and prevents the unsafe shutdown count from * incriminating. Once we delete the qpairs, we have to disable them - * before shutting down. The delay is out of paranoia in - * nvme_ctrlr_hw_reset, and is repeated here (though we should have no - * pending I/O that the delay copes with). + * before shutting down. */ nvme_ctrlr_delete_qpairs(ctrlr); nvme_ctrlr_disable_qpairs(ctrlr); - pause("nvmesusp", hz / 10); nvme_ctrlr_shutdown(ctrlr); return (0);