Re: [RFC PATCH 4/5] mm/damon/paddr: skip free pageblocks in migration walk

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Calling `PageBuddy()` on a tail page triggers a kernel panic.
- [High] Unsafe address advancement skips valid memory when starting from an unaligned offset within a folio.
- [High] Speculative access of folio flags on an unreferenced folio risks `VM_BUG_ON_FOLIO`.
- [Medium] The optimization fails to skip buddy blocks larger than `pageblock_order` efficiently.
- [Medium] Skipping an entire pageblock when `pfn_to_online_page()` returns NULL misses valid memory.
--

commit 66e620b27c62d5fcd72a28187d8b381c33d31172
Author: Ravi Jonnalagadda <ravis.opensrc@gmail.com>
Subject: mm/damon/paddr: skip free pageblocks in migration walk

This patch optimizes DAMON physical memory walks on sparse address spaces
by checking if a pageblock is entirely free or offline, skipping the entire
block to reduce kdamond tick time.

> diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> index c4738cd5e221e..e844c990987b9 100644
> --- a/mm/damon/paddr.c
> +++ b/mm/damon/paddr.c
> @@ -258,13 +258,32 @@ static unsigned long damon_pa_migrate(struct damon_region *r,
>  		unsigned long addr_unit, struct damos *s,
>  		unsigned long *sz_filter_passed)
>  {
> -	phys_addr_t addr, applied;
> +	phys_addr_t addr, end, applied;
>  	LIST_HEAD(folio_list);
>  	struct folio *folio = NULL;
> +	unsigned long pfn;
>  
>  	addr = damon_pa_phys_addr(r->ar.start, addr_unit);
> -	while (addr < damon_pa_phys_addr(r->ar.end, addr_unit)) {
> -		folio = damon_get_folio(PHYS_PFN(addr));
> +	end = damon_pa_phys_addr(r->ar.end, addr_unit);
> +	while (addr < end) {
> +		pfn = PHYS_PFN(addr);
> +
> +		/* Skip pageblocks that are entirely free. */
> +		if (IS_ALIGNED(pfn, pageblock_nr_pages)) {
> +			struct page *page = pfn_to_online_page(pfn);
> +
> +			if (!page) {
> +				addr += pageblock_nr_pages * PAGE_SIZE;
> +				continue;
> +			}

If a memory hole is smaller than a pageblock, can skipping the entire
pageblock miss valid, online pages in the remainder of the block?

> +			if (PageBuddy(page) &&
> +			    buddy_order_unsafe(page) >= pageblock_order) {
> +				addr += pageblock_nr_pages * PAGE_SIZE;
> +				continue;
> +			}

Does this code risk a kernel panic when calling PageBuddy() on a tail page?

If a pageblock-aligned PFN points to a tail page of a larger compound page
(such as a 1GB HugeTLB page), PageBuddy() would trigger a VM_BUG_ON_PGFLAGS
due to the PF_NO_TAIL policy.

Also, if a free buddy block is larger than pageblock_order, does the
optimization fall back to stepping by PAGE_SIZE?

The first pageblock is skipped, but on the next iteration, pfn points to the
start of the second pageblock, which is a tail page of the large buddy block.
Because the PG_buddy flag is only set on the head page, PageBuddy(page) will
evaluate to false.

> +		}
> +
> +		folio = damon_get_folio(pfn);

This isn't a bug introduced by this patch, but does damon_get_folio()
speculatively access folio flags before acquiring a reference?

mm/damon/ops-common.c:damon_get_folio() {
    ...
    if (!folio_test_lru(folio) || !folio_try_get(folio))
        return NULL;
    ...
}

If the page is concurrently freed and reallocated as a tail page of a new
compound page, calling folio_test_lru() before folio_try_get() risks a
VM_BUG_ON_FOLIO.

Additionally, this wasn't introduced by this patch, but can the physical
address advancement skip valid memory when starting from an unaligned offset?

If damon_pa_migrate() starts at an unaligned offset within a large folio, pfn
points to a tail page, and damon_get_folio() resolves this to the head folio.
The loop then unconditionally advances by the full folio size:

    put_folio:
        addr += folio_size(folio);
        folio_put(folio);

Adding the total folio size to an unaligned starting offset would push addr
past the actual end of the folio, skipping pages in the subsequent memory
region.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260516210357.2247-1-ravis.opensrc@gmail.com?part=4