From 0708be85bd49adb8bd0f49be152938f00ef1996e Mon Sep 17 00:00:00 2001 From: Lalit Maganti Date: Tue, 6 Oct 2026 20:55:19 +0100 Subject: [PATCH] traced_perf: scope callstacks to a target's descendants Profiling a program by its pid unwinds that one process. What it forks is sampled and then dropped: the children's pids are not targets, and their names need not match a target_cmdline. A server's workers and a build's compilers are the usual case, and perf record follows children by default, so people expect it. Scope.target_pid_descendants puts a process in scope when one of its ancestors is a target pid. Ancestry is read from the parent chain in /proc//stat when the process is first sampled, which is where the unwinding decision is already made and cached per pid. This is the first option of #7630 and has the weakness the issue names for it: a process whose parent exited before its first sample has been reparented, and is not covered. Following forks through ftrace, or through perf's own inheritance with per-process events, would close that at the costs the issue describes. This change does not rule either out. Test: perfetto_unittests TargetFilterTest.TargetPidDescendants. nginx under h2load for 8 s, scoped to the master's pid, two recordings each: no callstacks from 27.7k and 27.9k samples without the option; 5,885 and 5,970 callstacks from four worker processes with it. --- .../config/profiling/perf_event_config.proto | 8 ++++ src/profiling/perf/event_config.cc | 3 ++ src/profiling/perf/event_config.h | 2 + src/profiling/perf/perf_producer.cc | 44 +++++++++++++++++- src/profiling/perf/perf_producer.h | 5 +- src/profiling/perf/perf_producer_unittest.cc | 46 ++++++++++++++++++- 6 files changed, 103 insertions(+), 5 deletions(-) diff --git a/protos/perfetto/config/profiling/perf_event_config.proto b/protos/perfetto/config/profiling/perf_event_config.proto index e4ee40f37f8..87b3268a304 100644 --- a/protos/perfetto/config/profiling/perf_event_config.proto +++ b/protos/perfetto/config/profiling/perf_event_config.proto @@ -229,6 +229,14 @@ message PerfEventConfig { // it to the same value. The profiler will choose one bin for all those data // sources. optional uint32 process_shard_count = 6; + + // If set, a process is also in scope if one of its ancestors is in + // |target_pid|: the workers a server forks, the compilers a build runs. + // Ancestry is read from the parent pid chain when the process is first + // sampled, so a process whose parent exited before then has been + // reparented and is not covered. The names of the descendants do not + // matter, unlike |target_cmdline|. + optional bool target_pid_descendants = 7; } // Userspace unwinding mode. diff --git a/src/profiling/perf/event_config.cc b/src/profiling/perf/event_config.cc index 8e9ce206b0e..7ada554293e 100644 --- a/src/profiling/perf/event_config.cc +++ b/src/profiling/perf/event_config.cc @@ -563,6 +563,9 @@ std::optional EventConfig::CreateSampling( process_sharding) : ParseTargetFilter(pb_config, process_sharding); // backwards compatibility + // Only the Scope message has it; the legacy top-level fields do not. + target_filter.pid_descendants = + pb_config.callstack_sampling().scope().target_pid_descendants(); // Kernel callstacks. kernel_frames = pb_config.callstack_sampling().kernel_frames() || diff --git a/src/profiling/perf/event_config.h b/src/profiling/perf/event_config.h index 7a3d68671ce..9bda1c6f8d1 100644 --- a/src/profiling/perf/event_config.h +++ b/src/profiling/perf/event_config.h @@ -53,6 +53,8 @@ struct TargetFilter { base::FlatSet exclude_pids; std::optional process_sharding; uint32_t additional_cmdline_count = 0; + // Also keep the descendants of |pids|. + bool pid_descendants = false; }; // Describes a perf event for two purposes: diff --git a/src/profiling/perf/perf_producer.cc b/src/profiling/perf/perf_producer.cc index 05c07ae2ab5..aedb874442f 100644 --- a/src/profiling/perf/perf_producer.cc +++ b/src/profiling/perf/perf_producer.cc @@ -19,9 +19,11 @@ #include #include #include +#include #include #include +#include #include #include @@ -254,6 +256,31 @@ void WritePerfEventDefaultsPacket(const EventConfig& event_config, } } +// How far up the parent chain a process is looked for in the target's +// descendants. Deeper than any real process tree; stops a loop on a pid +// reused mid-walk. +constexpr int kMaxAncestorDepth = 64; + +// The parent pid of |pid|, from /proc//stat, whose fourth field it is. +// The second field is the command name in parentheses, which can itself +// hold spaces and parentheses, so the fields after it are found from the +// last ')'. +std::optional ReadParentPid(pid_t pid) { + std::string stat; + if (!base::ReadFile("/proc/" + std::to_string(pid) + "/stat", &stat)) + return std::nullopt; + size_t end = stat.rfind(')'); + if (end == std::string::npos || end + 4 > stat.size()) + return std::nullopt; + // ") S 1234 ..." + const char* fields = stat.c_str() + end + 4; + char* parsed_end = nullptr; + long ppid = strtol(fields, &parsed_end, 10); + if (parsed_end == fields) + return std::nullopt; + return static_cast(ppid); +} + uint32_t TimeToNextReadTickMs(DataSourceInstanceID ds_id, uint32_t period_ms) { // Normally, we'd schedule the next tick at the next |period_ms| // boundary of the boot clock. However, to avoid aligning the read tasks of @@ -372,7 +399,8 @@ bool PerfProducer::ShouldRejectDueToFilter( const TargetFilter& filter, bool skip_cmdline, base::FlatSet* additional_cmdlines, - std::function read_proc_pid_cmdline) { + std::function read_proc_pid_cmdline, + std::function(pid_t)> read_ppid) { PERFETTO_CHECK(additional_cmdlines); std::string cmdline; @@ -414,6 +442,17 @@ bool PerfProducer::ShouldRejectDueToFilter( if (filter.pids.count(pid)) { return false; } + if (filter.pid_descendants && !filter.pids.empty()) { + pid_t p = pid; + for (int depth = 0; depth < kMaxAncestorDepth; depth++) { + std::optional ppid = read_ppid(p); + if (!ppid.has_value() || *ppid <= 1) + break; + if (filter.pids.count(*ppid)) + return false; + p = *ppid; + } + } // Empty allow filter means keep everything that isn't explicitly excluded. if (filter.cmdlines.empty() && filter.pids.empty() && @@ -925,7 +964,8 @@ bool PerfProducer::ReadAndParsePerCpuBuffer(EventReader* reader, pid, event_config.filter(), is_kthread, &ds.additional_cmdlines, [pid](std::string* cmdline) { return glob_aware::ReadProcCmdlineForPID(pid, cmdline); - })) { + }, + ReadParentPid)) { process_state = ProcessTrackingStatus::kRejected; EmitSkippedSample(ds_id, std::move(sample.value()), SampleSkipReason::kRejected); diff --git a/src/profiling/perf/perf_producer.h b/src/profiling/perf/perf_producer.h index 324b6e4c4bb..86d040d9210 100644 --- a/src/profiling/perf/perf_producer.h +++ b/src/profiling/perf/perf_producer.h @@ -99,12 +99,15 @@ class PerfProducer : public Producer, } // public for testing: + // |read_ppid| returns a process's parent pid, or nullopt if it cannot be + // read (the process is gone). static bool ShouldRejectDueToFilter( pid_t pid, const TargetFilter& filter, bool skip_cmdline, base::FlatSet* additional_cmdlines, - std::function read_proc_pid_cmdline); + std::function read_proc_pid_cmdline, + std::function(pid_t)> read_ppid); private: // State of the producer's connection to tracing service (traced). diff --git a/src/profiling/perf/perf_producer_unittest.cc b/src/profiling/perf/perf_producer_unittest.cc index 76b615af79b..b90bdf00aed 100644 --- a/src/profiling/perf/perf_producer_unittest.cc +++ b/src/profiling/perf/perf_producer_unittest.cc @@ -17,6 +17,7 @@ #include "src/profiling/perf/perf_producer.h" #include +#include #include #include "perfetto/base/logging.h" @@ -30,11 +31,19 @@ bool ShouldReject(pid_t pid, std::string cmdline, const TargetFilter& filter, bool skip_cmd, - base::FlatSet* additional_cmdlines) { + base::FlatSet* additional_cmdlines, + std::map parents = {}) { return PerfProducer::ShouldRejectDueToFilter( - pid, filter, skip_cmd, additional_cmdlines, [cmdline](std::string* out) { + pid, filter, skip_cmd, additional_cmdlines, + [cmdline](std::string* out) { *out = cmdline; return true; + }, + [parents](pid_t p) -> std::optional { + auto it = parents.find(p); + if (it == parents.end()) + return std::nullopt; + return it->second; }); } @@ -197,6 +206,39 @@ TEST(TargetFilterTest, AdditionalCmdlines) { EXPECT_EQ(extra_cmds.count("/bin/top"), 0u); } +TEST(TargetFilterTest, TargetPidDescendants) { + bool skip_cmd = false; + base::FlatSet extra_cmds; + TargetFilter filter; + filter.pids.insert(100); + // 100 forks 200 (a worker that renamed itself), which forks 300; 400 is + // unrelated and 500 was reparented to init. + std::map parents = { + {100, 1}, {200, 100}, {300, 200}, {400, 1}, {500, 1}}; + + // Without the option, only the pid itself. + EXPECT_FALSE( + ShouldReject(100, "nginx", filter, skip_cmd, &extra_cmds, parents)); + EXPECT_TRUE(ShouldReject(200, "nginx: worker process", filter, skip_cmd, + &extra_cmds, parents)); + + filter.pid_descendants = true; + EXPECT_FALSE(ShouldReject(200, "nginx: worker process", filter, skip_cmd, + &extra_cmds, parents)); + EXPECT_FALSE( + ShouldReject(300, "cc1", filter, skip_cmd, &extra_cmds, parents)); + EXPECT_TRUE( + ShouldReject(400, "bash", filter, skip_cmd, &extra_cmds, parents)); + EXPECT_TRUE(ShouldReject(500, "nginx: worker process", filter, skip_cmd, + &extra_cmds, parents)); + // A process already gone has no parent to read. + EXPECT_TRUE( + ShouldReject(600, "gone", filter, skip_cmd, &extra_cmds, parents)); + // Exclusions still win. + filter.exclude_pids.insert(300); + EXPECT_TRUE(ShouldReject(300, "cc1", filter, skip_cmd, &extra_cmds, parents)); +} + } // namespace } // namespace profiling } // namespace perfetto