Skip to content

Commit 4e5e340

Browse files
yoon0805Compute-Runtime-Automation
yoon0805
authored and
Compute-Runtime-Automation
committed
fix: add rootDeviceIndex for BufferObjectHandleWrapper
Modified sharedBoHandles to use the (boHandle, rootDeviceIndex) pair as a key instead of boHandle, and added rootDeviceIndex variable in the BufferObjectHandleWrapper class of drm_buffer_object.h. Previously sharedBoHandles uses boHandle as a key. However, this will not work with the system using multiple devices, because each devices can return the same handle to refer different memory region. Related-To: GSD-9024 Signed-off-by: Young Jin Yoon <[email protected]> Source: 675ec13
1 parent 4e31f23 commit 4e5e340

File tree

6 files changed

+156
-49
lines changed

6 files changed

+156
-49
lines changed

‎shared/source/os_interface/linux/drm_buffer_object.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ BufferObjectHandleWrapper BufferObjectHandleWrapper::acquireSharedOwnership() {
3535
std::lock_guard lock{controlBlock->blockMutex};
3636
controlBlock->refCount++;
3737

38-
return BufferObjectHandleWrapper{boHandle, Ownership::strong, controlBlock};
38+
return BufferObjectHandleWrapper{boHandle, rootDeviceIndex, Ownership::strong, controlBlock};
3939
}
4040

4141
BufferObjectHandleWrapper BufferObjectHandleWrapper::acquireWeakOwnership() {
@@ -46,7 +46,7 @@ BufferObjectHandleWrapper BufferObjectHandleWrapper::acquireWeakOwnership() {
4646
std::lock_guard lock{controlBlock->blockMutex};
4747
controlBlock->weakRefCount++;
4848

49-
return BufferObjectHandleWrapper{boHandle, Ownership::weak, controlBlock};
49+
return BufferObjectHandleWrapper{boHandle, rootDeviceIndex, Ownership::weak, controlBlock};
5050
}
5151

5252
BufferObjectHandleWrapper::~BufferObjectHandleWrapper() {
@@ -78,7 +78,7 @@ bool BufferObjectHandleWrapper::canCloseBoHandle() {
7878
}
7979

8080
BufferObject::BufferObject(uint32_t rootDeviceIndex, Drm *drm, uint64_t patIndex, int handle, size_t size, size_t maxOsContextCount)
81-
: BufferObject(rootDeviceIndex, drm, patIndex, BufferObjectHandleWrapper{handle}, size, maxOsContextCount) {}
81+
: BufferObject(rootDeviceIndex, drm, patIndex, BufferObjectHandleWrapper{handle, rootDeviceIndex}, size, maxOsContextCount) {}
8282

8383
BufferObject::BufferObject(uint32_t rootDeviceIndex, Drm *drm, uint64_t patIndex, BufferObjectHandleWrapper &&handle, size_t size, size_t maxOsContextCount)
8484
: drm(drm), handle(std::move(handle)), size(size), refCount(1), rootDeviceIndex(rootDeviceIndex) {

‎shared/source/os_interface/linux/drm_buffer_object.h‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,11 @@ class BufferObjectHandleWrapper {
4545
};
4646

4747
public:
48-
explicit BufferObjectHandleWrapper(int boHandle) noexcept
49-
: boHandle{boHandle} {}
48+
explicit BufferObjectHandleWrapper(int boHandle, uint32_t rootDeviceIndex) noexcept
49+
: boHandle{boHandle}, rootDeviceIndex(rootDeviceIndex) {}
5050

5151
BufferObjectHandleWrapper(BufferObjectHandleWrapper &&other) noexcept
52-
: boHandle(std::exchange(other.boHandle, -1)), ownership(other.ownership), controlBlock(std::exchange(other.controlBlock, nullptr)) {}
52+
: boHandle(std::exchange(other.boHandle, -1)), rootDeviceIndex(std::exchange(other.rootDeviceIndex, UINT32_MAX)), ownership(other.ownership), controlBlock(std::exchange(other.controlBlock, nullptr)) {}
5353

5454
~BufferObjectHandleWrapper();
5555

@@ -65,16 +65,23 @@ class BufferObjectHandleWrapper {
6565
int getBoHandle() const {
6666
return boHandle;
6767
}
68+
uint32_t getRootDeviceIndex() const {
69+
return rootDeviceIndex;
70+
}
6871

6972
void setBoHandle(int handle) {
7073
boHandle = handle;
7174
}
75+
void setRootDeviceIndex(uint32_t index) {
76+
rootDeviceIndex = index;
77+
}
7278

7379
protected:
74-
BufferObjectHandleWrapper(int boHandle, Ownership ownership, ControlBlock *controlBlock)
75-
: boHandle{boHandle}, ownership{ownership}, controlBlock{controlBlock} {}
80+
BufferObjectHandleWrapper(int boHandle, uint32_t rootDeviceIndex, Ownership ownership, ControlBlock *controlBlock)
81+
: boHandle{boHandle}, rootDeviceIndex{rootDeviceIndex}, ownership{ownership}, controlBlock{controlBlock} {}
7682

7783
int boHandle{};
84+
uint32_t rootDeviceIndex{UINT32_MAX};
7885
Ownership ownership{Ownership::strong};
7986
ControlBlock *controlBlock{};
8087
};

‎shared/source/os_interface/linux/drm_memory_manager.cpp‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -241,13 +241,13 @@ uint32_t DrmMemoryManager::unreference(NEO::BufferObject *bo, bool synchronousDe
241241
if (bo->peekIsReusableAllocation()) {
242242
eraseSharedBufferObject(bo);
243243
}
244-
244+
auto rootDeviceIndex = bo->getRootDeviceIndex();
245245
int boHandle = bo->getHandle();
246246
bo->close();
247247

248248
if (bo->isBoHandleShared() && bo->getHandle() != boHandle) {
249249
// Shared BO was closed - handle was invalidated. Remove weak reference from container.
250-
eraseSharedBoHandleWrapper(boHandle);
250+
eraseSharedBoHandleWrapper(boHandle, rootDeviceIndex);
251251
}
252252

253253
if (lock) {
@@ -928,7 +928,7 @@ GraphicsAllocation *DrmMemoryManager::createGraphicsAllocationFromMultipleShared
928928
totalSize += size;
929929

930930
auto patIndex = drm.getPatIndex(nullptr, properties.allocationType, CacheRegion::defaultRegion, CachePolicy::writeBack, false, MemoryPoolHelper::isSystemMemoryPool(memoryPool));
931-
auto boHandleWrapper = reuseSharedAllocation ? BufferObjectHandleWrapper{boHandle} : tryToGetBoHandleWrapperWithSharedOwnership(boHandle);
931+
auto boHandleWrapper = reuseSharedAllocation ? BufferObjectHandleWrapper{boHandle, properties.rootDeviceIndex} : tryToGetBoHandleWrapperWithSharedOwnership(boHandle, properties.rootDeviceIndex);
932932

933933
bo = new (std::nothrow) BufferObject(properties.rootDeviceIndex, &drm, patIndex, std::move(boHandleWrapper), size, maxOsContextCount);
934934
i++;
@@ -1009,32 +1009,32 @@ void DrmMemoryManager::registerSharedBoHandleAllocation(DrmAllocation *drmAlloca
10091009
}
10101010

10111011
auto &bos = drmAllocation->getBOs();
1012+
auto rootDeviceIndex = drmAllocation->getRootDeviceIndex();
10121013

10131014
for (auto *bo : bos) {
10141015
if (bo == nullptr) {
10151016
continue;
10161017
}
10171018

1018-
auto foundHandleWrapperIt = sharedBoHandles.find(bo->getHandle());
1019+
auto foundHandleWrapperIt = sharedBoHandles.find(std::pair<int, uint32_t>(bo->getHandle(), rootDeviceIndex));
10191020
if (foundHandleWrapperIt == std::end(sharedBoHandles)) {
1020-
sharedBoHandles.emplace(bo->getHandle(), bo->acquireWeakOwnershipOfBoHandle());
1021+
sharedBoHandles.emplace(std::make_pair(bo->getHandle(), rootDeviceIndex), bo->acquireWeakOwnershipOfBoHandle());
10211022
} else {
10221023
bo->markAsSharedBoHandle();
10231024
}
10241025
}
10251026
}
10261027

1027-
BufferObjectHandleWrapper DrmMemoryManager::tryToGetBoHandleWrapperWithSharedOwnership(int boHandle) {
1028-
auto foundHandleWrapperIt = sharedBoHandles.find(boHandle);
1028+
BufferObjectHandleWrapper DrmMemoryManager::tryToGetBoHandleWrapperWithSharedOwnership(int boHandle, uint32_t rootDeviceIndex) {
1029+
auto foundHandleWrapperIt = sharedBoHandles.find(std::make_pair(boHandle, rootDeviceIndex));
10291030
if (foundHandleWrapperIt == std::end(sharedBoHandles)) {
1030-
return BufferObjectHandleWrapper{boHandle};
1031+
return BufferObjectHandleWrapper{boHandle, rootDeviceIndex};
10311032
}
1032-
10331033
return foundHandleWrapperIt->second.acquireSharedOwnership();
10341034
}
10351035

1036-
void DrmMemoryManager::eraseSharedBoHandleWrapper(int boHandle) {
1037-
auto foundHandleWrapperIt = sharedBoHandles.find(boHandle);
1036+
void DrmMemoryManager::eraseSharedBoHandleWrapper(int boHandle, uint32_t rootDeviceIndex) {
1037+
auto foundHandleWrapperIt = sharedBoHandles.find(std::make_pair(boHandle, rootDeviceIndex));
10381038
if (foundHandleWrapperIt != std::end(sharedBoHandles)) {
10391039
sharedBoHandles.erase(foundHandleWrapperIt);
10401040
}
@@ -1081,7 +1081,7 @@ GraphicsAllocation *DrmMemoryManager::createGraphicsAllocationFromSharedHandle(c
10811081
UNRECOVERABLE_IF(size == std::numeric_limits<size_t>::max());
10821082

10831083
auto patIndex = drm.getPatIndex(nullptr, properties.allocationType, CacheRegion::defaultRegion, CachePolicy::writeBack, false, MemoryPoolHelper::isSystemMemoryPool(memoryPool));
1084-
auto boHandleWrapper = reuseSharedAllocation ? BufferObjectHandleWrapper{boHandle} : tryToGetBoHandleWrapperWithSharedOwnership(boHandle);
1084+
auto boHandleWrapper = reuseSharedAllocation ? BufferObjectHandleWrapper{boHandle, properties.rootDeviceIndex} : tryToGetBoHandleWrapperWithSharedOwnership(boHandle, properties.rootDeviceIndex);
10851085

10861086
bo = new (std::nothrow) BufferObject(properties.rootDeviceIndex, &drm, patIndex, std::move(boHandleWrapper), size, maxOsContextCount);
10871087

@@ -2623,7 +2623,7 @@ DrmAllocation *DrmMemoryManager::createUSMHostAllocationFromSharedHandle(osHandl
26232623
}
26242624

26252625
auto boHandle = static_cast<int>(openFd.handle);
2626-
auto boHandleWrapper = reuseSharedAllocation ? BufferObjectHandleWrapper{boHandle} : tryToGetBoHandleWrapperWithSharedOwnership(boHandle);
2626+
auto boHandleWrapper = reuseSharedAllocation ? BufferObjectHandleWrapper{boHandle, properties.rootDeviceIndex} : tryToGetBoHandleWrapperWithSharedOwnership(boHandle, properties.rootDeviceIndex);
26272627

26282628
const bool useBooMmap = drm.getMemoryInfo() && properties.useMmapObject;
26292629
if (!useBooMmap) {

‎shared/source/os_interface/linux/drm_memory_manager.h‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,12 @@ enum class AtomicAccessMode : uint32_t;
2424

2525
enum class GemCloseWorkerMode;
2626

27+
struct BoHandleDeviceIndexPairComparer {
28+
bool operator()(std::pair<int, uint32_t> const &lhs, std::pair<int, uint32_t> const &rhs) const {
29+
return (lhs.first < rhs.first) || (lhs.second < rhs.second);
30+
}
31+
};
32+
2733
class DrmMemoryManager : public MemoryManager {
2834
public:
2935
DrmMemoryManager(GemCloseWorkerMode mode,
@@ -112,8 +118,8 @@ class DrmMemoryManager : public MemoryManager {
112118

113119
protected:
114120
void registerSharedBoHandleAllocation(DrmAllocation *drmAllocation);
115-
BufferObjectHandleWrapper tryToGetBoHandleWrapperWithSharedOwnership(int boHandle);
116-
void eraseSharedBoHandleWrapper(int boHandle);
121+
BufferObjectHandleWrapper tryToGetBoHandleWrapperWithSharedOwnership(int boHandle, uint32_t rootDeviceIndex);
122+
void eraseSharedBoHandleWrapper(int boHandle, uint32_t rootDeviceIndex);
117123

118124
MOCKABLE_VIRTUAL BufferObject *findAndReferenceSharedBufferObject(int boHandle, uint32_t rootDeviceIndex);
119125
void eraseSharedBufferObject(BufferObject *bo);
@@ -184,7 +190,7 @@ class DrmMemoryManager : public MemoryManager {
184190
std::vector<BufferObject *> sharingBufferObjects;
185191
std::mutex mtx;
186192

187-
std::map<int, BufferObjectHandleWrapper> sharedBoHandles;
193+
std::map<std::pair<int, uint32_t>, BufferObjectHandleWrapper, BoHandleDeviceIndexPairComparer> sharedBoHandles;
188194
std::vector<std::vector<GraphicsAllocation *>> localMemAllocs;
189195
std::vector<size_t> localMemBanksCount;
190196
std::vector<GraphicsAllocation *> sysMemAllocs;

‎shared/test/unit_test/os_interface/linux/drm_buffer_object_tests.cpp‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1002,30 +1002,33 @@ TEST_F(DrmBufferObjectTest, whenBoRequiresExplicitResidencyThenTheCorrespondingQ
10021002

10031003
TEST(DrmBufferObjectHandleWrapperTest, GivenWrapperConstructedFromNonSharedHandleThenControlBlockIsNotCreatedAndInternalHandleIsStored) {
10041004
constexpr int boHandle{5};
1005-
MockBufferObjectHandleWrapper boHandleWrapper{boHandle};
1005+
MockBufferObjectHandleWrapper boHandleWrapper{boHandle, 1u};
10061006

10071007
EXPECT_EQ(nullptr, boHandleWrapper.controlBlock);
10081008
EXPECT_EQ(boHandle, boHandleWrapper.getBoHandle());
1009+
EXPECT_EQ(1u, boHandleWrapper.getRootDeviceIndex());
10091010
}
10101011

10111012
TEST(DrmBufferObjectHandleWrapperTest, GivenWrapperConstructedFromNonSharedHandleWhenAskingIfCanBeClosedThenReturnTrue) {
10121013
constexpr int boHandle{21};
1013-
MockBufferObjectHandleWrapper boHandleWrapper{boHandle};
1014+
MockBufferObjectHandleWrapper boHandleWrapper{boHandle, 1u};
10141015

10151016
EXPECT_TRUE(boHandleWrapper.canCloseBoHandle());
10161017
}
10171018

10181019
TEST(DrmBufferObjectHandleWrapperTest, GivenWrapperWhenSettingNewValueThenStoreIt) {
10191020
constexpr int boHandle{13};
1020-
MockBufferObjectHandleWrapper boHandleWrapper{boHandle};
1021+
MockBufferObjectHandleWrapper boHandleWrapper{boHandle, 1u};
10211022

10221023
boHandleWrapper.setBoHandle(-1);
1024+
boHandleWrapper.setRootDeviceIndex(4u);
10231025
EXPECT_EQ(-1, boHandleWrapper.getBoHandle());
1026+
EXPECT_EQ(4u, boHandleWrapper.getRootDeviceIndex());
10241027
}
10251028

10261029
TEST(DrmBufferObjectHandleWrapperTest, GivenWrapperConstructedFromNonSharedHandleWhenMakingItSharedThenControlBlockIsCreatedAndReferenceCounterIsValid) {
10271030
constexpr int boHandle{85};
1028-
MockBufferObjectHandleWrapper firstBoHandleWrapper{boHandle};
1031+
MockBufferObjectHandleWrapper firstBoHandleWrapper{boHandle, 1u};
10291032
MockBufferObjectHandleWrapper secondBoHandleWrapper = firstBoHandleWrapper.acquireSharedOwnership();
10301033

10311034
ASSERT_NE(nullptr, firstBoHandleWrapper.controlBlock);
@@ -1039,7 +1042,7 @@ TEST(DrmBufferObjectHandleWrapperTest, GivenWrapperConstructedFromNonSharedHandl
10391042

10401043
TEST(DrmBufferObjectHandleWrapperTest, GivenMoreThanOneSharedHandleWrapperWhenAskingIfHandleCanBeClosedThenReturnFalse) {
10411044
constexpr int boHandle{121};
1042-
MockBufferObjectHandleWrapper firstBoHandleWrapper{boHandle};
1045+
MockBufferObjectHandleWrapper firstBoHandleWrapper{boHandle, 1u};
10431046
MockBufferObjectHandleWrapper secondBoHandleWrapper = firstBoHandleWrapper.acquireSharedOwnership();
10441047

10451048
EXPECT_FALSE(firstBoHandleWrapper.canCloseBoHandle());
@@ -1048,7 +1051,7 @@ TEST(DrmBufferObjectHandleWrapperTest, GivenMoreThanOneSharedHandleWrapperWhenAs
10481051

10491052
TEST(DrmBufferObjectHandleWrapperTest, GivenControlBlockCreatedWhenOnlyOneReferenceLeftThenHandleCanBeClosed) {
10501053
constexpr int boHandle{121};
1051-
MockBufferObjectHandleWrapper firstBoHandleWrapper{boHandle};
1054+
MockBufferObjectHandleWrapper firstBoHandleWrapper{boHandle, 1u};
10521055

10531056
{
10541057
MockBufferObjectHandleWrapper secondBoHandleWrapper = firstBoHandleWrapper.acquireSharedOwnership();
@@ -1065,7 +1068,7 @@ TEST(DrmBufferObjectHandleWrapperTest, GivenControlBlockCreatedWhenOnlyOneRefere
10651068

10661069
TEST(DrmBufferObjectHandleWrapperTest, GivenControlBlockCreatedWhenOnlyWeakReferencesLeftThenItIsNotDestroyed) {
10671070
constexpr int boHandle{777};
1068-
auto firstBoHandleWrapper = std::make_unique<MockBufferObjectHandleWrapper>(boHandle);
1071+
auto firstBoHandleWrapper = std::make_unique<MockBufferObjectHandleWrapper>(boHandle, 1u);
10691072
MockBufferObjectHandleWrapper weakHandleWrapper = firstBoHandleWrapper->acquireWeakOwnership();
10701073

10711074
ASSERT_NE(nullptr, firstBoHandleWrapper->controlBlock);
@@ -1080,7 +1083,7 @@ TEST(DrmBufferObjectHandleWrapperTest, GivenControlBlockCreatedWhenOnlyWeakRefer
10801083

10811084
TEST(DrmBufferObjectHandleWrapperTest, GivenControlBlockCreatedWhenWeakReferencesLeftAndOnlyOneStrongReferenceLeftThenHandleCanBeClosed) {
10821085
constexpr int boHandle{353};
1083-
MockBufferObjectHandleWrapper firstBoHandleWrapper{boHandle};
1086+
MockBufferObjectHandleWrapper firstBoHandleWrapper{boHandle, 1u};
10841087
MockBufferObjectHandleWrapper firstWeakHandleWrapper = firstBoHandleWrapper.acquireWeakOwnership();
10851088
MockBufferObjectHandleWrapper secondWeakHandleWrapper = firstBoHandleWrapper.acquireWeakOwnership();
10861089

@@ -1099,7 +1102,7 @@ TEST(DrmBufferObjectHandleWrapperTest, GivenControlBlockCreatedWhenWeakReference
10991102

11001103
TEST(DrmBufferObjectHandleWrapperTest, GivenWrapperWhenConstructingMoreThanTwoSharedResourcesControlBlockRemainsTheSameAndReferenceCounterIsUpdatedOnCreationAndDestruction) {
11011104
constexpr int boHandle{85};
1102-
MockBufferObjectHandleWrapper firstBoHandleWrapper{boHandle};
1105+
MockBufferObjectHandleWrapper firstBoHandleWrapper{boHandle, 1u};
11031106
MockBufferObjectHandleWrapper secondBoHandleWrapper = firstBoHandleWrapper.acquireSharedOwnership();
11041107

11051108
ASSERT_EQ(firstBoHandleWrapper.controlBlock, secondBoHandleWrapper.controlBlock);
@@ -1121,7 +1124,7 @@ TEST(DrmBufferObjectHandleWrapperTest, GivenWrapperWhenConstructingMoreThanTwoSh
11211124

11221125
TEST(DrmBufferObjectHandleWrapperTest, GivenWrapperWhenMoveConstructingAnotherObjectThenInternalDataIsCleared) {
11231126
constexpr int boHandle{27};
1124-
MockBufferObjectHandleWrapper firstBoHandleWrapper{boHandle};
1127+
MockBufferObjectHandleWrapper firstBoHandleWrapper{boHandle, 1u};
11251128
MockBufferObjectHandleWrapper secondBoHandleWrapper = firstBoHandleWrapper.acquireSharedOwnership();
11261129

11271130
auto oldControlBlock = firstBoHandleWrapper.controlBlock;
@@ -1190,4 +1193,4 @@ TEST_F(DrmBufferObjectTest, givenBufferObjectWhenSetIsLockableIsCalledThenIsLock
11901193
bo.setIsLockable(isLockable);
11911194
EXPECT_EQ(isLockable, bo.isLockable());
11921195
}
1193-
}
1196+
}

0 commit comments

Comments
 (0)