From daa0e96062064747a8d1108546edc0ed70a6df2d Mon Sep 17 00:00:00 2001 From: foreman Date: Thu, 29 Aug 2019 18:58:53 -0400 Subject: [PATCH] P4 to Git Change 1991259 by gandryey@gera-win10 on 2019/08/29 18:45:50 SWDEV-200614 - [Schneider] Crash in Agisoft when run in mGPU environment - Add a workaround for memory pinning path. It will perform 2-step copy to make sure memory pinning doesn't occur on the first unaligned page, because in Windows memory manager can have CPU access to the allocation header in another thread and a race condition is possible - change some default setting for staging and pinned paths, because PCIE gen3 performance. Affected files ... ... //depot/stg/opencl/drivers/opencl/runtime/device/pal/palsettings.cpp#96 edit ... //depot/stg/opencl/drivers/opencl/runtime/device/pal/palvirtual.cpp#150 edit ... //depot/stg/opencl/drivers/opencl/runtime/utils/flags.hpp#317 edit --- rocclr/runtime/device/pal/palsettings.cpp | 9 ++++- rocclr/runtime/device/pal/palvirtual.cpp | 47 +++++++++++++++++++++-- rocclr/runtime/utils/flags.hpp | 6 +-- 3 files changed, 54 insertions(+), 8 deletions(-) diff --git a/rocclr/runtime/device/pal/palsettings.cpp b/rocclr/runtime/device/pal/palsettings.cpp index ecd7165e38..d364b5f784 100644 --- a/rocclr/runtime/device/pal/palsettings.cpp +++ b/rocclr/runtime/device/pal/palsettings.cpp @@ -61,9 +61,14 @@ Settings::Settings() { // By default use host blit blitEngine_ = BlitEngineHost; - const static size_t MaxPinnedXferSize = 32; + constexpr size_t MaxPinnedXferSize = 64; pinnedXferSize_ = std::min(GPU_PINNED_XFER_SIZE, MaxPinnedXferSize) * Mi; - pinnedMinXferSize_ = std::min(GPU_PINNED_MIN_XFER_SIZE * Ki, pinnedXferSize_); + + constexpr size_t PinnedMinXferSize = 4 * Mi; + pinnedMinXferSize_ = flagIsDefault(GPU_PINNED_MIN_XFER_SIZE) + ? PinnedMinXferSize + : GPU_PINNED_MIN_XFER_SIZE * Ki; + pinnedMinXferSize_ = std::min(pinnedMinXferSize_, pinnedXferSize_); // Disable FP_FAST_FMA defines by default reportFMAF_ = false; diff --git a/rocclr/runtime/device/pal/palvirtual.cpp b/rocclr/runtime/device/pal/palvirtual.cpp index bdc94e1c68..9223f084d2 100644 --- a/rocclr/runtime/device/pal/palvirtual.cpp +++ b/rocclr/runtime/device/pal/palvirtual.cpp @@ -1145,8 +1145,28 @@ void VirtualGPU::submitReadMemory(amd::ReadMemoryCommand& vcmd) { result = blitMgr().copyBuffer(*memory, *hostMemory, origin, dstOrigin, size, vcmd.isEntireMemory()); } else { - result = - blitMgr().readBuffer(*memory, vcmd.destination(), origin, size, vcmd.isEntireMemory()); + // The logic below will perform 2 step copy to make sure memory pinning doesn't + // occur on the first unaligned page, because in Windows memory manager can + // have CPU access to the allocation header in another thread + // and a race condition is possible. + char* tmpHost = amd::alignUp( + reinterpret_cast(vcmd.destination()), PinnedMemoryAlignment); + + // Find the partial size for unaligned copy + size_t partial = tmpHost - reinterpret_cast(vcmd.destination()); + result = true; + // Check if it's staging copy, then ignore unaligned address + if (size[0] <= dev().settings().pinnedMinXferSize_) { + partial = size[0]; + } + // Make first step transfer + if (partial > 0) { + result = blitMgr().readBuffer(*memory, vcmd.destination(), origin, partial); + } + // Second step transfer if something left to copy + if (partial < size[0]) { + result &= blitMgr().readBuffer(*memory, tmpHost, origin[0] + partial, size[0] - partial); + } } if (nullptr != bufferFromImage) { bufferFromImage->release(); @@ -1258,7 +1278,28 @@ void VirtualGPU::submitWriteMemory(amd::WriteMemoryCommand& vcmd) { result = blitMgr().copyBuffer(*hostMemory, *memory, srcOrigin, origin, size, vcmd.isEntireMemory()); } else { - result = blitMgr().writeBuffer(vcmd.source(), *memory, origin, size, vcmd.isEntireMemory()); + // The logic below will perform 2 step copy to make sure memory pinning doesn't + // occur on the first unaligned page, because in Windows memory manager can + // have CPU access to the allocation header in another thread + // and a race condition is possible. + const char* tmpHost = + amd::alignUp(reinterpret_cast(vcmd.source()), PinnedMemoryAlignment); + + // Find the partial size for unaligned copy + size_t partial = tmpHost - reinterpret_cast(vcmd.source()); + result = true; + // Check if it's staging copy, then ignore unaligned address + if (size[0] <= dev().settings().pinnedMinXferSize_) { + partial = size[0]; + } + // Make first step transfer + if (partial > 0) { + result = blitMgr().writeBuffer(vcmd.source(), *memory, origin, partial); + } + // Second step transfer if something left to copy + if (partial < size[0]) { + result &= blitMgr().writeBuffer(tmpHost, *memory, origin[0] + partial, size[0] - partial); + } } if (nullptr != bufferFromImage) { bufferFromImage->release(); diff --git a/rocclr/runtime/utils/flags.hpp b/rocclr/runtime/utils/flags.hpp index e77ac4d8f2..9463bef5f7 100644 --- a/rocclr/runtime/utils/flags.hpp +++ b/rocclr/runtime/utils/flags.hpp @@ -44,7 +44,7 @@ release(bool, REMOTE_ALLOC, false, \ "Use remote memory for the global heap allocation") \ release(uint, GPU_MAX_HEAP_SIZE, 100, \ "Set maximum size of the GPU heap to % of board memory") \ -release(uint, GPU_STAGING_BUFFER_SIZE, 512, \ +release(uint, GPU_STAGING_BUFFER_SIZE, 1024, \ "Size of the GPU staging buffer in KiB") \ release(bool, GPU_DUMP_BLIT_KERNELS, false, \ "Dump the kernels for blit manager") \ @@ -68,9 +68,9 @@ release(cstring, AMD_OCL_SC_LIB, 0, \ "Set shader compiler shared library name or path") \ debug(bool, AMD_OCL_ENABLE_MESSAGE_BOX, false, \ "Enable the error dialog on Windows") \ -release(size_t, GPU_PINNED_XFER_SIZE, 16, \ +release(size_t, GPU_PINNED_XFER_SIZE, 32, \ "The pinned buffer size for pinning in read/write transfers") \ -release(size_t, GPU_PINNED_MIN_XFER_SIZE, 512, \ +release(size_t, GPU_PINNED_MIN_XFER_SIZE, 1024, \ "The minimal buffer size for pinned read/write transfers in KBytes") \ release(size_t, GPU_RESOURCE_CACHE_SIZE, 64, \ "The resource cache size in MB") \