Skip to content

Conversation

@dragonJACson
Copy link
Contributor

No description provided.

@codecov
Copy link

codecov bot commented Feb 19, 2025

Codecov Report

Attention: Patch coverage is 95.83333% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/ibverbs/memory_region.rs 95.00% 1 Missing ⚠️
Files with missing lines Coverage Δ
src/ibverbs/protection_domain.rs 100.00% <100.00%> (+8.10%) ⬆️
src/ibverbs/memory_region.rs 96.77% <95.00%> (+0.22%) ⬆️

@dragonJACson dragonJACson force-pushed the dev/stride-support branch 4 times, most recently from bdea8bd to c90fb93 Compare February 21, 2025 16:26
@dragonJACson dragonJACson changed the title feat(ibverbs): provide more interfaces for registering user managed MR feat(ibverbs): only provide unsafe interface for registering MR Feb 22, 2025
/// and that the memory remains accessible and unmodified as needed.
pub(crate) unsafe fn reg_mr(
pd: &'pd ProtectionDomain, ptr: usize, len: usize, access: AccessFlags,
) -> Result<Self, RegisterMemoryRegionError> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Types like Vec<T> would reallocate and move memory around, which would break the rule of MR (need Pinned memory), but use something like Pin<T> as parameter is also not pretty feasible for CUDA memory etc.. Do you think provide an interface for primitive types with Pin<T> and lifetime bound, another interface with bare usize as pointer a good idea? @FujiZ

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not familiar with the Pin<T> API, could you demostrate the usage of Pin<T> with reg_mr in another MR once this one is merged?

Signed-off-by: Luke Yue <lukedyue@gmail.com>
@dragonJACson dragonJACson merged commit d06c01e into main Feb 26, 2025
5 checks passed
@dragonJACson dragonJACson deleted the dev/stride-support branch December 6, 2025 14:28
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.

3 participants