From 704d9c5e19318ff9f19d414e959fc7fa0395bedf Mon Sep 17 00:00:00 2001 From: Jonathan Kim Date: Thu, 17 Aug 2023 07:24:55 -0400 Subject: [PATCH] Fix SDMA ganging circular deadlock in oversubscription When oversubscribing SDMA gangs, a circular deadlock can occur since gang enqueue is staggered with respect to SDMA engine leader based on source to destination. As a result, an enqueued leader may be waiting on a gang item that is waiting on another enqueued leader or gang item and so on. To prevent this, first lock the submission to ensure dma status query and submissions are atomic. Once this is in place, be more stringent with ganging in that all SDMA engines must be available in order to gang. Finally, re-enable SDMA ganging by default. Change-Id: I4511e3487db9d26475b5aece4897f10168cc5322 [ROCm/ROCR-Runtime commit: 8f21793a3e6781744a18a88b30731abfb3230c76] --- .../runtime/hsa-runtime/core/inc/amd_blit_kernel.h | 2 -- .../runtime/hsa-runtime/core/inc/amd_blit_sdma.h | 2 -- .../runtime/hsa-runtime/core/inc/amd_gpu_agent.h | 3 +++ .../runtime/hsa-runtime/core/inc/blit.h | 2 -- .../hsa-runtime/core/runtime/amd_gpu_agent.cpp | 14 +++++++------- 5 files changed, 10 insertions(+), 13 deletions(-) diff --git a/projects/rocr-runtime/runtime/hsa-runtime/core/inc/amd_blit_kernel.h b/projects/rocr-runtime/runtime/hsa-runtime/core/inc/amd_blit_kernel.h index 185a279ab0..b68f03ee9c 100644 --- a/projects/rocr-runtime/runtime/hsa-runtime/core/inc/amd_blit_kernel.h +++ b/projects/rocr-runtime/runtime/hsa-runtime/core/inc/amd_blit_kernel.h @@ -116,8 +116,6 @@ class BlitKernel : public core::Blit { void GangLeader(bool gang_leader) {} bool GangLeader() const { return false; } - void GangStatus(bool is_ganged) {} - bool GangStatus() const { return false; } private: union KernelArgs { diff --git a/projects/rocr-runtime/runtime/hsa-runtime/core/inc/amd_blit_sdma.h b/projects/rocr-runtime/runtime/hsa-runtime/core/inc/amd_blit_sdma.h index e1e1cfbe8e..a4feabc900 100644 --- a/projects/rocr-runtime/runtime/hsa-runtime/core/inc/amd_blit_sdma.h +++ b/projects/rocr-runtime/runtime/hsa-runtime/core/inc/amd_blit_sdma.h @@ -147,8 +147,6 @@ class BlitSdma : public BlitSdmaBase { virtual uint64_t PendingBytes() override; void GangLeader(bool gang_leader) { gang_leader_ = gang_leader; } bool GangLeader() const { return gang_leader_; } - void GangStatus(bool is_ganged) { is_ganged_ = is_ganged; } - bool GangStatus() const { return is_ganged_; } private: /// @brief Acquires the address into queue buffer where a new command 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 c8b8017387..39ed8c04a6 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 @@ -468,6 +468,9 @@ class GpuAgent : public GpuAgentInt { // @brief Mutex to protect access to blit objects. KernelMutex blit_lock_; + // @brief Mutex to protect sdma gang submissions. + KernelMutex sdma_gang_lock_; + // @brief GPU tick on initialization. HsaClockCounters t0_; diff --git a/projects/rocr-runtime/runtime/hsa-runtime/core/inc/blit.h b/projects/rocr-runtime/runtime/hsa-runtime/core/inc/blit.h index 94e9fe892d..f2ba647910 100644 --- a/projects/rocr-runtime/runtime/hsa-runtime/core/inc/blit.h +++ b/projects/rocr-runtime/runtime/hsa-runtime/core/inc/blit.h @@ -119,8 +119,6 @@ class Blit { virtual void GangLeader(bool gang_leader) = 0; virtual bool GangLeader() const { return false; }; - virtual void GangStatus(bool is_ganged) = 0; - virtual bool GangStatus() const { return false; }; }; } // namespace core } // namespace rocr 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 b007c0fbd9..7ddf19ca69 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 @@ -876,6 +876,8 @@ hsa_status_t GpuAgent::DmaCopy(void* dst, core::Agent& dst_agent, out_signal.async_copy_agent(core::Agent::Convert(this->public_handle())); } + ScopedAcquire lock(&sdma_gang_lock_); + // Calculate the number of gang items unsigned int tmp_gang_factor = 1; for (auto peer_info : gang_peers_info_) { @@ -884,7 +886,7 @@ hsa_status_t GpuAgent::DmaCopy(void* dst, core::Agent& dst_agent, Flag::SDMA_OVERRIDE sdma_override = core::Runtime::runtime_singleton_->flag().enable_sdma(); // Blit copies already saturate xGMI - if (sdma_override == Flag::SDMA_DISABLE || sdma_gang_override != Flag::SDMA_ENABLE) { + if (sdma_override == Flag::SDMA_DISABLE || sdma_gang_override == Flag::SDMA_DISABLE) { break; } @@ -906,8 +908,8 @@ hsa_status_t GpuAgent::DmaCopy(void* dst, core::Agent& dst_agent, bool has_aux_gang = tmp_gang_factor >= properties_.NumSdmaEngines && !!!properties_.NumSdmaXgmiEngines; tmp_gang_factor = has_aux_gang ? tmp_gang_factor : std::min(tmp_gang_factor, properties_.NumSdmaXgmiEngines); for (int i = 0; i < tmp_gang_factor; i++) { - if (has_aux_gang) { - if (!DmaEngineIsFree(i + 1) && !blits_[i + 1]->GangStatus()) continue; + if (has_aux_gang && !DmaEngineIsFree(i + 1)) { + break; } else { uint32_t engine_offset = 0; for (uint32_t idx = 0; idx < xgmi_peer_list_.size(); idx++) { @@ -918,9 +920,8 @@ hsa_status_t GpuAgent::DmaCopy(void* dst, core::Agent& dst_agent, } // Avoid oversubscribing unavailable blit engines that are not already ganged - if (!!engine_offset && tmp_gang_factor > 1 && !DmaEngineIsFree(engine_offset) && - !blits_[engine_offset]->GangStatus()) { - continue; + if (!!engine_offset && tmp_gang_factor > 1 && !DmaEngineIsFree(engine_offset)) { + break; } } @@ -968,7 +969,6 @@ hsa_status_t GpuAgent::DmaCopy(void* dst, core::Agent& dst_agent, lazy_ptr& blit = has_aux_gang ? blits_[i + 1] : GetBlitObject(dst_agent, src_agent, size, i); blit->GangLeader(gang_factor > 1 && !gang_leader_set); - blit->GangStatus(gang_factor > 1); hsa_status_t stat; size_t chunk = std::min(remainder_size, (size + gang_factor - 1)/gang_factor);