Commit 0e10fd4b authored by Andrii Nakryiko's avatar Andrii Nakryiko
Browse files

Merge branch 'Open-coded task_vma iter'

Dave Marchevsky says:

====================
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 763ecb03 ("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 f39af059 ("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:

v6 -> v7: https://lore.kernel.org/bpf/20231010185944.3888849-1-davemarchevsky@fb.com/

Patch numbers correspond to their position in v6

Patch 2 ("selftests/bpf: Rename bpf_iter_task_vma.c to bpf_iter_task_vmas.c")
  * Add Andrii ack
Patch 3 ("bpf: Introduce task_vma open-coded iterator kfuncs")
  * Add Andrii ack
  * Add missing __diag_ignore_all for -Wmissing-prototypes (Song)
Patch 4 ("selftests/bpf: Add tests for open-coded task_vma iter")
  * Remove two unnecessary header includes (Andrii)
  * Remove extraneous !vmas_seen check (Andrii)
New Patch ("bpf: Add BPF_KFUNC_{START,END}_defs macros")
  * After talking to Andrii, this is an attempt to clean up __diag_ignore_all
    spam everywhere kfuncs are defined. If nontrivial changes are needed,
    let's apply the other 4 and I'll respin as a standalone patch.

v5 -> v6: https://lore.kernel.org/bpf/20231010175637.3405682-1-davemarchevsky@fb.com/

Patch 4 ("selftests/bpf: Add tests for open-coded task_vma iter")
  * Remove extraneous blank line. I did this manually to the .patch file
    for v5, which caused BPF CI to complain about failing to apply the
    series

v4 -> v5: https://lore.kernel.org/bpf/20231002195341.2940874-1-davemarchevsky@fb.com/

Patch numbers correspond to their position in v4

New Patch ("selftests/bpf: Rename bpf_iter_task_vma.c to bpf_iter_task_vmas.c")
  * Patch 2's renaming of this selftest, and associated changes in the
    userspace runner, are split out into this separate commit (Andrii)

Patch 2 ("bpf: Introduce task_vma open-coded iterator kfuncs")
  * Remove bpf_iter_task_vma kfuncs from libbpf's bpf_helpers.h, they'll be
    added to selftests' bpf_experimental.h in selftests patch below (Andrii)
  * Split bpf_iter_task_vma.c renaming into separate commit (Andrii)

Patch 3 ("selftests/bpf: Add tests for open-coded task_vma iter")
  * Add bpf_iter_task_vma kfuncs to bpf_experimental.h (Andrii)
  * Remove '?' from prog SEC, open_and_load the skel in one operation (Andrii)
  * Ensure that fclose() always happens in test runner (Andrii)
  * Use global var w/ 1000 (vm_start, vm_end) structs instead of two
    MAP_TYPE_ARRAY's w/ 1k u64s each (Andrii)

v3 -> v4: https://lore.kernel.org/bpf/20230822050558.2937659-1-davemarchevsky@fb.com/

Patch 1 ("bpf: Don't explicitly emit BTF for struct btf_iter_num")
  * Add Andrii ack
Patch 2 ("bpf: Introduce task_vma open-coded iterator kfuncs")
  * Mark bpf_iter_task_vma_new args KF_RCU and remove now-unnecessary !task
    check (Yonghong)
    * Although KF_RCU is a function-level flag, in reality it only applies to
      the task_struct *task parameter, as the other two params are a scalar int
      and a specially-handled KF_ARG_PTR_TO_ITER
   * Remove struct bpf_iter_task_vma definition from uapi headers, define in
     kernel/bpf/task_iter.c instead (Andrii)
Patch 3 ("selftests/bpf: Add tests for open-coded task_vma iter")
  * Use a local var when looping over vmas to track map idx. Update vmas_seen
    global after done iterating. Don't start iterating or update vmas_seen if
    vmas_seen global is nonzero. (Andrii)
  * Move getpgid() call to correct spot - above skel detach. (Andrii)

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

  [0]: https://lore.kernel.org/bpf/20230801145414.418145-1-davemarchevsky@fb.com/


====================

Signed-off-by: default avatarAndrii Nakryiko <andrii@kernel.org>
parents ba8ea723 e0e1a7a5
Loading
Loading
Loading
Loading
+0 −2
Original line number Diff line number Diff line
@@ -793,8 +793,6 @@ __bpf_kfunc int bpf_iter_num_new(struct bpf_iter_num *it, int start, int end)
	BUILD_BUG_ON(sizeof(struct bpf_iter_num_kern) != sizeof(struct bpf_iter_num));
	BUILD_BUG_ON(__alignof__(struct bpf_iter_num_kern) != __alignof__(struct bpf_iter_num));

	BTF_TYPE_EMIT(struct btf_iter_num);

	/* start == end is legit, it's an empty range and we'll just get NULL
	 * on first (and any subsequent) bpf_iter_num_next() call
	 */
+3 −0
Original line number Diff line number Diff line
@@ -2552,6 +2552,9 @@ BTF_ID_FLAGS(func, bpf_dynptr_slice_rdwr, KF_RET_NULL)
BTF_ID_FLAGS(func, bpf_iter_num_new, KF_ITER_NEW)
BTF_ID_FLAGS(func, bpf_iter_num_next, KF_ITER_NEXT | KF_RET_NULL)
BTF_ID_FLAGS(func, bpf_iter_num_destroy, KF_ITER_DESTROY)
BTF_ID_FLAGS(func, bpf_iter_task_vma_new, KF_ITER_NEW | KF_RCU)
BTF_ID_FLAGS(func, bpf_iter_task_vma_next, KF_ITER_NEXT | KF_RET_NULL)
BTF_ID_FLAGS(func, bpf_iter_task_vma_destroy, KF_ITER_DESTROY)
BTF_ID_FLAGS(func, bpf_dynptr_adjust)
BTF_ID_FLAGS(func, bpf_dynptr_is_null)
BTF_ID_FLAGS(func, bpf_dynptr_is_rdonly)
+91 −0
Original line number Diff line number Diff line
@@ -7,7 +7,9 @@
#include <linux/fs.h>
#include <linux/fdtable.h>
#include <linux/filter.h>
#include <linux/bpf_mem_alloc.h>
#include <linux/btf_ids.h>
#include <linux/mm_types.h>
#include "mmap_unlock_work.h"

static const char * const iter_task_type_names[] = {
@@ -803,6 +805,95 @@ const struct bpf_func_proto bpf_find_vma_proto = {
	.arg5_type	= ARG_ANYTHING,
};

struct bpf_iter_task_vma_kern_data {
	struct task_struct *task;
	struct mm_struct *mm;
	struct mmap_unlock_irq_work *work;
	struct vma_iterator vmi;
};

struct bpf_iter_task_vma {
	/* opaque iterator state; having __u64 here allows to preserve correct
	 * alignment requirements in vmlinux.h, generated from BTF
	 */
	__u64 __opaque[1];
} __attribute__((aligned(8)));

/* Non-opaque version of bpf_iter_task_vma */
struct bpf_iter_task_vma_kern {
	struct bpf_iter_task_vma_kern_data *data;
} __attribute__((aligned(8)));

__diag_push();
__diag_ignore_all("-Wmissing-prototypes",
		  "Global functions as their definitions will be in vmlinux BTF");

__bpf_kfunc int bpf_iter_task_vma_new(struct bpf_iter_task_vma *it,
				      struct task_struct *task, u64 addr)
{
	struct bpf_iter_task_vma_kern *kit = (void *)it;
	bool irq_work_busy = false;
	int err;

	BUILD_BUG_ON(sizeof(struct bpf_iter_task_vma_kern) != sizeof(struct bpf_iter_task_vma));
	BUILD_BUG_ON(__alignof__(struct bpf_iter_task_vma_kern) != __alignof__(struct bpf_iter_task_vma));

	/* is_iter_reg_valid_uninit guarantees that kit hasn't been initialized
	 * before, so non-NULL kit->data doesn't point to previously
	 * bpf_mem_alloc'd bpf_iter_task_vma_kern_data
	 */
	kit->data = bpf_mem_alloc(&bpf_global_ma, sizeof(struct bpf_iter_task_vma_kern_data));
	if (!kit->data)
		return -ENOMEM;

	kit->data->task = get_task_struct(task);
	kit->data->mm = task->mm;
	if (!kit->data->mm) {
		err = -ENOENT;
		goto err_cleanup_iter;
	}

	/* kit->data->work == NULL is valid after bpf_mmap_unlock_get_irq_work */
	irq_work_busy = bpf_mmap_unlock_get_irq_work(&kit->data->work);
	if (irq_work_busy || !mmap_read_trylock(kit->data->mm)) {
		err = -EBUSY;
		goto err_cleanup_iter;
	}

	vma_iter_init(&kit->data->vmi, kit->data->mm, addr);
	return 0;

err_cleanup_iter:
	if (kit->data->task)
		put_task_struct(kit->data->task);
	bpf_mem_free(&bpf_global_ma, kit->data);
	/* NULL kit->data signals failed bpf_iter_task_vma initialization */
	kit->data = NULL;
	return err;
}

__bpf_kfunc struct vm_area_struct *bpf_iter_task_vma_next(struct bpf_iter_task_vma *it)
{
	struct bpf_iter_task_vma_kern *kit = (void *)it;

	if (!kit->data) /* bpf_iter_task_vma_new failed */
		return NULL;
	return vma_next(&kit->data->vmi);
}

__bpf_kfunc void bpf_iter_task_vma_destroy(struct bpf_iter_task_vma *it)
{
	struct bpf_iter_task_vma_kern *kit = (void *)it;

	if (kit->data) {
		bpf_mmap_unlock_mm(kit->data->work, kit->data->mm);
		put_task_struct(kit->data->task);
		bpf_mem_free(&bpf_global_ma, kit->data);
	}
}

__diag_pop();

DEFINE_PER_CPU(struct mmap_unlock_irq_work, mmap_unlock_work);

static void do_mmap_read_unlock(struct irq_work *entry)
+8 −0
Original line number Diff line number Diff line
@@ -159,6 +159,14 @@ extern void *bpf_percpu_obj_new_impl(__u64 local_type_id, void *meta) __ksym;
 */
extern void bpf_percpu_obj_drop_impl(void *kptr, void *meta) __ksym;

struct bpf_iter_task_vma;

extern int bpf_iter_task_vma_new(struct bpf_iter_task_vma *it,
				 struct task_struct *task,
				 unsigned long addr) __ksym;
extern struct vm_area_struct *bpf_iter_task_vma_next(struct bpf_iter_task_vma *it) __ksym;
extern void bpf_iter_task_vma_destroy(struct bpf_iter_task_vma *it) __ksym;

/* Convenience macro to wrap over bpf_obj_drop_impl */
#define bpf_percpu_obj_drop(kptr) bpf_percpu_obj_drop_impl(kptr, NULL)

+13 −13
Original line number Diff line number Diff line
@@ -10,7 +10,7 @@
#include "bpf_iter_task.skel.h"
#include "bpf_iter_task_stack.skel.h"
#include "bpf_iter_task_file.skel.h"
#include "bpf_iter_task_vma.skel.h"
#include "bpf_iter_task_vmas.skel.h"
#include "bpf_iter_task_btf.skel.h"
#include "bpf_iter_tcp4.skel.h"
#include "bpf_iter_tcp6.skel.h"
@@ -1399,19 +1399,19 @@ static void str_strip_first_line(char *str)
static void test_task_vma_common(struct bpf_iter_attach_opts *opts)
{
	int err, iter_fd = -1, proc_maps_fd = -1;
	struct bpf_iter_task_vma *skel;
	struct bpf_iter_task_vmas *skel;
	int len, read_size = 4;
	char maps_path[64];

	skel = bpf_iter_task_vma__open();
	if (!ASSERT_OK_PTR(skel, "bpf_iter_task_vma__open"))
	skel = bpf_iter_task_vmas__open();
	if (!ASSERT_OK_PTR(skel, "bpf_iter_task_vmas__open"))
		return;

	skel->bss->pid = getpid();
	skel->bss->one_task = opts ? 1 : 0;

	err = bpf_iter_task_vma__load(skel);
	if (!ASSERT_OK(err, "bpf_iter_task_vma__load"))
	err = bpf_iter_task_vmas__load(skel);
	if (!ASSERT_OK(err, "bpf_iter_task_vmas__load"))
		goto out;

	skel->links.proc_maps = bpf_program__attach_iter(
@@ -1462,25 +1462,25 @@ static void test_task_vma_common(struct bpf_iter_attach_opts *opts)
out:
	close(proc_maps_fd);
	close(iter_fd);
	bpf_iter_task_vma__destroy(skel);
	bpf_iter_task_vmas__destroy(skel);
}

static void test_task_vma_dead_task(void)
{
	struct bpf_iter_task_vma *skel;
	struct bpf_iter_task_vmas *skel;
	int wstatus, child_pid = -1;
	time_t start_tm, cur_tm;
	int err, iter_fd = -1;
	int wait_sec = 3;

	skel = bpf_iter_task_vma__open();
	if (!ASSERT_OK_PTR(skel, "bpf_iter_task_vma__open"))
	skel = bpf_iter_task_vmas__open();
	if (!ASSERT_OK_PTR(skel, "bpf_iter_task_vmas__open"))
		return;

	skel->bss->pid = getpid();

	err = bpf_iter_task_vma__load(skel);
	if (!ASSERT_OK(err, "bpf_iter_task_vma__load"))
	err = bpf_iter_task_vmas__load(skel);
	if (!ASSERT_OK(err, "bpf_iter_task_vmas__load"))
		goto out;

	skel->links.proc_maps = bpf_program__attach_iter(
@@ -1533,7 +1533,7 @@ static void test_task_vma_dead_task(void)
out:
	waitpid(child_pid, &wstatus, 0);
	close(iter_fd);
	bpf_iter_task_vma__destroy(skel);
	bpf_iter_task_vmas__destroy(skel);
}

void test_bpf_sockmap_map_iter_fd(void)
Loading