core/verbs: Add native API support for DMA-buf regions #9343

shefty · 2023-09-16T00:02:53Z

This expands the memory registration API to support passing in a DMA-buf region. DMA buf regions are specified using a struct fi_mr_dmabuf. The struct fi_mr_attr is updated to accept passing in the dmabuf structure as input, with a flag used to indicate that the registration is for a DMA-buf region.

The verbs provider is updated to support DMA-buf registration.

Changes are compile tested only. There's no test yet available to verify functionality.

j-xiong · 2023-09-16T00:14:12Z

prov/verbs/src/verbs_mr.c

+		return vrb_reg_dmabuf(domain, attr->dmabuf->fd,
+				      attr->dmabuf->offset, attr->dmabuf->len,
+				      vrb_mr_ofi2ibv_access(attr->access, domain),
+				      mr);


This bypasses the reg cache.

That was intentional because I don't know how we deal with a dma-buf in the cache or if we need to.

Good for now. In the long run, we may still want the registration be cached, probably by using a separate mr cache that use <fd, offset> instead of addr as the key.

The problem is that I don't know how you verify that the entry is still valid or eject it from the cache.

shefty · 2023-09-16T00:29:39Z

@j-xiong - It looks like we might be able to adapt fi_mr_reg_xe and fi_rdmabw-xe to verify the new registration call.

j-xiong · 2023-09-16T00:34:39Z

Yes, the existing component tests can be extended to have an option to use the new API.

j-xiong · 2023-09-16T00:41:12Z

man/fi_mr.3.md

@@ -740,6 +775,11 @@ The follow flag may be specified to any memory registration call.
  API AllocHost call (as opposed to using malloc-like host allocation,
  unified/shared memory allocation, or AllocDevice).

+*FI_MR_DMABUF*


Should this also be a capability bit so that the application can check if this supported by the provider?

Maybe, and I had the same thought. Can the current FI_HMEM work, which would require providers support FI_HMEM to also support dmabuf registration? I'd like to avoid capability bit overload when possible. But I'm not sure if FI_HMEM is sufficient.

I was thinking that a provider could call mmap() to map the region into the VA space, then treat the dmabuf the same as a VA registered region. The provider may need to discover what hmem API was used to allocate the region. Is there an issue if the provider were forced down this path? IOW, dmabuf support becomes an optimization that the provider could leverage, otherwise it needs to fallback to whatever its standard HMEM support is.

DMA-buf support is handled internally by some providers. However, there's no mechanism for an application to pass dma-buf mapped buffers directly to them. Add this to the MR API using the HMEM infrastructure. Signed-off-by: Sean Hefty <[email protected]>

shefty · 2023-09-18T18:07:35Z

Updated: I changed the value of FI_MR_DMABUF, so that it could be defined as a capability bit in the future, if needed. However, I left it defined only as a memory registration flag. I added a requirement that the flag requires FI_HMEM support.

j-xiong

Good enough for starting to write tests.

shefty · 2023-09-18T21:55:44Z

@j-xiong - I added an update to fi-mr-reg-xe. It's not tested (I couldn't even build that on my system, so I need to move my patches.) The Intel CI passed, but I'm not sure that test is built or run. Only verbs supports the new API directly, but it looks like rxm should pass the call through okay.

j-xiong · 2023-09-18T23:39:21Z

The test passed:

$ ./fi_xe_mr_reg -m device -S 1048576
Using first driver: 0x2fc8950 (total >= 1)
using GPU device 0: 0x2d89fd0 (total 2)
Using OFI device: verbs;ofi_rxm (mlx5_0)
mr 0x30c4dc0, buf 0xff007fffffef0000, rkey 0x175916, len 1048576
mr 0x30c4dc0, buf 0xff007fffffef0000, rkey 0x175916, len 1048576

j-xiong · 2023-09-18T23:42:11Z

fabtests/component/dmabuf-rdma/fi-mr-reg-xe.c

+	printf("mr %p, buf %p, rkey 0x%lx, len %zd\n",
+		mr, buf, fi_mr_key(mr), buf_size);


should print out dmabuf_mr instead of mr, same for the mr key.

Thanks - updated. And thanks for testing. :)

j-xiong · 2023-09-18T23:53:35Z

The rkey doen't look right:

 ./fi_xe_mr_reg -m device -S 1048576
Using first driver: 0x2bda950 (total >= 1)
using GPU device 0: 0x299bfd0 (total 2)
Using OFI device: verbs;ofi_rxm (mlx5_0)
mr 0x2d034e0, buf 0xff007fffffef0000, rkey 0x1738f7, len 1048576
mr 0x2d03690, buf 0xff007fffffef0000, rkey 0x0, len 1048576

shefty · 2023-09-18T23:59:14Z

I see the issue. The libfabric fid_mr wasn't initialized after the register call returned.

shefty · 2023-09-19T00:00:55Z

I need to either go through vrb_mr_reg_common() or duplicate the functionality after ibv_reg_mr().

Support callers using dma-buf registatration directly. Signed-off-by: Sean Hefty <[email protected]>

Add direct testing of dmabuf memory registration. Signed-off-by: Sean Hefty <[email protected]>

shefty · 2023-09-20T18:49:51Z

Restructured the verbs changes to go through vrb_mr_reg_common(), so that the memory region is properly initialized.

j-xiong · 2023-09-20T19:24:38Z

Works fine now:

$ ./fi_xe_mr_reg -m device -S 1048576
Using first driver: 0x27c9950 (total >= 1)
using GPU device 0: 0x258afd0 (total 2)
Using OFI device: verbs;ofi_rxm (mlx5_0)
mr 0x28df930, buf 0xff007fffffef0000, rkey 0x1726e5, len 1048576
mr 0x28f2450, buf 0xff007fffffef0000, rkey 0x17f6b5, len 1048576

shefty · 2023-09-21T16:15:34Z

bot:aws:retest

shefty requested a review from j-xiong September 16, 2023 00:02

j-xiong reviewed Sep 16, 2023

View reviewed changes

shefty force-pushed the main branch from 129089d to f59d9c8 Compare September 18, 2023 18:05

j-xiong approved these changes Sep 18, 2023

View reviewed changes

j-xiong reviewed Sep 18, 2023

View reviewed changes

shefty force-pushed the main branch from 3c1f952 to ac7115c Compare September 18, 2023 23:44

shefty added the ⚠️ Do not merge label Sep 19, 2023

shefty added 2 commits September 20, 2023 11:47

prov/verbs: Add support for DMA-buf memory registration

a32cadd

Support callers using dma-buf registatration directly. Signed-off-by: Sean Hefty <[email protected]>

fabtests/fi-mr-reg-xe: Add testing for dmabuf registration

8a914c7

Add direct testing of dmabuf memory registration. Signed-off-by: Sean Hefty <[email protected]>

shefty force-pushed the main branch from ac7115c to 8a914c7 Compare September 20, 2023 18:48

shefty removed the ⚠️ Do not merge label Sep 20, 2023

shefty merged commit d8e987a into ofiwg:main Sep 21, 2023
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

core/verbs: Add native API support for DMA-buf regions #9343

core/verbs: Add native API support for DMA-buf regions #9343

shefty commented Sep 16, 2023

j-xiong Sep 16, 2023

shefty Sep 16, 2023

j-xiong Sep 16, 2023

shefty Sep 16, 2023

shefty commented Sep 16, 2023

j-xiong commented Sep 16, 2023

j-xiong Sep 16, 2023

shefty Sep 16, 2023

shefty commented Sep 18, 2023

j-xiong left a comment

shefty commented Sep 18, 2023

j-xiong commented Sep 18, 2023

j-xiong Sep 18, 2023

shefty Sep 18, 2023

j-xiong commented Sep 18, 2023

shefty commented Sep 18, 2023

shefty commented Sep 19, 2023

shefty commented Sep 20, 2023

j-xiong commented Sep 20, 2023

shefty commented Sep 21, 2023

		printf("mr %p, buf %p, rkey 0x%lx, len %zd\n",
		mr, buf, fi_mr_key(mr), buf_size);

core/verbs: Add native API support for DMA-buf regions #9343

core/verbs: Add native API support for DMA-buf regions #9343

Conversation

shefty commented Sep 16, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shefty commented Sep 16, 2023

j-xiong commented Sep 16, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shefty commented Sep 18, 2023

j-xiong left a comment

Choose a reason for hiding this comment

shefty commented Sep 18, 2023

j-xiong commented Sep 18, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

j-xiong commented Sep 18, 2023

shefty commented Sep 18, 2023

shefty commented Sep 19, 2023

shefty commented Sep 20, 2023

j-xiong commented Sep 20, 2023

shefty commented Sep 21, 2023