diff --git a/runtime/hsa-runtime/core/inc/amd_memory_region.h b/runtime/hsa-runtime/core/inc/amd_memory_region.h index adc2d16452..49fdac348c 100644 --- a/runtime/hsa-runtime/core/inc/amd_memory_region.h +++ b/runtime/hsa-runtime/core/inc/amd_memory_region.h @@ -82,7 +82,7 @@ class MemoryRegion : public core::MemoryRegion { size_t size); /// @brief Free agent accessible memory (system / local memory). - static void FreeKfdMemory(void* ptr, size_t size); + static bool FreeKfdMemory(void* ptr, size_t size); static bool RegisterMemory(void* ptr, size_t size, const HsaMemFlags& MemFlags); @@ -93,7 +93,7 @@ class MemoryRegion : public core::MemoryRegion { size_t size, uint64_t* alternate_va, HsaMemMapFlags map_flag); /// @brief Unpin memory. - static void MakeKfdMemoryUnresident(const void* ptr); + static bool MakeKfdMemoryUnresident(const void* ptr); MemoryRegion(bool fine_grain, bool kernarg, bool full_profile, bool extended_scope_fine_grain, bool user_visible, core::Agent* owner, const HsaMemoryProperties& mem_props); diff --git a/runtime/hsa-runtime/core/runtime/amd_memory_region.cpp b/runtime/hsa-runtime/core/runtime/amd_memory_region.cpp index 9001b2d628..d24d6d717a 100644 --- a/runtime/hsa-runtime/core/runtime/amd_memory_region.cpp +++ b/runtime/hsa-runtime/core/runtime/amd_memory_region.cpp @@ -65,13 +65,17 @@ void* MemoryRegion::AllocateKfdMemory(const HsaMemFlags& flag, HSAuint32 node_id return (status == HSAKMT_STATUS_SUCCESS) ? ret : NULL; } -void MemoryRegion::FreeKfdMemory(void* ptr, size_t size) { +bool MemoryRegion::FreeKfdMemory(void* ptr, size_t size) { if (ptr == NULL || size == 0) { - return; + debug_print("Invalid free ptr:%p size:%lu\n", ptr, size); + return true; } - HSAKMT_STATUS status = hsaKmtFreeMemory(ptr, size); - assert(status == HSAKMT_STATUS_SUCCESS); + if (hsaKmtFreeMemory(ptr, size) != HSAKMT_STATUS_SUCCESS) { + debug_print("Failed to free ptr:%p size:%lu\n", ptr, size); + return false; + } + return true; } bool MemoryRegion::RegisterMemory(void* ptr, size_t size, const HsaMemFlags& MemFlags) { @@ -97,8 +101,9 @@ bool MemoryRegion::MakeKfdMemoryResident(size_t num_node, const uint32_t* nodes, return (status == HSAKMT_STATUS_SUCCESS); } -void MemoryRegion::MakeKfdMemoryUnresident(const void* ptr) { - hsaKmtUnmapMemoryToGPU(const_cast(ptr)); +bool MemoryRegion::MakeKfdMemoryUnresident(const void* ptr) { + const HSAKMT_STATUS status = hsaKmtUnmapMemoryToGPU(const_cast(ptr)); + return (status == HSAKMT_STATUS_SUCCESS); } MemoryRegion::MemoryRegion(bool fine_grain, bool kernarg, bool full_profile, @@ -311,9 +316,7 @@ hsa_status_t MemoryRegion::FreeImpl(void* address, size_t size) const { MakeKfdMemoryUnresident(address); - FreeKfdMemory(address, size); - - return HSA_STATUS_SUCCESS; + return FreeKfdMemory(address, size) ? HSA_STATUS_SUCCESS : HSA_STATUS_ERROR; } // TODO: Look into a better name and/or making this process transparent to exporting. diff --git a/runtime/hsa-runtime/core/runtime/runtime.cpp b/runtime/hsa-runtime/core/runtime/runtime.cpp index ead5c1f711..ed136e4038 100644 --- a/runtime/hsa-runtime/core/runtime/runtime.cpp +++ b/runtime/hsa-runtime/core/runtime/runtime.cpp @@ -370,7 +370,53 @@ hsa_status_t Runtime::FreeMemory(void* ptr) { if (alloc_flags & core::MemoryRegion::AllocateAsan) assert(hsaKmtReturnAsanHeaderPage(ptr) == HSAKMT_STATUS_SUCCESS); - return region->Free(ptr, size); + const hsa_status_t err = region->Free(ptr, size); + if (err != HSA_STATUS_SUCCESS) { + // hsaKmtFreeMemory failed to free this pointer. Throw a memory error event + + // Note: This should be treated as a fatal exception by the System Event Handler because: + // - This leaves allocation_map_ in an inconsistent state as this pointer entry has already + // been removed. + // - We already called back the notifier, but did not actually free. + // - We removed the ASAN Header but did not actually free. + // + // But this is a very unlikely use case and calling region->Free(..) before updating + // allocation_map_ would require us to hold the memory_lock_ for much longer and we would not be + // able to call hsaKmtReturnAsanHeaderPage after calling region->Free(..) + + const core::Agent* agentOwner = region->owner(); + hsa_status_t custom_handler_status = HSA_STATUS_ERROR; + auto system_event_handlers = runtime_singleton_->GetSystemEventHandlers(); + + if (!system_event_handlers.empty()) { + hsa_amd_event_t memory_error_event; + memory_error_event.event_type = HSA_AMD_GPU_MEMORY_ERROR_EVENT; + hsa_amd_gpu_memory_error_info_t& error_info = memory_error_event.memory_error; + + error_info.virtual_address = reinterpret_cast(ptr); + error_info.error_reason_mask = HSA_AMD_MEMORY_ERROR_MEMORY_IN_USE; + error_info.agent = Agent::Convert(agentOwner); + + for (auto& callback : system_event_handlers) { + hsa_status_t err = callback.first(&memory_error_event, callback.second); + if (err == HSA_STATUS_SUCCESS) custom_handler_status = HSA_STATUS_SUCCESS; + } + } + // No custom VM fault handler registered or it failed. + if (custom_handler_status != HSA_STATUS_SUCCESS) { + fprintf(stderr, + "Memory critical error by agent node-%u (Agent handle: %p) on address %p. Reason: " + "Memory in use. \n", + agentOwner->node_id(), reinterpret_cast(agentOwner->public_handle().handle), + ptr); + + assert(false && "GPU memory error."); + std::abort(); + } + return HSA_STATUS_ERROR; + } + + return HSA_STATUS_SUCCESS; } hsa_status_t Runtime::RegisterReleaseNotifier(void* ptr, hsa_amd_deallocation_callback_t callback,