Differential D32211

nvme: Add sanity check for phase on startup.
ClosedPublic
Actions

Authored by imp on Sep 28 2021, 8:06 PM.

Tags

None

Referenced Files

	Unknown Object (File)
	Wed, May 8, 9:30 AM

	Unknown Object (File)
	Wed, Apr 24, 3:51 AM

	Unknown Object (File)
	Tue, Apr 23, 2:54 AM

	Unknown Object (File)
	Mar 30 2024, 6:21 PM

	Unknown Object (File)
	Mar 10 2024, 8:15 AM

	Unknown Object (File)
	Mar 7 2024, 11:35 PM

	Unknown Object (File)
	Feb 10 2024, 12:30 AM

	Unknown Object (File)
	Feb 2 2024, 9:19 AM

View All 34 Files

Subscribers

Details

Reviewers

mav
chuck
cperciva

Commits

rG7144882ae15c: nvme: Add sanity check for phase on startup.
rG7d5eebe0f4a0: nvme: Add sanity check for phase on startup.

Summary

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.

Sponsored by: Netflix

Test Plan

Added Colin to double check this assert won't fire for him.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

imp created this revision.Sep 28 2021, 8:06 PM

Herald added a subscriber: dab. · View Herald TranscriptSep 28 2021, 8:06 PM

imp requested review of this revision.Sep 28 2021, 8:06 PM

Harbormaster completed remote builds in B41819: Diff 95890.Sep 28 2021, 8:06 PM

imp added reviewers: mav, chuck.Sep 28 2021, 8:07 PM

I hope num_intr_handler_calls will newer wrap around.

This revision is now accepted and ready to land.Sep 28 2021, 8:16 PM

Reset the counter to 1

This revision now requires review to proceed.Sep 28 2021, 8:37 PM

Harbormaster completed remote builds in B41821: Diff 95892.Sep 28 2021, 8:37 PM

It is a good addition, but now I am thinking whether it can "reset" to 1 something that is 0. ;)

imp mentioned this in D32210: nvme: start qpair in state RECOVERY_WAITING.Sep 28 2021, 9:55 PM

imp edited the test plan for this revision. (Show Details)

imp added a reviewer: cperciva.

In EC2, D32211 + D32210 boots and works fine.

D32211 *without* D32210 panics with "Phase wrong for first interrupt call".

So I think we're good for both patches.

This revision is now accepted and ready to land.Sep 28 2021, 10:37 PM

Closed by commit rG7d5eebe0f4a0: nvme: Add sanity check for phase on startup. (authored by imp). · Explain WhySep 29 2021, 3:24 AM

This revision was automatically updated to reflect the committed changes.

imp added a commit: rG7d5eebe0f4a0: nvme: Add sanity check for phase on startup..

mav added a commit: rG7144882ae15c: nvme: Add sanity check for phase on startup..Jan 21 2022, 2:27 AM

Revision Contents
Changeset List

Path

Size

sys/

dev/

nvme/

21 lines

7 lines

Diff 95897

sys/dev/nvme/nvme_qpair.c

Loading...

sys/dev/nvme/nvme_sysctl.c

Loading...