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

[Jakob Johnson via Phabricator via lldb-commits]

jj10306 accepted this revision.
jj10306 added a comment.
This revision is now accepted and ready to land.

thanks for answering those questions, lgtm



================
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);
----------------
wallace wrote:
> jj10306 wrote:
> > 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"
> > 
> I think that's fine for system paths. You can have multiple consecutive //// and the system collapses them
TIL


================
Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCpuDecoder.cpp:122
             } else {
-              m_unattributed_intelpt_subtraces++;
+              m_unattributed_psb_blocks++;
             }
----------------
wallace wrote:
> jj10306 wrote:
> > 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.
> the current code is correct, I'll explain:
> 
> - on_new_thread_execution is executed on all threads of the same CPU in chronological order
> - as on_new_thread_execution is being invoked repeatedly, the `it` iterator is being traversed and is always moving forwards
> - when on_new_thread_execution is invoked, the `it` iterator will look for psb blocks that happened before the given execution. These blocks do not belong to any thread execution. 
> 
> Graphically, we have
> 
> ----exec 1----           ---exec 2----             ---exec 3----
> PSB1    PSB2    PSB3     PSB4           PSB5    PSB6
> 
> when on_new_thread_execution is invoked for exec2, `it` will be pointing at PSB3, which is the first PSB after exec 1. PSB3 comes before exec 2, so it'll be unattributed, then it will move to PSB4 and so on
makes sense, thanks for the explanation!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129257