nvme: Allow requestors to suppress error reporting
Needs ReviewPublic
Actions

Authored by imp on Aug 3 2023, 10:41 PM.

Details

Reviewers

jhb
mav
chuck
chs

Summary

Add ability to supress reporting of the errors by the nvme layer. nda,
in the future, will turn this off since it does its own reporting and we
don't need both layers reporting. nvd won't change this, and this layer
will continue to report its errors in the nvme layer. This should be no
change (since we always turn on error reporting, and nda isn't turning
it off yet).

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 52975
Build 49866: arc lint + arc unit

Event Timeline

imp created this revision.Aug 3 2023, 10:41 PM

Herald added a subscriber: dab. · View Herald TranscriptAug 3 2023, 10:41 PM

imp requested review of this revision.Aug 3 2023, 10:41 PM

Harbormaster completed remote builds in B52975: Diff 125551.Aug 3 2023, 10:41 PM

imp added a parent revision: D41315: nvme: Update comment.Aug 3 2023, 10:41 PM

I have no objections, but it made me think that since retries are handed here, in case of CAM nda may not see some retried errors to report them.

In D41316#940746, @mav wrote:

I have no objections, but it made me think that since retries are handed here, in case of CAM nda may not see some retried errors to report them.

This is a very good observation. I need to think this through a little, I think. For my current plans, I don't think it will matter a lot, but thinking about it a little, I can see a number of possibilities that I didn't before (which might wind up changing things longer term).

jhb added inline comments.Aug 5 2023, 12:17 AM

sys/dev/nvme/nvme_private.h
124	I would go ahead and spell out `report_error`

When nda turns off reporting, how are pass-through commands treated? Will this change suppress errors from those commands as well?

In D41316#941288, @chuck wrote:

When nda turns off reporting, how are pass-through commands treated? Will this change suppress errors from those commands as well?

That's for us to decide. For SCSI and ATA, we don't report them unless we've been asked to do recovery.
But for SCSI/ATA we also report the errors to devctl regardless.
I plan on making nvme/nda similar in this respect.

I'm thinking maybe I need to have a few more commits down that path given some of the feedback so far (eg commit the ones w/o feedback and then relaunch/refresh when I've done the next round of changes).