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

[walter erquinigo via Phabricator via lldb-commits]

wallace added inline comments.


================
Comment at: lldb/source/Plugins/Process/Linux/IntelPTSingleBufferTrace.cpp:234
+IntelPTSingleBufferTrace::GetTraceBuffer(size_t offset, size_t size,
+                                         bool flush) {
   std::vector<uint8_t> data(size, 0);
----------------
jj10306 wrote:
> What's the purpose of this new `flush` flag? When would you want to call this method with it set to false? I can only think of cases when you want to flush the buffer if you're trying to read its data
This is nonsense, I shouldn't have created this flag =P


================
Comment at: lldb/source/Plugins/Process/Linux/Perf.h:216-222
+  llvm::Error DisableWithIoctl() const;
+
+  /// Use the ioctl API to enable the perf event.
+  ///
+  /// \return
+  ///   An Error if the perf event couldn't be enabled.
+  llvm::Error EnableWithIoctl() const;
----------------
jj10306 wrote:
> Nice, happy to see we are extending the mini perf API (:
> In the future we also can update this to control whether or not the event is enabled at perf_event_open time because that's the default but there may be cases where you don't want the event to be enabled until you explicitly enable it w ioctl
that's a good idea. I think I'll do it now


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124962