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

[walter erquinigo via Phabricator via lldb-commits]

wallace added inline comments.


================
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);
----------------
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


================
Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTMultiCpuDecoder.cpp:122
             } else {
-              m_unattributed_intelpt_subtraces++;
+              m_unattributed_psb_blocks++;
             }
----------------
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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129257