Skip to content

UCM/CUDA: Correct region size for pitched CUDA allocation hooks #10527

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

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

Conversation

uranix
Copy link

@uranix uranix commented Mar 1, 2025

What?

For pitched CUDA memory allocations (cuMemAllocPitch, cuMemAllocPitch_v2, cudaMallocPitch, cudaMalloc3D) the allocated region may be bigger than requested. The UCM code incorrectly marked those regions to have width * height[ * depth] size, but their actual size is pitch * height[ * depth].

Why?

Incorrect allocated region size leads to part of it having incorrect UCS_MEMORY_TYPE and further attempts to access device memory as host memory.

Closes #10526

@uranix uranix force-pushed the master branch 2 times, most recently from 3c9dcc2 to 985bcc6 Compare March 1, 2025 19:34
@uranix uranix marked this pull request as ready for review March 2, 2025 15:28
@uranix
Copy link
Author

uranix commented Mar 3, 2025

@hoopoepg Please have a look at this PR when you get a chance. There are some failed checks, but I don't think they are related

@hoopoepg hoopoepg requested a review from brminich March 3, 2025 09:33
@tvegas1
Copy link
Contributor

tvegas1 commented Mar 3, 2025

hello, have you or your organization signed the CLA https://openucx.org/license/?

@uranix
Copy link
Author

uranix commented Mar 3, 2025

No, not yet. Planning to sign an individual CLA, because PR is unrelated to my organization. Is it still possible to send a signed version via email as described here?

UPD: Sorry, found it at the very top of "Guidance for contributors" page

yosefe
yosefe previously approved these changes Mar 5, 2025
rakhmets
rakhmets previously approved these changes Mar 6, 2025
@uranix uranix dismissed stale reviews from rakhmets and yosefe via bc5fc17 March 6, 2025 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Segfault in uct_am_short_fill_data when transferring cudaMalloc3D allocated regions
4 participants