[Lldb-commits] [PATCH] D124962: [trace][intelpt] Support system-wide tracing [5] - Disable/enable per-core tracing based on the process state

[Jakob Johnson via Phabricator via lldb-commits]

jj10306 accepted this revision.
jj10306 added inline comments.
This revision is now accepted and ready to land.


================
Comment at: lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp:211
 
+Error IntelPTSingleBufferTrace::SetCollectionState(
+    TraceCollectionState new_state) {
----------------
nit: from the "Set" name, my first impression was this method was simply setting `m_collection_state`, but in reality it's doing some nontrivial operations, namely changing the state of the perf event via ioctl. Consider changing the name from "Set" to avoid giving the impression of a trivial operation


================
Comment at: lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp:309
     return attr.takeError();
+  attr->disabled = initial_state == TraceCollectionState::Paused;
 
----------------
nice


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124962