Re: [PATCH] mm/damon/core: do not merge regions when the number of regions is small

[SeongJae Park <sj@kernel.org>]

On Mon, 11 Aug 2025 16:46:12 +0900 Akinobu Mita <akinobu.mita@gmail.com> wrote:

> This change prevents adaptive region merging when the total number of
> regions is less than the minimum number of regions setting.

I agree having the number of regions lower than min_nr_regions is not a good
user experience.  We actually made a fix of a case where the number be higher
than the max_nr_regions, namely commit 310d6c15e910 ("mm/damon/core: merge
regions aggressively when max_nr_regions is unmet").

But, could you please elaborate when the situation can happen, why it is bad,
and if you see this from a real world?

I can expect this situation can happen when the user increases min_nr_regions
to somewhat higher than current number of regions while DAMON is running, using
runtime commit feature.  And it is bad because anyway it is confusing and could
take time to real expected minimum accuracy until DAMON splits regions to keep
the min_nr_regions?  I have no idea for the last question, though.

Anyway, I think this kind of details would be good to be added to the
changelog of this patch, in the next version.

> 
> Even in such cases, the kdamond_merge_regions() function must proceed by
> setting the upper limit of the region size to zero to prevent any actual
> region merging, since it is responsible for updating the region ages rather
> than just merging regions.
> 
> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
> ---
>  mm/damon/core.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/damon/core.c b/mm/damon/core.c
> index 52a48c9316bc..34f1844094cf 100644
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -2247,9 +2247,15 @@ static void kdamond_merge_regions(struct damon_ctx *c, unsigned int threshold,
>  				  unsigned long sz_limit)
>  {
>  	struct damon_target *t;
> -	unsigned int nr_regions;
> +	unsigned int nr_regions = 0;
>  	unsigned int max_thres;
>  
> +	damon_for_each_target(t, c)
> +		nr_regions += damon_nr_regions(t);
> +
> +	if (nr_regions < c->attrs.min_nr_regions)
> +		sz_limit = 0;
> +

sz_limit is already set based on the current min_nr_regions.  Setting this
smaller may help avoiding a few corner case merges, but ain't really avoid
having number of regions lower than min_nr_regions?

Instead, what about making kdamond_split_regions() repeat splitting in the case
until min_nr_regions is met, like kdamond_merge_regions() does for
max_nr_regions?

>  	max_thres = c->attrs.aggr_interval /
>  		(c->attrs.sample_interval ?  c->attrs.sample_interval : 1);
>  	do {
> -- 
> 2.43.0


Thanks,
SJ