Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nvme: allocate aligned payloads for all nvme commands #2051

Merged
merged 6 commits into from
Sep 20, 2023

Conversation

igaw
Copy link
Collaborator

@igaw igaw commented Sep 14, 2023

The kernel supports since v5.2 direct mapped DMA buffers to userspace.
Up to this point a bounce buffer was involved. Because the buffers are
now directly accessed by the device, the rules of alignment also apply
for the payloads.

Furthermore, ensure that the buffer is a multiple of 4k, because there
are devices on the market which will always transfer a multiple of 4k,
even if we ask for less, e.g 512 bytes. This avoid stack smashes.

Fixes: #linux-nvme/libnvme#684

The nvme_alloc has already a built in check which version of the
allocation strategy should be used based on the len argument. There is
no need for fw_download to do this as well.

Signed-off-by: Daniel Wagner <dwagner@suse.de>
These two function are abstracting the libhugebtl API. Let's rename
Rename these functions because we want to introduce a generic alloc
function with alignment support.

Signed-off-by: Daniel Wagner <dwagner@suse.de>
Introduce a cleanup helper function to free generic memory allocation
from malloc, calloc, etc.

Signed-off-by: Daniel Wagner <dwagner@suse.de>
The kernel supports since v5.2 direct mapped DMA buffers to userspace.
Up to this point a bounce buffer was involved. Because the buffers are
now directly accessed by the device, the rules of alignment also apply
for the payloads.

Introduce a helper to allocate all correctly buffers aligned.

Furthermore, ensure that the buffer is a multiple of 4k, because there
are devices on the market which will always transfer a multiple of 4k,
even if we ask for less, e.g 512 bytes. This avoid stack smashes.

Signed-off-by: Daniel Wagner <dwagner@suse.de>
@igaw igaw force-pushed the fix-alloc branch 4 times, most recently from 11f3426 to 372cdf9 Compare September 15, 2023 10:08
The kernel supports since v5.2 direct mapped DMA buffers to userspace.
Up to this point a bounce buffer was involved. Because the buffers are
now directly accessed by the device, the rules of alignment also apply
for the payloads.

Use the newly introduced nvme_alloc helper to allocate correctly aligned
payloads for all nvme commands.

Signed-off-by: Daniel Wagner <dwagner@suse.de>
Use the cleanup infra structure to free the nvme_dev resource. This
allows us to use return directly and makes the code a bit more readable.

While at it also do the same for trivial filedescriptor cases. The more
complex cases such in submit_io() needs a bit more tinkering to get
right.

Signed-off-by: Daniel Wagner <dwagner@suse.de>
@igaw
Copy link
Collaborator Author

igaw commented Sep 15, 2023

I've added a cleanup patch on top for the nvme_dev resource. This makes the code slightly more cleaner and easier to follow. Obviously, there is more things we can tidy up with the auto cleanup hooks. I think we could also close all file handles this way in submit_io and co which could make the whole error handling way simpler. Currently, this function is really really hard to read and I wouldn't surprised if there are still bugs lurking in these code paths.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant