Re: [PATCH v3 0/7] mm/page_owner: add bpf_iter and kfuncs

["zhen.ni" <zhen.ni@easystack.cn>]

在 2026/10/22 06:00, Alexei Starovoitov 写道:
> On Wed, Oct 21, 2026 at 04:24 AM Zhen Ni <zhen.ni@easystack.cn> wrote:
>> Exposing the same walk to eBPF lets a program filter and aggregate in
>> the kernel and surface only the records it cares about. This series
>> adds that access as a seq_file bpf_iter target and an open-coded
>> iterator.
> 
> This is overkill. Two iterators, two kfuncs that print strings,
> a rewrite of read_page_owner(), a new file and a new header.
> 

Thanks for the detailed review and the sketch. I agree with the
principle that a thin data kfunc would be ideal if it were sufficient,
but I’d like to explain why the current design ended up this way.

On the two iterator surfaces:
They aren’t duplicate implementations. Both are thin shells over the
single page_owner_next_eligible() extracted from read_page_owner(), so
the eligibility rules exist exactly once. There’s no code redundancy;
the two iters only differ in access patterns while sharing the same
underlying iteration logic. This shape is also what Documentation/bpf/
bpf_iterators.rst prescribes:

When adding a new BPF iterator program, it is expected that similar
functionality will be added as open-coded iterator for maximum
flexibility.
It's also expected that iteration logic and code will be maximally 
shared and reused between two iterator API surfaces.

On the two “print strings” kfuncs:
These are things a prog genuinely cannot do on its own. The record only
carries an opaque stack-depot handle, and resolving it is not reachable
from BPF. Likewise, the cgroup path is built by cgroup_path() walking
the kernfs parent chain — a prog has no way to format it. Given the 512-
byte BPF stack limit, the caller-provides-buffer pattern (as in
bpf_d_path() and bpf_snprintf_btf()) is the standard shape for this kind
of kfunc. I agree they could have been mentioned earlier rather than
left to the diff.

On “a rewrite of read_page_owner()”:
This is really an extraction of common functions, not a rewrite. Aside
from the fix for mem-hotplug locking (which addresses a pre-existing bug
in the debugfs interface), there are no substantial logic changes. It’s
not a large-scale refactoring.

On the new file and header:
This is for code clarity — to distinguish the traditional interface from
the BPF interface and make maintenance easier.

> All that the prog needs from the kernel is the page_owner record
> of a pfn. That's one kfunc. The walk, the eligibility rules and
> the filters belong in the bpf prog:
> 
>    bpf_for(i, 0, ctx->nr) {
>            pfn = ctx->start + i;
>            if (bpf_page_owner_read(pfn, &po) ||
>                pfn & ((1ul << po.order) - 1))
>                    continue;
>            if (po.pid != pid || bpf_page_cgroup_ino(pfn) != cgrp)
>                    continue;
>            /* add 1 << po.order to a hash map keyed by po.handle */
>    }
> 

On why the prog can't actually execute it:

1. What range to scan. Where do ctx->start and ctx->nr come from? No
in-tree iterator context provides them, and a prog cannot derive them:
min_low_pfn..max_pfn, memblock holes, and offline sections are all
kernel state. The loop bounds alone require either another kfunc or a
kernel-side walk — which is exactly what the iter target already is.

2. What makes a page eligible. The alignment test in the sketch is the
only part the kernel agrees is expressible in a prog. Everything before
it reads state a prog doesn’t have a handle to: pfn_valid() over
MAX_ORDER_NR_PAGES areas, pfn_to_online_page() under get_online_mems(),
skip_buddy_pages(), the PAGE_EXT_OWNER / PAGE_EXT_OWNER_ALLOCATED flags
in page_ext, and taking the record snapshot inside the RCU window so the
checked value and the reported value are the same copy.

3. The walk is deliberately lockless, so exposing struct page to the
prog is not safe. The pfn validity check and the page_ext dereference
must happen under the same kernel-controlled RCU window. Otherwise the
prog could observe offlined pages or stale page_ext data, breaking
consistency between the check and the snapshot. Keeping the walk in the
kernel guarantees that the page_owner record is read atomically with
respect to mem-hotplug and RCU protection.

4. One record doesn’t give the prog the useful data. The record holds
only opaque handle/free_handle tokens; stack-depot resolution exists
solely in the kernel. The memcg is neither in the record nor trivially
reachable — the path is built by cgroup_path() walking the kernfs parent
chain. The NUMA node comes from page_owner_get_nid(), which bypasses
page_to_nid()’s poison check. And so on.


Thanks again for pushing on this. I’m still concerned that the sketch
may not be workable in practice, and that it would also lose some useful
capabilities. I hope the above clarifies the reasoning. Happy to discuss
further.



Thanks,
Zhen