[Feature][Core] Support Fabric detection to adapt the MNNVL protocol for the GB series - #33540
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds support for detecting NVIDIA Fabric capabilities to adapt the MNNVL protocol, which should improve performance on systems with GB-series GPUs as shown by the benchmarks. The implementation correctly queries for fabric support and sets the appropriate memory allocation property. However, the new CUDA attributes used were introduced in CUDA 12.2. To maintain compatibility with older CUDA toolkits, the new code block should be conditionally compiled based on the CUDA version.
| @@ -115,6 +115,12 @@ void create_and_map(unsigned long long device, ssize_t size, CUdeviceptr d_mem, | |||
| if (flag) { // support GPUDirect RDMA if possible | |||
| prop.allocFlags.gpuDirectRDMACapable = 1; | |||
| } | |||
| int fab_flag = 0; | |||
| CUDA_CHECK(cuDeviceGetAttribute( | |||
| &fab_flag, CU_DEVICE_ATTRIBUTE_HANDLE_TYPE_FABRIC_SUPPORTED, device)); | |||
There was a problem hiding this comment.
There are cases where although Fabric seems supported, the actual allocation fails, can be tested with program below. So we should probably either have fallback or maybe better environment variable.
// gcc -L$CUDA_HOME/lib64 -lcuda -I$CUDA_HOME/include fabri.c -lcuda
#include <cuda.h>
#include <stdio.h>
#include <string.h>
#define CHECK_CUDA(call) \
do { \
CUresult _status = (call); \
if (_status != CUDA_SUCCESS) { \
const char *err; \
cuGetErrorString(_status, &err); \
fprintf(stderr, "%s:%d: CUDA error: %s\n", __FUNCTION__, __LINE__, err); \
return -1; \
} \
} while (0)
int main() {
CHECK_CUDA(cuInit(0));
CUdevice dev;
CHECK_CUDA(cuDeviceGet(&dev, 0));
int fabricSupported = 0;
CHECK_CUDA(cuDeviceGetAttribute( &fabricSupported,
CU_DEVICE_ATTRIBUTE_HANDLE_TYPE_FABRIC_SUPPORTED,
dev));
if (!fabricSupported) {
fprintf(stderr,
"FABRIC handle type NOT supported on this device\n");
return 0;
}
CUcontext ctx;
CHECK_CUDA(cuCtxCreate(&ctx, NULL, 0, dev));
const size_t size = 64 * 1024 * 1024; // 64 MB
CUmemAllocationProp prop;
memset(&prop, 0, sizeof(prop));
prop.type = CU_MEM_ALLOCATION_TYPE_PINNED;
prop.location.type = CU_MEM_LOCATION_TYPE_DEVICE;
prop.location.id = dev;
prop.requestedHandleTypes = CU_MEM_HANDLE_TYPE_FABRIC;
CUmemGenericAllocationHandle handle;
CHECK_CUDA(cuMemCreate(&handle, size, &prop, 0));
CUdeviceptr dptr;
CHECK_CUDA(cuMemAddressReserve(&dptr, size, 0, 0, 0));
CHECK_CUDA(cuMemMap(dptr, size, 0, handle, 0));
CUmemAccessDesc accessDesc;
memset(&accessDesc, 0, sizeof(accessDesc));
accessDesc.location.type = CU_MEM_LOCATION_TYPE_DEVICE;
accessDesc.location.id = dev;
accessDesc.flags = CU_MEM_ACCESS_FLAGS_PROT_READWRITE;
CHECK_CUDA(cuMemSetAccess(dptr, size, &accessDesc, 1));
printf("FABRIC pinned memory mapped at %p\n", (void*)dptr);
CHECK_CUDA(cuMemUnmap(dptr, size));
CHECK_CUDA(cuMemAddressFree(dptr, size));
CHECK_CUDA(cuMemRelease(handle));
CHECK_CUDA(cuCtxDestroy(ctx));
return 0;
}There was a problem hiding this comment.
I found this issue and discussed it with @youkaichao. I added a fallback strategy.
There was a problem hiding this comment.
CU_DEVICE_ATTRIBUTE_HANDLE_TYPE_FABRIC_SUPPORTED is more like querying the cuda version to see if the cuda supports this functionality. it does not check if multi-node nvlink is available.
There was a problem hiding this comment.
See https://docs.nvidia.com/cuda/cuda-driver-api/group__CUDA__TYPES.html
CU_DEVICE_ATTRIBUTE_HANDLE_TYPE_FABRIC_SUPPORTED = 128
Device supports exporting memory to a fabric handle with cuMemExportToShareableHandle() or requested with cuMemCreate()
Looking at this attr's description, it should be for detecting the capabilities of the device, but indeed on the Hopper GPU, even when this value is 1, it cannot create CU_MEM_HANDLE_TYPE_FABRIC. Should we remove this detection? But it seems that doing this detection doesn't matter either.
71b99a7 to
d5222fd
Compare
| #endif | ||
|
|
||
| #ifndef USE_ROCM | ||
| // Allocate memory using cuMemCreate | ||
| CUDA_CHECK(cuMemCreate(p_memHandle, size, &prop, 0)); | ||
| CUresult ret = (CUresult)cuMemCreate(p_memHandle, size, &prop, 0); | ||
| if (ret) { |
There was a problem hiding this comment.
can you fine-tune the error checking? only retry when ret is operation not permitted
There was a problem hiding this comment.
these two errors can be related: CUDA_ERROR_NOT_PERMITTED and CUDA_ERROR_NOT_SUPPORTED
|
I just started #33558 that contains C defines, vLLM environment and also activates KV cache with Fabric memory without sleep mode being needed. Feel free to take from it and put it here in this PR. |
d5222fd to
45d2d92
Compare
@tvegas1 I'd prefer this PR without an environment variable (the auto-retry has better user experience). @kebe7jun will separate the code out to be independent of sleep mode. |
| if (fab_flag && ret == cudaErrorNotPermitted) { | ||
| // Fabric allocation may fail on architectures other than Blackwell GPU, | ||
| // fallback to POSIX file descriptor | ||
| prop.requestedHandleTypes = CU_MEM_HANDLE_TYPE_POSIX_FILE_DESCRIPTOR; |
There was a problem hiding this comment.
Just for my understanding, why falling back on CU_MEM_HANDLE_TYPE_POSIX_FILE_DESCRIPTOR? Previously we would implicitly use CU_MEM_HANDLE_TYPE_NONE I think.
There was a problem hiding this comment.
CU_MEM_HANDLE_TYPE_NONE disables any IPC, CU_MEM_HANDLE_TYPE_POSIX_FILE_DESCRIPTOR makes it available for IPC via posix fd, which is available in all poxis systems.
I see, makes sense. |
…for the GB series Signed-off-by: Kebe <mail@kebe7jun.com> Co-authored-by: Thomas Vegas <tvegas@nvidia.com>
45d2d92 to
c972f13
Compare
Signed-off-by: youkaichao <youkaichao@gmail.com>
…for the GB series (vllm-project#33540) Signed-off-by: Kebe <mail@kebe7jun.com> Signed-off-by: youkaichao <youkaichao@gmail.com> Co-authored-by: Thomas Vegas <tvegas@nvidia.com> Co-authored-by: youkaichao <youkaichao@gmail.com> Signed-off-by: Pai <416932041@qq.com>
…for the GB series (vllm-project#33540) Signed-off-by: Kebe <mail@kebe7jun.com> Signed-off-by: youkaichao <youkaichao@gmail.com> Co-authored-by: Thomas Vegas <tvegas@nvidia.com> Co-authored-by: youkaichao <youkaichao@gmail.com> Signed-off-by: Pai <416932041@qq.com>
…for the GB series (vllm-project#33540) Signed-off-by: Kebe <mail@kebe7jun.com> Signed-off-by: youkaichao <youkaichao@gmail.com> Co-authored-by: Thomas Vegas <tvegas@nvidia.com> Co-authored-by: youkaichao <youkaichao@gmail.com> Signed-off-by: felix01.yu <felix01.yu@vipshop.com>
…for the GB series (vllm-project#33540) Signed-off-by: Kebe <mail@kebe7jun.com> Signed-off-by: youkaichao <youkaichao@gmail.com> Co-authored-by: Thomas Vegas <tvegas@nvidia.com> Co-authored-by: youkaichao <youkaichao@gmail.com>
…for the GB series (vllm-project#33540) Signed-off-by: Kebe <mail@kebe7jun.com> Signed-off-by: youkaichao <youkaichao@gmail.com> Co-authored-by: Thomas Vegas <tvegas@nvidia.com> Co-authored-by: youkaichao <youkaichao@gmail.com>
…for the GB series (vllm-project#33540) Signed-off-by: Kebe <mail@kebe7jun.com> Signed-off-by: youkaichao <youkaichao@gmail.com> Co-authored-by: Thomas Vegas <tvegas@nvidia.com> Co-authored-by: youkaichao <youkaichao@gmail.com>
Purpose
Fix #33539
Thank you very much for the solution provided by @tvegas1
Test Plan
Note that, currently, to use a custom allocator, sleep-mode must to be enabled.
Bench:
I have tested it on both Hopper and Blackwell GPUs and it passed.
Test Result
In my env, the throughput increased threefold:
Main:
This PR:
The reason for the improved performance is that my GB200 environment is not configured with RoCE network, resulting in abnormal performance. But at this point, switching directly to the MNNVL protocol will become very stable
From the log, we can clearly see that it has been using
cuda_ipc(MNNVL) as the cross-node transmission protocol.Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.