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

UCT/CUDA/CUDA_COPY: Enabled memory attributes query after switching CUDA GPU. #10388

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

rakhmets
Copy link
Contributor

@rakhmets rakhmets commented Dec 17, 2024

What?

Enabled memory attributes query by cuda_cpy memory domain after switching CUDA GPU.
Added test.

Without the changes in cuda_cpy memory domain the test fails with the following error:

cuda_copy_md.c:649  UCX  ERROR cuMemGetAddressRange(0x7f8cd9e00000) error: named symbol not found

@rakhmets rakhmets force-pushed the topic/gtest-switch-gpu branch from d31bb61 to 1adcb5e Compare December 17, 2024 18:09
class test_ucp_mmap_mgpu : public ucs::test {
};

UCS_TEST_F(test_ucp_mmap_mgpu, switch_gpu) {
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need to add tests for transfer cuda_copy or cuda_ipc, mem type cuda or vmm/mallocasync:

  • buf1 on device1, buf2 on device2, copy happening under progress with context of unrelated device3

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree, we will need more test cases.
The test in this PR is the simplest one. And it fails. Once it is fixed, we will add more scenarios.

@rakhmets rakhmets marked this pull request as ready for review January 15, 2025 17:50
@brminich
Copy link
Contributor

brminich commented Feb 7, 2025

/azp run UCX PR

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rakhmets rakhmets force-pushed the topic/gtest-switch-gpu branch from 4adf57b to 6d38f73 Compare February 11, 2025 19:36
@rakhmets rakhmets requested review from tvegas1 and brminich February 12, 2025 11:33
@rakhmets rakhmets changed the title TEST/GTEST: Added cuda gpu switching testing. UCT/CUDA/CUDA_COPY: Enabled switching CUDA GPUs between memory allocation and memory mapping. Feb 12, 2025
src/uct/cuda/cuda_copy/cuda_copy_md.c Outdated Show resolved Hide resolved
src/uct/cuda/cuda_copy/cuda_copy_md.c Outdated Show resolved Hide resolved
src/uct/cuda/cuda_copy/cuda_copy_md.c Outdated Show resolved Hide resolved
brminich
brminich previously approved these changes Feb 12, 2025
@rakhmets rakhmets added the WIP-DNM Work in progress / Do not review label Feb 13, 2025
@rakhmets rakhmets removed the WIP-DNM Work in progress / Do not review label Feb 13, 2025
@rakhmets rakhmets changed the title UCT/CUDA/CUDA_COPY: Enabled switching CUDA GPUs between memory allocation and memory mapping. UCT/CUDA/CUDA_COPY: Enabled memory attributes query after switching CUDA GPU. Feb 13, 2025
ASSERT_EQ(cudaGetDevice(&device), cudaSuccess);
ASSERT_EQ(cudaSetDevice((device + 1) % num_devices), cudaSuccess);

const int size = 16;
Copy link
Contributor

Choose a reason for hiding this comment

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

cosmetic: size_t looks more appropriate

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.

4 participants