Re: (sashiko review) [RFC PATCH v5 02/11] mm/damon/core: merge quota-sliced regions back

[SeongJae Park <sj@kernel.org>]

On Fri, 10 Apr 2026 16:55:26 -0700 SeongJae Park <sj@kernel.org> wrote:
[...]
> > Should the merge condition verify that the contiguous slices share identical
> > state before merging, perhaps by checking if (orig_region->age != r->age)?
> 
> Good finding.  I was thinking the scheme will anyway be applied, once the
> quota-split is happened.  But I was wrong, since DAMOS core filter is applied
> after the split operation.  Also, age is not reset if the action is DAMOS_STAT.
> 
> I will address this in the next revision, like below.
> 
> '''
> --- a/mm/damon/core.c
> +++ b/mm/damon/core.c
> @@ -2439,35 +2439,32 @@ static void damon_do_apply_schemes(struct damon_ctx *c,
> 
>  static void damos_apply_target(struct damon_ctx *c, struct damon_target *t)
>  {
> -       struct damon_region *r, *orig_region = NULL;
> -       unsigned long orig_end_addr;
> +       struct damon_region *r;
> 
>         damon_for_each_region(r, t) {
> +               struct damon_region *prev_r;
> +
> +               damon_do_apply_schemes(c, t, r);
>                 /*
> -                * damon_do_apply_schemes() could split the region for the
> +                * damon_do_apply_scheems() could split the region for the
>                  * quota.  Keeping the new slices is an overhead.  Merge back
> -                * the slices into the original region if there is no reason to
> -                * keep those.
> +                * the slices into the previous region if it doesn't lose any
> +                * information.
>                  */
> -               if (!orig_region || orig_end_addr <= r->ar.start) {
> -                       orig_region = r;
> -                       orig_end_addr = r->ar.end;
> -               }
> -               damon_do_apply_schemes(c, t, r);
> -               if (r == orig_region)
> +               if (damon_first_region(t) == r)
>                         continue;
> -               /*
> -                * If no scheme was applied to the sliced region, the age of
> -                * the slice ain't be reset.  Don't merge that back.
> -                * Otherwise, the monitored information of the region is lost.
> -                */
> -               if (r->age) {
> -                       orig_region = NULL;
> +               prev_r = damon_prev_region(r);
> +               if (prev_r->ar.end != r->ar.start)
>                         continue;
> -               }
> -               orig_region->ar.end = r->ar.end;
> +               if (prev_r->age != r->age)
> +                       continue;
> +               if (prev_r->last_nr_accesses != r->last_nr_accesses)
> +                       continue;
> +               if (prev_r->nr_accesses != r->nr_accesses)
> +                       continue;
> +               prev_r->ar.end = r->ar.end;
>                 damon_destroy_region(r, t);
> -               r = orig_region;
> +               r = prev_r;
>         }
>  }

This could result in too aggressive merging, that can make the number of
regions lower than the min_nr_regions.  Actually this makes one of damo tests
fail.  I will fix this by using damon_region_sz_limit().


Thanks,
SJ

[...]