Minor re-factor tweaks#9759
Conversation
Some small re-factor changes to xrt_hw_context.cpp with few more comments. Signed-off-by: Soren Soe <2106410+stsoe@users.noreply.github.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Signed-off-by: Soren Soe <2106410+stsoe@users.noreply.github.com>
|
clang-tidy review says "All clean, LGTM! 👍" |
|
|
||
| bool m_elf_flow = false; | ||
|
|
||
| // update_elf_map() - Insert all kernels in elf |
There was a problem hiding this comment.
Minor fix
Mismatch b/w function name and the name in comment
| // Add kernels available in ELF to elf map | ||
| // This function throws if kernel with same name is already present | ||
| create_elf_map(elf); | ||
| std::lock_guard lk(m_mutex); |
There was a problem hiding this comment.
init_from_elf() is currently invoked while holding the mutex and performs a driver call to create the HW context. This can potentially extend the lock hold time due to kernel latency.
So I was thinking whether this needs to be under the lock or if it could be done outside the critical section.
Ideally this should be okay as hw ctx creation is not in critical path but just a thought.
There was a problem hiding this comment.
@rbramand-xilinx . Thanks for the comments. My concern was that add_config() could theoretically be called from different threads, and without locking m_hdl creation could be created under race (multiple handles created). I don't think we have choice but to lock as I did?
Signed-off-by: Soren Soe <2106410+stsoe@users.noreply.github.com>
|
clang-tidy review says "All clean, LGTM! 👍" |
Signed-off-by: Soren Soe <2106410+stsoe@users.noreply.github.com>
|
clang-tidy review says "All clean, LGTM! 👍" |
1 similar comment
|
clang-tidy review says "All clean, LGTM! 👍" |
2 participants
Some small re-factor changes to xrt_hw_context.cpp with few more comments.