Page MenuHomeFreeBSD

nvme: On ioctl, use nvme_allocate_request_buf
AcceptedPublic

Authored by imp on Aug 22 2025, 1:15 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Oct 12, 9:22 PM
Unknown Object (File)
Sun, Oct 12, 9:22 PM
Unknown Object (File)
Sun, Oct 12, 9:48 AM
Unknown Object (File)
Wed, Oct 1, 6:55 AM
Unknown Object (File)
Tue, Sep 30, 8:50 AM
Unknown Object (File)
Wed, Sep 24, 2:18 AM
Unknown Object (File)
Wed, Sep 24, 2:18 AM
Unknown Object (File)
Mon, Sep 15, 3:29 PM
Subscribers

Details

Reviewers
jhb
mav
markj
kib
Summary

Use the new nvme_allocate_request_buf after we vmapbuf the user data for
passthrough ioctls.

Sponsored by: Netflix

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 66442
Build 63325: arc lint + arc unit

Event Timeline

imp requested review of this revision.Aug 22 2025, 1:15 AM
This revision is now accepted and ready to land.Aug 22 2025, 12:49 PM

Btw, I do find it odd that the driver uses vmapbuf. Other code tends to instead use vm_fault_quick_hold_pages (or whatever it is called now) (see for example the AIO code) and then uses the vm_pages array from that directly. That avoids creating temporary kernel mappings with the associated cost of TLB shutdowns, etc.

In D52111#1190298, @jhb wrote:

Btw, I do find it odd that the driver uses vmapbuf. Other code tends to instead use vm_fault_quick_hold_pages (or whatever it is called now) (see for example the AIO code) and then uses the vm_pages array from that directly. That avoids creating temporary kernel mappings with the associated cost of TLB shutdowns, etc.

As discussed elsewhere, the next review will fix this by always unmapping. We'll eliminate the buf dance after the branch since we just need an array of pages, which in theory could live here on the stack... For. my use case, though, I'll likely just have two pages on the stack in the wrappers that will go around this...

(pick a random one I had opened): https://reviews.freebsd.org/D52149 has an update to this and other parts of this stack. We ditch the buf usage entirely, so all but related things aren't needed.