From 95d55fdfa8ae08e5242b9298870a3e1522859bc7 Mon Sep 17 00:00:00 2001 From: German Andryeyev Date: Thu, 17 Feb 2022 12:51:15 -0500 Subject: [PATCH] SWDEV-323702 - Use active queue for transfer Pass active queue for transfers in the cache coherency layer. That will allow to use device transfer queue only for cases when active queue isn't available, because using device transfer queue from another active queue may cause a deadlock Change-Id: Ifbe7e0303b77dbf6eeda3939ffbc25a3df7472de --- rocclr/device/device.hpp | 3 ++- rocclr/device/pal/palmemory.cpp | 32 +++++++++++++++++-------------- rocclr/device/pal/palmemory.hpp | 7 ++++--- rocclr/device/pal/palvirtual.cpp | 8 ++++---- rocclr/device/rocm/rocmemory.cpp | 32 +++++++++++++++++-------------- rocclr/device/rocm/rocmemory.hpp | 6 +++--- rocclr/device/rocm/rocvirtual.cpp | 4 ++-- rocclr/platform/memory.cpp | 10 +++++----- rocclr/platform/memory.hpp | 4 ++-- 9 files changed, 58 insertions(+), 48 deletions(-) diff --git a/rocclr/device/device.hpp b/rocclr/device/device.hpp index ba0182e630..4380665731 100644 --- a/rocclr/device/device.hpp +++ b/rocclr/device/device.hpp @@ -741,7 +741,8 @@ class Memory : public amd::HeapObject { //! Immediate blocking write from device cache to owners's backing store. //! Marks owner as "current" by resetting the last writer to NULL. - virtual void syncHostFromCache(SyncFlags syncFlags = SyncFlags()) {} + virtual void syncHostFromCache(device::VirtualDevice* vDev, + SyncFlags syncFlags = SyncFlags()) {} //! Allocate memory for API-level maps virtual void* allocMapTarget(const amd::Coord3D& origin, //!< The map location in memory diff --git a/rocclr/device/pal/palmemory.cpp b/rocclr/device/pal/palmemory.cpp index 0cbcc76e75..6259df9f58 100644 --- a/rocclr/device/pal/palmemory.cpp +++ b/rocclr/device/pal/palmemory.cpp @@ -1,4 +1,4 @@ -/* Copyright (c) 2015 - 2021 Advanced Micro Devices, Inc. +/* Copyright (c) 2015 - 2022 Advanced Micro Devices, Inc. Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal @@ -459,7 +459,7 @@ void Memory::syncCacheFromHost(VirtualGPU& gpu, device::Memory::SyncFlags syncFl // If the last writer was another GPU, then make a writeback if (isChacheCoherencySync() && (owner()->getLastWriter() != nullptr) && (&dev() != owner()->getLastWriter())) { - mgpuCacheWriteBack(); + mgpuCacheWriteBack(gpu); } // If host memory doesn't have direct access, then we have to synchronize @@ -571,7 +571,8 @@ void Memory::syncCacheFromHost(VirtualGPU& gpu, device::Memory::SyncFlags syncFl } } -void Memory::syncHostFromCache(device::Memory::SyncFlags syncFlags) { +void Memory::syncHostFromCache(device::VirtualDevice* vDev, device::Memory::SyncFlags syncFlags) { + VirtualGPU* gpu = (vDev != nullptr) ? reinterpret_cast(vDev) : dev().xferQueue(); // Sanity checks assert(owner() != nullptr); @@ -594,7 +595,7 @@ void Memory::syncHostFromCache(device::Memory::SyncFlags syncFlags) { // If the app uses multiple subbuffers from multiple queues, // then the parent sync can be called from multiple threads amd::ScopedLock lock(owner()->parent()->lockMemoryOps()); - m->syncHostFromCache(syncFlagsTmp); + m->syncHostFromCache(gpu, syncFlagsTmp); //! \note Don't do early exit here, since we still have to sync //! this view, if the parent sync operation was a NOP. //! If parent was synchronized, then this view sync will be a NOP @@ -630,7 +631,7 @@ void Memory::syncHostFromCache(device::Memory::SyncFlags syncFlags) { device::Memory* devSub = sub->getDeviceMemory(dev(), AllocSubBuffer); if (nullptr != devSub) { pal::Memory* gpuSub = reinterpret_cast(devSub); - gpuSub->syncHostFromCache(syncFlagsTmp); + gpuSub->syncHostFromCache(gpu, syncFlagsTmp); } } } @@ -650,17 +651,20 @@ void Memory::syncHostFromCache(device::Memory::SyncFlags syncFlags) { bool result = false; static const bool Entire = true; amd::Coord3D origin(0, 0, 0); + // If device on the provided queue doesn't match the device memory was allocated, + // then use blit manager on device + const auto& bltMgr = (&gpu->dev() != &dev()) ? dev().xferMgr() : gpu->blitMgr(); // If backing store was pinned then make a transfer if (flags_ & PinnedMemoryAlloced) { if (desc().buffer_) { amd::Coord3D region(owner()->getSize()); - result = dev().xferMgr().copyBuffer(*this, *pinnedMemory_, origin, origin, region, Entire); + result = bltMgr.copyBuffer(*this, *pinnedMemory_, origin, origin, region, Entire); } else { amd::Image& image = static_cast(*owner()); - result = dev().xferMgr().copyImageToBuffer(*this, *pinnedMemory_, origin, origin, - image.getRegion(), Entire, image.getRowPitch(), - image.getSlicePitch()); + result = bltMgr.copyImageToBuffer(*this, *pinnedMemory_, origin, origin, + image.getRegion(), Entire, image.getRowPitch(), + image.getSlicePitch()); } } @@ -668,11 +672,11 @@ void Memory::syncHostFromCache(device::Memory::SyncFlags syncFlags) { if (!result) { if (desc().buffer_) { amd::Coord3D region(owner()->getSize()); - result = dev().xferMgr().readBuffer(*this, owner()->getHostMem(), origin, region, Entire); + result = bltMgr.readBuffer(*this, owner()->getHostMem(), origin, region, Entire); } else { amd::Image& image = static_cast(*owner()); - result = dev().xferMgr().readImage(*this, owner()->getHostMem(), origin, image.getRegion(), - image.getRowPitch(), image.getSlicePitch(), Entire); + result = bltMgr.readImage(*this, owner()->getHostMem(), origin, image.getRegion(), + image.getRowPitch(), image.getSlicePitch(), Entire); } } @@ -958,7 +962,7 @@ Memory* Memory::mapMemory() const { return map; } -void Memory::mgpuCacheWriteBack() { +void Memory::mgpuCacheWriteBack(VirtualGPU& gpu) { // Lock memory object, so only one write back can occur amd::ScopedLock lock(owner()->lockMemoryOps()); @@ -977,7 +981,7 @@ void Memory::mgpuCacheWriteBack() { if (owner()->getHostMem() != nullptr) { //! \note Ignore pinning result bool ok = pinSystemMemory(owner()->getHostMem(), owner()->getSize()); - owner()->cacheWriteBack(); + owner()->cacheWriteBack(&gpu); } } diff --git a/rocclr/device/pal/palmemory.hpp b/rocclr/device/pal/palmemory.hpp index bc49c0f65a..6d8d0cd832 100644 --- a/rocclr/device/pal/palmemory.hpp +++ b/rocclr/device/pal/palmemory.hpp @@ -1,4 +1,4 @@ -/* Copyright (c) 2015 - 2021 Advanced Micro Devices, Inc. +/* Copyright (c) 2015 - 2022 Advanced Micro Devices, Inc. Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal @@ -126,8 +126,9 @@ class Memory : public device::Memory, public Resource { //! Updates the owner's host allocation from device memory virtual void syncHostFromCache( + device::VirtualDevice* vDev, //! Synchronization flags - device::Memory::SyncFlags syncFlags = device::Memory::SyncFlags()); + device::Memory::SyncFlags syncFlags = device::Memory::SyncFlags()) override; //! Creates a view from current resource virtual Memory* createBufferView( @@ -141,7 +142,7 @@ class Memory : public device::Memory, public Resource { } //! Allocates host memory for synchronization with MGPU context - void mgpuCacheWriteBack(); + void mgpuCacheWriteBack(VirtualGPU& gpu); //! Accessors for indirect map memory object Memory* mapMemory() const; diff --git a/rocclr/device/pal/palvirtual.cpp b/rocclr/device/pal/palvirtual.cpp index fad4b874d3..3e18ccadde 100644 --- a/rocclr/device/pal/palvirtual.cpp +++ b/rocclr/device/pal/palvirtual.cpp @@ -1,4 +1,4 @@ -/* Copyright (c) 2015 - 2021 Advanced Micro Devices, Inc. +/* Copyright (c) 2015 - 2022 Advanced Micro Devices, Inc. Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal @@ -1611,7 +1611,7 @@ void VirtualGPU::submitMapMemory(amd::MapMemoryCommand& vcmd) { } // Target is the backing store, so just ensure that owner is up-to-date - memory->owner()->cacheWriteBack(); + memory->owner()->cacheWriteBack(this); // Add memory to VA cache, so rutnime can detect direct access to VA dev().addVACache(memory); @@ -2002,7 +2002,7 @@ void VirtualGPU::submitSvmMapMemory(amd::SvmMapMemoryCommand& vcmd) { } // Target is the backing store, so just ensure that owner is up-to-date - memory->owner()->cacheWriteBack(); + memory->owner()->cacheWriteBack(this); } else { LogError("Unhandled svm map!"); } @@ -2089,7 +2089,7 @@ void VirtualGPU::submitMigrateMemObjects(amd::MigrateMemObjectsCommand& vcmd) { pal::Memory* memory = dev().getGpuMemory(it); if (vcmd.migrationFlags() & CL_MIGRATE_MEM_OBJECT_HOST) { - memory->mgpuCacheWriteBack(); + memory->mgpuCacheWriteBack(*this); } else if (vcmd.migrationFlags() & CL_MIGRATE_MEM_OBJECT_CONTENT_UNDEFINED) { // Synchronize memory from host if necessary. // The sync function will perform memory migration from diff --git a/rocclr/device/rocm/rocmemory.cpp b/rocclr/device/rocm/rocmemory.cpp index 9637898af2..544d63116d 100644 --- a/rocclr/device/rocm/rocmemory.cpp +++ b/rocclr/device/rocm/rocmemory.cpp @@ -1,4 +1,4 @@ -/* Copyright (c) 2008 - 2021 Advanced Micro Devices, Inc. +/* Copyright (c) 2008 - 2022 Advanced Micro Devices, Inc. Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal @@ -356,7 +356,7 @@ void Memory::syncCacheFromHost(VirtualGPU& gpu, device::Memory::SyncFlags syncFl (&dev() != owner()->getLastWriter())) { // Make sure GPU finished operation before synchronization with the backing store gpu.releaseGpuMemoryFence(); - mgpuCacheWriteBack(); + mgpuCacheWriteBack(gpu); } // If host memory doesn't have direct access, then we have to synchronize @@ -466,7 +466,9 @@ void Memory::syncCacheFromHost(VirtualGPU& gpu, device::Memory::SyncFlags syncFl } } -void Memory::syncHostFromCache(device::Memory::SyncFlags syncFlags) { +void Memory::syncHostFromCache(device::VirtualDevice* vDev, + device::Memory::SyncFlags syncFlags) { + VirtualGPU* gpu = (vDev != nullptr) ? reinterpret_cast(vDev) : dev().xferQueue(); // Sanity checks assert(owner() != nullptr); @@ -490,7 +492,7 @@ void Memory::syncHostFromCache(device::Memory::SyncFlags syncFlags) { // If the app uses multiple subbuffers from multiple queues, // then the parent sync can be called from multiple threads amd::ScopedLock lock(owner()->parent()->lockMemoryOps()); - m->syncHostFromCache(syncFlagsTmp); + m->syncHostFromCache(gpu, syncFlagsTmp); //! \note Don't do early exit here, since we still have to sync //! this view, if the parent sync operation was a NOP. //! If parent was synchronized, then this view sync will be a NOP @@ -526,7 +528,7 @@ void Memory::syncHostFromCache(device::Memory::SyncFlags syncFlags) { device::Memory* devSub = sub->getDeviceMemory(dev(), AllocSubBuffer); if (nullptr != devSub) { Memory* gpuSub = reinterpret_cast(devSub); - gpuSub->syncHostFromCache(syncFlagsTmp); + gpuSub->syncHostFromCache(gpu, syncFlagsTmp); } } } @@ -546,18 +548,20 @@ void Memory::syncHostFromCache(device::Memory::SyncFlags syncFlags) { bool result = false; static const bool Entire = true; amd::Coord3D origin(0, 0, 0); + // If device on the provided queue doesn't match the device memory was allocated, + // then use blit manager on device + const auto& bltMgr = (&gpu->dev() != &dev()) ? dev().xferMgr() : gpu->blitMgr(); // If backing store was pinned then make a transfer if (flags_ & PinnedMemoryAlloced) { Memory& pinned = *dev().getRocMemory(pinnedMemory_); if (owner()->getType() == CL_MEM_OBJECT_BUFFER) { amd::Coord3D region(owner()->getSize()); - result = dev().xferMgr().copyBuffer(*this, pinned, origin, origin, region, Entire); + result = bltMgr.copyBuffer(*this, pinned, origin, origin, region, Entire); } else { amd::Image& image = static_cast(*owner()); - result = - dev().xferMgr().copyImageToBuffer(*this, pinned, origin, origin, image.getRegion(), - Entire, image.getRowPitch(), image.getSlicePitch()); + result = bltMgr.copyImageToBuffer(*this, pinned, origin, origin, image.getRegion(), + Entire, image.getRowPitch(), image.getSlicePitch()); } } @@ -565,11 +569,11 @@ void Memory::syncHostFromCache(device::Memory::SyncFlags syncFlags) { if (!result) { if (owner()->getType() == CL_MEM_OBJECT_BUFFER) { amd::Coord3D region(owner()->getSize()); - result = dev().xferMgr().readBuffer(*this, owner()->getHostMem(), origin, region, Entire); + result = bltMgr.readBuffer(*this, owner()->getHostMem(), origin, region, Entire); } else { amd::Image& image = static_cast(*owner()); - result = dev().xferMgr().readImage(*this, owner()->getHostMem(), origin, image.getRegion(), - image.getRowPitch(), image.getSlicePitch(), Entire); + result = bltMgr.readImage(*this, owner()->getHostMem(), origin, image.getRegion(), + image.getRowPitch(), image.getSlicePitch(), Entire); } } @@ -578,7 +582,7 @@ void Memory::syncHostFromCache(device::Memory::SyncFlags syncFlags) { } } -void Memory::mgpuCacheWriteBack() { +void Memory::mgpuCacheWriteBack(VirtualGPU& gpu) { // Lock memory object, so only one write back can occur amd::ScopedLock lock(owner()->lockMemoryOps()); @@ -597,7 +601,7 @@ void Memory::mgpuCacheWriteBack() { if (owner()->getHostMem() != nullptr) { //! \note Ignore pinning result bool ok = pinSystemMemory(owner()->getHostMem(), owner()->getSize()); - owner()->cacheWriteBack(); + owner()->cacheWriteBack(&gpu); } } diff --git a/rocclr/device/rocm/rocmemory.hpp b/rocclr/device/rocm/rocmemory.hpp index 91731207fe..a9b558d0d8 100644 --- a/rocclr/device/rocm/rocmemory.hpp +++ b/rocclr/device/rocm/rocmemory.hpp @@ -1,4 +1,4 @@ -/* Copyright (c) 2016 - 2021 Advanced Micro Devices, Inc. +/* Copyright (c) 2016 - 2022 Advanced Micro Devices, Inc. Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal @@ -74,10 +74,10 @@ class Memory : public device::Memory { // Immediate blocking write from device cache to owners's backing store. // Marks owner as "current" by resetting the last writer to nullptr. - void syncHostFromCache(SyncFlags syncFlags = SyncFlags()) override; + void syncHostFromCache(device::VirtualDevice* vDev, SyncFlags syncFlags = SyncFlags()) override; //! Allocates host memory for synchronization with MGPU context - void mgpuCacheWriteBack(); + void mgpuCacheWriteBack(VirtualGPU& gpu); // Releases indirect map surface void releaseIndirectMap() override { decIndMapCount(); } diff --git a/rocclr/device/rocm/rocvirtual.cpp b/rocclr/device/rocm/rocvirtual.cpp index f12e8cb9c1..4b002cd7d2 100644 --- a/rocclr/device/rocm/rocvirtual.cpp +++ b/rocclr/device/rocm/rocvirtual.cpp @@ -2044,7 +2044,7 @@ void VirtualGPU::submitMapMemory(amd::MapMemoryCommand& cmd) { releaseGpuMemoryFence(); } // Target is the backing store, so just ensure that owner is up-to-date - devMemory->owner()->cacheWriteBack(); + devMemory->owner()->cacheWriteBack(this); if (devMemory->isHostMemDirectAccess()) { // Add memory to VA cache, so rutnime can detect direct access to VA @@ -2464,7 +2464,7 @@ void VirtualGPU::submitMigrateMemObjects(amd::MigrateMemObjectsCommand& vcmd) { // Make sure GPU finished operation before synchronization with the backing store releaseGpuMemoryFence(); } - memory->mgpuCacheWriteBack(); + memory->mgpuCacheWriteBack(*this); } else if (vcmd.migrationFlags() & CL_MIGRATE_MEM_OBJECT_CONTENT_UNDEFINED) { // Synchronize memory from host if necessary. // The sync function will perform memory migration from diff --git a/rocclr/platform/memory.cpp b/rocclr/platform/memory.cpp index 2aeaa40fb9..3361ca4978 100644 --- a/rocclr/platform/memory.cpp +++ b/rocclr/platform/memory.cpp @@ -1,4 +1,4 @@ -/* Copyright (c) 2010 - 2021 Advanced Micro Devices, Inc. +/* Copyright (c) 2010 - 2022 Advanced Micro Devices, Inc. Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal @@ -412,7 +412,7 @@ Memory::~Memory() { if (NULL != parent_) { // Update cache if runtime destroys a subbuffer if (NULL != parent_->getHostMem() && (vDev_ == NULL)) { - cacheWriteBack(); + cacheWriteBack(nullptr); } parent_->removeSubBuffer(this); } @@ -475,20 +475,20 @@ void Memory::signalWrite(const Device* writer) { } } -void Memory::cacheWriteBack() { +void Memory::cacheWriteBack(device::VirtualDevice* vDev) { if (NULL != lastWriter_) { device::Memory* dmem = getDeviceMemory(*lastWriter_); //! @note It's a special condition, when a subbuffer was created, //! but never used. Thus dev memory is still NULL and lastWriter_ //! was passed from the parent. if (NULL != dmem) { - dmem->syncHostFromCache(); + dmem->syncHostFromCache(vDev); } } else if (isParent()) { // On CPU parent can't be synchronized, because lastWriter_ could be NULL // and syncHostFromCache() won't be called. for (uint i = 0; i < numDevices_; ++i) { - deviceMemories_[i].value_->syncHostFromCache(); + deviceMemories_[i].value_->syncHostFromCache(vDev); } } } diff --git a/rocclr/platform/memory.hpp b/rocclr/platform/memory.hpp index b24450db49..7f1d8372bc 100644 --- a/rocclr/platform/memory.hpp +++ b/rocclr/platform/memory.hpp @@ -1,4 +1,4 @@ -/* Copyright (c) 2010 - 2021 Advanced Micro Devices, Inc. +/* Copyright (c) 2010 - 2022 Advanced Micro Devices, Inc. Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal @@ -329,7 +329,7 @@ class Memory : public amd::RuntimeObject { //! Signal that a write has occurred to a cached version void signalWrite(const Device* writer); //! Force an asynchronous writeback from the most-recent dirty cache to host - void cacheWriteBack(void); + void cacheWriteBack(device::VirtualDevice* vDev); //! Returns true if the specified area covers memory intirely virtual bool isEntirelyCovered(const Coord3D& origin, //!< Origin location of the covered region