> mm/page_owner: add bpf_iter target "page_owner"

TL;DR:
- Missing BPF_ITER_RESCHED may let one read() loop up to 1M objects without
  yielding, risking RCU stalls on !PREEMPT
- Commit message bpftrace example counts allocations, not pages

>     mm/page_owner: add bpf_iter target "page_owner"
>
>     Register a bpf_iter target "page_owner" that walks the pages eligible
>     for page_owner output, with the same eligibility rules as the
>     /sys/kernel/debug/page_owner read path.
>
>     For each eligible page the attached BPF program receives the pfn, the
>     page and the page_owner record, and can freely filter and format output.
>
>     The po_snap record in the scan cursor is taken inside the page_ext
>     RCU window and handed to the BPF program as a copy, so no RCU window
>     is needed at consumption time. Torn reads remain possible if the
>     record is concurrently updated while it is copied, but this is the
>     same best-effort behavior as the read path has always had.
>
>     The target is registered from pageowner_init() when both
>     CONFIG_PAGE_OWNER and CONFIG_BPF_SYSCALL are enabled; a registration
>     failure only disables the bpf_iter target.
>
>     Userspace tools such as bpftrace can attach to this target like any
>     other bpf_iter. For example, to count pages owned by init (pid 1):
>
>         bpftrace -e 'iter:page_owner / ctx->po->pid == 1 / { @ = count(); }'
>
>     Signed-off-by: Zhen Ni <[email protected]>

The example in the commit message says it counts pages owned by init:

    For example, to count pages owned by init (pid 1):
        bpftrace -e 'iter:page_owner / ctx->po->pid == 1 / { @ = count(); }'

Is that accurate?  The iterator produces one record per allocation, not one
per page, because page_owner_next_eligible() skips the tail pages of
high-order allocations:

    /*
     * Don't print "tail" pages of high-order allocations as that
     * would inflate the stats.
     */
    if (!IS_ALIGNED(pfn, 1 << page_owner->order))
        goto ext_put_continue;

So '@ = count()' counts allocations made by pid 1.  An order-9 THP
allocation is counted as 1 instead of 512 pages.

Could the example or its description be corrected so that it reports what
it claims, for instance with '@ = sum(1 << ctx->po->order)' to count pages?

> diff --git a/mm/page_owner_iter.c b/mm/page_owner_iter.c
> new file mode 100644
> index 000000000000..938cb42ab7d7
> --- /dev/null
> +++ b/mm/page_owner_iter.c
> @@ -0,0 +1,124 @@

[ ... ]

> +static struct bpf_iter_reg page_owner_iter_reg_info = {
> +     .target                 = "page_owner",
> +     .ctx_arg_info_size      = 2,
> +     .ctx_arg_info           = {
> +             { offsetof(struct bpf_iter__page_owner, page),
> +               PTR_TO_BTF_ID_OR_NULL },
> +             { offsetof(struct bpf_iter__page_owner, po),
> +               PTR_TO_BTF_ID_OR_NULL },
> +     },
> +     .seq_info               = &page_owner_iter_seq_info,
> +};

Should page_owner_iter_reg_info set .feature = BPF_ITER_RESCHED?

Without it, bpf_seq_read() never calls cond_resched() between objects for
this target, because can_resched = bpf_iter_support_resched(seq) is false.

page_owner_next_eligible() has no scheduling point on its hit path either:

    scan->po_snap = *page_owner;
    if (!scan->po_snap.handle)
        goto ext_put_continue;
    page_ext_put(page_ext);
    return true;

cond_resched() only runs on the ext_put_continue miss path.

When hits are back to back (runs of allocated order-0 pages, which is the
normal case for anon and page cache memory) and the BPF program writes less
than the user buffer size (the commit message's own '@ = count()' example
writes nothing to the seq_file), a single read() loops up to
MAX_ITER_OBJECTS (1,000,000) times.  Each pass does page_ext_get/put, a
struct copy and a BPF program run, with nothing yielding in between.  On
!PREEMPT kernels that holds the CPU and delays RCU quiescent states for the
whole loop.

This is the situation commit cf83b2d2e2b6 ("bpf: Permit cond_resched for
some iterators") hit in the same bpf_seq_read() loop, an rcu_sched
self-detected stall under load.  The debugfs read path doesn't have this
problem because read_page_owner() returns to userspace after every record.

The requirement for the flag looks satisfied, since nothing is held across
next() -> show(): page_owner_next_eligible() drops the page_ext RCU read
lock before returning, and already calls cond_resched() itself.  The other
iterators that can safely reschedule (task, task_file, task_vma, cgroup,
kmem_cache, dmabuf, kallsyms) all set BPF_ITER_RESCHED.

Later commits in this series (54a9d79ddbf5..e52345a241ae) do not add it.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/38403671988

Reply via email to