nvme: program the arbitration burst from Identify RAB
AcceptedPublic
Actions

Authored by seuros on Mon, Aug 24, 6:20 PM.

Details

Reviewers

adrian
dteske
imp
ngie
ziaee

Summary

The controller's Recommended Arbitration Burst was ignored; the
specification advises matching the Arbitration feature's burst to it
even under plain round robin. Some controllers report unusable
values: programming the Apple S3X with its reported RAB wedged
sustained I/O, so quirked controllers keep their default
configuration. The hw.nvme.arb_burst tunable overrides the
recommendation.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 76389
Build 73272: arc lint + arc unit

Event Timeline

seuros created this revision.Mon, Aug 24, 6:20 PM

Herald added subscribers: ziaee, dab, imp. · View Herald TranscriptMon, Aug 24, 6:20 PM

seuros requested review of this revision.Mon, Aug 24, 6:20 PM

Harbormaster completed remote builds in B76060: Diff 184923.Mon, Aug 24, 6:20 PM

seuros added reviewers: adrian, dteske, imp.Mon, Aug 24, 6:23 PM

seuros added a reviewer: ngie.

seuros added a child revision: D59156: nvme: drop the S3X single-outstanding-I/O quirk.

seuros added a parent revision: D59154: nvme: limit I/O queue trackers to Identify MAXCMD.

seuros added a reviewer: ziaee.Mon, Aug 24, 6:38 PM

Thanks for tagging me!

Oh no, the way this is manual is written is excluding the nvme tunables from the search database. Let me fix that real quick.

@ziaee sadly the date change in b863d38437af11c18a88de656404af8cefa35e63 makes this change stack difficult to apply, this should be rebased on top of the manpage change

In D59155#1356950, @saper_saper.info wrote:

@ziaee sadly the date change in b863d38437af11c18a88de656404af8cefa35e63 makes this change stack difficult to apply, this should be rebased on top of the manpage change

Yes, that's what I meant by "let me fix that real quick". It also has a followup that could use a quick look over for errors or omissions, I tagged you and everyone else in this thread except ngie because she's on vacation.

rebased. we can wait till ngie is back from their break for a second check.

This stack gives a 720% speed boost in the S3X driver.

The most important are the 2 first commit of this stack, those are latent bugs i discovered while benchmarking dozen of nvmes vendors.

Harbormaster completed remote builds in B76114: Diff 184991.Tue, Aug 25, 6:35 AM

Will look at this with the rest of the stack when I get back to my personal mini "lab" in a few days :).

sys/dev/nvme/nvme_ctrlr.c
896	(hope the formatted message fits in <80 columns)
908	Minor tweak for clarity.
sys/dev/nvme/nvme_private.h
400–403	Is this indentation correct per style(9)? This should be hard-tab indented, then indented by 4 (the existing style doesn't seem to match style(9), but it would be nice to have this be consistent).