Re: [PATCH 2/2] mm/damon/paddr: prefetch struct page of the next region

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [High] Unsafe use of `pfn_to_page()` on unvalidated PFNs causes kernel panic on SPARSEMEM configurations without VMEMMAP.
--

commit 8abf0dd953e91eace9153afe00585b922bbe2751
Author: Jiayuan Chen <jiayuan.chen@shopee.com>

mm/damon/paddr: prefetch struct page of the next region

This commit introduces software prefetching for the next region's struct
page in DAMON's physical address mode to reduce cache misses. It uses
pfn_to_page() instead of pfn_to_online_page() to avoid the serialization
costs of looking up the memory section, expecting prefetchw() to safely
drop hints for unmapped addresses.

>   - pfn_to_page() rather than pfn_to_online_page(): on
>     CONFIG_SPARSEMEM_VMEMMAP the former is pure arithmetic
>     (vmemmap + pfn), while the latter walks the mem_section table.  The
>     mem_section lookup itself incurs a DRAM miss for random PFNs, which
>     would serialize what is supposed to be a non-blocking hint - an
>     earlier attempt that used pfn_to_online_page() saw the prefetch
>     path's internal stall dominate perf (~91% skid on the converge
>     point after the call).  Prefetching an unmapped vmemmap entry is
>     safe: the hint is dropped without faulting.

Does this logic hold for systems using CONFIG_SPARSEMEM without VMEMMAP?

While it is true that pfn_to_page() is pure arithmetic on VMEMMAP, under
classic SPARSEMEM it requires a section lookup to evaluate.

> diff --git a/mm/damon/paddr.c b/mm/damon/paddr.c
> index b5e1197f2ba22..99bdf2b88cf19 100644
> --- a/mm/damon/paddr.c
> +++ b/mm/damon/paddr.c
[ ... ]
> @@ -48,10 +49,29 @@ static void damon_pa_mkold(phys_addr_t paddr)
>  	folio_put(folio);
>  }
>  
> +static void damon_pa_prefetch_page(unsigned long sampling_addr,
> +				   unsigned long addr_unit)
> +{
> +	phys_addr_t paddr = damon_pa_phys_addr(sampling_addr, addr_unit);
> +
> +	prefetchw(pfn_to_page(PHYS_PFN(paddr)));

Can this cause a kernel panic on CONFIG_SPARSEMEM configurations without
VMEMMAP?

In DAMON paddr mode, target regions can span physical memory holes. Since
damon_rand_fast() selects a random physical address, it might select an
unpopulated area.

If the random PFN falls into an unallocated section, __pfn_to_section() will
be invalid. Attempting to read the section_mem_map from it will lead to an
immediate NULL pointer dereference in C code before the prefetchw instruction
is ever reached.

> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260423122340.138880-1-jiayuan.chen@linux.dev?part=2