[Lldb-commits] [PATCH] D124858: [trace][intelpt] Support system-wide tracing [4] - Support per core tracing on lldb-server

[walter erquinigo via Phabricator via lldb-commits]

wallace marked 4 inline comments as done.
wallace added inline comments.


================
Comment at: lldb/docs/lldb-gdb-remote.txt:248
 //     "name": <string>,
-//         Tracing technology name, e.g. intel-pt, arm-coresight.
+//         Tracing technology name, e.g. intel-pt, arm-etm.
 //     "description": <string>,
----------------
this should be "Embedded Trace Macrocells" (ETM) instead of coresight


================
Comment at: lldb/docs/lldb-gdb-remote.txt:465-474
 //      "tid": <decimal integer>,
 //      "binaryData": [
 //        {
 //          "kind": <string>,
 //              Identifier for some binary data related to this thread to
 //              fetch with the jLLDBTraceGetBinaryData packet.
 //          "size": <decimal integer>,
----------------
wallace wrote:
> jj10306 wrote:
> > Should the thread info be optional now that we have an optional `cores`? If not, can you explain how this output works in the case that you are doing per core tracing? Will both the tid binary data and the cores binary data section be populated?
> I'll make this field optional and include more documentation below
in the end i didn't make this optional but instead forced the per-core case to return all threads 


================
Comment at: lldb/include/lldb/Utility/TraceGDBRemotePackets.h:157-160
+  std::vector<TraceThreadState> traced_threads;
+  std::vector<TraceBinaryData> process_binary_data;
+  llvm::Optional<std::vector<TraceCoreState>> cores;
 };
----------------
jj10306 wrote:
> Similar question here as above on the GetState documentation - how do the cores and traced_threads options play together?
traced_threads will contain all the threads in the per core case but without any binary data. The cores field will then be returned with per-core trace buffers


================
Comment at: lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp:149
 
 #ifdef PERF_ATTR_SIZE_VER5
 static Expected<uint64_t>
----------------
i'm the location of this ifdef so that perf_event_attr is used only if PERF_ATTR_SIZE_VER5 is set


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124858/new/

https://reviews.llvm.org/D124858