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

@@ -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;

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<char*>(vcmd.destination()), PinnedMemoryAlignment);
+
+        // Find the partial size for unaligned copy
+        size_t partial = tmpHost - reinterpret_cast<char*>(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<const char*>(vcmd.source()), PinnedMemoryAlignment);
+
+        // Find the partial size for unaligned copy
+        size_t partial = tmpHost - reinterpret_cast<const char*>(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();

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")                                      \