Skip to content

Commit f7482ef

Browse files
iassiouriassiour
authored
SWDEV-529449 - Bug fix when retrieving a memobj from the IPC mem handle
1 parent 5606deb commit f7482ef

File tree

7 files changed

+55
-53
lines changed

7 files changed

+55
-53
lines changed

‎hipamd/src/hip_internal.hpp‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,6 @@
4848
#define KCYN "\x1B[36m"
4949
#define KWHT "\x1B[37m"
5050

51-
/*! IHIP IPC MEMORY Structure */
52-
#define IHIP_IPC_MEM_HANDLE_SIZE 32
53-
#define IHIP_IPC_MEM_RESERVED_SIZE LP64_SWITCH(20,12)
5451
namespace hip{
5552
extern std::once_flag g_ihipInitialized;
5653
}
@@ -89,13 +86,6 @@ struct GraphNode;
8986
struct GraphExec;
9087
struct UserObject;
9188
class Stream;
92-
typedef struct ihipIpcMemHandle_st {
93-
char ipc_handle[IHIP_IPC_MEM_HANDLE_SIZE]; ///< ipc memory handle on ROCr
94-
size_t psize;
95-
size_t poffset;
96-
int owners_process_id;
97-
char reserved[IHIP_IPC_MEM_RESERVED_SIZE];
98-
} ihipIpcMemHandle_t;
9989

10090
#define IHIP_IPC_EVENT_HANDLE_SIZE 32
10191
#define IHIP_IPC_EVENT_RESERVED_SIZE LP64_SWITCH(28,24)

‎hipamd/src/hip_memory.cpp‎

Lines changed: 11 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3236,16 +3236,16 @@ hipError_t hipIpcGetMemHandle(hipIpcMemHandle_t* handle, void* dev_ptr) {
32363236
HIP_INIT_API(hipIpcGetMemHandle, handle, dev_ptr);
32373237

32383238
amd::Device* device = nullptr;
3239-
ihipIpcMemHandle_t* ihandle = nullptr;
3239+
amd::MemObjMap::IpcMemHandle* ihandle = nullptr;
32403240

32413241
if ((handle == nullptr) || (dev_ptr == nullptr)) {
32423242
HIP_RETURN(hipErrorInvalidValue);
32433243
}
32443244

32453245
device = hip::getCurrentDevice()->devices()[0];
3246-
ihandle = reinterpret_cast<ihipIpcMemHandle_t *>(handle);
3246+
ihandle = reinterpret_cast<amd::MemObjMap::IpcMemHandle*>(handle);
32473247

3248-
if(!device->IpcCreate(dev_ptr, &(ihandle->psize), &(ihandle->ipc_handle), &(ihandle->poffset))) {
3248+
if (!device->IpcCreate(dev_ptr, &(ihandle->psize), ihandle->ipc_handle, &(ihandle->poffset))) {
32493249
LogPrintfError("IPC memory creation failed for memory: 0x%x", dev_ptr);
32503250
HIP_RETURN(hipErrorInvalidValue);
32513251
}
@@ -3256,10 +3256,9 @@ hipError_t hipIpcGetMemHandle(hipIpcMemHandle_t* handle, void* dev_ptr) {
32563256

32573257
hipError_t hipIpcOpenMemHandle(void** dev_ptr, hipIpcMemHandle_t handle, unsigned int flags) {
32583258
HIP_INIT_API(hipIpcOpenMemHandle, dev_ptr, &handle, flags);
3259-
32603259
amd::Memory* amd_mem_obj = nullptr;
32613260
amd::Device* device = nullptr;
3262-
ihipIpcMemHandle_t* ihandle = nullptr;
3261+
amd::MemObjMap::IpcMemHandle* ihandle = nullptr;
32633262
size_t offset = 0;
32643263

32653264
if (dev_ptr == nullptr || flags != hipIpcMemLazyEnablePeerAccess) {
@@ -3268,7 +3267,7 @@ hipError_t hipIpcOpenMemHandle(void** dev_ptr, hipIpcMemHandle_t handle, unsigne
32683267

32693268
/* Call the IPC Attach from Device class */
32703269
device = hip::getCurrentDevice()->devices()[0];
3271-
ihandle = reinterpret_cast<ihipIpcMemHandle_t *>(&handle);
3270+
ihandle = reinterpret_cast<amd::MemObjMap::IpcMemHandle*>(&handle);
32723271

32733272
if (ihandle->psize == 0) {
32743273
HIP_RETURN(hipErrorInvalidValue);
@@ -3278,9 +3277,9 @@ hipError_t hipIpcOpenMemHandle(void** dev_ptr, hipIpcMemHandle_t handle, unsigne
32783277
HIP_RETURN(hipErrorInvalidContext);
32793278
}
32803279

3281-
amd_mem_obj = amd::MemObjMap::FindIpcHandleMemObj(ihandle);
3280+
amd_mem_obj = amd::MemObjMap::FindIpcHandleMemObj(*ihandle);
32823281
if (amd_mem_obj == nullptr) {
3283-
if (!device->IpcAttach(&(ihandle->ipc_handle), ihandle->psize, ihandle->poffset, flags,
3282+
if (!device->IpcAttach(ihandle->ipc_handle, ihandle->psize, ihandle->poffset, flags,
32843283
dev_ptr)) {
32853284
LogPrintfError(
32863285
"Cannot attach ipc_handle: with ipc_size: %u"
@@ -3289,7 +3288,7 @@ hipError_t hipIpcOpenMemHandle(void** dev_ptr, hipIpcMemHandle_t handle, unsigne
32893288
HIP_RETURN(hipErrorInvalidDevicePointer);
32903289
}
32913290
amd_mem_obj = getMemoryObject(*dev_ptr, offset);
3292-
amd::MemObjMap::AddIpcHandleMemObj(ihandle, amd_mem_obj);
3291+
amd::MemObjMap::AddIpcHandleMemObj(*ihandle, amd_mem_obj);
32933292
} else {
32943293
// Handle was already opened by the same process
32953294
*dev_ptr = amd_mem_obj->getSvmPtr();
@@ -3330,18 +3329,10 @@ hipError_t hipIpcCloseMemHandle(void* dev_ptr) {
33303329
HIP_RETURN(hipErrorInvalidValue);
33313330
}
33323331

3333-
// If handle is opened more than once, do detach on last release call
3334-
if (amd_mem_obj->referenceCount() == 1) {
3335-
auto device_id = amd_mem_obj->getUserData().deviceId;
3336-
g_devices[device_id]->SyncAllStreams();
3332+
auto device_id = amd_mem_obj->getUserData().deviceId;
3333+
g_devices[device_id]->SyncAllStreams();
33373334

3338-
// Remove entry from the map
3339-
amd::MemObjMap::RemoveIpcHandleMemObj(amd_mem_obj);
3340-
/* detach the memory */
3341-
device->IpcDetach(dev_ptr);
3342-
} else {
3343-
amd_mem_obj->release();
3344-
}
3335+
amd_mem_obj->release();
33453336

33463337
HIP_RETURN(hipSuccess);
33473338
}

‎hipamd/src/hip_mempool_impl.hpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ class Stream;
3535
struct SharedMemPointer {
3636
size_t offset_;
3737
size_t size_;
38-
char handle_[IHIP_IPC_MEM_HANDLE_SIZE];
38+
char handle_[AMD_IPC_MEM_HANDLE_SIZE];
3939
};
4040

4141
struct MemoryTimestamp {

‎rocclr/device/device.cpp‎

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,8 @@ bool Device::device_not_usable_ = false;
341341
std::shared_mutex MemObjMap::AllocatedLock_ ROCCLR_INIT_PRIORITY(101);
342342
std::map<uintptr_t, amd::Memory*> MemObjMap::MemObjMap_ ROCCLR_INIT_PRIORITY(101);
343343
std::map<uintptr_t, amd::Memory*> MemObjMap::VirtualMemObjMap_ ROCCLR_INIT_PRIORITY(101);
344-
std::unordered_map<uintptr_t, amd::Memory*> MemObjMap::IpcHandleMemObjMap_ ROCCLR_INIT_PRIORITY(101);
344+
std::map<MemObjMap::IpcMemHandle, amd::Memory*> MemObjMap::IpcHandleMemObjMap_ ROCCLR_INIT_PRIORITY(101);
345+
345346

346347
void MemObjMap::AddMemObj(const void* k, amd::Memory* v) {
347348
std::unique_lock lock(AllocatedLock_);
@@ -449,12 +450,12 @@ amd::Memory* MemObjMap::FindVirtualMemObj(const void* k) {
449450
}
450451
}
451452

452-
void MemObjMap::AddIpcHandleMemObj(const void* k, amd::Memory* v) {
453+
void MemObjMap::AddIpcHandleMemObj(const IpcMemHandle& k, amd::Memory* v) {
453454
std::unique_lock lock(AllocatedLock_);
454-
auto rval = IpcHandleMemObjMap_.insert({reinterpret_cast<uintptr_t>(k), v});
455+
auto rval = IpcHandleMemObjMap_.insert({k, v});
455456
if (!rval.second) {
456-
DevLogPrintfError("IpcHandle Memobj map already has an entry for ptr: 0x%x",
457-
reinterpret_cast<uintptr_t>(k));
457+
DevLogPrintfError(
458+
"Error adding entry for Memobj 0x%x in IpcHandle map. The handle already exists.", v);
458459
}
459460
}
460461

@@ -469,11 +470,10 @@ void MemObjMap::RemoveIpcHandleMemObj(amd::Memory* v) {
469470
}
470471
}
471472

472-
amd::Memory* MemObjMap::FindIpcHandleMemObj(const void* k) {
473+
amd::Memory* MemObjMap::FindIpcHandleMemObj(const IpcMemHandle& k) {
473474
std::shared_lock lock(AllocatedLock_);
474-
uintptr_t key = reinterpret_cast<uintptr_t>(k);
475475

476-
auto it = IpcHandleMemObjMap_.find(key);
476+
auto it = IpcHandleMemObjMap_.find(k);
477477
if (it == IpcHandleMemObjMap_.cend()) {
478478
return nullptr;
479479
}
@@ -1020,7 +1020,7 @@ char* Device::getExtensionString() {
10201020
}
10211021

10221022
// ================================================================================================
1023-
bool Device::IpcCreate(void* dev_ptr, size_t* mem_size, void* handle, size_t* mem_offset) const {
1023+
bool Device::IpcCreate(void* dev_ptr, size_t* mem_size, char* handle, size_t* mem_offset) const {
10241024
amd::Memory* amd_mem_obj = amd::MemObjMap::FindMemObj(dev_ptr);
10251025
if (amd_mem_obj == nullptr) {
10261026
DevLogPrintfError("Cannot retrieve amd_mem_obj for dev_ptr: 0x%x", dev_ptr);
@@ -1059,7 +1059,7 @@ bool Device::IpcCreate(void* dev_ptr, size_t* mem_size, void* handle, size_t* me
10591059
}
10601060

10611061
// ================================================================================================
1062-
bool Device::IpcAttach(const void* handle, size_t mem_size, size_t mem_offset, unsigned int flags,
1062+
bool Device::IpcAttach(const char* handle, size_t mem_size, size_t mem_offset, unsigned int flags,
10631063
void** dev_ptr) const {
10641064
amd::Memory* amd_mem_obj = nullptr;
10651065

@@ -1093,8 +1093,7 @@ bool Device::IpcAttach(const void* handle, size_t mem_size, size_t mem_offset, u
10931093
}
10941094

10951095
// ================================================================================================
1096-
void Device::IpcDetach(void* dev_ptr) const {
1097-
amd::Memory* amd_mem_obj = amd::MemObjMap::FindMemObj(dev_ptr);
1096+
void Device::IpcDetach(amd::Memory* amd_mem_obj) const {
10981097

10991098
// Get the original pointer from the amd::Memory object
11001099
void* orig_dev_ptr = nullptr;
@@ -1105,8 +1104,7 @@ void Device::IpcDetach(void* dev_ptr) const {
11051104
} else {
11061105
ShouldNotReachHere();
11071106
}
1108-
1109-
if (amd_mem_obj->release() == 0) {
1107+
if (amd::MemObjMap::FindMemObj(orig_dev_ptr)) {
11101108
amd::MemObjMap::RemoveMemObj(orig_dev_ptr);
11111109
}
11121110
}

‎rocclr/device/device.hpp‎

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1374,10 +1374,25 @@ class VirtualDevice : public amd::HeapObject {
13741374
} // namespace amd::device
13751375

13761376
namespace amd {
1377-
1377+
/*! IHIP IPC MEMORY Structure */
1378+
#define AMD_IPC_MEM_HANDLE_SIZE 32
13781379
//! MemoryObject map lookup class
13791380
class MemObjMap : public AllStatic {
13801381
public:
1382+
struct IpcMemHandle {
1383+
char ipc_handle[AMD_IPC_MEM_HANDLE_SIZE]; ///< ipc memory handle on ROCr
1384+
size_t psize; ///< Total size of the device memory allocation
1385+
size_t poffset; ///< Offset within the allocation
1386+
int owners_process_id; ///< ID of the process that owns the allocation
1387+
char reserved[LP64_SWITCH(20, 12)]; ///< Reserved for future extensions
1388+
1389+
bool operator<(const IpcMemHandle& h) const {
1390+
int cmp = std::memcmp(ipc_handle, h.ipc_handle, AMD_IPC_MEM_HANDLE_SIZE);
1391+
if (cmp != 0) return cmp < 0;
1392+
1393+
return poffset < h.poffset;
1394+
}
1395+
};
13811396
//!< add the host mem pointer and buffer in the container
13821397
static void AddMemObj(const void* k, amd::Memory* v);
13831398

@@ -1398,11 +1413,11 @@ class MemObjMap : public AllStatic {
13981413
static amd::Memory* FindVirtualMemObj(const void* k);
13991414

14001415
//!< Same as AddMemObj but for virtual ipc handle to MemObj mapping
1401-
static void AddIpcHandleMemObj(const void* k, amd::Memory* v);
1416+
static void AddIpcHandleMemObj(const IpcMemHandle& k, amd::Memory* v);
14021417
//!< Remove entry from the map by searching values
14031418
static void RemoveIpcHandleMemObj(amd::Memory* v);
14041419
//!< Same as FindMemObj but for ipc handle to MemObj mapping
1405-
static amd::Memory* FindIpcHandleMemObj(const void* k);
1420+
static amd::Memory* FindIpcHandleMemObj(const IpcMemHandle& k);
14061421

14071422
private:
14081423
//!< the mem object<->hostptr information container
@@ -1412,7 +1427,7 @@ class MemObjMap : public AllStatic {
14121427
//!< Shared read/write lock
14131428
static std::shared_mutex AllocatedLock_;
14141429
//!< the ipc handle<->mem object information container
1415-
static std::unordered_map<uintptr_t, amd::Memory*> IpcHandleMemObjMap_;
1430+
static std::map<IpcMemHandle, amd::Memory*> IpcHandleMemObjMap_;
14161431
};
14171432

14181433
/// @brief Instruction Set Architecture properties.
@@ -2080,12 +2095,12 @@ class Device : public RuntimeObject {
20802095
//! Checks if OCL runtime can use hsail for compilation
20812096
bool ValidateHsail();
20822097

2083-
bool IpcCreate(void* dev_ptr, size_t* mem_size, void* handle, size_t* mem_offset) const;
2098+
bool IpcCreate(void* dev_ptr, size_t* mem_size, char* handle, size_t* mem_offset) const;
20842099

2085-
bool IpcAttach(const void* handle, size_t mem_size, size_t mem_offset, unsigned int flags,
2100+
bool IpcAttach(const char* handle, size_t mem_size, size_t mem_offset, unsigned int flags,
20862101
void** dev_ptr) const;
20872102

2088-
void IpcDetach(void* dev_ptr) const;
2103+
void IpcDetach(amd::Memory* amd_mem_obj) const;
20892104

20902105
//! Return context
20912106
amd::Context& context() const { return *context_; }

‎rocclr/device/pal/paldevice.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2505,8 +2505,8 @@ void Device::svmFree(void* ptr) const {
25052505
} else {
25062506
amd::Memory* svmMem = amd::MemObjMap::FindMemObj(ptr);
25072507
if (nullptr != svmMem) {
2508-
svmMem->release();
25092508
amd::MemObjMap::RemoveMemObj(ptr);
2509+
svmMem->release();
25102510
}
25112511
}
25122512
}

‎rocclr/platform/memory.cpp‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,14 @@ device::Memory* Memory::getDeviceMemory(const Device& dev, bool alloc) {
416416

417417
// ================================================================================================
418418
Memory::~Memory() {
419+
420+
if (ipcShared()) {
421+
amd::MemObjMap::RemoveIpcHandleMemObj(this);
422+
auto device = context_().devices()[0];
423+
if (device != nullptr) {
424+
device->IpcDetach(this);
425+
}
426+
}
419427
// For_each destructor callback:
420428
DestructorCallBackEntry* entry;
421429
for (entry = destructorCallbacks_; entry != nullptr; entry = entry->next_) {

0 commit comments

Comments
 (0)