Re: [RFC PATCH 1/7] mm/damon: introduce damon_set_region_system_rams_default()

[SeongJae Park <sj@kernel.org>]

TL; DR: Sashiko found no real issue of this patch.

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

> Sashiko AI review found 1 potential issue(s):
> - [High] Creation of a single bounding box region for all System RAM severely degrades monitoring accuracy on sparse memory topologies by including non-RAM holes.
> --
> 
> commit 8f8f43643f81a1ce41ad12d7eb7416faa6e23114
> Author: SeongJae Park <sj@kernel.org>
> 
> mm/damon: introduce damon_set_region_system_rams_default()
> 
> This commit introduces a function to set the default target region to
> cover all System RAM resources instead of just the largest one. It finds
> the start of the first resource and the end of the last resource, and
> sets this bounding box as the target region.
> 
> > diff --git a/mm/damon/core.c b/mm/damon/core.c
> > index 12544c60531d3..91e14caa4f6a1 100644
> > --- a/mm/damon/core.c
> > +++ b/mm/damon/core.c
> [ ... ]
> > +static int damon_system_ram_walk_fn(struct resource *res, void *arg)
> > +{
> > +	struct damon_system_ram_range_walk_arg *a = arg;
> > +
> > +	if (!a->walked) {
> > +		a->walked = true;
> > +		a->res.start = res->start;
> > +	}
> > +	a->res.end = res->end;
> > +	return 0;
> > +}
> [ ... ]
> > +int damon_set_region_system_rams_default(struct damon_target *t,
> > +			unsigned long *start, unsigned long *end,
> > +			unsigned long addr_unit, unsigned long min_region_sz)
> > +{
> > +	struct damon_addr_range addr_range;
> > +
> > +	if (*start > *end)
> > +		return -EINVAL;
> > +
> > +	if (!*start && !*end &&
> > +		!damon_find_system_rams_range(start, end, addr_unit))
> > +		return -EINVAL;
> > +
> > +	addr_range.start = *start;
> > +	addr_range.end = *end;
> > +	return damon_set_regions(t, &addr_range, 1, min_region_sz);
> 
> Could this create a single bounding box region that includes large non-RAM
> holes on systems with sparse memory layouts?
> 
> Since damon_system_ram_walk_fn() only records the minimum start address and
> maximum end address across all System RAM resources, creating a single
> contiguous damon_addr_range from this bounding box includes any
> intervening address gaps.
> 
> On multi-socket NUMA systems or devices with large MMIO holes, this
> could result in massive address ranges not backed by actual RAM. Because
> DAMON uniformly samples random addresses within each region, sampling
> might predominantly hit these unbacked memory holes.
> 
> Would this dilute the access sampling and cause actively used memory to be
> incorrectly classified as cold? Should the code gather discrete System RAM
> ranges into an array of struct damon_addr_range and pass them to
> damon_set_regions() with nr_ranges > 1 to exclude holes?

This is true.  But it shouldn't cause real problems because of the adaptive
regions adjustment mechniasm of DAMON, as also mentioned on the cover letter.

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


Thanks,
SJ