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

prov/efa: fix leak of dmabuf fd in cuda p2p probe #10708

Merged
merged 1 commit into from
Jan 23, 2025

Conversation

aws-nslick
Copy link
Contributor

prior to this patch, when efa_hmem_info_check_p2p_support_cuda elected to attempt dmabuf for p2p, we previously leaked the file descriptor returned by cuMemGetHandleForAddressRange in all cases. This ultimately meant the dmabuf stuck around for the lifetime of the process, even after dereg and after releasing the memory back to the device mempool.

All calls to cuda_get_dmabuf_fd need a corresponding close call.

prior to this patch, when efa_hmem_info_check_p2p_support_cuda elected
to attempt dmabuf for p2p, we previously leaked the file descriptor
returned by cuMemGetHandleForAddressRange in all cases. This ultimately
meant the dmabuf stuck around for the lifetime of the process, even
after dereg and after releasing the memory back to the device mempool.

All calls to cuda_get_dmabuf_fd need a corresponding close call.

Signed-off-by: Nicholas Sielicki <[email protected]>
@darrylabbate darrylabbate requested a review from a team January 17, 2025 22:10
@shijin-aws
Copy link
Contributor

AWS CI failure is likely an infra issue. Will fix it and retest

@shijin-aws
Copy link
Contributor

bot:aws:retest

@iziemba
Copy link
Contributor

iziemba commented Jan 22, 2025

In #10716, I have defined ofi_hmem_put_dmabuf_fd() for CUDA which could be used instead of close().

@shijin-aws shijin-aws merged commit 83ab8a3 into ofiwg:main Jan 23, 2025
12 of 13 checks passed
@aws-nslick aws-nslick deleted the fdreakaleak branch January 23, 2025 20:04
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