Re: [RFC PATCH 2/7] mm/damon/reclaim: cover all system rams

[SeongJae Park <sj@kernel.org>]

On Wed, 15 Apr 2026 01:58:29 +0000 sashiko-bot@kernel.org wrote:

> Sashiko AI review found 3 potential issue(s):
> - [High] Unbounded iteration over physical memory holes causes severe scheduling latency and soft lockups.
> - [High] Inclusion of physical holes permanently exhausts DAMOS quotas and starves actual memory reclaim.
> - [High] Bounding box over sparse memory dilutes sampling accuracy and traps RAM in cold regions.

I don't think none of these are real issues for this patch.  See below for more
detailed answers.

> --
> 
> commit 1ebe17b8690373c96bc931bcacccad9cbc735e44
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon/reclaim: cover all system rams
> 
> This commit changes the default DAMON_RECLAIM monitoring region from the
> biggest System RAM resource to a single bounding box that spans all System
> RAM resources on the system.
> 
> > diff --git a/mm/damon/reclaim.c b/mm/damon/reclaim.c
> > index 86da147786583..2a2343d751f3e 100644
> > --- a/mm/damon/reclaim.c
> > +++ b/mm/damon/reclaim.c
> [ ... ]
> > @@ -242,11 +244,9 @@ static int damon_reclaim_apply_parameters(void)
> >  		damos_add_filter(scheme, filter);
> >  	}
> >  
> > -	err = damon_set_region_biggest_system_ram_default(param_target,
> > -					&monitor_region_start,
> > -					&monitor_region_end,
> > -					param_ctx->addr_unit,
> > -					param_ctx->min_region_sz);
> > +	err = damon_set_region_system_rams_default(param_target,
> > +			&monitor_region_start, &monitor_region_end,
> > +			param_ctx->addr_unit, param_ctx->min_region_sz);
> 
> When using a single bounding box that covers all System RAM, does this
> introduce large physical address holes (like PCIe MMIO gaps) into the
> monitored region? On sparse NUMA topologies, these holes can be terabytes
> in size.
> 
> If operations like DAMOS_PAGEOUT iterate linearly over the region
> page-by-page, will they process these massive holes? For physical holes,
> damon_get_folio() will return NULL, and the loop might continue without
> yielding:
> 
> damon_pa_pageout() {
>     ...
>     while (addr < end) {
>         ...
>         folio = damon_get_folio(PHYS_PFN(addr));
>         if (!folio) {
>             addr += PAGE_SIZE;
>             continue;
>         }
>         ...
>     }
> }
> 
> Since cond_resched() is often placed outside or after such loops, could this
> continuous iteration over unbacked holes trigger soft lockups and severe
> scheduling latency?

Theoretically that's possible.  But I believe damon_get_folio() should be
lighweight enough to not cause such problems in common cases.  Also, we allow
users to manually set the monitoring regions for special cases.

So unless this cause such real issues in common cases, I think this patch is
good to go as is.

> 
> How does the inclusion of massive physical holes interact with DAMOS quotas?
> 
> In damos_apply_scheme(), the scheme's charged size is incremented by the
> evaluated region size (quota->charged_sz += sz). If regions span terabytes of
> unbacked holes, evaluating them will quickly exhaust the quota (e.g., 128
> MiB/sec for DAMON_RECLAIM) without reclaiming any actual pages.
> 
> Could this starve actual memory reclaim and inflate the sz_tried statistics?

Yes, this could exhaust the quota.  But, again, users can manually set the
regions if this becomes a real problem on their setup.  We are also working on
the action failed regions quota charge ratio [1], which could also help the
corner case.

> 
> Does the bounding box approach also affect sampling accuracy?
> 
> DAMON samples a random address within a region using
> damon_rand(r->ar.start, r->ar.end). If the region is mostly composed of
> physical holes, the sample will frequently land in unbacked space and return
> zero accesses.
> 
> Since the region will continuously appear cold, could
> damon_merge_two_regions() merge it with adjacent valid RAM regions? This
> might trap valid RAM inside massive cold regions, diluting the sampling
> probability for actual memory.

Adaptive regions adjustment should avoid things going on such extreme case.

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260415012048.76508-1-sj@kernel.org?part=2

[1] https://lore.kernel.org/20260412161957.82835-1-sj@kernel.org


Thanks,
SJ

[...]