mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Ian Rogers <irogers@google.com>
To: irogers@google.com, acme@kernel.org, namhyung@kernel.org,
	 Howard Chu <howardchu95@gmail.com>
Cc: adrian.hunter@intel.com, james.clark@linaro.org,
	jolsa@kernel.org,  linux-kernel@vger.kernel.org,
	linux-perf-users@vger.kernel.org,  mingo@redhat.com,
	peterz@infradead.org
Subject: [PATCH v4 09/18] perf trace: Enumerate the target again once BPF is attached
Date: Fri, 18 Sep 2026 14:19:23 -0700	[thread overview]
Message-ID: <20260918211932.2966061-10-irogers@google.com> (raw)
In-Reply-To: <20260918211932.2966061-1-irogers@google.com>

evlist__create_maps() reads the target out of /proc, and the BPF
sched_process_fork() program only sees what is cloned once it is
attached. A task the target creates between the two is in neither, and
since cmd_trace() drops the sys_enter evsel in favour of
__augmented_syscalls__ there is no other source of enter events. Such a
task, and in turn everything it forks, goes unreported for the rest of
the session.

Read the target out of /proc once more, after the attach. Whatever
existed before the programs went live is there to be found, and whatever
is created after it is sched_process_fork()'s to add, so between them
nothing is left out. Doing this before the attach instead would only
move the window rather than close it.

The enumeration follows the same rule as the BPF programs, which is that
the maps name individual tasks:

 - -p names a process, so its thread group is read from
   /proc/<pid>/task.
 - -t names a thread, which is taken on its own. Expanding it to its
   thread group would trace the siblings it asked to leave out.
 - Descendants come from task->children, read through
   /proc/<pid>/task/<tid>/children. A forked task leads a thread group
   of its own, so each one found is walked in turn and a tree of any
   depth is covered. New threads are not listed there, copy_process()
   gives a CLONE_THREAD child the real_parent of its creator rather than
   the creator itself, but the thread group walk above has them.

The other two ways of choosing what to trace need nothing, for the same
reasons the window never affected them: 'perf trace -a' does not filter
on pid at all, and evlist__prepare_workload() keeps a workload blocked
on a pipe until evlist__start_workload(), well after the attach.

A target that exits during startup is not an error. Reading /proc for a
task that has gone fails with ENOENT, and a task directory that is read
but has nothing in it sets nothing at all, so errno is cleared before the
enumeration and only an allocation failure is passed back. Anything else
leaves the tasks that evlist__create_maps() already found in the map,
which sched_process_exit() takes out again as they die, and the session
runs on rather than being ended over a target that was going to stop
producing events anyway.

What is left is smaller and no longer lasts. A task found here may have
made syscalls between sys_enter going live and it being added to the
map, and those are not reported, but it is traced from that point on. On
a kernel built without CONFIG_PROC_CHILDREN the children files are
absent and descendants cannot be named, leaving the threads of the
target, which are still picked up.

pid_t, PATH_MAX, FILE and the directory reading are all used directly by
the new code, so <sys/types.h>, <limits.h>, <stdio.h> and <dirent.h> are
included rather than relied upon to arrive through another header.

Nothing is read out of /proc twice. A pid is in the collection only
because the task directory holding it was read, and that directory holds
the whole thread group, so both the directory and the children files of
everything in it have been read already. 'perf trace -p' needs this: the
thread map names every thread of the target, each of them is queued as
something to expand, and they all stand for the same directory, so a
target of N threads was read N times over and a children file was opened
N squared times. On a 64 thread target that is 4097 of them, against 65
once the ones already read are left alone.

Assisted-by: Antigravity:gemini-3.1-pro
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-trace.c | 248 +++++++++++++++++++++++++++++++++++++
 1 file changed, 248 insertions(+)

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 003fc13ab6d5..8a5dbf144540 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -15,6 +15,7 @@
  */
 
 #include "util/record.h"
+#include <api/fs/fs.h>
 #include <api/fs/tracing_path.h>
 #ifdef HAVE_LIBBPF_SUPPORT
 #include <bpf/bpf.h>
@@ -65,11 +66,15 @@
 #include "trace_augment.h"
 #include "dwarf-regs.h"
 
+#include <dirent.h>
 #include <errno.h>
 #include <sys/stat.h>
+#include <sys/types.h>
 #include <inttypes.h>
+#include <limits.h>
 #include <poll.h>
 #include <signal.h>
+#include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <linux/err.h>
@@ -4648,6 +4653,239 @@ static int trace__set_filter_pids(struct trace *trace)
 	return err;
 }
 
+/* A list of pids that grows as it is added to, holding each pid once. */
+struct pid_list {
+	pid_t	*entries;
+	size_t	nr;
+	size_t	allocated;
+};
+
+static bool pid_list__has(const struct pid_list *list, pid_t pid)
+{
+	for (size_t i = 0; i < list->nr; i++) {
+		if (list->entries[i] == pid)
+			return true;
+	}
+	return false;
+}
+
+/* Append pid, unless it is already there. */
+static int pid_list__add(struct pid_list *list, pid_t pid)
+{
+	if (pid_list__has(list, pid))
+		return 0;
+
+	if (list->nr == list->allocated) {
+		size_t allocated = list->allocated ? list->allocated * 2 : 32;
+		pid_t *entries = realloc(list->entries, allocated * sizeof(*entries));
+
+		if (entries == NULL)
+			return -ENOMEM;
+
+		list->entries = entries;
+		list->allocated = allocated;
+	}
+
+	list->entries[list->nr++] = pid;
+	return 0;
+}
+
+static void pid_list__exit(struct pid_list *list)
+{
+	zfree(&list->entries);
+	list->nr = 0;
+	list->allocated = 0;
+}
+
+/*
+ * Append the tasks tid has forked to tgids.
+ *
+ * task->children holds what a task forked, and a forked task leads a thread
+ * group of its own, so each is something to expand in turn. New threads are
+ * not listed: copy_process() gives a CLONE_THREAD child the real_parent of
+ * its creator rather than the creator itself, so a thread is a sibling of the
+ * task that created it. Those are enumerated from the task directory instead.
+ */
+static int pid_list__add_children(struct pid_list *tgids, pid_t tid)
+{
+	char path[PATH_MAX];
+	pid_t child;
+	FILE *fp;
+	int err = 0;
+
+	scnprintf(path, sizeof(path), "%s/%d/task/%d/children",
+		  procfs__mountpoint(), tid, tid);
+	fp = fopen(path, "r");
+	/*
+	 * Absent if the task exited, and on a kernel built without
+	 * CONFIG_PROC_CHILDREN. Neither is worth failing for: what is missed
+	 * is a task that has gone away, or descendants the kernel will not
+	 * name.
+	 */
+	if (fp == NULL)
+		return 0;
+
+	while (fscanf(fp, "%d", &child) == 1) {
+		err = pid_list__add(tgids, child);
+		if (err)
+			break;
+	}
+
+	fclose(fp);
+	return err;
+}
+
+/*
+ * Collect the tasks to trace: the target, its threads, and everything they
+ * have forked.
+ *
+ * tgids is the queue of thread groups still to expand. It is walked as it
+ * grows, so a child found here has its own children picked up in a later
+ * pass and the depth of the tree does not matter. The walk terminates
+ * because a task cannot be its own ancestor and pid_list__add() ignores a
+ * pid that is already listed.
+ */
+static int trace__collect_target_pids(struct trace *trace, struct pid_list *pids)
+{
+	struct target *target = &trace->opts.target;
+	/*
+	 * -p names processes, so the whole thread group is a target. -t names
+	 * threads, and expanding one to its group would trace the siblings
+	 * that were deliberately left out.
+	 */
+	bool whole_group = target->pid != NULL;
+	struct perf_thread_map *threads;
+	struct pid_list tgids = {};
+	int err = 0;
+
+	/* Enumerate the target as evlist__create_maps() did, but now. */
+	errno = 0;
+	threads = thread_map__new_str(target->pid, target->tid, target->per_thread);
+	if (threads == NULL) {
+		char bf[128];
+
+		/*
+		 * A target that exited while perf trace was starting up shows
+		 * up here as a failure to read /proc/<pid>/task, with scandir()
+		 * setting ENOENT, or as a task directory that is read but has
+		 * nothing in it, which sets nothing at all and is why errno is
+		 * cleared above. Neither is worth ending the session for: the
+		 * tasks the target had are already in the map from
+		 * evlist__create_maps() and sched_process_exit() takes them out
+		 * again as they die. Carry on with what is known and let only
+		 * an allocation failure through, matching how the pid_list
+		 * additions below are treated.
+		 */
+		if (errno == ENOMEM)
+			return -ENOMEM;
+
+		pr_debug("Couldn't enumerate the target again (%s), tracing the tasks already known\n",
+			 errno == 0 ? "it exited" : str_error_r(errno, bf, sizeof(bf)));
+		return 0;
+	}
+
+	for (int i = 0; i < perf_thread_map__nr(threads); i++) {
+		pid_t pid = perf_thread_map__pid(threads, i);
+
+		err = pid_list__add(whole_group ? &tgids : pids, pid);
+		/* A thread named by -t is not expanded, but its children are. */
+		if (!err && !whole_group)
+			err = pid_list__add_children(&tgids, pid);
+		if (err)
+			goto out;
+	}
+
+	for (size_t i = 0; i < tgids.nr; i++) {
+		pid_t tgid = tgids.entries[i];
+		char path[PATH_MAX];
+		struct dirent *dent;
+		DIR *tasks;
+
+		/*
+		 * A pid is in pids only because the task directory holding it
+		 * was read, and that directory holds the whole thread group,
+		 * so this one has been read already. 'perf trace -p' relies on
+		 * this: the thread map names every thread of the target, each
+		 * of them is queued here, and they all stand for the same
+		 * directory, which would otherwise be read once per thread.
+		 */
+		if (pid_list__has(pids, tgid))
+			continue;
+
+		scnprintf(path, sizeof(path), "%s/%d/task", procfs__mountpoint(), tgid);
+		tasks = opendir(path);
+		if (tasks == NULL)
+			continue;	/* Exited between being named and being read. */
+
+		while ((dent = readdir(tasks)) != NULL) {
+			char *end;
+			pid_t tid = strtol(dent->d_name, &end, 10);
+
+			/* Skip "." and "..". */
+			if (*end != '\0')
+				continue;
+
+			/* Seen before, so its children have been read too. */
+			if (pid_list__has(pids, tid))
+				continue;
+
+			err = pid_list__add(pids, tid);
+			if (!err)
+				err = pid_list__add_children(&tgids, tid);
+			if (err)
+				break;
+		}
+
+		closedir(tasks);
+		if (err)
+			goto out;
+	}
+out:
+	perf_thread_map__put(threads);
+	pid_list__exit(&tgids);
+	return err;
+}
+
+/*
+ * Trace the tasks that appeared while perf trace was starting up.
+ *
+ * evlist__create_maps() enumerated the target from /proc, and
+ * sched_process_fork() only sees what is cloned once it is attached. A task
+ * created in between is in neither, and since cmd_trace() drops the sys_enter
+ * evsel in favour of __augmented_syscalls__ it would go unreported for the
+ * whole session.
+ *
+ * Read the target out of /proc again now that the programs are attached.
+ * Whatever came before the attach is in /proc to be found, and whatever comes
+ * after it is sched_process_fork()'s to add, so between them nothing is left
+ * out.
+ *
+ * Doing this after the attach rather than before it is what makes that true;
+ * before it would only move the window. The cost is that a task found here
+ * may have made syscalls between sys_enter going live and it being added
+ * below, and those are not reported. It is traced from that point on.
+ */
+static int trace__set_startup_pids(struct trace *trace)
+{
+	struct pid_list pids = {};
+	int err;
+
+	/*
+	 * Nothing to do without a target: 'perf trace -a' does not filter on
+	 * pid at all, and evlist__prepare_workload() keeps a workload blocked
+	 * on a pipe until evlist__start_workload(), well after the attach.
+	 */
+	if (!target__has_task(&trace->opts.target))
+		return 0;
+
+	err = trace__collect_target_pids(trace, &pids);
+	if (!err)
+		err = augmented_syscalls__set_target_pids(pids.nr, pids.entries);
+
+	pid_list__exit(&pids);
+	return err;
+}
+
 static int __trace__deliver_event(struct trace *trace, union perf_event *event)
 {
 	struct evlist *evlist = trace->evlist;
@@ -5014,6 +5252,16 @@ static int trace__run(struct trace *trace, int argc, const char **argv)
 	if (err < 0)
 		goto out_error_attach;
 
+	/*
+	 * With the programs live, anything the target created while they were
+	 * being set up is now sched_process_fork()'s to keep track of, but it
+	 * was not there to see it appear. Enumerate the target once more to
+	 * pick those up.
+	 */
+	err = trace__set_startup_pids(trace);
+	if (err < 0)
+		goto out_error_filter_pids;
+
 	/*
 	 * If the "close" syscall is not traced, then we will not have the
 	 * opportunity to, in syscall_arg__scnprintf_close_fd() invalidate the
-- 
2.55.0.1082.g2b9226bbc0-goog


  parent reply	other threads:[~2026-09-18 21:20 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-17  6:42 [PATCH v1 00/13] perf trace: Fix BPF filtering and make tracing tests non-exclusive Ian Rogers
2026-09-17  6:42 ` [PATCH v1 01/13] perf trace: Start BPF summary before starting workload Ian Rogers
2026-09-17  6:42 ` [PATCH v1 02/13] perf trace: Skip internal tracepoint fields in formatting and beauty map Ian Rogers
2026-09-17  6:42 ` [PATCH v1 03/13] perf trace: Do not set unaugmented BPF program on sys_exit map Ian Rogers
2026-09-17  6:42 ` [PATCH v1 04/13] perf trace: Filter events in BPF and avoid tracepoint vetoes Ian Rogers
2026-09-17  6:42 ` [PATCH v1 05/13] perf trace: Handle fork and exit directly in BPF filter maps Ian Rogers
2026-09-17  6:42 ` [PATCH v1 06/13] perf test test_task_analyzer: Isolate in temporary directory and make non-exclusive Ian Rogers
2026-09-17  6:42 ` [PATCH v1 07/13] perf test common: Do not globally disable tracing events in clear_all_probes Ian Rogers
2026-09-17  6:42 ` [PATCH v1 08/13] perf test probe_vfs_getname: Scope probe name to PID and make non-exclusive Ian Rogers
2026-09-17  6:42 ` [PATCH v1 09/13] perf test record+probe_libc_inet_pton: Scope event to PID, add retries, " Ian Rogers
2026-09-17  6:42 ` [PATCH v1 10/13] perf test trace_summary: Improve error diagnostics Ian Rogers
2026-09-17  6:42 ` [PATCH v1 11/13] perf test trace_btf_general: Drop --max-events=1 and make non-exclusive Ian Rogers
2026-09-17  6:42 ` [PATCH v1 12/13] perf test trace_summary: Make non-exclusive Ian Rogers
2026-09-17  6:42 ` [PATCH v1 13/13] perf test uprobe_from_different_cu: Scope probe name to PID Ian Rogers
2026-09-17 16:38 ` [PATCH v2 00/14] perf trace: Fix BPF filtering and make tracing tests non-exclusive Ian Rogers
2026-09-17 16:38   ` [PATCH v2 01/14] perf trace: Include the headers declaring pid_t and strcmp Ian Rogers
2026-09-17 16:38   ` [PATCH v2 02/14] perf trace: Start BPF summary before starting workload Ian Rogers
2026-09-17 16:38   ` [PATCH v2 03/14] perf trace: Skip internal tracepoint fields in formatting and beauty map Ian Rogers
2026-09-17 16:38   ` [PATCH v2 04/14] perf trace: Do not set unaugmented BPF program on sys_exit map Ian Rogers
2026-09-17 16:38   ` [PATCH v2 05/14] perf trace: Filter events in BPF and avoid tracepoint vetoes Ian Rogers
2026-09-17 16:38   ` [PATCH v2 06/14] perf trace: Handle fork and exit directly in BPF filter maps Ian Rogers
2026-09-17 16:38   ` [PATCH v2 07/14] perf test test_task_analyzer: Isolate in temporary directory and make non-exclusive Ian Rogers
2026-09-17 16:38   ` [PATCH v2 08/14] perf test common: Do not globally disable tracing events in clear_all_probes Ian Rogers
2026-09-17 16:38   ` [PATCH v2 09/14] perf test probe_vfs_getname: Scope probe name to PID and make non-exclusive Ian Rogers
2026-09-17 16:38   ` [PATCH v2 10/14] perf test record+probe_libc_inet_pton: Scope event to PID, add retries, " Ian Rogers
2026-09-17 16:38   ` [PATCH v2 11/14] perf test trace_summary: Improve error diagnostics Ian Rogers
2026-09-17 16:39   ` [PATCH v2 12/14] perf test trace_btf_general: Drop --max-events=1 and make non-exclusive Ian Rogers
2026-09-17 16:39   ` [PATCH v2 13/14] perf test trace_summary: Make non-exclusive Ian Rogers
2026-09-17 16:39   ` [PATCH v2 14/14] perf test uprobe_from_different_cu: Scope probe name to PID Ian Rogers
2026-09-18 14:06   ` [PATCH v3 00/16] perf trace: Fix BPF filtering and make tracing tests non-exclusive Ian Rogers
2026-09-18 14:06     ` [PATCH v3 01/16] perf trace: Include the headers declaring pid_t, strcmp and assert Ian Rogers
2026-09-18 14:06     ` [PATCH v3 02/16] perf trace: Free the whole evsel_trace in evsel__put_and_free_priv Ian Rogers
2026-09-18 14:06     ` [PATCH v3 03/16] perf trace: Start BPF summary before starting workload Ian Rogers
2026-09-18 14:06     ` [PATCH v3 04/16] perf trace: Skip internal tracepoint fields in formatting and beauty map Ian Rogers
2026-09-18 14:06     ` [PATCH v3 05/16] perf trace: Do not set unaugmented BPF program on sys_exit map Ian Rogers
2026-09-18 14:06     ` [PATCH v3 06/16] perf trace: Filter events in BPF and avoid tracepoint vetoes Ian Rogers
2026-09-18 14:06     ` [PATCH v3 07/16] perf trace: Handle fork and exit directly in BPF filter maps Ian Rogers
2026-09-18 14:06     ` [PATCH v3 08/16] perf trace: Enumerate the target again once BPF is attached Ian Rogers
2026-09-18 14:06     ` [PATCH v3 09/16] perf test test_task_analyzer: Isolate in temporary directory and make non-exclusive Ian Rogers
2026-09-18 14:06     ` [PATCH v3 10/16] perf test common: Only disable probes in clear_all_probes Ian Rogers
2026-09-18 14:06     ` [PATCH v3 11/16] perf test probe_vfs_getname: Scope probe name to PID and make non-exclusive Ian Rogers
2026-09-18 14:06     ` [PATCH v3 12/16] perf test record+probe_libc_inet_pton: Scope event to PID, add retries, " Ian Rogers
2026-09-18 14:06     ` [PATCH v3 13/16] perf test trace_summary: Improve error diagnostics Ian Rogers
2026-09-18 14:06     ` [PATCH v3 14/16] perf test trace_btf_general: Drop --max-events=1 and make non-exclusive Ian Rogers
2026-09-18 14:06     ` [PATCH v3 15/16] perf test trace_summary: Make non-exclusive Ian Rogers
2026-09-18 14:06     ` [PATCH v3 16/16] perf test uprobe_from_different_cu: Scope probe name to PID Ian Rogers
2026-09-18 21:19     ` [PATCH v4 00/18] perf trace: Fix BPF filtering and make tracing tests non-exclusive Ian Rogers
2026-09-18 21:19       ` [PATCH v4 01/18] perf trace: Include the headers declaring pid_t, strcmp and assert Ian Rogers
2026-09-18 21:19       ` [PATCH v4 02/18] perf trace: Free the whole evsel_trace in evsel__put_and_free_priv Ian Rogers
2026-09-18 21:19       ` [PATCH v4 03/18] perf evsel: Report an allocation failure as ENOMEM when setting filters Ian Rogers
2026-09-18 21:19       ` [PATCH v4 04/18] perf trace: Start BPF summary before starting workload Ian Rogers
2026-09-18 21:19       ` [PATCH v4 05/18] perf trace: Skip internal tracepoint fields in formatting and beauty map Ian Rogers
2026-09-18 21:19       ` [PATCH v4 06/18] perf trace: Do not set unaugmented BPF program on sys_exit map Ian Rogers
2026-09-18 21:19       ` [PATCH v4 07/18] perf trace: Filter events in BPF and avoid tracepoint vetoes Ian Rogers
2026-09-18 21:19       ` [PATCH v4 08/18] perf trace: Handle fork and exit directly in BPF filter maps Ian Rogers
2026-09-18 21:19       ` Ian Rogers [this message]
2026-09-18 21:19       ` [PATCH v4 10/18] perf trace: Drop targets that died before they were filtered Ian Rogers
2026-09-18 21:19       ` [PATCH v4 11/18] perf test test_task_analyzer: Isolate in temporary directory and make non-exclusive Ian Rogers
2026-09-18 21:19       ` [PATCH v4 12/18] perf test common: Only disable probes in clear_all_probes Ian Rogers
2026-09-18 21:19       ` [PATCH v4 13/18] perf test probe_vfs_getname: Scope probe name to PID and make non-exclusive Ian Rogers
2026-09-18 21:19       ` [PATCH v4 14/18] perf test record+probe_libc_inet_pton: Scope event to PID, add retries, " Ian Rogers
2026-09-18 21:19       ` [PATCH v4 15/18] perf test trace_summary: Improve error diagnostics Ian Rogers
2026-09-18 21:19       ` [PATCH v4 16/18] perf test trace_btf_general: Drop --max-events=1 and make non-exclusive Ian Rogers
2026-09-18 21:19       ` [PATCH v4 17/18] perf test trace_summary: Make non-exclusive Ian Rogers
2026-09-18 21:19       ` [PATCH v4 18/18] perf test uprobe_from_different_cu: Scope probe name to PID Ian Rogers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260918211932.2966061-10-irogers@google.com \
    --to=irogers@google.com \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=howardchu95@gmail.com \
    --cc=james.clark@linaro.org \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®