d/freebsd-nq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

nvme: Add sanity check for phase on startup.

Browse Source 
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
This commit is contained in:
Warner Losh 2021-09-28 21:11:17 -06:00
parent fa81f3731d
commit 7d5eebe0f4
2 changed files with 24 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
sys/dev/nvme/nvme_qpair.c
View File

@ -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);
/*

7
sys/dev/nvme/nvme_sysctl.c
View File

@ -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;
}