From: Dave Marchevsky <davemarchevsky@fb.com>
To: <bpf@vger.kernel.org>
Cc: Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Andrii Nakryiko <andrii@kernel.org>,
Martin KaFai Lau <martin.lau@kernel.org>,
Kernel Team <kernel-team@fb.com>, <yonghong.song@linux.dev>,
<sdf@google.com>, Dave Marchevsky <davemarchevsky@fb.com>
Subject: [PATCH v3 bpf-next 0/3] Open-coded task_vma iter
Date: Mon, 21 Aug 2023 22:05:55 -0700 [thread overview]
Message-ID: <20230822050558.2937659-1-davemarchevsky@fb.com> (raw)
At Meta we have a profiling daemon which periodically collects
information on many hosts. This collection usually involves grabbing
stacks (user and kernel) using perf_event BPF progs and later symbolicating
them. For user stacks we try to use BPF_F_USER_BUILD_ID and rely on
remote symbolication, but BPF_F_USER_BUILD_ID doesn't always succeed. In
those cases we must fall back to digging around in /proc/PID/maps to map
virtual address to (binary, offset). The /proc/PID/maps digging does not
occur synchronously with stack collection, so the process might already
be gone, in which case it won't have /proc/PID/maps and we will fail to
symbolicate.
This 'exited process problem' doesn't occur very often as
most of the prod services we care to profile are long-lived daemons, but
there are enough usecases to warrant a workaround: a BPF program which
can be optionally loaded at data collection time and essentially walks
/proc/PID/maps. Currently this is done by walking the vma list:
struct vm_area_struct* mmap = BPF_CORE_READ(mm, mmap);
mmap_next = BPF_CORE_READ(rmap, vm_next); /* in a loop */
Since commit 763ecb035029 ("mm: remove the vma linked list") there's no
longer a vma linked list to walk. Walking the vma maple tree is not as
simple as hopping struct vm_area_struct->vm_next. Luckily,
commit f39af05949a4 ("mm: add VMA iterator"), another commit in that series,
added struct vma_iterator and for_each_vma macro for easy vma iteration. If
similar functionality was exposed to BPF programs, it would be perfect for our
usecase.
This series adds such functionality, specifically a BPF equivalent of
for_each_vma using the open-coded iterator style.
Notes:
* This approach was chosen after discussion on a previous series [0] which
attempted to solve the same problem by adding a BPF_F_VMA_NEXT flag to
bpf_find_vma.
* Unlike the task_vma bpf_iter, the open-coded iterator kfuncs here do not
drop the vma read lock between iterations. See Alexei's response in [0].
* The [vsyscall] page isn't really part of task->mm's vmas, but
/proc/PID/maps returns information about it anyways. The vma iter added
here does not do the same. See comment on selftest in patch 3.
* bpf_iter_task_vma allocates a _data struct which contains - among other
things - struct vma_iterator, using BPF allocator and keeps a pointer to
the bpf_iter_task_vma_data. This is done in order to prevent changes to
struct ma_state - which is wrapped by struct vma_iterator - from
necessitating changes to uapi struct bpf_iter_task_vma.
Changelog:
v2 -> v3: https://lore.kernel.org/bpf/20230821173415.1970776-1-davemarchevsky@fb.com/
Patch 1 ("bpf: Don't explicitly emit BTF for struct btf_iter_num")
* Add Yonghong ack
Patch 2 ("bpf: Introduce task_vma open-coded iterator kfuncs")
* UAPI bpf header and tools/ version should match
* Add bpf_iter_task_vma_kern_data which bpf_iter_task_vma_kern points to,
bpf_mem_alloc/free it instead of just vma_iterator. (Alexei)
* Inner data ptr == NULL implies initialization failed
v1 -> v2: https://lore.kernel.org/bpf/20230810183513.684836-1-davemarchevsky@fb.com/
* Patch 1
* Now removes the unnecessary BTF_TYPE_EMIT instead of changing the
type (Yonghong)
* Patch 2
* Don't do unnecessary BTF_TYPE_EMIT (Yonghong)
* Bump task refcount to prevent ->mm reuse (Yonghong)
* Keep a pointer to vma_iterator in bpf_iter_task_vma, alloc/free
via BPF mem allocator (Yonghong, Stanislav)
* Patch 3
Patch summary:
* Patch 1 is a tiny fix I ran into while implementing the vma iter in this
series. It can be applied independently.
* Patch 2 is the meat of the implementation
* Patch 3 adds tests for the new functionality
* Existing iter tests exercise failure cases (e.g. prog that doesn't call
_destroy()). I didn't replicate them in this series, but am happy to add
them in v2 if folks feel that it would be worthwhile.
[0]: https://lore.kernel.org/bpf/20230801145414.418145-1-davemarchevsky@fb.com/
Dave Marchevsky (3):
bpf: Don't explicitly emit BTF for struct btf_iter_num
bpf: Introduce task_vma open-coded iterator kfuncs
selftests/bpf: Add tests for open-coded task_vma iter
include/uapi/linux/bpf.h | 4 +
kernel/bpf/bpf_iter.c | 2 -
kernel/bpf/helpers.c | 3 +
kernel/bpf/task_iter.c | 84 +++++++++++++++++++
tools/include/uapi/linux/bpf.h | 4 +
tools/lib/bpf/bpf_helpers.h | 8 ++
.../selftests/bpf/prog_tests/bpf_iter.c | 26 +++---
.../testing/selftests/bpf/prog_tests/iters.c | 71 ++++++++++++++++
...f_iter_task_vma.c => bpf_iter_task_vmas.c} | 0
.../selftests/bpf/progs/iters_task_vma.c | 56 +++++++++++++
10 files changed, 243 insertions(+), 15 deletions(-)
rename tools/testing/selftests/bpf/progs/{bpf_iter_task_vma.c => bpf_iter_task_vmas.c} (100%)
create mode 100644 tools/testing/selftests/bpf/progs/iters_task_vma.c
--
2.34.1
next reply other threads:[~2023-08-22 5:06 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-22 5:05 Dave Marchevsky [this message]
2023-08-22 5:05 ` [PATCH v3 bpf-next 1/3] bpf: Don't explicitly emit BTF for struct btf_iter_num Dave Marchevsky
2023-08-22 23:37 ` Andrii Nakryiko
2023-08-22 5:05 ` [PATCH v3 bpf-next 2/3] bpf: Introduce task_vma open-coded iterator kfuncs Dave Marchevsky
2023-08-22 17:42 ` Yonghong Song
2023-08-22 19:19 ` David Marchevsky
2023-08-22 20:14 ` Yonghong Song
2023-08-22 22:36 ` Alexei Starovoitov
2023-08-22 23:57 ` Andrii Nakryiko
2023-08-23 0:11 ` Yonghong Song
2023-08-23 0:04 ` Andrii Nakryiko
2023-08-23 5:42 ` David Marchevsky
2023-08-23 14:57 ` Alexei Starovoitov
2023-08-23 16:55 ` Andrii Nakryiko
2023-08-22 23:52 ` Andrii Nakryiko
2023-08-23 7:26 ` David Marchevsky
2023-08-23 15:03 ` Alexei Starovoitov
2023-08-23 17:14 ` Andrii Nakryiko
2023-08-23 17:53 ` Alexei Starovoitov
2023-08-23 18:13 ` Andrii Nakryiko
2023-08-23 17:07 ` Andrii Nakryiko
2023-08-23 17:26 ` Alexei Starovoitov
2023-08-23 17:43 ` Andrii Nakryiko
2023-08-22 5:05 ` [PATCH v3 bpf-next 3/3] selftests/bpf: Add tests for open-coded task_vma iter Dave Marchevsky
2023-08-23 0:13 ` Andrii Nakryiko
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=20230822050558.2937659-1-davemarchevsky@fb.com \
--to=davemarchevsky@fb.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kernel-team@fb.com \
--cc=martin.lau@kernel.org \
--cc=sdf@google.com \
--cc=yonghong.song@linux.dev \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.