fix hipMalloc() issue when running in multithreads. #8127
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Chiming Zhang <[email protected]>
zhangchiming
requested review from
sonals,
chvamshi-xilinx and
stsoe
as code owners
stsoe
requested changes
May 1, 2024
src/runtime_src/hip/core/memory.cpp
Outdated
| @@ -171,12 +171,14 @@ namespace xrt::core::hip | |||
| void | |||
| memory_database::insert(uint64_t addr, size_t size, std::shared_ptr<xrt::core::hip::memory> hip_mem) | |||
| { | |||
| std::lock_guard<std::mutex> lock(m_mutex); | |||
There was a problem hiding this comment.
Suggested change
| std::lock_guard<std::mutex> lock(m_mutex); | |
| std::lock_guard lock(m_mutex); |
src/runtime_src/hip/core/memory.cpp
Outdated
| m_addr_map.insert({address_range_key(addr, size), hip_mem}); | ||
| } | ||
|
|
||
| void | ||
| memory_database::remove(uint64_t addr) | ||
| { | ||
| std::lock_guard<std::mutex> lock(m_mutex); |
There was a problem hiding this comment.
Suggested change
| std::lock_guard<std::mutex> lock(m_mutex); | |
| std::lock_guard lock(m_mutex); |
Signed-off-by: Chiming Zhang <[email protected]>
stsoe
approved these changes
May 1, 2024
chvamshi-xilinx
approved these changes
May 2, 2024
There was a problem hiding this comment.
@zhangchiming , You need acquire lock in get_hip_mem_from_addr function also.
Please send a PR for that.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fix hipMalloc() issue when running in multithreads.
Problem solved by the commit
crash on calling hipMalloc() from multiple threads.
Bug / issue (if any) fixed, which PR introduced the bug, how it was discovered
Issue was found during developing unit testing case.
How problem was solved, alternative solutions (if any) and why they were rejected
Add lock for memory_database::remove() and memory_database::insert(). Allow device_init() be called once per thread.
Risks (if any) associated the changes in the commit
None. Code will only be built with switch "-hip".
What has been tested and how, request additional testing if necessary
Compiled and tested on Ubuntu 22.04 running on Ryzen 7840.
Documentation impact (if any)
None.