Dynamic trace support in ve2 #8721
Conversation
rbramand-xilinx
requested review from
stsoe,
rozumx and
uday610
as code owners
rbramand-xilinx
requested review from
xuhz and
chvamshi-xilinx
and removed request for
uday610 and
rozumx
| struct dlib_handle | ||
| { | ||
| void* handle = nullptr; | ||
|
|
||
| dlib_handle(const std::string& path) | ||
| { | ||
| handle = xrt_core::dlopen(path.c_str(), RTLD_LAZY); | ||
| } | ||
|
|
||
| ~dlib_handle() | ||
| { | ||
| if (handle) | ||
| xrt_core::dlclose(handle); | ||
| } | ||
| }; |
There was a problem hiding this comment.
This is marginally silly, but you may want to consider hiding the unique_ptr within struct dlib_handle. Take a look at runtime_src/core/common/runner/cpu.cpp:dllwrap.
If this is temporary code based on AIEBU comment below, then please ignore this comment.
There was a problem hiding this comment.
It is temporary code but I will refactor it as code of dllwrap makes more sense, Thanks Soren.
| void | ||
| initialize_dtrace_buf(const module_impl* parent) | ||
| { |
There was a problem hiding this comment.
Since this function is rather long, can we insert some comments in between the various blocks as to what exactly is being initialized?
| msg = std::string{"[dtrace] : dtrace buffers initialization failed, "} + e.what(); | ||
| } | ||
|
|
||
| xrt_core::message::send(xrt_core::message::severity_level::debug, "xrt_module", msg); |
There was a problem hiding this comment.
Remove and put on line 2053 and 2057. Move std::string from line 2030 to local variable if at all needed.
| msg = std::string{"[dtrace] : dtrace buffer dump failed, "} + e.what(); | ||
| } | ||
|
|
||
| xrt_core::message::send(xrt_core::message::severity_level::debug, "xrt_module", msg); |
| static bool | ||
| init_dtrace_helper(const module_impl* parent, dtrace_util& dtrace_obj) |
There was a problem hiding this comment.
Return codes are not usually desired, but in this case make sense after reading how it is used. This deserves a comment. Basically, the exception alternative is not good when the success path allows init_dtrace_helper to be a no-op.
Signed-off-by: rbramand <[email protected]>
Signed-off-by: rbramand <[email protected]>
rbramand-xilinx
force-pushed
the
dtrace1
branch
from
d66e5e7 to
702f1e0
Compare
chvamshi-xilinx
changed the title
Problem solved by the commit
Enabled support for dumping dynamic trace. This feature is supported in Telluride only for now.
Trace is dumped only when ini option is enabled.
Input is a control script based on which trace info will be filled. Spec for same can be found at - https://confluence.amd.com/pages/viewpage.action?pageId=1665323347
Bug / issue (if any) fixed, which PR introduced the bug, how it was discovered
NA
How problem was solved, alternative solutions (if any) and why they were rejected
Trace is dumped into a file only when required ini options are enabled.
Also we are calling APIs from dtrace library and the library path is taken as an ini option, ideally the library code should be part of AIEBU repo but as Telluride based AIEBU is still not made public we are taking library path as ini option and calling dlopen / dlsym to use APIs from the library. This should go away once AIEBU has the APIs for dtrace integrated into it.
Risks (if any) associated the changes in the commit
Low as dtrace is dumped only when ini option is enabled.
What has been tested and how, request additional testing if necessary
Tested dtrace dump on Telluride qemu flow and the output was as expected.
Documentation impact (if any)
NA