From 59685f4492f0b49d4df6153e8e2213a78e2995a8 Mon Sep 17 00:00:00 2001 From: David Yat Sin Date: Wed, 4 Jan 2023 20:26:25 +0000 Subject: [PATCH] Add flag for external memory allocations ROCr internally uses the same allocation_map_ list to track memory allocations that are both for internal allocations and allocations by users of ROCr library. In some edge cases, the library user would call hsa_amd_pointer_info on an invalid pointer, but ROCR would return the pointer as valid because this pointer belongs to a memory range that was allocated internally within ROCr. Adding a flag to differentiate between internal and external allocations. Change-Id: I98c52bd85f3985d1ba1b0e3101d2254b003412cf --- runtime/hsa-runtime/core/inc/runtime.h | 18 +++++++++++++----- .../core/runtime/amd_memory_region.cpp | 4 ++-- runtime/hsa-runtime/core/runtime/hsa.cpp | 2 +- .../hsa-runtime/core/runtime/hsa_ext_amd.cpp | 5 +++-- runtime/hsa-runtime/core/runtime/runtime.cpp | 16 +++++++++------- 5 files changed, 28 insertions(+), 17 deletions(-) diff --git a/runtime/hsa-runtime/core/inc/runtime.h b/runtime/hsa-runtime/core/inc/runtime.h index 6ca4e52253..f612f2c534 100644 --- a/runtime/hsa-runtime/core/inc/runtime.h +++ b/runtime/hsa-runtime/core/inc/runtime.h @@ -176,11 +176,13 @@ class Runtime { /// @param [in] size Allocation size in bytes. /// @param [in] alloc_flags Modifiers to pass to MemoryRegion allocator. /// @param [out] address Pointer to store the allocation result. + /// @param [in] user_request Set to true if this is an external allocation, i.e this address + /// is directly visible to the user of this library /// /// @retval ::HSA_STATUS_SUCCESS If allocation is successful. hsa_status_t AllocateMemory(const MemoryRegion* region, size_t size, - MemoryRegion::AllocateFlags alloc_flags, - void** address); + MemoryRegion::AllocateFlags alloc_flags, void** address, + bool user_request = false); /// @brief Free memory previously allocated with AllocateMemory. /// @@ -288,7 +290,7 @@ class Runtime { hsa_status_t PtrInfo(const void* ptr, hsa_amd_pointer_info_t* info, void* (*alloc)(size_t), uint32_t* num_agents_accessible, hsa_agent_t** accessible, - PtrInfoBlockData* block_info = nullptr); + PtrInfoBlockData* block_info = nullptr, bool user_request = false); hsa_status_t SetPtrInfoData(const void* ptr, void* userptr); @@ -374,8 +376,13 @@ class Runtime { struct AllocationRegion { AllocationRegion() : region(NULL), size(0), size_requested(0), user_ptr(nullptr) {} - AllocationRegion(const MemoryRegion* region_arg, size_t size_arg, size_t size_requested) - : region(region_arg), size(size_arg), size_requested(size_requested), user_ptr(nullptr) {} + AllocationRegion(const MemoryRegion* region_arg, size_t size_arg, size_t size_requested, + bool user_request = false) + : region(region_arg), + size(size_arg), + size_requested(size_requested), + user_ptr(nullptr), + user_request(user_request) {} struct notifier_t { void* ptr; @@ -388,6 +395,7 @@ class Runtime { size_t size_requested; /* size requested by user */ void* user_ptr; std::unique_ptr> notifiers; + bool user_request; }; struct AsyncEventsControl { diff --git a/runtime/hsa-runtime/core/runtime/amd_memory_region.cpp b/runtime/hsa-runtime/core/runtime/amd_memory_region.cpp index e1a339f495..a45f73dbc5 100644 --- a/runtime/hsa-runtime/core/runtime/amd_memory_region.cpp +++ b/runtime/hsa-runtime/core/runtime/amd_memory_region.cpp @@ -547,8 +547,8 @@ hsa_status_t MemoryRegion::AllowAccess(uint32_t num_agents, ScopedAcquire lock(&access_lock_); if (core::Runtime::runtime_singleton_->PtrInfo(const_cast(ptr), &info, malloc, - &agent_count, &accessible, - &blockInfo) == HSA_STATUS_SUCCESS) { + &agent_count, &accessible, &blockInfo, + true) == HSA_STATUS_SUCCESS) { /* Thunk may return type = HSA_EXT_POINTER_TYPE_UNKNOWN for userptrs */ if (info.type != HSA_EXT_POINTER_TYPE_UNKNOWN && (blockInfo.length != size || info.sizeInBytes != size)) { diff --git a/runtime/hsa-runtime/core/runtime/hsa.cpp b/runtime/hsa-runtime/core/runtime/hsa.cpp index 3dc854bbd4..8756bf720f 100644 --- a/runtime/hsa-runtime/core/runtime/hsa.cpp +++ b/runtime/hsa-runtime/core/runtime/hsa.cpp @@ -1092,7 +1092,7 @@ hsa_status_t IS_VALID(mem_region); return core::Runtime::runtime_singleton_->AllocateMemory( - mem_region, size, core::MemoryRegion::AllocateNoFlags, ptr); + mem_region, size, core::MemoryRegion::AllocateNoFlags, ptr, true); CATCH; } diff --git a/runtime/hsa-runtime/core/runtime/hsa_ext_amd.cpp b/runtime/hsa-runtime/core/runtime/hsa_ext_amd.cpp index 5555546060..162041c388 100644 --- a/runtime/hsa-runtime/core/runtime/hsa_ext_amd.cpp +++ b/runtime/hsa-runtime/core/runtime/hsa_ext_amd.cpp @@ -701,7 +701,7 @@ hsa_status_t hsa_amd_memory_pool_allocate(hsa_amd_memory_pool_t memory_pool, siz if (flags == HSA_AMD_MEMORY_POOL_PCIE_FLAG) alloc_flag |= core::MemoryRegion::AllocatePCIeRW; - return core::Runtime::runtime_singleton_->AllocateMemory(mem_region, size, alloc_flag, ptr); + return core::Runtime::runtime_singleton_->AllocateMemory(mem_region, size, alloc_flag, ptr, true); CATCH; } @@ -852,7 +852,8 @@ hsa_status_t hsa_amd_pointer_info(const void* ptr, hsa_amd_pointer_info_t* info, IS_OPEN(); IS_BAD_PTR(ptr); IS_BAD_PTR(info); - return core::Runtime::runtime_singleton_->PtrInfo(ptr, info, alloc, num_accessible, accessible); + return core::Runtime::runtime_singleton_->PtrInfo(ptr, info, alloc, num_accessible, accessible, + NULL, true); CATCH; } diff --git a/runtime/hsa-runtime/core/runtime/runtime.cpp b/runtime/hsa-runtime/core/runtime/runtime.cpp index f26b862a9e..61cdb04786 100644 --- a/runtime/hsa-runtime/core/runtime/runtime.cpp +++ b/runtime/hsa-runtime/core/runtime/runtime.cpp @@ -283,14 +283,14 @@ hsa_status_t Runtime::IterateAgent(hsa_status_t (*callback)(hsa_agent_t agent, } hsa_status_t Runtime::AllocateMemory(const MemoryRegion* region, size_t size, - MemoryRegion::AllocateFlags alloc_flags, - void** address) { + MemoryRegion::AllocateFlags alloc_flags, void** address, + bool user_request) { size_t size_requested = size; // region->Allocate(...) may align-up size to granularity hsa_status_t status = region->Allocate(size, alloc_flags, address); // Track the allocation result so that it could be freed properly. if (status == HSA_STATUS_SUCCESS) { ScopedAcquire lock(&memory_lock_); - allocation_map_[*address] = AllocationRegion(region, size, size_requested); + allocation_map_[*address] = AllocationRegion(region, size, size_requested, user_request); } return status; @@ -753,7 +753,7 @@ hsa_status_t Runtime::InteropUnmap(void* ptr) { hsa_status_t Runtime::PtrInfo(const void* ptr, hsa_amd_pointer_info_t* info, void* (*alloc)(size_t), uint32_t* num_agents_accessible, hsa_agent_t** accessible, - PtrInfoBlockData* block_info) { + PtrInfoBlockData* block_info, bool user_request) { static_assert(static_cast(HSA_POINTER_UNKNOWN) == static_cast(HSA_EXT_POINTER_TYPE_UNKNOWN), "Thunk pointer info mismatch"); static_assert(static_cast(HSA_POINTER_ALLOCATED) == static_cast(HSA_EXT_POINTER_TYPE_HSA), @@ -824,11 +824,13 @@ hsa_status_t Runtime::PtrInfo(const void* ptr, hsa_amd_pointer_info_t* info, voi auto fragment = allocation_map_.upper_bound(ptr); if (fragment != allocation_map_.begin()) { fragment--; - if ((fragment->first <= ptr) && - (ptr < reinterpret_cast(fragment->first) + fragment->second.size_requested)) { + if ((fragment->first <= ptr) && (user_request && fragment->second.user_request) && + (ptr < + reinterpret_cast(fragment->first) + fragment->second.size_requested)) { // agent and host address must match here. Only lock memory is allowed to have differing // addresses but lock memory has type HSA_EXT_POINTER_TYPE_LOCKED and cannot be // suballocated. + retInfo.agentBaseAddress = const_cast(fragment->first); retInfo.hostBaseAddress = retInfo.agentBaseAddress; retInfo.sizeInBytes = fragment->second.size_requested; @@ -923,7 +925,7 @@ hsa_status_t Runtime::IPCCreate(void* ptr, size_t len, hsa_amd_ipc_memory_t* han PtrInfoBlockData block; hsa_amd_pointer_info_t info; info.size = sizeof(info); - if (PtrInfo(ptr, &info, nullptr, nullptr, nullptr, &block) != HSA_STATUS_SUCCESS) + if (PtrInfo(ptr, &info, nullptr, nullptr, nullptr, &block, true) != HSA_STATUS_SUCCESS) return HSA_STATUS_ERROR_INVALID_ARGUMENT; // Temporary: Previous versions of HIP will call hsa_amd_ipc_memory_create with the len aligned to