Page MenuHomeFreeBSD
Differential D59151

nvme: complete requests when payload DMA mapping fails
AcceptedPublic
Actions

Authored by seuros on Mon, Aug 24, 6:19 PM.
Tags
None
Referenced Files
F170055017: D59151.diff
Thu, Sep 3, 9:25 AM
F170048024: D59151.id184987.diff
Thu, Sep 3, 8:54 AM
F169985155: D59151.diff
Thu, Sep 3, 4:44 AM
F169981625: D59151.diff
Thu, Sep 3, 4:26 AM
F169944620: D59151.id184919.diff
Thu, Sep 3, 1:31 AM
F169938167: D59151.diff
Thu, Sep 3, 1:01 AM
F169909664: D59151.id184987.diff
Wed, Sep 2, 10:44 PM
F169840594: D59151.diff
Wed, Sep 2, 5:38 PM
View All 37 Files
Subscribers
dab
imp
ngie

Details

Reviewers
adrian
imp
ngie
Summary

bus_dmamap_load_mem() reports most mapping failures, including EFBIG,
only through its callback and then returns zero. nvme_payload_map()
logged the error without telling the submission path, so the tracker
stayed on the outstanding list with no command submitted and no
timeout armed, stalling all later I/O on the queue behind it.

Approved by: ngie (co-mentor)
MFC after: 1 week

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 76388
Build 73271: arc lint + arc unit

Event Timeline

seuros created this revision.Mon, Aug 24, 6:19 PM
Herald added subscribers: dab, imp. · View Herald TranscriptMon, Aug 24, 6:19 PM
seuros requested review of this revision.Mon, Aug 24, 6:19 PM
Harbormaster completed remote builds in B76056: Diff 184919.Mon, Aug 24, 6:19 PM
seuros added reviewers: adrian, imp.Mon, Aug 24, 6:20 PM
seuros added a child revision: D59152: nvme: limit namespace character-device I/O size.
seuros updated this revision to Diff 184987.Tue, Aug 25, 6:34 AM

rebase

Harbormaster completed remote builds in B76110: Diff 184987.Tue, Aug 25, 6:34 AM
imp added inline comments.Tue, Aug 25, 7:03 PM
sys/dev/nvme/nvme_qpair.c
1181

Should we set this to EINPROGRESS here? Are there paths out which don't do the DMA that aren't a mapping failure?

1195

I'd think that we don't want to wait here.
If it's just to detect this error, can't we detect it in the callback instead?

ngie added a subscriber: ngie.Sat, Aug 29, 2:24 AM
ngie added inline comments.
sys/dev/nvme/nvme_qpair.c
1195

+1. Doing this in the callback (if possible) seems much cleaner so you don't have to deal with the truth table of err x tr->map_err conditional combinations and the associated complexity in all of the downstream callers.

seuros updated this revision to Diff 185582.Tue, Sep 1, 6:41 PM

update flow

Harbormaster completed remote builds in B76388: Diff 185582.Tue, Sep 1, 6:41 PM
imp accepted this revision.Tue, Sep 1, 7:43 PM
imp added inline comments.
sys/dev/nvme/nvme_qpair.c
1195

OK. The callback is always called, so this is good.

This revision is now accepted and ready to land.Tue, Sep 1, 7:43 PM
ngie accepted this revision.Wed, Sep 2, 2:32 AM

Approved by: ngie (co-mentor)

seuros edited the summary of this revision. (Show Details)Wed, Sep 2, 12:05 PM

Revision Contents
Changeset List

PathSize
sys/
dev/
nvme/
37 lines

Diff 185582

View Options

sys/dev/nvme/nvme_qpair.c

Loading...