From edcc3a1ed580f40bb861c88478e53452d2712dde Mon Sep 17 00:00:00 2001 From: David Yat Sin Date: Mon, 24 Mar 2025 16:04:42 +0000 Subject: [PATCH] rocr: Release agent resources before pools Adding a general stage for agents to release their resources on shutdown. This avoids a circular dependency during shutdown because we have to delete allocated resources before deleting memory pools, but we also have to delete memory pools before destroying agents. [ROCm/ROCR-Runtime commit: 947391deac0c0cca3029378a922dba817e3fb12f] --- .../core/driver/kfd/amd_kfd_driver.cpp | 4 +- .../runtime/hsa-runtime/core/inc/agent.h | 4 ++ .../hsa-runtime/core/inc/amd_gpu_agent.h | 5 ++ .../core/runtime/amd_gpu_agent.cpp | 60 ++++++++++--------- .../hsa-runtime/core/runtime/runtime.cpp | 5 ++ 5 files changed, 47 insertions(+), 31 deletions(-) diff --git a/projects/rocr-runtime/runtime/hsa-runtime/core/driver/kfd/amd_kfd_driver.cpp b/projects/rocr-runtime/runtime/hsa-runtime/core/driver/kfd/amd_kfd_driver.cpp index 6e47cb21ee..4a6dcd6001 100644 --- a/projects/rocr-runtime/runtime/hsa-runtime/core/driver/kfd/amd_kfd_driver.cpp +++ b/projects/rocr-runtime/runtime/hsa-runtime/core/driver/kfd/amd_kfd_driver.cpp @@ -524,9 +524,9 @@ hsa_status_t KfdDriver::IsModelEnabled(bool* enable) const { // AIE does not support streaming performance monitor. HSAKMT_STATUS status = HSAKMT_STATUS_ERROR; status = hsaKmtModelEnabled(enable); - if (status != HSAKMT_STATUS_SUCCESS) { + if (status != HSAKMT_STATUS_SUCCESS) return HSA_STATUS_ERROR; - } + return HSA_STATUS_SUCCESS; } diff --git a/projects/rocr-runtime/runtime/hsa-runtime/core/inc/agent.h b/projects/rocr-runtime/runtime/hsa-runtime/core/inc/agent.h index 853e305e6c..906ce869c5 100644 --- a/projects/rocr-runtime/runtime/hsa-runtime/core/inc/agent.h +++ b/projects/rocr-runtime/runtime/hsa-runtime/core/inc/agent.h @@ -320,10 +320,14 @@ class Agent : public Checked<0xF6BC25EB17E6F917> { __forceinline void Enable() { enabled_ = true; } + __forceinline void Disable() { enabled_ = false; } + virtual void Trim() { for (auto region : regions()) region->Trim(); } + virtual void ReleaseResources() { } + protected: // Intention here is to have a polymorphic update procedure for public_handle_ // which is callable on any Agent* but only from some class dervied from diff --git a/projects/rocr-runtime/runtime/hsa-runtime/core/inc/amd_gpu_agent.h b/projects/rocr-runtime/runtime/hsa-runtime/core/inc/amd_gpu_agent.h index 4aa65edaee..051879c4ba 100644 --- a/projects/rocr-runtime/runtime/hsa-runtime/core/inc/amd_gpu_agent.h +++ b/projects/rocr-runtime/runtime/hsa-runtime/core/inc/amd_gpu_agent.h @@ -87,6 +87,8 @@ class GpuAgentInt : public core::Agent { // @retval HSA_STATUS_SUCCESS if initialization is successful. virtual hsa_status_t PostToolsInit() = 0; + virtual void ReleaseResources() = 0; + // @brief Invoke the user provided callback for each region accessible by // this agent. // @@ -237,6 +239,9 @@ class GpuAgent : public GpuAgentInt { // @brief GPU agent destructor. ~GpuAgent(); + // @brief Release allocated resources and disables agent + void ReleaseResources() override; + // @brief Ensure blits are ready (performance hint). void PreloadBlits() override; diff --git a/projects/rocr-runtime/runtime/hsa-runtime/core/runtime/amd_gpu_agent.cpp b/projects/rocr-runtime/runtime/hsa-runtime/core/runtime/amd_gpu_agent.cpp index 5a7563acc5..dfb219f8ba 100644 --- a/projects/rocr-runtime/runtime/hsa-runtime/core/runtime/amd_gpu_agent.cpp +++ b/projects/rocr-runtime/runtime/hsa-runtime/core/runtime/amd_gpu_agent.cpp @@ -247,35 +247,6 @@ GpuAgent::GpuAgent(HSAuint32 node, const HsaNodeProperties& node_props, bool xna } GpuAgent::~GpuAgent() { - if (this->Enabled()) { - for (auto& blit : blits_) { - if (!blit.empty()) { - hsa_status_t status = blit->Destroy(*this); - assert(status == HSA_STATUS_SUCCESS); - } - } - - if (ape1_base_ != 0) { - _aligned_free(reinterpret_cast(ape1_base_)); - } - - scratch_cache_.trim(true); - scratch_cache_.free_reserve(); - - if (scratch_pool_.base() != NULL) { - hsaKmtFreeMemory(scratch_pool_.base(), scratch_pool_.size()); - } - - for (int i = 0; i < QueueCount; i++) - queues_[i].reset(); - - system_deallocator()(doorbell_queue_map_); - - if (trap_code_buf_ != NULL) { - ReleaseShader(trap_code_buf_, trap_code_buf_size_); - } - } - std::for_each(regions_.begin(), regions_.end(), DeleteObject()); regions_.clear(); } @@ -947,6 +918,37 @@ void GpuAgent::PreloadBlits() { } } +void GpuAgent::ReleaseResources() { + if (this->Enabled()) { + this->Disable(); + for (auto& blit : blits_) { + if (!blit.empty()) { + hsa_status_t status = blit->Destroy(*this); + assert(status == HSA_STATUS_SUCCESS); + } + } + + if (ape1_base_ != 0) { + _aligned_free(reinterpret_cast(ape1_base_)); + } + + scratch_cache_.trim(true); + scratch_cache_.free_reserve(); + + if (scratch_pool_.base() != NULL) { + hsaKmtFreeMemory(scratch_pool_.base(), scratch_pool_.size()); + } + + for (int i = 0; i < QueueCount; i++) + queues_[i].reset(); + + system_deallocator()(doorbell_queue_map_); + + if (trap_code_buf_ != NULL) + system_deallocator()(trap_code_buf_); + } +} + hsa_status_t GpuAgent::PostToolsInit() { // Defer memory allocation until agents have been discovered. InitAllocators(); diff --git a/projects/rocr-runtime/runtime/hsa-runtime/core/runtime/runtime.cpp b/projects/rocr-runtime/runtime/hsa-runtime/core/runtime/runtime.cpp index 5314cc87fd..9a9ca83c83 100644 --- a/projects/rocr-runtime/runtime/hsa-runtime/core/runtime/runtime.cpp +++ b/projects/rocr-runtime/runtime/hsa-runtime/core/runtime/runtime.cpp @@ -2098,6 +2098,11 @@ void Runtime::Unload() { amd::hsa::loader::Loader::Destroy(loader_); loader_ = nullptr; + for(auto nodeAgent: agents_by_node_) { + for (auto agent: nodeAgent.second) + agent->ReleaseResources(); + } + asyncSignals_.control.Shutdown(); asyncExceptions_.control.Shutdown();