Skip to content

Improvement potentials & questions regarding MemoryRegion<T> implementation #57

@kyle-yh-kim

Description

@kyle-yh-kim

Hi folks, I have a few questions regarding struct MemoryRegion<T> implementation;

1. Why the strict 1:1 relation between data: Vec<T> to MemoryRegion?.

Theoretically, RDMA-core/libverbs allows for complex relationships between buffer and MemoryRegion. There are beneficial use cases to this, such as a shared buffer between two MemoryRegions with different access flags.

Reading impl MemoryRegion, this is not allowed for the case of rust-ibverbs. As for its tradeoff, I would assume the safety (no allowance of shared buffers represented under *mut c_void) was prioritized over supporting complex buffer to MemoryRegion relationships. Would be great to gain confirmation on whether this was the thought process behind.

2. Why data: Vec<T> and not data: T?

Say the user intends to pass a single MyStruct (custom class, not a primitive data-type such as u64), this would still have to be wrapped under a Vec<MyStruct> of length 1, adding an unnecessary layer of indirection.

Whereas, if the data could represent a generic type data: T, both Vec<u64> and MyStruct would satisfy the generic data: T parameter. And rather than enforcing the Type to implement Sized Trait, the library would ask the caller to provide an additional size: usize parameter to satisfy ffi::ibv_reg_mr(..., size_t length, ...) argument.

I'm wondering why this alternative was not selected. A few potential explanations could be;

  1. Vector is just being used as a simple container for Heap space allocation, equivalent to malloc(sizeof(T) * N) in C.
  2. Standardized *mut c_void pointer casting via Vector's data.as_mut_ptr(). While this could be overcome by a custom trait, say - Trait IntoRawMut, that would require all types passed into data: T to implement this custom trait - which is not ideal. POC implementation is attached below [Rust playground link].
use std::ffi::c_void;

pub trait IntoRawMut {
    fn into_raw_mut(&mut self) -> *mut c_void;
}

#[derive(Debug)]
struct MyStruct {
    key: u64,
    val: u64,
}

#[derive(Debug)]
// Type T must implement IntoRaw trait for raw pointer coersion.
struct MemoryRegion<T: IntoRawMut> {
    // mr: *mut ffi::ibv_mr, // For example's sake, ignoring the ffi raw pointer for now.
    data: T,
    size: usize,
}

impl IntoRawMut for MyStruct {
    fn into_raw_mut(&mut self) -> *mut c_void {
        self as *mut Self as *mut _
    }
}

impl<T> IntoRawMut for Vec<T> {
    fn into_raw_mut(&mut self) -> *mut c_void {
        self.as_mut_ptr() as *mut _
    }
}

use std::ops::{Deref, DerefMut};
impl<T: IntoRawMut> Deref for MemoryRegion<T> {
    type Target = T;
    fn deref(&self) -> &Self::Target {
        &self.data
    }
}

impl<T: IntoRawMut> DerefMut for MemoryRegion<T> {
    fn deref_mut(&mut self) -> &mut Self::Target {
        &mut self.data
    }
}

fn main() {
    // Custom struct.
    let mut mr = MemoryRegion {
        data: MyStruct { key: 0, val: 0 },
        size: size_of::<MyStruct>(),
    };
    (*mr).key = 1;
    println!("{:?}", mr); // MemoryRegion { data: MyStruct { key: 1, val: 0 }, size: 16 }
    println!("{:?}", (*mr).into_raw_mut()); // 0x7ffcf75223d0. This could be then fed into ffi::ibv_reg_mr(...);
    
    // Vector.
    let mut mr = MemoryRegion {
        data: vec![1, 1, 1, 1, 1],
        size: 5 * size_of::<i32>(),
    };
    mr[0] = 2;
    println!("{:?}", mr); // MemoryRegion { data: [2, 1, 1, 1, 1], size: 20 }
    println!("{:?}", (*mr).into_raw_mut()); // 0x5d01ed96bb10. This could be then fed into ffi::ibv_reg_mr(...);

Reference. Existing MemoryRegion<T> implementation.

pub struct MemoryRegion<T> {
    mr: *mut ffi::ibv_mr,
    data: Vec<T>,
}

impl<T> Deref for MemoryRegion<T> {
    type Target = [T];
    fn deref(&self) -> &Self::Target {
        &self.data[..]
    }
}

impl<T> DerefMut for MemoryRegion<T> {
    fn deref_mut(&mut self) -> &mut Self::Target {
        &mut self.data[..]
    }
}

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions