Re: [PATCH v3 3/7] mm/page_owner: add bpf_iter target "page_owner"

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mm/page_owner: rescan and post-stop show() in page_owner_seq_start() at EOF
--

commit a6ba125169596a18417440d179f83a946227810e
Author: Zhen Ni <zhen.ni@easystack.cn>

mm/page_owner: add bpf_iter target "page_owner"

This commit adds a "page_owner" bpf_iter target that iterates over pages
eligible for page_owner output using page_owner_next_eligible() and passes
the PFN and a snapshot of struct page_owner to the attached BPF program.

> diff --git a/mm/page_owner_iter.c b/mm/page_owner_iter.c
> new file mode 100644
> index 0000000000000..529b2c4a589a9
> --- /dev/null
> +++ b/mm/page_owner_iter.c
> @@ -0,0 +1,118 @@

[ ... ]

> +static void *page_owner_seq_start(struct seq_file *seq, loff_t *posp)
> +{
> +	struct page_owner_scan *scan = seq->private;
> +
> +	if (*posp == 0)
> +		scan->pfn = min_low_pfn;
> +
> +	if (!page_owner_next_eligible(scan))
> +		return NULL;
> +	return &scan->po_snap;
> +}
> +
> +static void *page_owner_seq_next(struct seq_file *seq, void *v, loff_t *posp)
> +{
> +	struct page_owner_scan *scan = seq->private;
> +
> +	++*posp;
> +	scan->pfn++;
> +	if (!page_owner_next_eligible(scan))
> +		return NULL;
> +
> +	return &scan->po_snap;
> +}

[Severity: Medium]
Can this rescan PFNs and invoke page_owner_seq_show() again after the final
page_owner_seq_stop() callback has already run?

In mm/page_owner.c:page_owner_next_eligible(), scan->pfn is copied into a
local variable and only written back inside the loop before checking
scan->po_snap.handle:

mm/page_owner.c:page_owner_next_eligible() {
	unsigned long pfn = scan->pfn;
	...
	for (; pfn < max_pfn; pfn++) {
		...
		scan->pfn = pfn;
		scan->po_snap = *page_owner;

		if (!scan->po_snap.handle)
			goto ext_put_continue;
		...
		return true;
		...
	}

	return false;
}

When page_owner_seq_next() advances scan->pfn past the last eligible page
and calls page_owner_next_eligible(scan), the loop reaches max_pfn and
returns false without updating scan->pfn to max_pfn. As a result, scan->pfn
is left at the PFN right after the last hit (or at an intermediate PFN where
po_snap.handle was zero).

In kernel/bpf/bpf_iter.c:bpf_seq_read(), when page_owner_seq_next() returns
NULL, bpf_seq_read() calls page_owner_seq_stop(seq, NULL) (running the BPF
program with ctx.po == NULL), marks iter_priv->done_stop = true, and flushes
any buffered output in seq->buf to userspace with a positive return value.

When userspace issues the next read() call to observe EOF, bpf_seq_read()
calls page_owner_seq_start(), which calls page_owner_next_eligible(scan) and
rescans from the stale scan->pfn up to max_pfn. If any page in that range
was allocated between the two read() calls, page_owner_next_eligible(scan)
returns true and page_owner_seq_show() invokes the BPF program again with
ctx.po != NULL after the terminal ctx.po == NULL stop callback has already
run.

Similarly, if the initial scan finds no eligible pages (*posp remains 0) and
the BPF program writes output during page_owner_seq_stop(seq, NULL), the
follow-up read() enters page_owner_seq_start() with *posp == 0, resets
scan->pfn = min_low_pfn, and rescans the entire PFN range.

Should page_owner_next_eligible() update scan->pfn = pfn when the loop
terminates at max_pfn (and only store scan->pfn = pfn on a hit after checking
scan->po_snap.handle), and should page_owner_seq_start() also ensure *posp is
advanced so a follow-up read() after an empty scan does not reset scan->pfn
to min_low_pfn?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261021112437.1799213-1-zhen.ni@easystack.cn?part=3