From owner-dev-commits-src-all@freebsd.org Wed Sep 29 03:24: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 D27AA669B59; Wed, 29 Sep 2021 03:24: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 4HK1vC4z45z3qs6; Wed, 29 Sep 2021 03:24: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 8BCBB10CCE; Wed, 29 Sep 2021 03:24: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 18T3ON8k075560; Wed, 29 Sep 2021 03:24:23 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 18T3ONUd075559; Wed, 29 Sep 2021 03:24:23 GMT (envelope-from git) Date: Wed, 29 Sep 2021 03:24:23 GMT Message-Id: <202109290324.18T3ONUd075559@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: 7d5eebe0f4a0 - main - nvme: Add sanity check for phase on startup. 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: 7d5eebe0f4a0f2aa5c8c7dfdd1a9ce1513849da8 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: Wed, 29 Sep 2021 03:24:24 -0000 The branch main has been updated by imp: URL: https://cgit.FreeBSD.org/src/commit/?id=7d5eebe0f4a0f2aa5c8c7dfdd1a9ce1513849da8 commit 7d5eebe0f4a0f2aa5c8c7dfdd1a9ce1513849da8 Author: Warner Losh AuthorDate: 2021-09-29 03:11:17 +0000 Commit: Warner Losh CommitDate: 2021-09-29 03:18:00 +0000 nvme: Add sanity check for phase on startup. The proper phase for the qpiar right after reset in the first interrupt is 1. For it, make sure that we're not still in phase 0. This is an illegal state to be processing interrupts and indicates that we've failed to properly protect against a race between initializing our state and processing interrupts. Modify stat resetting code so it resets the number of interrpts to 1 instead of 0 so we don't trigger a false positive panic. Sponsored by: Netflix Reviewed by: cperciva, mav (prior version) Differential Revision: https://reviews.freebsd.org/D32211 --- sys/dev/nvme/nvme_qpair.c | 21 ++++++++++++++++++--- sys/dev/nvme/nvme_sysctl.c | 7 ++++++- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/sys/dev/nvme/nvme_qpair.c b/sys/dev/nvme/nvme_qpair.c index 6ee5fa9d4c30..827054efd48e 100644 --- a/sys/dev/nvme/nvme_qpair.c +++ b/sys/dev/nvme/nvme_qpair.c @@ -536,16 +536,31 @@ nvme_qpair_process_completions(struct nvme_qpair *qpair) int done = 0; bool in_panic = dumping || SCHEDULER_STOPPED(); - qpair->num_intr_handler_calls++; - /* * qpair is not enabled, likely because a controller reset is is in * progress. Ignore the interrupt - any I/O that was associated with - * this interrupt will get retried when the reset is complete. + * this interrupt will get retried when the reset is complete. Any + * pending completions for when we're in startup will be completed + * as soon as initialization is complete and we start sending commands + * to the device. */ if (qpair->recovery_state != RECOVERY_NONE) return (false); + /* + * Sanity check initialization. After we reset the hardware, the phase + * is defined to be 1. So if we get here with zero prior calls and the + * phase is 0, it means that we've lost a race between the + * initialization and the ISR running. With the phase wrong, we'll + * process a bunch of completions that aren't really completions leading + * to a KASSERT below. + */ + KASSERT(!(qpair->num_intr_handler_calls == 0 && qpair->phase == 0), + ("%s: Phase wrong for first interrupt call.", + device_get_nameunit(qpair->ctrlr->dev))); + + qpair->num_intr_handler_calls++; + bus_dmamap_sync(qpair->dma_tag, qpair->queuemem_map, BUS_DMASYNC_POSTREAD | BUS_DMASYNC_POSTWRITE); /* diff --git a/sys/dev/nvme/nvme_sysctl.c b/sys/dev/nvme/nvme_sysctl.c index 9ec1f14511f5..58db38dc373d 100644 --- a/sys/dev/nvme/nvme_sysctl.c +++ b/sys/dev/nvme/nvme_sysctl.c @@ -155,8 +155,13 @@ static void nvme_qpair_reset_stats(struct nvme_qpair *qpair) { + /* + * Reset the values. Due to sanity checks in + * nvme_qpair_process_completions, we reset the number of interrupt + * calls to 1. + */ qpair->num_cmds = 0; - qpair->num_intr_handler_calls = 0; + qpair->num_intr_handler_calls = 1; qpair->num_retries = 0; qpair->num_failures = 0; }