nvme: Eliminate RECOVERY_FAILED state
ClosedPublic
Actions

Authored by imp on Oct 2 2023, 9:20 PM.

Details

Reviewers

jhb
mav
chuck
gallatin
chs

Commits

rG9cd7b624732c: nvme: Eliminate RECOVERY_FAILED state

Summary

While it seemed like a good idea to have this state, we can do
everything we wanted with the state by checking ctrlr->is_failed since
that's set before we start failing the qpairs. Add some comments about
racing when we're failing the controller, though in practice I'm not
sure that kind of race could even be lost.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 53802
Build 50693: arc lint + arc unit

Event Timeline

imp created this revision.Oct 2 2023, 9:20 PM

Herald added a subscriber: dab. · View Herald TranscriptOct 2 2023, 9:20 PM

imp requested review of this revision.Oct 2 2023, 9:20 PM

imp added a parent revision: D42050: nvme: Remove stale comment.

Harbormaster completed remote builds in B53802: Diff 128114.Oct 2 2023, 9:20 PM

chuck added inline comments.Oct 2 2023, 11:24 PM

sys/dev/nvme/nvme_qpair.c
1038	Would it be good hygiene to add `qpair->timer_armed = false;` here or will that already be true?

imp added inline comments.Oct 2 2023, 11:39 PM

sys/dev/nvme/nvme_qpair.c
1038	It won't already be true. In fact, we rely on the timeout to disarm itself now. I'll add it here for this review... But now that you point this out, and I got to thinking... This could lead to a crash if we're unloading the driver under heavy load maybe (since we won't be idle). I'll followup with a separate patch to fix that since we're currently never cancelling the timer (I was sure we were, but a quick grep suggests it was overlooked). I'll have to study to find which of the destroy, destruct, etc methods would be right.