[Lldb-commits] [PATCH] D129257: [trace][intel pt] Add a cgroup filter

[Jakob Johnson via Phabricator via lldb-commits]

jj10306 requested changes to this revision.
jj10306 added a comment.
This revision now requires changes to proceed.

looks good overall, just a couple questions from my end



================
Comment at: lldb/source/Plugins/Process/Linux/IntelPTCollector.cpp:87-90
+    std::string slice = line.substr(line.find_first_of("/"));
+    if (slice.empty())
+      return None;
+    std::string cgroup_file = formatv("/sys/fs/cgroup/{0}", slice);
----------------
isn't the cgroup_file path going to have two slashes since slice starts with a slash?
{F23780471}
in the case of the image above, wouldn't cgroup_file be "/sys/fs/cgroup//foo.slice/bar.service" instead of "/sys/fs/cgroup/foo.slice/bar.service"



================
Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCpuDecoder.cpp:122
             } else {
-              m_unattributed_intelpt_subtraces++;
+              m_unattributed_psb_blocks++;
             }
----------------
Help me understand this. To me this seems like it is counting all of the PSBs that don't belong to **the current thread**, whereas I would expect this to only count the PSBs that don't belong to **any thread**?
So based on my understanding we would be severely overcounting the number of unattributed PSB, but I think I'm just misunderstanding how this code flows.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129257