nvme_driver: allocate different DMA memory sizes if not bounce buffering #1306
Conversation
chris-oo
changed the title
| ); | ||
| let alloc: PageAllocator = | ||
| PageAllocator::new(mem.subblock(data_offset, QueuePair::PER_QUEUE_PAGES * PAGE_SIZE)); | ||
| let alloc = if bounce_buffer { |
There was a problem hiding this comment.
Feels like this expression can be shortened? E.g. use one PageAllocator::new after checking all asserts
There was a problem hiding this comment.
This assert is probably not needed at all?
There was a problem hiding this comment.
hmm the assert is checking the minimum size is large enough for the given commands right? I don't know enough to say if the assert should be removed or not, but it still seems useful?
There was a problem hiding this comment.
Sorry, what I meant is - we are asserting if one constant is greater than another constant. Can be done statically outside of this function, I think. But okay, leave it.
There was a problem hiding this comment.
this is a static assert, since it's a const evaluation at compile time. A bit confusing, but it doesn't require taking a dependency on static_assert which i think does the same thing under the hood.
| @@ -266,7 +266,7 @@ async fn test_nvme_save_restore_inner(driver: DefaultDriver) { | |||
| .unwrap(); | |||
|
|
|||
| let device = NvmeTestEmulatedDevice::new(nvme_ctrl, msi_x, dma_client.clone()); | |||
| let mut nvme_driver = NvmeDriver::new(&driver_source, CPU_COUNT, device) | |||
| let mut nvme_driver = NvmeDriver::new(&driver_source, CPU_COUNT, device, false) | |||
There was a problem hiding this comment.
TODO: add a test case with true
| // buffering / double buffering is needed. | ||
| let alloc_len = if bounce_buffer { | ||
| const SIZE: usize = QueuePair::PER_QUEUE_PAGES_BOUNCE_BUFFER * PAGE_SIZE; | ||
| const _: () = assert!( |
There was a problem hiding this comment.
Can we package this up as a local const fn? E.g.,
const fn pages_to_size(pages: usize) -> usize {
let size = pages * PAGE_SIZE;
assert!(size >= ...);
size
}
// Note: using const {} to ensure the internal assertion checks are at compile time
let alloc_len = if bounce_buffer {
const { pages_to_size(QueuePair::PER_QUEUE_PAGES_BOUNCE_BUFFER) }
} else {
const { pages_to_size(QueuePair::PER_QUEUE_PAGES_NO_BOUNCE_BUFFER) }
};
A previous change increased the DMA memory allocated per queue to 128 pages, to handle the need for additional pages to bounce buffer on CVMs. Make this configurable, so that the pages allocated go back to the previous of 64, when not isolated.