Skip to content

Commit 128bf45

Browse files
Remove debug flag ForceResourceLockOnTransferCalls
Unlock locked resoures in freeGraphicsMemory method Change-Id: I2baae7b7f9d8260f19a4b083849c5bf0d1a764f3 Signed-off-by: Mateusz Jablonski <[email protected]>
1 parent 82046c2 commit 128bf45

File tree

13 files changed

+66
-49
lines changed

13 files changed

+66
-49
lines changed

runtime/command_queue/enqueue_read_buffer.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,6 @@ cl_int CommandQueueHw<GfxFamily>::enqueueReadBuffer(
5454
TransferProperties transferProperties(buffer, CL_COMMAND_READ_BUFFER, 0, true, &offset, &size, ptr);
5555
EventsRequest eventsRequest(numEventsInWaitList, eventWaitList, event);
5656
cpuDataTransferHandler(transferProperties, eventsRequest, retVal);
57-
if (DebugManager.flags.ForceResourceLockOnTransferCalls.get()) {
58-
if (transferProperties.lockedPtr != nullptr) {
59-
buffer->getMemoryManager()->unlockResource(buffer->getGraphicsAllocation());
60-
}
61-
}
6257
return retVal;
6358
}
6459
MultiDispatchInfo dispatchInfo;

runtime/command_queue/enqueue_write_buffer.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,6 @@ cl_int CommandQueueHw<GfxFamily>::enqueueWriteBuffer(
5252
TransferProperties transferProperties(buffer, CL_COMMAND_WRITE_BUFFER, 0, true, &offset, &size, const_cast<void *>(ptr));
5353
EventsRequest eventsRequest(numEventsInWaitList, eventWaitList, event);
5454
cpuDataTransferHandler(transferProperties, eventsRequest, retVal);
55-
if (DebugManager.flags.ForceResourceLockOnTransferCalls.get()) {
56-
if (transferProperties.lockedPtr != nullptr) {
57-
buffer->getMemoryManager()->unlockResource(buffer->getGraphicsAllocation());
58-
}
59-
}
60-
6155
return retVal;
6256
}
6357

runtime/helpers/properties_helper.cpp

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2018 Intel Corporation
2+
* Copyright (C) 2018-2019 Intel Corporation
33
*
44
* SPDX-License-Identifier: MIT
55
*
@@ -21,10 +21,8 @@ TransferProperties::TransferProperties(MemObj *memObj, cl_command_type cmdType,
2121
if (memObj->peekClMemObjType() == CL_MEM_OBJECT_BUFFER) {
2222
size[0] = *sizePtr;
2323
offset[0] = *offsetPtr;
24-
if (DebugManager.flags.ForceResourceLockOnTransferCalls.get()) {
25-
if ((false == MemoryPool::isSystemMemoryPool(memObj->getGraphicsAllocation()->getMemoryPool())) && (memObj->getMemoryManager() != nullptr)) {
26-
this->lockedPtr = memObj->getMemoryManager()->lockResource(memObj->getGraphicsAllocation());
27-
}
24+
if ((false == MemoryPool::isSystemMemoryPool(memObj->getGraphicsAllocation()->getMemoryPool())) && (memObj->getMemoryManager() != nullptr)) {
25+
this->lockedPtr = memObj->getMemoryManager()->lockResource(memObj->getGraphicsAllocation());
2826
}
2927
} else {
3028
size = {{sizePtr[0], sizePtr[1], sizePtr[2]}};

runtime/memory_manager/graphics_allocation.h

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,9 +110,10 @@ class GraphicsAllocation : public IDNode<GraphicsAllocation> {
110110
void setEvictable(bool evictable) { this->evictable = evictable; }
111111
bool peekEvictable() const { return evictable; }
112112

113-
bool isResident(uint32_t contextId) const { return GraphicsAllocation::objectNotResident != getResidencyTaskCount(contextId); }
114-
void setLocked(bool locked) { this->locked = locked; }
115-
bool isLocked() const { return locked; }
113+
void lock(void *ptr) { this->lockedPtr = ptr; }
114+
void unlock() { this->lockedPtr = nullptr; }
115+
bool isLocked() const { return lockedPtr != nullptr; }
116+
void *getLockedPtr() const { return lockedPtr; }
116117

117118
void incReuseCount() { reuseCount++; }
118119
void decReuseCount() { reuseCount--; }
@@ -128,6 +129,7 @@ class GraphicsAllocation : public IDNode<GraphicsAllocation> {
128129
uint32_t getInspectionId(uint32_t contextId) { return usageInfos[contextId].inspectionId; }
129130
void setInspectionId(uint32_t newInspectionId, uint32_t contextId) { usageInfos[contextId].inspectionId = newInspectionId; }
130131

132+
bool isResident(uint32_t contextId) const { return GraphicsAllocation::objectNotResident != getResidencyTaskCount(contextId); }
131133
void updateResidencyTaskCount(uint32_t newTaskCount, uint32_t contextId) { usageInfos[contextId].residencyTaskCount = newTaskCount; }
132134
uint32_t getResidencyTaskCount(uint32_t contextId) const { return usageInfos[contextId].residencyTaskCount; }
133135
void releaseResidencyInOsContext(uint32_t contextId) { updateResidencyTaskCount(objectNotResident, contextId); }
@@ -155,7 +157,7 @@ class GraphicsAllocation : public IDNode<GraphicsAllocation> {
155157
uint64_t gpuAddress = 0;
156158
bool coherent = false;
157159
osHandle sharedHandle = Sharing::nonSharedResource;
158-
bool locked = false;
160+
void *lockedPtr = nullptr;
159161
uint32_t reuseCount = 0; // GraphicsAllocation can be reused by shared resources
160162
bool evictable = true;
161163
MemoryPool::Type memoryPool = MemoryPool::MemoryNull;

runtime/memory_manager/memory_manager.cpp

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,9 @@ void MemoryManager::freeGraphicsMemory(GraphicsAllocation *gfxAllocation) {
137137
if (!gfxAllocation) {
138138
return;
139139
}
140+
if (gfxAllocation->isLocked()) {
141+
unlockResource(gfxAllocation);
142+
}
140143
freeGraphicsMemoryImpl(gfxAllocation);
141144
}
142145
//if not in use destroy in place
@@ -353,9 +356,11 @@ void *MemoryManager::lockResource(GraphicsAllocation *graphicsAllocation) {
353356
if (!graphicsAllocation) {
354357
return nullptr;
355358
}
356-
DEBUG_BREAK_IF(graphicsAllocation->isLocked());
359+
if (graphicsAllocation->isLocked()) {
360+
return graphicsAllocation->getLockedPtr();
361+
}
357362
auto retVal = lockResourceImpl(*graphicsAllocation);
358-
graphicsAllocation->setLocked(true);
363+
graphicsAllocation->lock(retVal);
359364
return retVal;
360365
}
361366

@@ -365,6 +370,6 @@ void MemoryManager::unlockResource(GraphicsAllocation *graphicsAllocation) {
365370
}
366371
DEBUG_BREAK_IF(!graphicsAllocation->isLocked());
367372
unlockResourceImpl(*graphicsAllocation);
368-
graphicsAllocation->setLocked(false);
373+
graphicsAllocation->unlock();
369374
}
370375
} // namespace OCLRT

runtime/os_interface/debug_variables_base.inl

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ DECLARE_DEBUG_VARIABLE(bool, UseNewHeapAllocator, true, "Custom 4GB heap allocat
7676
DECLARE_DEBUG_VARIABLE(bool, UseNoRingFlushesKmdMode, true, "Windows only, passes flag to KMD that informs KMD to not emit any ring buffer flushes.")
7777
DECLARE_DEBUG_VARIABLE(bool, DisableZeroCopyForUseHostPtr, false, "When active all buffer allocations created with CL_MEM_USE_HOST_PTR flag will not share memory with CPU.")
7878
DECLARE_DEBUG_VARIABLE(bool, DisableZeroCopyForBuffers, false, "When active all buffer allocations will not share memory with CPU.")
79-
DECLARE_DEBUG_VARIABLE(bool, ForceResourceLockOnTransferCalls, false, "Forces resource locking on memory transfer calls")
8079
DECLARE_DEBUG_VARIABLE(bool, EnableHostPtrTracking, true, "Enable host ptr tracking")
8180

8281
/*FEATURE FLAGS*/

runtime/os_interface/windows/wddm_memory_manager.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -329,9 +329,6 @@ void WddmMemoryManager::freeGraphicsMemoryImpl(GraphicsAllocation *gfxAllocation
329329
allocationHandles = &input->handle;
330330
allocationCount = 1;
331331
}
332-
if (input->isLocked()) {
333-
unlockResource(input);
334-
}
335332
auto status = tryDeferDeletions(allocationHandles, allocationCount, resourceHandle);
336333
DEBUG_BREAK_IF(!status);
337334
alignedFreeWrapper(input->driverAllocatedCpuPointer);

unit_tests/command_queue/enqueue_read_buffer_tests.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -454,9 +454,8 @@ HWTEST_F(EnqueueReadBufferTypeTest, givenCommandQueueWhenEnqueueReadBufferIsCall
454454
EXPECT_TRUE(mockCmdQ->notifyEnqueueReadBufferCalled);
455455
}
456456

457-
HWTEST_F(EnqueueReadBufferTypeTest, givenEnqueueReadBufferCalledWhenLockedPtrInTransferPropertisIsAvailableThenItIsUnlocked) {
457+
HWTEST_F(EnqueueReadBufferTypeTest, givenEnqueueReadBufferCalledWhenLockedPtrInTransferPropertisIsAvailableThenItIsNotUnlocked) {
458458
DebugManagerStateRestore dbgRestore;
459-
DebugManager.flags.ForceResourceLockOnTransferCalls.set(true);
460459
DebugManager.flags.DoCpuCopyOnReadBuffer.set(true);
461460

462461
ExecutionEnvironment executionEnvironment;
@@ -479,12 +478,11 @@ HWTEST_F(EnqueueReadBufferTypeTest, givenEnqueueReadBufferCalledWhenLockedPtrInT
479478
nullptr);
480479

481480
EXPECT_EQ(CL_SUCCESS, retVal);
482-
EXPECT_EQ(1u, memoryManager.unlockResourceCalled);
481+
EXPECT_EQ(0u, memoryManager.unlockResourceCalled);
483482
}
484483

485484
HWTEST_F(EnqueueReadBufferTypeTest, gicenEnqueueReadBufferCalledWhenLockedPtrInTransferPropertisIsNotAvailableThenItIsNotUnlocked) {
486485
DebugManagerStateRestore dbgRestore;
487-
DebugManager.flags.ForceResourceLockOnTransferCalls.set(true);
488486
DebugManager.flags.DoCpuCopyOnReadBuffer.set(true);
489487

490488
ExecutionEnvironment executionEnvironment;

unit_tests/command_queue/enqueue_write_buffer_tests.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -363,9 +363,8 @@ HWTEST_F(EnqueueWriteBufferTypeTest, givenInOrderQueueAndEnabledSupportCpuCopies
363363
EXPECT_EQ(pCmdQ->taskLevel, 1u);
364364
}
365365

366-
HWTEST_F(EnqueueWriteBufferTypeTest, givenEnqueueWriteBufferCalledWhenLockedPtrInTransferPropertisIsAvailableThenItIsUnlocked) {
366+
HWTEST_F(EnqueueWriteBufferTypeTest, givenEnqueueWriteBufferCalledWhenLockedPtrInTransferPropertisIsAvailableThenItIsNotUnlocked) {
367367
DebugManagerStateRestore dbgRestore;
368-
DebugManager.flags.ForceResourceLockOnTransferCalls.set(true);
369368
DebugManager.flags.DoCpuCopyOnWriteBuffer.set(true);
370369

371370
ExecutionEnvironment executionEnvironment;
@@ -388,12 +387,11 @@ HWTEST_F(EnqueueWriteBufferTypeTest, givenEnqueueWriteBufferCalledWhenLockedPtrI
388387
nullptr);
389388

390389
EXPECT_EQ(CL_SUCCESS, retVal);
391-
EXPECT_EQ(1u, memoryManager.unlockResourceCalled);
390+
EXPECT_EQ(0u, memoryManager.unlockResourceCalled);
392391
}
393392

394393
HWTEST_F(EnqueueWriteBufferTypeTest, givenEnqueueWriteBufferCalledWhenLockedPtrInTransferPropertisIsNotAvailableThenItIsNotUnlocked) {
395394
DebugManagerStateRestore dbgRestore;
396-
DebugManager.flags.ForceResourceLockOnTransferCalls.set(true);
397395
DebugManager.flags.DoCpuCopyOnWriteBuffer.set(true);
398396

399397
ExecutionEnvironment executionEnvironment;

unit_tests/helpers/transfer_properties_tests.cpp

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2018 Intel Corporation
2+
* Copyright (C) 2018-2019 Intel Corporation
33
*
44
* SPDX-License-Identifier: MIT
55
*
@@ -21,9 +21,7 @@ TEST(TransferPropertiesTest, givenTransferPropertiesCreatedWhenDefaultDebugSetti
2121
EXPECT_EQ(nullptr, transferProperties.lockedPtr);
2222
}
2323

24-
TEST(TransferPropertiesTest, givenTransferPropertiesCreatedWhenForceResourceLockOnTransferCallsSetThenLockPtrIsSet) {
25-
DebugManagerStateRestore dbgRestore;
26-
DebugManager.flags.ForceResourceLockOnTransferCalls.set(true);
24+
TEST(TransferPropertiesTest, givenAllocationInNonSystemPoolWhenTransferPropertiesAreCreatedForMapBufferThenLockPtrIsSet) {
2725
ExecutionEnvironment executionEnvironment;
2826
OsAgnosticMemoryManager memoryManager(false, true, executionEnvironment);
2927

@@ -40,9 +38,7 @@ TEST(TransferPropertiesTest, givenTransferPropertiesCreatedWhenForceResourceLock
4038
EXPECT_NE(nullptr, transferProperties.lockedPtr);
4139
}
4240

43-
TEST(TransferPropertiesTest, givenTransferPropertiesCreatedWhenForceResourceLockOnTransferCallsSetAndMemoryPoolIsSystemMemoryThenLockPtrIsNotSet) {
44-
DebugManagerStateRestore dbgRestore;
45-
DebugManager.flags.ForceResourceLockOnTransferCalls.set(true);
41+
TEST(TransferPropertiesTest, givenAllocationInSystemPoolWhenTransferPropertiesAreCreatedForMapBufferThenLockPtrIsNotSet) {
4642
ExecutionEnvironment executionEnvironment;
4743
OsAgnosticMemoryManager memoryManager(false, true, executionEnvironment);
4844

@@ -59,10 +55,7 @@ TEST(TransferPropertiesTest, givenTransferPropertiesCreatedWhenForceResourceLock
5955
EXPECT_EQ(nullptr, transferProperties.lockedPtr);
6056
}
6157

62-
TEST(TransferPropertiesTest, givenTransferPropertiesCreatedWhenForceResourceLockOnTransferCallsSetAndMemoryManagerInMemObjectIsNotSetThenLockPtrIsNotSet) {
63-
DebugManagerStateRestore dbgRestore;
64-
DebugManager.flags.ForceResourceLockOnTransferCalls.set(true);
65-
58+
TEST(TransferPropertiesTest, givenTransferPropertiesCreatedWhenMemoryManagerInMemObjectIsNotSetThenLockPtrIsNotSet) {
6659
MockBuffer buffer;
6760

6861
size_t offset = 0;
@@ -72,8 +65,6 @@ TEST(TransferPropertiesTest, givenTransferPropertiesCreatedWhenForceResourceLock
7265
}
7366

7467
TEST(TransferPropertiesTest, givenTransferPropertiesWhenLockedPtrIsSetThenItIsReturnedForReadWrite) {
75-
DebugManagerStateRestore dbgRestore;
76-
DebugManager.flags.ForceResourceLockOnTransferCalls.set(true);
7768
ExecutionEnvironment executionEnvironment;
7869
OsAgnosticMemoryManager memoryManager(false, true, executionEnvironment);
7970

0 commit comments

Comments
 (0)