From 8f878609dfb9fba4d195e5a728d3d52d9eabb6eb Mon Sep 17 00:00:00 2001 From: Michal Shalev Date: Tue, 16 Sep 2025 18:08:45 +0300 Subject: [PATCH 1/4] Revert to gpuXferReqH Signed-off-by: Michal Shalev --- src/plugins/ucx/ucx_backend.cpp | 10 +--- ...device_mem_list.cpp => gpu_xfer_req_h.cpp} | 55 +++++++++---------- .../{device_mem_list.h => gpu_xfer_req_h.h} | 22 ++++---- src/utils/ucx/meson.build | 2 +- 4 files changed, 41 insertions(+), 48 deletions(-) rename src/utils/ucx/{device_mem_list.cpp => gpu_xfer_req_h.cpp} (67%) rename src/utils/ucx/{device_mem_list.h => gpu_xfer_req_h.h} (74%) diff --git a/src/plugins/ucx/ucx_backend.cpp b/src/plugins/ucx/ucx_backend.cpp index 0d3ac24409..d306cecd7e 100644 --- a/src/plugins/ucx/ucx_backend.cpp +++ b/src/plugins/ucx/ucx_backend.cpp @@ -19,7 +19,7 @@ #include "common/nixl_log.h" #include "serdes/serdes.h" #include "common/nixl_log.h" -#include "ucx/device_mem_list.h" +#include "ucx/gpu_xfer_req_h.h" #include #include @@ -1666,10 +1666,7 @@ nixlUcxEngine::createGpuXferReq(const nixlBackendReqH &req_hndl, } try { - auto device_mem_list = - std::make_unique(*ep, local_mems, remote_rkeys); - gpu_req_hndl = device_mem_list->get(); - device_mem_list.release(); + gpu_req_hndl = nixl::ucx::gpuXferReqH::create(*ep, local_mems, remote_rkeys); return NIXL_SUCCESS; } catch (const std::exception &e) { @@ -1680,8 +1677,7 @@ nixlUcxEngine::createGpuXferReq(const nixlBackendReqH &req_hndl, void nixlUcxEngine::releaseGpuXferReq(nixlGpuXferReqH gpu_req_hndl) const { - nixl::ucx::deviceMemList device_mem_list{gpu_req_hndl}; - gpu_req_hndl = nullptr; + nixl::ucx::gpuXferReqH::release(gpu_req_hndl); } nixl_status_t diff --git a/src/utils/ucx/device_mem_list.cpp b/src/utils/ucx/gpu_xfer_req_h.cpp similarity index 67% rename from src/utils/ucx/device_mem_list.cpp rename to src/utils/ucx/gpu_xfer_req_h.cpp index ab6460da94..83174cad1b 100644 --- a/src/utils/ucx/device_mem_list.cpp +++ b/src/utils/ucx/gpu_xfer_req_h.cpp @@ -15,10 +15,7 @@ * limitations under the License. */ -#include "device_mem_list.h" - -#include - +#include "gpu_xfer_req_h.h" #include "common/nixl_log.h" #include "ucx_utils.h" #include "rkey.h" @@ -28,25 +25,30 @@ namespace nixl::ucx { #ifdef HAVE_UCX_GPU_DEVICE_API -deviceMemList::deviceMemList(const nixlUcxEp &ep, - const std::vector &local_mems, - const std::vector &remote_rkeys) - : deviceMemList_{createDeviceMemList(ep, local_mems, remote_rkeys), - &ucp_device_mem_list_release} {} +nixlGpuXferReqH +gpuXferReqH::create(const nixlUcxEp &ep, + const std::vector &local_mems, + const std::vector &remote_rkeys) { + ucp_device_mem_list_handle_h ucx_handle = createDeviceMemList(ep, local_mems, remote_rkeys); + return reinterpret_cast(ucx_handle); +} -deviceMemList::deviceMemList(const nixlGpuXferReqH gpu_req) noexcept - : deviceMemList_{reinterpret_cast(gpu_req), - &ucp_device_mem_list_release} {} +void +gpuXferReqH::release(nixlGpuXferReqH gpu_req) { + if (gpu_req == nullptr) { + NIXL_WARN << "Attempting to release null GPU transfer request handle"; + return; + } -nixlGpuXferReqH -deviceMemList::get() const noexcept { - return reinterpret_cast(deviceMemList_.get()); + ucp_device_mem_list_handle_h ucx_handle = + reinterpret_cast(gpu_req); + ucp_device_mem_list_release(ucx_handle); } ucp_device_mem_list_handle_h -deviceMemList::createDeviceMemList(const nixlUcxEp &ep, - const std::vector &local_mems, - const std::vector &remote_rkeys) { +gpuXferReqH::createDeviceMemList(const nixlUcxEp &ep, + const std::vector &local_mems, + const std::vector &remote_rkeys) { nixl_status_t status = ep.checkTxState(); if (status != NIXL_SUCCESS) { throw std::runtime_error("Endpoint not in valid state for creating memory list"); @@ -93,20 +95,17 @@ deviceMemList::createDeviceMemList(const nixlUcxEp &ep, #else -deviceMemList::deviceMemList(const nixlUcxEp &ep, - const std::vector &local_mems, - const std::vector &remote_rkeys) { +nixlGpuXferReqH +gpuXferReqH::create(const nixlUcxEp &ep, + const std::vector &local_mems, + const std::vector &remote_rkeys) { NIXL_ERROR << "UCX GPU device API not supported"; throw std::runtime_error("UCX GPU device API not available"); } -deviceMemList::deviceMemList(const nixlGpuXferReqH gpu_req) noexcept { - NIXL_WARN << "UCX GPU device API not supported - cannot create device memory list from handle"; -} - -nixlGpuXferReqH -deviceMemList::get() const noexcept { - return nullptr; +void +gpuXferReqH::release(nixlGpuXferReqH gpu_req) { + NIXL_WARN << "UCX GPU device API not supported - cannot release GPU transfer request handle"; } #endif diff --git a/src/utils/ucx/device_mem_list.h b/src/utils/ucx/gpu_xfer_req_h.h similarity index 74% rename from src/utils/ucx/device_mem_list.h rename to src/utils/ucx/gpu_xfer_req_h.h index 3d52b35c1d..63b8117762 100644 --- a/src/utils/ucx/device_mem_list.h +++ b/src/utils/ucx/gpu_xfer_req_h.h @@ -35,26 +35,24 @@ class nixlUcxMem; namespace nixl::ucx { class rkey; -class deviceMemList { +class gpuXferReqH { public: - deviceMemList() = delete; - deviceMemList(const nixlUcxEp &ep, - const std::vector &local_mems, - const std::vector &remote_rkeys); - explicit deviceMemList(const nixlGpuXferReqH) noexcept; + gpuXferReqH() = delete; - [[nodiscard]] nixlGpuXferReqH - get() const noexcept; + static nixlGpuXferReqH + create(const nixlUcxEp &ep, + const std::vector &local_mems, + const std::vector &remote_rkeys); + + static void + release(nixlGpuXferReqH gpu_req); -#ifdef HAVE_UCX_GPU_DEVICE_API private: +#ifdef HAVE_UCX_GPU_DEVICE_API [[nodiscard]] static ucp_device_mem_list_handle_h createDeviceMemList(const nixlUcxEp &ep, const std::vector &local_mems, const std::vector &remote_rkeys); - - const std::unique_ptr - deviceMemList_; #endif }; } // namespace nixl::ucx diff --git a/src/utils/ucx/meson.build b/src/utils/ucx/meson.build index a25316c246..cf70169441 100644 --- a/src/utils/ucx/meson.build +++ b/src/utils/ucx/meson.build @@ -21,7 +21,7 @@ endif ucx_utils_inc_dirs = include_directories('.') -ucx_utils_sources = ['ucx_utils.cpp', 'ucx_utils.h', 'config.h', 'config.cpp', 'rkey.cpp', 'rkey.h', 'device_mem_list.cpp', 'device_mem_list.h'] +ucx_utils_sources = ['ucx_utils.cpp', 'ucx_utils.h', 'config.h', 'config.cpp', 'rkey.cpp', 'rkey.h', 'gpu_xfer_req_h.cpp', 'gpu_xfer_req_h.h'] ucx_utils_lib = library('ucx_utils', ucx_utils_sources, From 0f42fd7d10efb13630a5c9a4fe703b34c845dbf3 Mon Sep 17 00:00:00 2001 From: Michal Shalev Date: Tue, 16 Sep 2025 20:15:06 +0300 Subject: [PATCH 2/4] PR fixes Signed-off-by: Michal Shalev --- src/plugins/ucx/ucx_backend.cpp | 4 +-- src/utils/ucx/gpu_xfer_req_h.cpp | 47 +++++++++++++------------------- src/utils/ucx/gpu_xfer_req_h.h | 24 +++------------- 3 files changed, 25 insertions(+), 50 deletions(-) diff --git a/src/plugins/ucx/ucx_backend.cpp b/src/plugins/ucx/ucx_backend.cpp index d306cecd7e..b2d9633ee9 100644 --- a/src/plugins/ucx/ucx_backend.cpp +++ b/src/plugins/ucx/ucx_backend.cpp @@ -1666,7 +1666,7 @@ nixlUcxEngine::createGpuXferReq(const nixlBackendReqH &req_hndl, } try { - gpu_req_hndl = nixl::ucx::gpuXferReqH::create(*ep, local_mems, remote_rkeys); + gpu_req_hndl = nixl::ucx::createGpuXferReq(*ep, local_mems, remote_rkeys); return NIXL_SUCCESS; } catch (const std::exception &e) { @@ -1677,7 +1677,7 @@ nixlUcxEngine::createGpuXferReq(const nixlBackendReqH &req_hndl, void nixlUcxEngine::releaseGpuXferReq(nixlGpuXferReqH gpu_req_hndl) const { - nixl::ucx::gpuXferReqH::release(gpu_req_hndl); + nixl::ucx::releaseGpuXferReq(gpu_req_hndl); } nixl_status_t diff --git a/src/utils/ucx/gpu_xfer_req_h.cpp b/src/utils/ucx/gpu_xfer_req_h.cpp index 83174cad1b..25cb2ba4ec 100644 --- a/src/utils/ucx/gpu_xfer_req_h.cpp +++ b/src/utils/ucx/gpu_xfer_req_h.cpp @@ -26,29 +26,9 @@ namespace nixl::ucx { #ifdef HAVE_UCX_GPU_DEVICE_API nixlGpuXferReqH -gpuXferReqH::create(const nixlUcxEp &ep, - const std::vector &local_mems, - const std::vector &remote_rkeys) { - ucp_device_mem_list_handle_h ucx_handle = createDeviceMemList(ep, local_mems, remote_rkeys); - return reinterpret_cast(ucx_handle); -} - -void -gpuXferReqH::release(nixlGpuXferReqH gpu_req) { - if (gpu_req == nullptr) { - NIXL_WARN << "Attempting to release null GPU transfer request handle"; - return; - } - - ucp_device_mem_list_handle_h ucx_handle = - reinterpret_cast(gpu_req); - ucp_device_mem_list_release(ucx_handle); -} - -ucp_device_mem_list_handle_h -gpuXferReqH::createDeviceMemList(const nixlUcxEp &ep, - const std::vector &local_mems, - const std::vector &remote_rkeys) { +createGpuXferReq(const nixlUcxEp &ep, + const std::vector &local_mems, + const std::vector &remote_rkeys) { nixl_status_t status = ep.checkTxState(); if (status != NIXL_SUCCESS) { throw std::runtime_error("Endpoint not in valid state for creating memory list"); @@ -90,21 +70,32 @@ gpuXferReqH::createDeviceMemList(const nixlUcxEp &ep, } NIXL_DEBUG << "Created device memory list handle with " << local_mems.size() << " elements"; - return ucx_handle; + return reinterpret_cast(ucx_handle); +} + +void +releaseGpuXferReq(nixlGpuXferReqH gpu_req) noexcept { + if (gpu_req == nullptr) { + NIXL_WARN << "Attempting to release null GPU transfer request handle"; + return; + } + + auto ucx_handle = reinterpret_cast(gpu_req); + ucp_device_mem_list_release(ucx_handle); } #else nixlGpuXferReqH -gpuXferReqH::create(const nixlUcxEp &ep, - const std::vector &local_mems, - const std::vector &remote_rkeys) { +createGpuXferReq(const nixlUcxEp &ep, + const std::vector &local_mems, + const std::vector &remote_rkeys) { NIXL_ERROR << "UCX GPU device API not supported"; throw std::runtime_error("UCX GPU device API not available"); } void -gpuXferReqH::release(nixlGpuXferReqH gpu_req) { +releaseGpuXferReq(nixlGpuXferReqH gpu_req) noexcept { NIXL_WARN << "UCX GPU device API not supported - cannot release GPU transfer request handle"; } diff --git a/src/utils/ucx/gpu_xfer_req_h.h b/src/utils/ucx/gpu_xfer_req_h.h index 63b8117762..2c88d9dd4d 100644 --- a/src/utils/ucx/gpu_xfer_req_h.h +++ b/src/utils/ucx/gpu_xfer_req_h.h @@ -18,7 +18,6 @@ #ifndef NIXL_SRC_UTILS_UCX_GPU_XFER_REQ_H_H #define NIXL_SRC_UTILS_UCX_GPU_XFER_REQ_H_H -#include #include extern "C" { @@ -35,26 +34,11 @@ class nixlUcxMem; namespace nixl::ucx { class rkey; -class gpuXferReqH { -public: - gpuXferReqH() = delete; +nixlGpuXferReqH createGpuXferReq(const nixlUcxEp &, + const std::vector &, + const std::vector &); - static nixlGpuXferReqH - create(const nixlUcxEp &ep, - const std::vector &local_mems, - const std::vector &remote_rkeys); - - static void - release(nixlGpuXferReqH gpu_req); - -private: -#ifdef HAVE_UCX_GPU_DEVICE_API - [[nodiscard]] static ucp_device_mem_list_handle_h - createDeviceMemList(const nixlUcxEp &ep, - const std::vector &local_mems, - const std::vector &remote_rkeys); -#endif -}; +void releaseGpuXferReq(nixlGpuXferReqH gpu_req) noexcept; } // namespace nixl::ucx #endif From 26b448919867c1881d63d19ac734b12bd898e091 Mon Sep 17 00:00:00 2001 From: Michal Shalev Date: Tue, 16 Sep 2025 20:46:33 +0300 Subject: [PATCH 3/4] PR fixes 2.0 Signed-off-by: Michal Shalev --- src/utils/ucx/gpu_xfer_req_h.cpp | 10 +++++----- src/utils/ucx/gpu_xfer_req_h.h | 6 ------ 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/src/utils/ucx/gpu_xfer_req_h.cpp b/src/utils/ucx/gpu_xfer_req_h.cpp index 25cb2ba4ec..92df96eb81 100644 --- a/src/utils/ucx/gpu_xfer_req_h.cpp +++ b/src/utils/ucx/gpu_xfer_req_h.cpp @@ -21,6 +21,11 @@ #include "rkey.h" #include "config.h" +extern "C" { +#ifdef HAVE_UCX_GPU_DEVICE_API +#include +#endif +} namespace nixl::ucx { #ifdef HAVE_UCX_GPU_DEVICE_API @@ -75,11 +80,6 @@ createGpuXferReq(const nixlUcxEp &ep, void releaseGpuXferReq(nixlGpuXferReqH gpu_req) noexcept { - if (gpu_req == nullptr) { - NIXL_WARN << "Attempting to release null GPU transfer request handle"; - return; - } - auto ucx_handle = reinterpret_cast(gpu_req); ucp_device_mem_list_release(ucx_handle); } diff --git a/src/utils/ucx/gpu_xfer_req_h.h b/src/utils/ucx/gpu_xfer_req_h.h index 2c88d9dd4d..d1d0749b37 100644 --- a/src/utils/ucx/gpu_xfer_req_h.h +++ b/src/utils/ucx/gpu_xfer_req_h.h @@ -20,12 +20,6 @@ #include -extern "C" { -#ifdef HAVE_UCX_GPU_DEVICE_API -#include -#endif -} - #include "nixl_types.h" class nixlUcxEp; From 32048269f4027e2f06ddb1c843e45b63cd4c263d Mon Sep 17 00:00:00 2001 From: Michal Shalev Date: Tue, 16 Sep 2025 20:48:44 +0300 Subject: [PATCH 4/4] Fix clang format Signed-off-by: Michal Shalev --- src/utils/ucx/gpu_xfer_req_h.cpp | 1 + src/utils/ucx/gpu_xfer_req_h.h | 10 ++++++---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/utils/ucx/gpu_xfer_req_h.cpp b/src/utils/ucx/gpu_xfer_req_h.cpp index 92df96eb81..f33b8b9e5f 100644 --- a/src/utils/ucx/gpu_xfer_req_h.cpp +++ b/src/utils/ucx/gpu_xfer_req_h.cpp @@ -26,6 +26,7 @@ extern "C" { #include #endif } + namespace nixl::ucx { #ifdef HAVE_UCX_GPU_DEVICE_API diff --git a/src/utils/ucx/gpu_xfer_req_h.h b/src/utils/ucx/gpu_xfer_req_h.h index d1d0749b37..11803e811f 100644 --- a/src/utils/ucx/gpu_xfer_req_h.h +++ b/src/utils/ucx/gpu_xfer_req_h.h @@ -28,11 +28,13 @@ class nixlUcxMem; namespace nixl::ucx { class rkey; -nixlGpuXferReqH createGpuXferReq(const nixlUcxEp &, - const std::vector &, - const std::vector &); +nixlGpuXferReqH +createGpuXferReq(const nixlUcxEp &, + const std::vector &, + const std::vector &); -void releaseGpuXferReq(nixlGpuXferReqH gpu_req) noexcept; +void +releaseGpuXferReq(nixlGpuXferReqH gpu_req) noexcept; } // namespace nixl::ucx #endif