hmem: Define ofi_hmem_put_dmabuf_fd #10716
Conversation
For some HMEM ifaces, ofi_hmem_get_dmabuf_fd() may result in a new FD being allocated. Define ofi_hmem_put_dmabuf_fd() to close FD. Signed-off-by: Ian Ziemba <[email protected]>
With ROCR, callers of ofi_hmem_get_dmabuf_fd() should call ofi_hmem_put_dmabuf_fd() once the DMA buf region is no longer used. Signed-off-by: Ian Ziemba <[email protected]>
|
would u mind implementing this for cuda as well? it should be simply a |
| @@ -131,6 +131,7 @@ struct ofi_hmem_ops { | |||
| const void *src, size_t size); | |||
| int (*get_dmabuf_fd)(const void *addr, uint64_t size, int *fd, | |||
| uint64_t *offset); | |||
| int (*put_dmabuf_fd)(int fd); | |||
There was a problem hiding this comment.
Why not just name it as close_dmabuf_fd ?
There was a problem hiding this comment.
I chose put_dmabuf_fd to semantically align with get_dmabuf_fd.
There was a problem hiding this comment.
is there any hmem type where a get_dmabuf_fd does not implicitly increase the open count on the file? if not, I'd recommend going the other way and renaming get to open so that it's clear that it's not simply resolving an already-open file descriptor, but is actually opening a resource which must be eventually closed.
There was a problem hiding this comment.
yes, the ZE hmem type always return the same fd.
With CUDA, callers of ofi_hmem_get_dmabuf_fd() should call ofi_hmem_put_dmabuf_fd() once the DMA buf region is no longer used. Signed-off-by: Ian Ziemba <[email protected]>
Signed-off-by: Ian Ziemba <[email protected]>
Performing multiple HSA allocations appears to result in a DMA buf offset. Verify that the CXI provider can register a DMA buf offset memory region. Signed-off-by: Ian Ziemba <[email protected]>
When a MR is freed, the CXI provider should free the DMA buf FD used for the ROCR region. Failing to do this will result in FDs being exhausted. Signed-off-by: Ian Ziemba <[email protected]>
When a MR is freed, the CXI provider should free the DMA buf FD used for the CUDA region. Failing to do this will result in FDs being exhausted. Signed-off-by: Ian Ziemba <[email protected]>
|
@shijin-aws : I added CUDA support. If you could review, that would be great. @swelch : If you could review CXI prov changes, that would be great. Thanks! |
|
@j-xiong is intel ci failure related? |
|
@shijin-aws We are having slurm issues right now. The tests didn't actually run. |
|
@j-xiong is Intel CI still having issues? |
|
There are two intermittent failures with oneCCL over tcp provider. They are unrelated to this PR. |
| @@ -575,6 +589,9 @@ static void cxip_unmap_nocache(struct cxip_md *md) | |||
| { | |||
| int ret; | |||
|
|
|||
| if (md->dmabuf_fd_valid) | |||
| ofi_hmem_put_dmabuf_fd(md->info.iface, md->dmabuf_fd); | |||
There was a problem hiding this comment.
I'd be curious to know why cxi requires that the close of the dmabuf is deferred until point of unmapping it? Why can't you just close it immediately after you do the map? I thought this was a property of the dmabuf kernel interface and not something that would be different between providers, but I don't know this code well and might be missing something.
There was a problem hiding this comment.
I assume the kernel driver would reference count properly so that closing right after mapping should work just fine. I don't see the drawback of doing either way.
For some HMEM ifaces, ofi_hmem_get_dmabuf_fd() may result in a new FD being allocated. Define ofi_hmem_put_dmabuf_fd() to close FD.
Support ofi_hmem_get_dmabuf_fd() with ROCR.
Update CXI provider accordingly.